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
: assertion, testcase
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Linux
-- critical (vote)
: mozilla16
Assigned To: Bill McCloskey (:billm)
: Jason Orendorff [:jorendorff]
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:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

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 User image 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 User image Christian Holler (:decoder) 2012-06-01 07:03:13 PDT
Still reproduces on rev 73783bf75c4c, with the new verifier assertion format now.
Comment 2 User image Bill McCloskey (:billm) 2012-06-12 16:03:27 PDT
Created attachment 632453 [details] [diff] [review]

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 User image Brian Hackett (:bhackett) 2012-06-14 15:11:28 PDT
Comment on attachment 632453 [details] [diff] [review]

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 User image 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 User image 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 User image David Mandelin [:dmandelin] 2012-06-19 17:42:08 PDT
Can you guys put a security rating on this?
Comment 7 User image 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 User image Gary Kwong [:gkw] [:nth10sd] 2012-06-26 17:50:31 PDT
Helped land this since this was bothering the fuzzers:
Comment 10 User image Gary Kwong [:gkw] [:nth10sd] 2012-06-26 21:26:03 PDT
Orange seems to related to verifybarriers not being defined?
Comment 12 User image Ed Morley [:emorley] 2012-06-28 01:09:06 PDT
Comment 13 User image 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.