Crash after evaluating RegExp twice on iOS & Android #121270
Assignees
Labels
c: crash
Stack traces logged to the console
c: fatal crash
Crashes that terminate the process
c: regression
It was better in the past than it is now
dependency: dart
Dart team may need to help us
engine
flutter/engine repository. See also e: labels.
found in release: 3.7
Found to occur in 3.7
found in release: 3.8
Found to occur in 3.8
has reproducible steps
The issue has been confirmed reproducible and is ready to work on
P0
Critical issues such as a build break or regression
platform-android
Android applications specifically
platform-ios
iOS applications specifically
r: fixed
Issue is closed as already fixed in a newer version
Comments
danagbemava-nc
changed the title
|
I can reproduce the issue using the code sample provided above. In my testing, this reproduced on Labeling for further investigation. logsiOS LogsFlutter 3.7.0 & belowFlutter 3.7.1 & aboveAndroid logsFlutter 3.7.0 & belowFlutter 3.7.1 & aboveflutter doctor -v |
danagbemava-nc
added
c: crash
|
@aam would you be able to take a look at this? |
|
Increasing the priority to investigate how this regressed on a hotfix release. @itsjustkevin Were there any Dart CPs in 3.7.1? |
zanderso
added
P2
and removed
P2
|
Most likely caused by dart-lang/sdk#51130 |
|
dart-lang/sdk#51130 exposed a problem in RegExp evaluation where repeated optimizing compilation of generated RegExp__matcher function produces different instructions, different deoptids compared to unoptimized compilation. Actual issue with divergence of the paths is tied to canonicalization of CharacterRanges, which happens during first run, but after dependent instructions were generated for the first run. The canonicalization causes "quick check" to get enabled for the second run, while it was disabled for the first run(https://github.com/dart-lang/sdk/blob/main/runtime/vm/regexp.cc#L1831). Hopefully will get more clarity on this soon. |
|
We should make sure the flow graph building is stable even if we can do some optimizations and cut some fragments out of a flow graph. In order to achieve that we usually build fragments of flow graph anyway, even if they are not needed, and then skip linking those fragments into the graph (so they are hanging independently and unused). This way we allocate the same deopt ids regardless of the decision to include or exclude these fragments. This is more easily doable if flow graph building uses |
|
https://dart-review.googlesource.com/c/sdk/+/286021 with proposed fix. @alexmarkov the issue here is not really difference between unoptimized vs optimized pipelines, rather that the repeated RegEx compilation sees different RegExp AST. Canonicalization of CharacterRanges happens in the middle of the first compilation run, but after some instructions that depended on not-yet-canonicalized ranges were generated(QuickCheck). Second run starts with the ranges all canonicalized, so decision to generate QuickCheck is taken differently. |
copybara-service bot
pushed a commit
to dart-lang/sdk
that referenced
this issue
Feb 28, 2023
… decision is made. First, canonicalized CharacterRanges allow for QuickCheck code to be generated, rather than skipped. Second, CharacterRanges could become canonicalized later leading to incompatible instructions generated on a second compilation run, leading to a crash. Fixes flutter/flutter#121270 TEST=flutter_regress_121270_test Change-Id: I075986b86b810ede96471bbe5fbe0b337714d215 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/286021 Commit-Queue: Alexander Aprelev <aam@google.com> Reviewed-by: Ryan Macnak <rmacnak@google.com>
|
@aam (the correct one for once) I see https://dart-review.googlesource.com/c/sdk/+/286021 is merged. If this works, could you open a cherry-pick to Dart stable? |
|
@itsjustkevin sure, will do |
|
@vsaase thanks again for reporting the problem, @danagbemava-nc - thanks for reproducing it. The fix landed on flutter main branch(e340d86), if you can verify it works for you, that would help. |
|
Hi @aam, the crash no longer reproduces on my Hi @vsaase, can you confirm if this is the case for you as well? master flutter doctor -v |
danagbemava-nc
added
the
waiting for customer response
|
@aam thank you for the fix! |
github-actions
bot
removed
the
waiting for customer response
|
Thanks for the confirmation. Closing this as fixed per our triage policy |
danagbemava-nc
added
the
r: fixed
copybara-service bot
pushed a commit
to dart-lang/sdk
that referenced
this issue
Mar 6, 2023
…uickCheck decision is made. First, canonicalized CharacterRanges allow for QuickCheck code to be generated, rather than skipped. Second, CharacterRanges could become canonicalized later leading to incompatible instructions generated on a second compilation run, leading to a crash. Bug=flutter/flutter#121270 TEST=flutter_regress_121270_test Fixes #51606 Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/286021 Change-Id: Ibf8e94da3703b8935d9bb55140f77170f3a8f411 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/286520 Commit-Queue: Alexander Aprelev <aam@google.com> Reviewed-by: Siva Annamalai <asiva@google.com>
|
This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of |
flutter-triage-bot
bot
added
P0
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Steps to Reproduce
I encountered this crash after upgrading from 3.7.0 (no crash there)
Since 3.7.1 all stable and beta versions are affected.
Targets macOS and Chrome and iOS Simulator are not affected.
There is no crash when evaluating the RegExp only once.
flutter runon the code sample , running on an iOS deviceExpected results: see the print outputs
Actual results: see the print outputs and then a crash
Code
Logs
The text was updated successfully, but these errors were encountered: