932796 - Assertion failure: efs->argCount == args.length() - 1, at vm/SelfHosting.cpp:87 or Crash [@ __strlen_sse2_bsf]

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The following testcase asserts on mozilla-central revision 829d7bef8b0a (run with --fuzzing-safe --ion-compile-try-catch):


var T = TypedObject;
  var Point = new T.ArrayType(T.float32, 3);
  var Line = new T.StructType({from: Point, to: Point});
  var Lines = new T.ArrayType(Line, 3);
  var lines = new Lines([
    {from: [1, 2, 3], to: [4, 5, 6]},
    {from: [7, 8, 9], to: [10, 11, 12]},
    {from: [13, 14, 15], to: [16, 17, 18]}
  ]);
  var handle0 = Line.handle("Math.floor", "null", 0);

Comment 1

[Christian Holler (:decoder)]

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

Comment 2

[Felix S. Klock II [:pnkfelix, :fklock]]

Smaller test case:

    var Point = new TypedObject.ArrayType(TypedObject.float32, 3);
    var handle0 = Point.handle("Math.floor", "null", 0);

Comment 3

[Felix S. Klock II [:pnkfelix, :fklock]]

Attachment: bz932796-pAv1-fix-ThrowError-call

Fix the bug by passing correct number of arguments for JSMSG_INCOMPATIBLE_PROTO.

Also, I looked over the rest of the ThrowError invocations and made sure that each invocation passed the correct number of arguments.  So we should not see any more problems from this version of the TypedObject.js code.

There is a follow-up bug 928943 to improve ThrowError further: even a simple python/perl/whatever script to grep through the self-hosted .js code and validate it against the specifications in js.msg would probably pay for itself here.

Comment 4

[Felix S. Klock II [:pnkfelix, :fklock]]

Pushed to inbound:

  https://hg.mozilla.org/integration/mozilla-inbound/rev/75d5fed1950e

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/75d5fed1950e

Comment 6

[Ioana (away)]

The assertion failure reproduced with both testcases using the 10/31 Fx28 JS shell on Ubuntu 12.10 x86_x64.

When using the 02/04 Fx28 shell in the same environment, I get "ReferenceError: TypedObject is not defined". Is there a chance this error might be hiding a still existing assertion failure?

Comment 7

[Ioana (away)]

Also search Socorro for searches with this signature (__strlen_sse2_bsf), but I haven't found any.

Comment 8

[Felix S. Klock II [:pnkfelix, :fklock]]

ioana: The TypedObject reference error sounds like you are trying to run the test in a context where that class is not provided.

My understanding is that the TypedObject class is only available on Nightly builds; I believe it is disabled on release builds.

Would that explain the behavior you are observing?

Comment 9

[Ioana (away)]

Thanks for the details Felix! I did get that error with the beta shell.

I also tested with the nightly (fx30) 02/05 js shell now and I get this:
TypeError: can't convert ArrayType to StructType field specifier

Comment 10

[Felix S. Klock II [:pnkfelix, :fklock]]

ioana: Yeah, there have been some API changes, so I'm not terribly surprised by that error message from the original test.

Even after updating the test to more closely match the current API, you'll still see JavaScript exceptions thrown (and that's not a bad thing; the crucial point is that the internal assertion failure that the original test exposed is no longer being triggered).

For example, this particular fuzz test seems like it was probably derived from this original test from the tree:

  https://hg.mozilla.org/integration/mozilla-inbound/annotate/b5e50392da12/js/src/tests/ecma_6/TypedObject/handle.js#l14

(note that that code has been revised to bring it up to date with the current TypedObjects API.)

Anyway, here is an updated version of the fuzz test, and the result I see from it:

  var T = TypedObject;
  var Point = T.float32.array(3); // new T.ArrayType(T.float32, 3);
  var Line = new T.StructType({from: Point, to: Point});
  var Lines = Line.array(3); // new T.ArrayType(Line, 3);
  var lines = new Lines([
      {from: [1, 2, 3], to: [4, 5, 6]},
      {from: [7, 8, 9], to: [10, 11, 12]},
      {from: [13, 14, 15], to: [16, 17, 18]}
  ]);
  var handle0 = Line.handle("Math.floor", "null", 0);

Result (which matches my expectation, since .handle is not being invoked on appropriate arguments):

  self-hosted:4763:4 TypeError: Handle.prototype.set called on incompatible value

Comment 11

[Felix S. Klock II [:pnkfelix, :fklock]]

nmatsakis: Should we continue accepting |new TypedArray.ArrayType(T, [len])|, even though the result is not acceptable as a StructType field specifier?

Comment 12

[Ioana (away)]

Marking as verified per the above comments.

Comment 13

[Niko Matsakis [:nmatsakis]]

Felix -- new ArrayType(T, len) is not accepted in the code as it stands. You have to do new ArrayType(T).dim(len), I think. But this is all changed by latest draft of the spec anyhow. Bringing us up to date is my next priority, actually, after bug 898356