1055818 - Make findReferences-02.js not perma-fail

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jeff Walden [:Waldo]]

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 <[e-mail]>
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.

Comment 1

[Jeff Walden [:Waldo]]

Attachment: Adjust paths to work again, add better slot names for a bunch of things for readability

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.

Comment 2

[Luke Wagner [: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.

Comment 3

[Jeff Walden [:Waldo]]

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

Comment 4

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

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