-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
crash on front-end-linux-release-x64-try during presubmit test run #47314
Comments
It looks like similar failure happened on front-end-mac-release-x64 ci bot resulting in purple bot
|
/cc @alexmarkov |
Tried running release, debug, TSAN and ASAN versions multiple times. Also tried running under |
The crash happens in the following code
when writing |
Happened once again on
|
This change adds debug prints for flaky crash in compiler which only appears on bots. TEST=ci Issue: #47314 Change-Id: I6653df11612d24e8a8e67e3e177484a70e308dcc Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/215152 Auto-Submit: Alexander Markov <alexmarkov@google.com> Commit-Queue: Siva Annamalai <asiva@google.com> Reviewed-by: Siva Annamalai <asiva@google.com>
Here's the analysis of such a crash: It crashes during unoptimized compilation of _Utf8Decoder. _Utf8Decoder(this.allowMalformed) : _state = initial; This unoptimized IL will have a LocationSummary* StoreInstanceFieldInstr::MakeLocationSummary(...) {
const intptr_t kNumInputs = 2;
const intptr_t kNumTemps = ... (IsPotentialUnboxedDartFieldStore() ? 3 : 0);
LocationSummary* summary = new (zone) LocationSummary(
zone, kNumInputs, kNumTemps,
(... || IsPotentialUnboxedDartFieldStore())
? LocationSummary::kCallOnSlowPath
: LocationSummary::kNoCall);
...
if (...) {
} else if (IsPotentialUnboxedDartFieldStore()) {
summary->set_in(kValuePos, ShouldEmitStoreBarrier()
? Location::WritableRegister()
: Location::RequiresRegister());
summary->set_temp(0, Location::RequiresRegister());
summary->set_temp(1, Location::RequiresRegister());
summary->set_temp(2, opt ? Location::RequiresFpuRegister()
: Location::FpuRegisterLocation(XMM1));
}
...
return summary;
} In the crash, the created Looking at bool StoreInstanceFieldInstr::IsPotentialUnboxedDartFieldStore() const {
return slot().representation() == kTagged && slot().IsDartField() &&
FlowGraphCompiler::IsPotentialUnboxedField(slot().field());
} and then bool FlowGraphCompiler::IsPotentialUnboxedField(const Field& field) {
if (FLAG_precompiled_mode) {
// kernel_loader.cc:ReadInferredType sets the guarded cid for fields based
// on inferred types from TFA (if available). The guarded cid is therefore
// proven to be correct.
return IsUnboxedField(field);
}
return field.is_unboxing_candidate() &&
(FlowGraphCompiler::IsUnboxedField(field) ||
(field.guarded_cid() == kIllegalCid));
} makes me believe the field is in This happens because we don't clone @aam Since you worked on the field guard support with |
@mkustermann cool analysis! How did you end up reproducing the failure?
Do you mean we could clone Field objects for unoptimized compilation and then reject compilation starting again? |
I've picked the commit where it happened last, run it the same way as the bot over night and it produced 3 core files in those 12 hours, one I looked at. |
This change replaces debug prints introduced in https://dart-review.googlesource.com/c/sdk/+/215152 with assertions. We no longer need the printed information because root cause of the flaky crashes is found. TEST=ci Issue: #47314 Change-Id: I2c37fa640bc64cf5295f8acd0fbfb2b63ec7cddf Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/215883 Auto-Submit: Alexander Markov <alexmarkov@google.com> Commit-Queue: Siva Annamalai <asiva@google.com> Reviewed-by: Siva Annamalai <asiva@google.com>
The crash also appears on one of the flaky tests (this could be easier to reproduce):
|
Fix in progress, will not make it for this milestone, moving it to next milestone. |
…guard information that is kept on a slot." This reverts commit fd31242 as the issue that caused revert should be fixed by removal of boxing heuristic removed in 8159c38. Fixes #47314 TEST=ci,g3 Change-Id: I42f3f0c6d25639fb326e74d96a51c4c6c0f88f78 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/221601 Reviewed-by: Ryan Macnak <rmacnak@google.com> Commit-Queue: Alexander Aprelev <aam@google.com>
from logs
The text was updated successfully, but these errors were encountered: