951346 - Assertion failure: obj->lastProperty() == p->value().shape, at jsinfer.cpp:2545

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision b980c2dee2e7 (run with --fuzzing-safe):


setObjectMetadataCallback(function(obj) {});
eval(uneval({'-1':true}));

Comment 1

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 2

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/bec71542c055
user:        Brian Hackett
date:        Sat Dec 14 16:29:43 2013 -0800
summary:     Bug 950118 - Don't allow the object metadata hook to reenter JS, r=jimb.

This iteration took 1.373 seconds to run.

Comment 3

[Brian Hackett [Laid off!]]

Attachment: patch

It should be fine to just skip fixObjectType (a TI optimization to improve type information) for objects with metadata.

Comment 4

[Jim Blandy :jimb]

We'd like to attach metadata to pretty much all objects, when the heap analysis tools are on. Is it hard to preserve the optimization?

Comment 5

[Christian Holler (:decoder)]

Attachment: [crash-signature] Machine-readable crash signature

Comment 6

[Brian Hackett [Laid off!]]

(In reply to Jim Blandy :jimb from comment #4)
> We'd like to attach metadata to pretty much all objects, when the heap
> analysis tools are on. Is it hard to preserve the optimization?

The metadata could be added to the lookup/key part of the table, which would work but would be pretty complicated to get right with generational GC --- if metadata objects are in the nursery (which would be nice to permit, but could be restricted I guess with some new JSAPI for the callback to use) then table entries referring to nursery objects would need to be rekeyed on minor GCs.  We do stuff like this for the new object type table (NewTypeObjectsSetRef), but it's pretty complicated and not well encapsulated and would be good to avoid for tables when possible.

Disabling the optimization won't affect performance that much and won't affect the object graph at all, and since we already inhibit optimizations when the metadata callback is on this simple option doesn't seem too out of character.

Comment 7

[Jim Blandy :jimb]

Comment on attachment 8350369 [details] [diff] [review]
patch

Review of attachment 8350369 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me --- but please add a brief comment explaining that we want to avoid including metadata objects in the key. (These chains of excluded cases get pretty obscure pretty fast...)

Comment 8

[Christian Holler (:decoder)]

Brian, can we land this? Thanks!

Comment 9

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/acfa8a65c4db

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/acfa8a65c4db