Closed Bug 1071001 Opened 5 years ago Closed 5 years ago

JS-shell-only function findReferences constructs result object incorrectly

Categories

(Core :: JavaScript Engine, defect, critical)

x86_64
Linux
defect
Not set
critical

Tracking

()

RESOLVED FIXED
Tracking Status
firefox32 --- ?
firefox33 --- affected
firefox34 --- affected
firefox35 --- affected

People

(Reporter: decoder, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, regression, testcase, Whiteboard: [jsbugmon:update,ignore])

Attachments

(1 file)

The following testcase asserts on mozilla-central revision 5e704397529b (run with --fuzzing-safe --thread-count=2 --ion-offthread-compile=off):


var p = Proxy.create({
    has : function(id) {
      return true ;
    },
    get : function(id) {
      var o = {0: 1, 1: 2, 3: 4, length: 4};
      return o;
    }
});
Object.prototype.__proto__ = p;
findReferences([]);
This could be shell-only but since findReferences is GC-related, I'm marking this s-s until investigated.
Whiteboard: [jsbugmon:update,bisect]
Whiteboard: [jsbugmon:update,bisect] → [jsbugmon:update]
JSBugMon: Bisection requested, result:
Due to skipped revisions, the first bad revision could be any of:
changeset:   https://hg.mozilla.org/mozilla-central/rev/a0dd5a83ba36
user:        Jan de Mooij
date:        Thu Jul 24 11:56:43 2014 +0200
summary:     Bug 1031529 part 2 - Remove JS_THREADSAFE #ifdefs everywhere. r=bhackett

changeset:   https://hg.mozilla.org/mozilla-central/rev/6426fef52f51
user:        Jan de Mooij
date:        Thu Jul 24 11:56:45 2014 +0200
summary:     Bug 1031529 part 3 - Step defining JS_THREADSAFE, remove --disable-threadsafe. r=glandium

This iteration took 120.154 seconds to run.
I'm sure :decoder would like me to dig more at the regressing changeset.
Flags: needinfo?(gary)
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/65617cb216fa
user:        Jim Blandy
date:        Wed Aug 03 20:19:38 2011 -0700
summary:     Bug 672736: Implement the 'findReferences' shell function. r=jorendorff

Jim, I guess this goes all the way back to the implementation of the "findReferences" shell function in bug 672736?
Flags: needinfo?(gary) → needinfo?(jimb)
To clarify the needinfo-- is this bug a problem only in the shell, or is it an underlying GC issue merely exposed in this instance by findReferences?
This is shell-only. It doesn't have any security implications, nor does it show a bug in the GC. It only shows a problem in my lousy code.

The code in ReferenceFinder::addReferrer assumes that it is the only thing creating properties on this->result, and hence since it only stores arrays as property values, then it should only find arrays as property values. Replacing Object.prototype.__proto__ with a proxy whose 'has' trap always returns true breaks these assumptions.
Flags: needinfo?(jimb)
Group: core-security
Summary: Assertion failure: JS_IsArrayObject(context, array), at shell/jsheaptools.cpp:523 → JS-shell-only function findReferences constructs result object incorrectly
Reading our policies, I see that the original reporter is usually the one to clear the S-S flag. Please let me know if I've caused difficulty.
No, that's fine :) But modifying the bug subject causes me trouble because I won't find the bug in my dashboard then. But for this particular bug it doesn't matter now since the attached signature covers it and it's shell-only.
(In reply to Christian Holler (:decoder) from comment #9)
> No, that's fine :) But modifying the bug subject causes me trouble because I
> won't find the bug in my dashboard then.

Okay. I'll avoid that in the future.
Whiteboard: [jsbugmon:update] → [jsbugmon:update,ignore]
JSBugMon: The testcase found in this bug no longer reproduces (tried revision 9dbb2d41bb2c).
Bug 1128603 removed findReferences.
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.