1068451 - Assertion failure: i < argc_, at dist/include/js/CallArgs.h

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

(function() {
    for (b in new SharedArrayBuffer) {}
}())

asserts js debug shell on m-c changeset 8252eae8278c with --no-ion --no-threads at Assertion failure: i < argc_, at dist/include/js/CallArgs.h.

Debug configure flags:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar sh /Users/skywalker/trees/mozilla-central/js/src/configure --target=x86_64-apple-darwin12.5.0 --enable-debug --enable-optimize --enable-nspr-build --enable-more-deterministic --with-ccache --enable-gczeal --enable-debug-symbols --disable-tests

=== Tinderbox Build Bisection Results by autoBisect ===

The "good" changeset has the timestamp "20140916090358" and the hash "d803279cf506".
The "bad" changeset has the timestamp "20140916094802" and the hash "cf9ed5c35329".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=d803279cf506&tochange=cf9ed5c35329

Lars, is bug 1054882 a possible regressor?

Comment 1

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

Attachment: stack

This is happening often enough to become a fuzzblocker.

Comment 2

[Jeff Walden [:Waldo]]

Unguarded args[0] can be changed to args.get(0) as a quick and dirty fix.  I might do it if my tree weren't halfway through a rebase, but it is, so no fix from me now.  rs=me to change to args.get(0) tho for anyone who wants it.

Comment 3

[Lars T Hansen [:lth]]

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

Comment 4

[Ryan VanderMeulen [:RyanVM]]

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

Any reason this landed minus a test?

Comment 5

[Lars T Hansen [:lth]]

(In reply to Ryan VanderMeulen [:RyanVM UTC-4] from comment #4)
> https://hg.mozilla.org/mozilla-central/rev/df9e359b34b9
> 
> Any reason this landed minus a test?

Yes: I was stressed out dealing with a bunch of fires resulting from the SharedArrayBuffer patch, and plain forgot to include the test case.

I'll cook one up now.

Comment 6

[Lars T Hansen [:lth]]

Attachment: bug1068451-testcase.diff

Test case.  This crashes a debug build without the patch, and works fine in a debug build with the patch.

Comment 7

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

Comment on attachment 8491359 [details] [diff] [review]
bug1068451-testcase.diff

Forwarding review? to Waldo (the original rubber-stamper-er).

Comment 8

[Jeff Walden [:Waldo]]

Comment on attachment 8491359 [details] [diff] [review]
bug1068451-testcase.diff

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

::: js/src/jit-test/tests/sharedbuf/bug1068451.js
@@ +1,1 @@
> +if (!this.SharedArrayBuffer)

Rename this test to construct-no-arguments.js or something like that.  Bug numbers in testcase names are uninformative when skimming directory listings, don't autocomplete well, are hard to remember when opening up a failing test in a file picker, etc.  Better to give it some sort of name describing what's being tested.

@@ +9,5 @@
> +
> +try {
> +    (function() {
> +        for (b in new SharedArrayBuffer) {}
> +    }())

This should just be |new SharedArrayBuffer();|.  The rest of this is just fuzzer nonsense that tests a bunch of irrelevancies and obscures what's actually being tested.

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/56c54c6dc1a6