Hit MOZ_CRASH(There should be room for registering the new object) at js/src/vm/TypeInference.cpp:3494
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr68 | --- | unaffected |
firefox71 | --- | unaffected |
firefox72 | --- | fixed |
firefox73 | --- | fixed |
People
(Reporter: gkw, Assigned: cfallin)
References
(Regression)
Details
(4 keywords, Whiteboard: [fuzzblocker][jsbugmon:update])
Crash Data
Attachments
(3 files)
6.81 KB,
text/plain
|
Details | |
2.57 KB,
text/plain
|
Details | |
47 bytes,
text/x-phabricator-request
|
jcristau
:
approval-mozilla-beta+
|
Details | Review |
The following testcase crashes on mozilla-central revision d1001fea6e4c (build with --enable-debug --disable-optimize, run with --fuzzing-safe --no-threads --no-baseline --no-ion --no-ggc):
for (let y of [
{ x: 1 },
{ x: 2 },
{ x: 3 },
{ x: 4 },
{ x: 5 },
{ x: 6 },
{ x: 7 },
{ x: 8 },
{ x: 9 },
{ x: 10 },
{ x: 11 },
]) {}
Backtrace:
#0 0x0000561deba5b0b1 in js::PreliminaryObjectArray::registerNewObject (this=0x7fa67da1fdf0, res=0x2de229e7c580) at js/src/vm/TypeInference.cpp:3494
#1 0x0000561deb3b0f0e in js::NewObjectOperation (cx=0x7fa67dc27000, script=..., pc=0x7fa67d4418c4 "\362\v", newKind=js::GenericObject) at js/src/vm/Interpreter.cpp:5266
#2 0x0000561deb3a0325 in Interpret (cx=0x7fa67dc27000, state=...) at js/src/vm/Interpreter.cpp:3801
#3 0x0000561deb38dffa in js::RunScript (cx=0x7fa67dc27000, state=...) at js/src/vm/Interpreter.cpp:424
#4 0x0000561deb3a8939 in js::ExecuteKernel (cx=0x7fa67dc27000, script=..., envChainArg=..., newTargetValue=..., evalInFrame=..., result=0x0) at js/src/vm/Interpreter.cpp:811
#5 0x0000561deb3a8d0d in js::Execute (cx=0x7fa67dc27000, script=..., envChainArg=..., rval=0x0) at js/src/vm/Interpreter.cpp:844
#6 0x0000561deb65bbd3 in ExecuteScript (cx=0x7fa67dc27000, scope=..., script=..., rval=0x0) at js/src/vm/CompilationAndEvaluation.cpp:453
/snip
For detailed crash information, see attachment.
Setting s-s as a start because opt ASan builds show an error too, log coming up.
Reporter | ||
Comment 1•5 years ago
|
||
Reporter | ||
Comment 2•5 years ago
|
||
Reporter | ||
Comment 3•5 years ago
|
||
Setting [fuzzblocker] as this is happening very often.
Reporter | ||
Comment 4•5 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/7bd0784e7a6f
user: Chris Fallin
date: Wed Nov 27 22:49:33 2019 +0000
summary: Bug 1598347, part 2: pass "inner singleton" to NEWOBJECT group logic. r=djvj,iain
Chris, is bug 1598347 a likely regressor?
Reporter | ||
Comment 5•5 years ago
|
||
Pernosco link:
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 6•5 years ago
|
||
Yes, this is almost certainly due to the recent change for bug 1598347. Looking into it!
Assignee | ||
Comment 7•5 years ago
|
||
The recent addition of the JSOP_NEWOBJECT_WITHGROUP opcode for bug
1598347 (itself a regression fix for 1580246) has led to an issue when
more than a certain number of array elements with the same group are
created within an array literal. In particular, when too many objects
are created, the preliminary-objects-set for the ObjectGroup becomes
full and hits a MOZ_CRASH. This patch checks whether the set is full
before adding to it.
Comment 8•5 years ago
|
||
Is this a security issue, or is this just going to be a MOZ_CRASH() any time we get in this bad state? Thanks.
Assignee | ||
Comment 9•5 years ago
|
||
This is just a MOZ_CRASH, not a security issue -- the PreliminaryObjectArray
will not overflow its bounds. (See https://searchfox.org/mozilla-central/source/js/src/vm/TypeInference.cpp#3481 for its implementation.) The commit that caused this issue had changed some of the conditions under which objects are added to this array, or which array they are added to, hence the possibility of trying to add to an already-full array. (It's perfectly fine to not include objects, as they only improve analysis precision, so the fix is just to check if full first and do nothing.)
Updated•5 years ago
|
Comment 10•5 years ago
|
||
Comment 11•5 years ago
|
||
bugherder |
Assignee | ||
Comment 12•5 years ago
|
||
Comment on attachment 9113328 [details]
Bug 1601074: Fix preliminary-objects-set issue wrt NEWOBJECT_WITHGROUP. r=iain,djvj
Beta/Release Uplift Approval Request
- User impact if declined: Slight regression in performance (fewer JS functions are inlined) in some cases.
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: Bug 1598347
- Risk to taking this patch: Medium
- Why is the change risky/not risky? (and alternatives if risky): (This is meant to go along with 1598347, so is the same text reproduced below:)
Changes to the guts of the JS engine are always slightly risky, but this has been on m-c for a few days, and the one issue that arose (1601074) was caught quickly by the fuzzing team and fixed. Given that no further issues have arisen, and given that the engine is well-covered by tests, we believe that this risk is not unacceptably high.
- String changes made/needed:
Updated•5 years ago
|
Updated•5 years ago
|
Comment 14•5 years ago
|
||
Comment on attachment 9113328 [details]
Bug 1601074: Fix preliminary-objects-set issue wrt NEWOBJECT_WITHGROUP. r=iain,djvj
companion fix for bug 1598347, approved for 72.0b4
Comment 15•5 years ago
|
||
bugherder uplift |
Description
•