Closed
Bug 1498980
Opened 2 years ago
Closed 2 years ago
Crash [@ js::gc::IsAboutToBeFinalizedInternal] or Assertion failure: false (IsAboutToBeFinalized(&scope_)), at js/src/vm/EnvironmentObject.cpp:1499
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
VERIFIED
FIXED
mozilla64
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox62 | --- | unaffected |
firefox63 | --- | unaffected |
firefox64 | --- | verified |
firefox65 | --- | verified |
People
(Reporter: gkw, Assigned: jonco)
References
Details
(6 keywords, Whiteboard: [jsbugmon:update][post-critsmash-triage])
Attachments
(2 files, 1 obsolete file)
12.00 KB,
text/plain
|
Details | |
6.50 KB,
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 3aca49b2df24 (build with --enable-debug, run with --fuzzing-safe --no-threads --no-baseline --no-ion): // Adapted from randomly chosen test: js/src/jit-test/tests/debug/bug1304553.js dbgGlobal = newGlobal(); dbg = new dbgGlobal.Debugger; dbg.addDebuggee(this); function f() { dbg.getNewestFrame().older.eval(""); } m = parseModule("f();"); m.declarationInstantiation(); m.evaluation(); // jsfunfuzz-generated gc(); Backtrace: #0 0x000055fac1b2d32a in js::LiveEnvironmentVal::needsSweep (this=<optimized out>) at js/src/vm/EnvironmentObject.cpp:1499 #1 JS::StructGCPolicy<js::LiveEnvironmentVal>::needsSweep (tp=<optimized out>) at /home/ubuntu/shell-cache/js-dbg-64-linux-3aca49b2df24/objdir-js/dist/include/js/GCPolicyAPI.h:84 #2 JS::DefaultMapSweepPolicy<js::ReadBarriered<JSObject*>, js::LiveEnvironmentVal>::needsSweep (key=0x7ffa071e3c68, value=<optimized out>) at /home/ubuntu/shell-cache/js-dbg-64-linux-3aca49b2df24/objdir-js/dist/include/js/GCHashTable.h:24 #3 JS::GCHashMap<js::ReadBarriered<JSObject*>, js::LiveEnvironmentVal, js::MovableCellHasher<js::ReadBarriered<JSObject*> >, js::ZoneAllocPolicy, JS::DefaultMapSweepPolicy<js::ReadBarriered<JSObject*>, js::LiveEnvironmentVal> >::sweep (this=<optimized out>) at /home/ubuntu/shell-cache/js-dbg-64-linux-3aca49b2df24/objdir-js/dist/include/js/GCHashTable.h:80 #4 0x000055fac1b17782 in js::DebugEnvironments::sweep (this=<optimized out>) at js/src/vm/EnvironmentObject.cpp:2643 /snip For detailed crash information, see attachment. Setting s-s because gc is involved, as a start.
![]() |
Reporter | |
Comment 1•2 years ago
|
||
![]() |
Reporter | |
Comment 2•2 years ago
|
||
autobisectjs shows this is probably related to the following changeset: The first bad revision is: changeset: https://hg.mozilla.org/mozilla-central/rev/f8b19c4105d2 user: Jon Coppeard date: Thu Oct 11 18:33:57 2018 +0100 summary: Bug 1489477 - Stop modules from entraining the top-level JSScript r=sfink Jon, is bug 1489477 a likely regressor? (Also variants crash [@ js::gc::IsAboutToBeFinalizedInternal] )
Blocks: 1489477
Flags: needinfo?(jcoppeard)
Summary: Assertion failure: false (IsAboutToBeFinalized(&scope_)), at js/src/vm/EnvironmentObject.cpp:1499 → Crash [@ js::gc::IsAboutToBeFinalizedInternal] or Assertion failure: false (IsAboutToBeFinalized(&scope_)), at js/src/vm/EnvironmentObject.cpp:1499
Comment 3•2 years ago
|
||
Debugger GC issues are unlikely to be s-s and DebugEnvironments is on the stack.
Assignee | ||
Comment 4•2 years ago
|
||
s-s but requires use of debugger to trigger the problem.
Assignee: nobody → jcoppeard
Flags: needinfo?(jcoppeard)
Updated•2 years ago
|
Keywords: sec-moderate
Assignee | ||
Comment 5•2 years ago
|
||
The problem is that module environments are currently not removed from DebugEnvironments::liveEnvs map. The patch is a minimal fix to do this explicitly after executing the body of a module. There's another problem though that we don't take a snapshot of the environment if we have a debug environment proxy for the module environment. I think this explains bug 1283534.
Attachment #9017217 -
Flags: review?(jorendorff)
Comment 6•2 years ago
|
||
Comment on attachment 9017217 [details] [diff] [review] bug1498980-live-env-sweep Review of attachment 9017217 [details] [diff] [review]: ----------------------------------------------------------------- Clearing review for now. ::: js/src/builtin/ModuleObject.cpp @@ +1171,5 @@ > JS_ReportErrorASCII(cx, "Module declarations have not yet been instantiated"); > return false; > } > > + bool result = Execute(cx, script, *scope, rval.address()); Please try fixing it here instead: https://searchfox.org/mozilla-central/rev/c9272ef398954288525e37196eada1e5a93d93bf/js/src/vm/Interpreter.cpp#1242-1243 It took me a while to find that, but I was suspicious immediately, because by the time Execute returns, the frame has already been popped. We have already left the scope. For modules, a typical interpreter call stack where this happens should (I think) be like: js::Execute js::ExecuteKernel js::RunScript js::Interpret <https://searchfox.org/mozilla-central/rev/c9272ef398954288525e37196eada1e5a93d93bf/js/src/vm/Interpreter.cpp#4826> js::InterpreterFrame::epilogue js::UnwindAllEnvironmentsInFrame PopEnvironment ::: js/src/jit-test/tests/modules/bug-1498980.js @@ +5,5 @@ > + dbg.getNewestFrame().older.eval(""); > +} > +m = parseModule("f();"); > +m.declarationInstantiation(); > +m.evaluation(); Great, thanks! Please also test when the module throws.
Attachment #9017217 -
Flags: review?(jorendorff)
Assignee | ||
Comment 7•2 years ago
|
||
Updated patch.
Attachment #9017217 -
Attachment is obsolete: true
Attachment #9017490 -
Flags: review?(jorendorff)
Updated•2 years ago
|
Attachment #9017490 -
Flags: review?(jorendorff) → review+
Assignee | ||
Comment 8•2 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/6b952be63f69e244ab688cdb7a84d121b009a162
https://hg.mozilla.org/mozilla-central/rev/6b952be63f69
Group: javascript-core-security → core-security-release
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Updated•2 years ago
|
status-firefox62:
--- → unaffected
status-firefox63:
--- → unaffected
status-firefox-esr60:
--- → unaffected
Flags: in-testsuite+
Updated•2 years ago
|
Flags: qe-verify-
Whiteboard: [jsbugmon:update] → [jsbugmon:update][post-critsmash-triage]
Updated•2 years ago
|
Status: RESOLVED → VERIFIED
status-firefox65:
--- → verified
Comment 10•2 years ago
|
||
JSBugMon: This bug has been automatically verified fixed.
Comment 11•2 years ago
|
||
JSBugMon: This bug has been automatically verified fixed on Fx64
Updated•1 year ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•