Closed Bug 1055818 Opened 6 years ago Closed 6 years ago

Make findReferences-02.js not perma-fail


(Core :: JavaScript Engine, defect)

Not set





(Reporter: Waldo, Assigned: Waldo)




(1 file)

findReferences-02.js is perma-fail right now.  I'd thought for awhile that it was just intermittent, but on closer look it's just completely broken.  Current output if you run it looks like this:

## js1_8_5/extensions/findReferences-02.js: rc = 3, run time = 0.275691
referent is not referenced via: "edge: fun_callscope; shape; base; parent"
but it is referenced via:       ["edge: shape; base; parent", "edge: fun_callscope; **UNKNOWN SLOT 0**"]
all incoming edges, from any object:
...much trimming here...

And it looks like that consistently, so it's not an intermittent thing.

Bisection took the longest time to even find when this ever worked, but eventually I got a good rev to bisect against a 25k-old bad revision.  (!)  That pointed at:

changeset 164525:363b31e32272: bad
The first bad revision is:
changeset:   164525:363b31e32272
user:        Luke Wagner <>
date:        Tue Jan 21 16:25:37 2014 -0600
summary:     Bug 961969 - Assert that all shapes in a lineage have the same numFixedSlots (r=billm)

which change is in the area but by that description sounds innocuous.  But if you look very closely we find this change in Bindings::initWithTemporaryStorage:

-        StackBaseShape base(cx, &CallObject::class_, cx->global(), nullptr,
-                            BaseShape::VAROBJ | BaseShape::DELEGATE);
-        UnownedBaseShape *nbase = BaseShape::getUnowned(cx, base);
-        if (!nbase)
+        StackBaseShape stackBase(cx, &CallObject::class_, nullptr, nullptr,
+                                 BaseShape::VAROBJ | BaseShape::DELEGATE);
+        UnownedBaseShape *base = BaseShape::getUnowned(cx, stackBase);
+        if (!base)
             return false;

Nestled in that is a wee little change to make BaseShapes (for bindings, presumably) have null parent instead of using the global object.  That would obviously cause the asserted path in the test, which runs through BaseShape, to not be found.  That breaks the desired path.
Blocks: 961969
You broke it, you get to review the patch to fix it.  :-P  Plus a bit more to give saner names to things.  I think "environment" is better than "callscope" given we have the field env_, the accessor environment(), and specs talk about "Environment Records".  The with-slot names are meh, but I've spent far too long on this bug already today.
Attachment #8475500 - Flags: review?(luke)
Comment on attachment 8475500 [details] [diff] [review]
Adjust paths to work again, add better slot names for a bunch of things for readability

Sorry about that.
Attachment #8475500 - Flags: review?(luke) → review+
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.