Last Comment Bug 757199 - Assertion failure: [barrier verifier] Unmarked edge: shape, at jsgc.cpp:4443
: Assertion failure: [barrier verifier] Unmarked edge: shape, at jsgc.cpp:4443
Status: RESOLVED FIXED
[js:p1:fx16]
: assertion, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Linux
: -- critical (vote)
: mozilla16
Assigned To: Bill McCloskey (:billm)
:
Mentors:
Depends on:
Blocks: langfuzz
  Show dependency treegraph
 
Reported: 2012-05-21 14:00 PDT by Christian Holler (:decoder)
Modified: 2013-01-14 08:35 PST (History)
5 users (show)
choller: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Test case for shell (run with -n -m -a) (2.05 KB, application/javascript)
2012-05-21 14:00 PDT, Christian Holler (:decoder)
no flags Details
fix (1.78 KB, patch)
2012-06-12 16:03 PDT, Bill McCloskey (:billm)
no flags Details | Diff | Splinter Review
patch v2 (1.68 KB, patch)
2012-06-15 16:18 PDT, Bill McCloskey (:billm)
no flags Details | Diff | Splinter Review
patch v3 (1.63 KB, patch)
2012-06-22 14:02 PDT, Bill McCloskey (:billm)
bhackett1024: review+
Details | Diff | Splinter Review

Description Christian Holler (:decoder) 2012-05-21 14:00:12 PDT
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.
Comment 1 Christian Holler (:decoder) 2012-06-01 07:03:13 PDT
Still reproduces on rev 73783bf75c4c, with the new verifier assertion format now.
Comment 2 Bill McCloskey (:billm) 2012-06-12 16:03:27 PDT
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.
Comment 3 Brian Hackett (:bhackett) 2012-06-14 15:11:28 PDT
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.
Comment 4 Bill McCloskey (:billm) 2012-06-15 16:18:58 PDT
Created attachment 633717 [details] [diff] [review]
patch v2

Is this what you mean?
Comment 5 Brian Hackett (:bhackett) 2012-06-18 08:12:33 PDT
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.
Comment 6 David Mandelin [:dmandelin] 2012-06-19 17:42:08 PDT
Can you guys put a security rating on this?
Comment 7 Bill McCloskey (:billm) 2012-06-22 14:02:38 PDT
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.
Comment 8 Gary Kwong [:gkw] [:nth10sd] 2012-06-26 17:50:31 PDT
Helped land this since this was bothering the fuzzers:

http://hg.mozilla.org/integration/mozilla-inbound/rev/431fa10c63a6
Comment 10 Gary Kwong [:gkw] [:nth10sd] 2012-06-26 21:26:03 PDT
Orange seems to related to verifybarriers not being defined?
Comment 12 Ed Morley [:emorley] 2012-06-28 01:09:06 PDT
https://hg.mozilla.org/mozilla-central/rev/d150934320be
Comment 13 Christian Holler (:decoder) 2013-01-14 08:35:23 PST
A testcase for this bug was automatically identified at js/src/jit-test/tests/basic/bug757199.js.

Note You need to log in before you can comment on or make changes to this bug.