Closed Bug 1101576 Opened 10 years ago Closed 10 years ago

Assertion failure: Integer input should be equal or higher than Lowerbound., at jit/IonMacroAssembler.cpp

Categories

(Core :: JavaScript Engine: JIT, defect)

x86_64
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla36
Tracking Status
firefox33 --- unaffected
firefox34 --- unaffected
firefox35 --- unaffected
firefox36 --- fixed
firefox-esr31 --- unaffected
b2g-v1.4 --- unaffected
b2g-v2.0 --- unaffected
b2g-v2.0M --- unaffected
b2g-v2.1 --- unaffected
b2g-v2.2 --- fixed

People

(Reporter: gkw, Assigned: h4writer)

References

Details

(4 keywords, Whiteboard: [jsbugmon:])

Attachments

(2 files)

// Random chosen test: js/src/jit-test/tests/ion/bug928423.js
o = {
    a: 1,
    b: 1
}
for (var x = 0; x < 2; x++) {
    assertEq(o["a1".substr(0, 1)], 1)
    assertEq(o["b1".substr(0, 1)], 1)
}
// jsfunfuzz
"a" + "b"

asserts js debug shell on m-c changeset d0d8c407efb5 with --fuzzing-safe --no-threads --ion-eager --ion-licm=off --ion-check-range-analysis at Assertion failure: Integer input should be equal or higher than Lowerbound., at jit/IonMacroAssembler.cpp.

Debug configure options:

CC="clang -Qunused-arguments" CXX="clang++ -Qunused-arguments" AR=ar AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213 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

This was found by combining random jit-tests together with jsfunfuzz, the specific file(s) is/are:

http://hg.mozilla.org/mozilla-central/file/d0d8c407efb5/js/src/jit-test/tests/ion/bug928423.js

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

The "good" changeset has the timestamp "20141118063028" and the hash "4f1382061059".
The "bad" changeset has the timestamp "20141118065627" and the hash "34859490061a".

Likely regression window: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=4f1382061059&tochange=34859490061a

(Setting s-s because a previous bug 973615 was marked s-s as well. Assuming sec-high for now.)

Hannes, is bug 1052839 a likely regressor?
Flags: needinfo?(hv1989)
Attached file stack
(lldb) bt
* thread #1: tid = 0x1436c5, 0x0000000101dc8b9a, queue = 'com.apple.main-thread', stop reason = EXC_BREAKPOINT (code=EXC_I386_BPT, subcode=0x0)
  * frame #0: 0x0000000101dc8b9a
(lldb)
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Assignee: nobody → hv1989
Flags: needinfo?(hv1989)
Attachment #8525656 - Flags: review?(efaustbmo)
Comment on attachment 8525656 [details] [diff] [review]
Don't overwrite inputs

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

r=me with one comment addressed.

::: js/src/jit/CodeGenerator.cpp
@@ +6060,3 @@
>          masm.computeEffectiveAddress(outputStorage, temp);
> +        CopyStringChars(masm, temp, temp2, length, temp3, sizeof(char16_t), sizeof(char16_t));
> +        masm.load32(Address(output, JSString::offsetOfLength()), length);

As briefly discussed, it's probably possible to not smash length. This is more explicitly obviously correct, so let's stay with this for the moment and consider other approaches as an optimization.

::: js/src/jit/Lowering.cpp
@@ +2178,5 @@
>                                           useRegister(ins->begin()),
>                                           useRegister(ins->length()),
> +                                         temp(),
> +                                         temp(),
> +                                         tempFixed(CallTempReg1));

Why does this need to be CallTempReg1 specifically? Are we out of random virtual registers?
Attachment #8525656 - Flags: review?(efaustbmo) → review+
(In reply to Eric Faust [:efaust] from comment #4)
> Comment on attachment 8525656 [details] [diff] [review]
> Don't overwrite inputs
> 
> Review of attachment 8525656 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me with one comment addressed.
> 
> ::: js/src/jit/CodeGenerator.cpp
> @@ +6060,3 @@
> >          masm.computeEffectiveAddress(outputStorage, temp);
> > +        CopyStringChars(masm, temp, temp2, length, temp3, sizeof(char16_t), sizeof(char16_t));
> > +        masm.load32(Address(output, JSString::offsetOfLength()), length);
> 
> As briefly discussed, it's probably possible to not smash length. This is
> more explicitly obviously correct, so let's stay with this for the moment
> and consider other approaches as an optimization.

opened bug 1102001
 
> ::: js/src/jit/Lowering.cpp
> @@ +2178,5 @@
> >                                           useRegister(ins->begin()),
> >                                           useRegister(ins->length()),
> > +                                         temp(),
> > +                                         temp(),
> > +                                         tempFixed(CallTempReg1));
> 
> Why does this need to be CallTempReg1 specifically? Are we out of random
> virtual registers?

Added comment (we need to have a register that handles 8bit moves)
https://hg.mozilla.org/mozilla-central/rev/43aceb996c3b
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Group: core-security
You need to log in before you can comment on or make changes to this bug.