981650 - Assertion failure: args[4].isInt32(), at builtin/TypedObject.cpp:2774

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

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


var T = TypedObject;
var AT = new T.ArrayType(T.int32, 10);
var v = new AT(10);
for (var i=0 ; new AT(v) < 1000 ; i++)
  test(12);

Comment 1

[Christian Holler (:decoder)]

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

Comment 2

[Christian Holler (:decoder)]

I don't know if this is really s-s, it does not crash in optimized builds. But we had previous asserts of this type, that lead to s-s bugs and the assertion is on a "size" variable, so I am not sure what the consequences are (or can be).

Comment 3

[Christian Holler (:decoder)]

JSBugMon: Bisection requested, result:
=== Tinderbox Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20140220102129" and the hash "7a5cbe4dadf8".
The "bad" changeset has the timestamp "20140220102430" and the hash "cc73b1f7a47d".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=7a5cbe4dadf8&tochange=cc73b1f7a47d

Comment 4

[Niko Matsakis [:nmatsakis]]

Attachment: Bug981650.diff

When we decide whether to short-circuit a type and use Memcpy, we have to skip over variable-sized type descriptors. The old code was incorrectly checking for a property variable on the type repr. We should check on the type descriptor. This code path will go away after bug 966575 lands, and doubly go away after bug 973238 lands, but for now let's patch it up.

Comment 5

[Steve Fink [:sfink] [:s:]]

Comment on attachment 8392823 [details] [diff] [review]
Bug981650.diff

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

It'd be nice to have a "use brittle" mode for self-hosted (and other!) code that would throw if you looked up an unknown property on specially flagged classes.

::: js/src/builtin/TypedObject.js
@@ -459,1 @@
>  TypedObjectPointer.prototype.set = function(fromValue) {

Not really related, but can you change the comment

  // Assigns `fromValue` to the memory pointed at by `this`...

to

  // Writes `fromValue` into the memory pointed at by `this`...

or if "assign" is more accurate, at least change s/to/into/. I read it backwards. "Assign x to y" is ambiguous; it could mean |x = y| or |y = x|.

Comment 6

[Niko Matsakis [:nmatsakis]]

sfink -- re: use brittle, I've been trying to use naming conventions, which helps, but of course mistakes slip in. It occurs to me though that I ought to redefine the DESCR_FOO() macros to also assert that the object has the correct type. (And maybe just remove the macros altogether and use ion inlining hints instead.)

Comment 7

[Niko Matsakis [:nmatsakis]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/35f3b8ae743c

Comment 8

[Niko Matsakis [:nmatsakis]]

Try run: https://tbpl.mozilla.org/?tree=Try&rev=171a0e96143e

Comment 9

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unknown exception (check manually)

Comment 10

[Christian Holler (:decoder)]

JSBugMon: Cannot process bug: Unknown exception (check manually)

Comment 11

[Christian Holler (:decoder)]

Sorry for the annoying bug traffic here, we're debugging a problem with the bot.

Comment 12

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

https://hg.mozilla.org/mozilla-central/rev/35f3b8ae743c

Comment 13

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed.

Comment 14

[Christian Holler (:decoder)]

JSBugMon: This bug has been automatically verified fixed on Fx31