853573 - Assertion failure: !cx->compartment->activeAnalysis, at jsinterp.cpp:354

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision 1d6fe70c79c5 (no options required):


var p = Proxy.create({
  has : function(id) {}
});
Object.prototype.__proto__ = p;
  var pa0 = new ParallelArray(range(0, 256));
  var pa1 = new ParallelArray(256, function (x) {
    return pa0.map(function(y) {});
  });
function range(n, m) {
  var result = [];
  for (var i = n; i < m; i++)
    result.push(i);
  return result;
}

Comment 1

[Niko Matsakis [:nmatsakis]]

I reduced the test case to this:

var p = Proxy.create({
  has : function(id) {}
});
Object.prototype.__proto__ = p;

  var pa1 = new ParallelArray(4, function(x) {
    return new ParallelArray(4, function() 0);
  });

Comment 2

[Niko Matsakis [:nmatsakis]]

Attachment: Use defineProperty and not setProperty

When we create a (self-hosted) ParallelArray object, we always define the same four properties. We expect these properties to always be "own" properties, and there are offsets are baked into TI so that they are treated as definite properties. The assertion failure that we see here was due to the fact that these offsets were not matching what we expected. This is because we were using JSObject::setProperty() to create the properties, which is vulnerable to proxies and other strange behavior based on modifications to Object.prototype. This patch switches us to use JSObject::defineProperty() which I believe should avoid the issue.

Comment 3

[Till Schneidereit [:till]]

Comment on attachment 728891 [details] [diff] [review]
Use defineProperty and not setProperty

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

Definitely, using defineProperty is required for correctness, here.

Comment 4

[Niko Matsakis [:nmatsakis]]

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

Comment 5

[Christian Holler (:decoder)]

Marking s-s because this bug is actually a security problem. A very similar test with the same assertion tripped an asan error on the fuzzer.

Comment 6

[Christian Holler (:decoder)]

JSBugMon: The testcase found in this bug no longer reproduces (tried revision 8d09f003e087).

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

This landed on m-c:

http://hg.mozilla.org/mozilla-central/rev/f7138044b0cf

What branches (if any) are affected by this?

Comment 8

[Niko Matsakis [:nmatsakis]]

No branches are affected.

Comment 9

[Niko Matsakis [:nmatsakis]]

Add test:

https://hg.mozilla.org/integration/mozilla-inbound/rev/adb4ae9d355a
https://hg.mozilla.org/integration/mozilla-inbound/rev/598542288568

Comment 10

[Ryan VanderMeulen [:RyanVM]]

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