OdinMonkey: Assertion failure: max_exponent_ + canHaveFractionalPart_ >= mozilla::FloorLog2(mozilla::Abs(upper_)), at jit/RangeAnalysis.h

RESOLVED FIXED in mozilla27

Status

()

--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: gkw, Assigned: sunfish)

Tracking

(Blocks: 2 bugs, {assertion, regression, testcase})

Trunk
mozilla27
x86_64
Mac OS X
assertion, regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox24 unaffected, firefox25 unaffected, firefox26 unaffected, firefox27 affected, firefox-esr17 unaffected, firefox-esr24 unaffected)

Details

(Whiteboard: [jsbugmon:])

Attachments

(3 attachments)

(Reporter)

Description

5 years ago
Created attachment 817814 [details]
stack

(function() {
    "use asm"
    function f() {
        +~~1.1
    }
})()

asserts js debug shell on m-c changeset c14ca6b27b30 with --ion-gvn=off at Assertion failure: max_exponent_ + canHaveFractionalPart_ >= mozilla::FloorLog2(mozilla::Abs(upper_)), at jit/RangeAnalysis.h

My configure flags are:

CC="clang -Qunused-arguments" AR=ar CXX="clang++ -Qunused-arguments" sh ./configure --target=x86_64-apple-darwin12.5.0 --enable-optimize --enable-debug --enable-profiling --enable-gczeal --enable-debug-symbols --enable-methodjit --enable-type-inference --disable-tests --with-ccache
(Reporter)

Updated

5 years ago
Blocks: 840284
Summary: Assertion failure: max_exponent_ + canHaveFractionalPart_ >= mozilla::FloorLog2(mozilla::Abs(upper_)), at jit/RangeAnalysis.h → OdinMonkey: Assertion failure: max_exponent_ + canHaveFractionalPart_ >= mozilla::FloorLog2(mozilla::Abs(upper_)), at jit/RangeAnalysis.h
Whiteboard: [jsbugmon:update] → [jsbugmon:]
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
(Reporter)

Comment 2

5 years ago
autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   http://hg.mozilla.org/mozilla-central/rev/42a20a0d4269
user:        Dan Gohman
date:        Tue Oct 15 20:49:44 2013 -0700
summary:     Bug 918607 - IonMonkey: Add a Range::setDouble which takes double arguments and use it to simplify and generalize several things.

Dan, is bug 918607 a likely regressor?
Blocks: 918607
status-firefox24: --- → unaffected
status-firefox25: --- → unaffected
status-firefox26: --- → unaffected
status-firefox27: --- → affected
status-firefox-esr17: --- → unaffected
status-firefox-esr24: --- → unaffected
Flags: needinfo?(sunfish)
(Assignee)

Comment 3

5 years ago
Yes, it is.

The bug here is that Range::wrapAroundToInt32 is setting hasFractionalPart_ to false without checking whether this allows lower_/upper_ to be improved, and the new strict checking is catching the fact that an opportunity for improvement was missed.
Assignee: general → sunfish
Flags: needinfo?(sunfish)

Updated

5 years ago
No longer blocks: 840284
(Assignee)

Comment 4

5 years ago
Created attachment 819378 [details] [diff] [review]
range-refine-bounds-after-clearing-fractional.patch

This patch refactors out the code for refining lower_/upper_ using max_exponent_ from Range::intersect into a helper function so that it can be used by Range::wrapAroundToInt32 too.
Attachment #819378 - Flags: review?(nicolas.b.pierron)
(Assignee)

Updated

5 years ago
Blocks: 928450
Comment on attachment 819378 [details] [diff] [review]
range-refine-bounds-after-clearing-fractional.patch

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

::: js/src/jit/RangeAnalysis.h
@@ +256,5 @@
>  
> +    // Compute the best lower and upper bounds that would be compatible with
> +    // max exponent value of e, if possible. If it is, set *impliedLower and
> +    // *impliedUpper and return true. Otherwise return false.
> +    static void refineInt32BoundsByExponent(uint16_t e, int32_t *l, int32_t *h) {

nit: Precise that this function is only useful when we get the exponent from a range which has a fractional part, as only in such case the exponent might be more precise than the int32 bound.
Attachment #819378 - Flags: review?(nicolas.b.pierron) → review+
No ASM.js required here, this reproduces for me as well:


(function () {
    var x = 0 || 1.1;
    x >> x;
})();
Blocks: 676763
Created attachment 819701 [details]
[crash-signature] Machine-readable crash signature
(Assignee)

Comment 8

5 years ago
(In reply to Nicolas B. Pierron [:nbp] from comment #5)
> Comment on attachment 819378 [details] [diff] [review]
> range-refine-bounds-after-clearing-fractional.patch
> 
> Review of attachment 819378 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jit/RangeAnalysis.h
> @@ +256,5 @@
> >  
> > +    // Compute the best lower and upper bounds that would be compatible with
> > +    // max exponent value of e, if possible. If it is, set *impliedLower and
> > +    // *impliedUpper and return true. Otherwise return false.
> > +    static void refineInt32BoundsByExponent(uint16_t e, int32_t *l, int32_t *h) {
> 
> nit: Precise that this function is only useful when we get the exponent from
> a range which has a fractional part, as only in such case the exponent might
> be more precise than the int32 bound.

Done.

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce54c42790f6
https://hg.mozilla.org/mozilla-central/rev/ce54c42790f6
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
(Reporter)

Updated

5 years ago
Component: JavaScript Engine → JavaScript Engine: JIT
You need to log in before you can comment on or make changes to this bug.