Closed
Bug 1233331
Opened 5 years ago
Closed 5 years ago
Crash [@ js::jit::IonScript::incrementInvalidationCount]
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: gkw, Assigned: nbp)
Details
(Keywords: crash, regression, testcase, Whiteboard: [jsbugmon:update])
Crash Data
Attachments
(2 files)
2.05 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
CodeGenerator: Prepare the invalidation of the recompileInfo as soon as the contraints are recorded.
6.25 KB,
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 0babaa3edcf9 (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --ion-eager -D): // jsfunfuzz-generated x = 0; try { a; b; } catch (e) {} // Adapted from randomly chosen test: js/src/jit-test/tests/gc/oomInDebugger.js var g = newGlobal(); oomTest(function() { return Debugger(g); }); // Adapted from randomly chosen test: js/src/tests/ecma_6/TemplateStrings/templLit.js eval("function g() {}"); Backtrace: #0 js::jit::IonScript::incrementInvalidationCount (this=<optimized out>) at js/src/jit/IonCode.h:542 #1 js::jit::Invalidate (types=..., fop=fop@entry=0x7fe230a4cae8, invalid=..., resetUses=resetUses@entry=true, cancelOffThread=cancelOffThread@entry=true) at js/src/jit/Ion.cpp:3068 #2 0x0000000000b3f467 in js::TypeZone::processPendingRecompiles (this=0x7fe22f78b630, fop=0x7fe230a4cae8, recompiles=...) at js/src/vm/TypeInference.cpp:2489 #3 0x00000000006ae60b in js::AutoEnterAnalysis::~AutoEnterAnalysis (this=0x7ffe1efeadd0, __in_chrg=<optimized out>) at js/src/vm/TypeInference-inl.h:309 #4 0x0000000000b51494 in js::AddTypePropertyId (cx=cx@entry=0x7fe230a18c00, group=0x7fe22f9602b0, obj=<optimized out>, id=..., type=...) at js/src/vm/TypeInference.cpp:2730 #5 0x0000000000b516d8 in js::AddTypePropertyId (cx=cx@entry=0x7fe230a18c00, group=<optimized out>, obj=obj@entry=0x7fe22f963060, id=..., id@entry=..., value=...) at js/src/vm/TypeInference.cpp:2773 #6 0x0000000000a83d2c in js::AddTypePropertyId (value=..., id=..., obj=0x7fe22f963060, cx=0x7fe230a18c00) at js/src/vm/TypeInference-inl.h:421 #7 js::NativeObject::setSlotWithType (overwriting=<optimized out>, value=..., shape=<optimized out>, cx=0x7fe230a18c00, this=<optimized out>) at js/src/vm/NativeObject-inl.h:286 #8 NativeSetExistingDataProperty (cx=cx@entry=0x7fe230a18c00, obj=..., obj@entry=..., shape=..., shape@entry=..., v=v@entry=..., result=..., receiver=...) at js/src/vm/NativeObject.cpp:2031 #9 0x0000000000a9b7b7 in SetExistingProperty (result=..., shape=..., pobj=..., receiver=..., v=..., id=..., obj=..., cx=0x7fe230a18c00) at js/src/vm/NativeObject.cpp:2255 #10 js::NativeSetProperty (cx=cx@entry=0x7fe230a18c00, obj=..., obj@entry=..., id=id@entry=..., value=..., receiver=..., qualified=qualified@entry=js::Qualified, result=...) at js/src/vm/NativeObject.cpp:2316 #11 0x00000000005e6a94 in js::SetProperty (result=..., receiver=..., v=..., id=..., obj=..., cx=0x7fe230a18c00) at js/src/vm/NativeObject.h:1485 #12 js::PutProperty (cx=0x7fe230a18c00, obj=..., id=..., v=..., strict=<optimized out>) at js/src/jsobj.h:935 #13 0x0000000000a9c2cc in js::DefFunOperation (cx=0x7fe230a18c00, script=..., scopeChain=..., funArg=...) at js/src/vm/Interpreter.cpp:4179 #14 0x00007fe23219ea39 in ?? () #15 0x00007ffe1efeb328 in ?? () #16 0x00007ffe1efeb2a8 in ?? () #17 0x00007ffe1efec780 in ?? () #18 0x0000000001b7a6a0 in InitPropGetterSetterInfo () #19 0x00007fe22f959880 in ?? () #20 0x00007fe231ffdafd in ?? () #21 0x0000000000000d01 in ?? () #22 0x00007fe22f9a0230 in ?? () #23 0x00007fe22f964070 in ?? () #24 0x00007fe22f9813c0 in ?? () #25 0x00007ffe1efeb328 in ?? () #26 0x0000000000000008 in ?? () #27 0x0000000000000007 in ?? () #28 0x0000000000000048 in ?? () #29 0x00007fe22f964070 in ?? () #30 0x00007fe22f9a0230 in ?? () #31 0x0000000000000000 in ?? ()
![]() |
Reporter | |
Comment 1•5 years ago
|
||
=== Treeherder Build Bisection Results by autoBisect === The "good" changeset has the timestamp "20151125161010" and the hash "bc9a43cbbdfad05a533c43dcecb59527e3b22b01". The "bad" changeset has the timestamp "20151125161413" and the hash "91c196b60306403eeb870ac65c9c81fe793e07e9". Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=bc9a43cbbdfad05a533c43dcecb59527e3b22b01&tochange=91c196b60306403eeb870ac65c9c81fe793e07e9 which points to bug 1226027, but it does not seem to be the regressor. I'll use the tips in: https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/Hacking_Tips#How_to_debug_oomTest%28%29_failures and see what I can find.
![]() |
Reporter | |
Comment 2•5 years ago
|
||
Running with OOM_VERBOSE=1, ReportOutOfMemory called allocation 517 ReportOutOfMemory called allocation 518 ReportOutOfMemory called ReportOutOfMemory called allocation 519 allocation 520 finished after 519 allocations thread 2 allocation 1 finished after 0 allocations thread 3 allocation 1 finished after 0 allocations thread 4 allocation 1 finished after 0 allocations thread 5 allocation 1 finished after 0 allocations thread 6 allocation 1 finished after 0 allocations thread 7 allocation 1 finished after 0 allocations Segmentation fault (core dumped) So I'm not sure which allocation to break at, as they all seem to be "finished", so setting needinfo? from Jon, who might know the best way to proceed.
Flags: needinfo?(jcoppeard)
Comment 3•5 years ago
|
||
It looks like the oomTest() part completes OK. The crash happens executing the last line of the testcase. The crash happens in js::jit::Invalidate(): 3065 // Keep the ion script alive during the invalidation and flag this 3066 // ionScript as being invalidated. This increment is removed by the 3067 // loop after the calls to InvalidateActivation. 3068 co->ion()->incrementInvalidationCount(); 3069 numInvalidations++; The problem is that co->ion() == ION_DISABLED_SCRIPT here. I couldn't see any obvious culprits with blame. Jan, any idea?
Flags: needinfo?(jcoppeard) → needinfo?(jdemooij)
Comment 4•5 years ago
|
||
I can't reproduce this with the revision, configure flags, shell flags in comment 0. Do you know if this is Linux-only?
Flags: needinfo?(gary)
![]() |
Reporter | |
Comment 5•5 years ago
|
||
Yes, this seems to be Linux-only. I reproduced this issue on m-c rev 9ddf0da90fb3 on Linux but not on Mac.
Flags: needinfo?(gary)
Comment 6•5 years ago
|
||
(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #5) > Yes, this seems to be Linux-only. I reproduced this issue on m-c rev > 9ddf0da90fb3 on Linux but not on Mac. Thanks. I'll forward this to nbp, maybe he can reproduce this.
Flags: needinfo?(jdemooij) → needinfo?(nicolas.b.pierron)
Assignee | ||
Comment 7•5 years ago
|
||
I am able to reproduce this issue, I will investigate more on Monday.
Assignee | ||
Comment 8•5 years ago
|
||
The problem is located inside CodeGenerator::link and comes from the fact that discardIonCode is not immediately created after FinishCompilation. Thus when the OOMTest fucntion cause a failure under encodeSafepoints function, we have not yet invalidated the RecompileInfo structure. I suggest that we move the FinishCompilation function call to a method call of AutoDiscardIonCode, such that this kind of error cannot happen in the future.
Assignee | ||
Comment 9•5 years ago
|
||
Attachment #8704620 -
Flags: review?(jdemooij)
Assignee | ||
Comment 10•5 years ago
|
||
This patch replace AutoDiscardIonCode by 2 uses of MakeScopeExit. I first tried to merge both into AutoDiscardIonCode, and failed to come-up with a proper name for AutoDiscardIonCode, as it does not discard any IonCode (directly). As there is only one use AutoDiscardIonCode, using MakeScopeExit instead should be easier to understand, and relies on RAII the same way to execute the lambda given as argument unless released.
Attachment #8704622 -
Flags: review?(jdemooij)
Updated•5 years ago
|
Attachment #8704620 -
Flags: review?(jdemooij) → review+
Comment 11•5 years ago
|
||
Comment on attachment 8704622 [details] [diff] [review] CodeGenerator: Prepare the invalidation of the recompileInfo as soon as the contraints are recorded. Review of attachment 8704622 [details] [diff] [review]: ----------------------------------------------------------------- Nice. ::: js/src/jit/CodeGenerator.cpp @@ +8084,5 @@ > // will trickle to jit::Compile() and return Method_Skipped. > uint32_t warmUpCount = script->getWarmUpCount(); > + > + // Record constraints. If an error occured, returns false and potentially > + // prevent future compilations. Otherwise, if the an invalidation occurs, Nit: s/the an/an/
Attachment #8704622 -
Flags: review?(jdemooij) → review+
Comment 12•5 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2dce0db8de74 https://hg.mozilla.org/integration/mozilla-inbound/rev/8dcf131a146d
Comment 14•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/2dce0db8de74 https://hg.mozilla.org/mozilla-central/rev/8dcf131a146d https://hg.mozilla.org/mozilla-central/rev/182c36f37bf2
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
![]() |
Reporter | |
Updated•5 years ago
|
Assignee: nobody → nicolas.b.pierron
Flags: needinfo?(nicolas.b.pierron)
You need to log in
before you can comment on or make changes to this bug.
Description
•