Assertion failure: [barrier verifier] Unmarked edge: shape, at jsgc.cpp:4443

RESOLVED FIXED in mozilla16

Status

()

Core
JavaScript Engine
--
critical
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: decoder, Assigned: billm)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla16
x86
Linux
assertion, testcase
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [js:p1:fx16])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
Created attachment 625755 [details]
Test case for shell (run with -n -m -a)

The attached test asserts on mozilla-central revision d55df2c9c037 (options -m -n -a). S-s due to GC-related assert with possible security impact.
(Assignee)

Updated

5 years ago
Assignee: general → wmccloskey
(Reporter)

Comment 1

5 years ago
Still reproduces on rev 73783bf75c4c, with the new verifier assertion format now.
Summary: Assertion failure: IsMarkedOrAllocated(static_cast<Cell *>(thing)), at jsgc.cpp:4406 → Assertion failure: [barrier verifier] Unmarked edge: shape, at jsgc.cpp:4443
(Assignee)

Comment 2

5 years ago
Created attachment 632453 [details] [diff] [review]
fix

This is a pretty fundamental problem, although unlikely to hurt us in practice. The barrier code for JSOP_SETPROP assumes that only the slot being updated needs a write barrier. However, when we add new properties, we're also changing the object's shape. So that also needs a barrier.

In theory we could call out to a little stub routine that marks the shape. However, I don't think that's worth it given how close IonMonkey is. This patch just takes the slow path if we need to add a property and if incremental GC is running.
Attachment #632453 - Flags: review?(bhackett1024)
Comment on attachment 632453 [details] [diff] [review]
fix

This should be done by disabling the cache if doing an addprop with barriers on, rather than generating stubs which are guaranteed to fail.  Otherwise we can keep hitting the same ADDPROP case in the slow path, and chain together a bunch of auto-failing stubs.  Look for the 'generateStub(initialShape, shape, true)' call for places where the cache can be disabled in the ADDPROP case.
Attachment #632453 - Flags: review?(bhackett1024)
(Assignee)

Comment 4

5 years ago
Created attachment 633717 [details] [diff] [review]
patch v2

Is this what you mean?
Attachment #632453 - Attachment is obsolete: true
Attachment #633717 - Flags: review?(bhackett1024)
Almost, the cache is being disabled but that should happen before we start trying to generate a stub for it.  If you put this test in the update() method, near the test below:

            if (obj->numDynamicSlots() != slots)
                return disable("insufficient slot capacity");

That should work fine.
Attachment #633717 - Flags: review?(bhackett1024)
Can you guys put a security rating on this?
Whiteboard: js-triage-needed → [js:p1:fx16]
(Assignee)

Comment 7

5 years ago
Created attachment 635898 [details] [diff] [review]
patch v3

I'm 99% certain that this is harmless. The danger would be if the old shape never got marked. I just don't see how that could happen, though, since the new shape's parent pointer should always point to the old one.
Attachment #633717 - Attachment is obsolete: true
Attachment #635898 - Flags: review?(bhackett1024)
(Assignee)

Updated

5 years ago
Group: core-security
Attachment #635898 - Flags: review?(bhackett1024) → review+
Helped land this since this was bothering the fuzzers:

http://hg.mozilla.org/integration/mozilla-inbound/rev/431fa10c63a6
Target Milestone: --- → mozilla16
Backed out for orange:

https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=431fa10c63a6

https://hg.mozilla.org/integration/mozilla-inbound/rev/77e2d6789e02
Target Milestone: mozilla16 → ---
Orange seems to related to verifybarriers not being defined?
(Assignee)

Comment 11

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/d150934320be
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/d150934320be
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Comment 13

4 years ago
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/bug757199.js.
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.