Closed Bug 799793 Opened 8 years ago Closed 8 years ago

Bounds computed by bit and are incorrect.


(Core :: JavaScript Engine, defect)

Not set



Tracking Status
firefox18 --- fixed
firefox19 --- fixed


(Reporter: mjrosenb, Unassigned)



(1 file)

It threw an assertion stating that upper >= lower, and it looks like in general, taking the min of the two max-values is wrong.  For future things, I may switch the meaning of upper/lower for the results of bitand, since computing ranges with binary operations is non-trivial.
Attachment #669828 - Flags: review?(dvander)
Comment on attachment 669828 [details] [diff] [review]

Review of attachment 669828 [details] [diff] [review]:

::: js/src/ion/RangeAnalysis.cpp
@@ +346,5 @@
>      // If both numbers can be negative, issues can be had.
>      if (lhs->lower_ < 0 && rhs->lower_ < 0)
>          lower = INT_MIN;
> +    int64_t upper = Max(lhs->upper_, rhs->upper_);
> +    //fprintf(stderr, "updating to: (%d, %d)\n", lower, upper);

r=me with this comment removed
Attachment #669828 - Flags: review?(dvander) → review+
This change seems unfortunate since it means you can't guarantee that "i & 3" is < 4 when i is unknown, which might be very useful for array bounds checks and stuff.

If either input to "and" is non-negative, then that input is an upper-bound for the result of "and". So if lhs->lower >= 0, lhs->upper is an upper bound, and likewise for rhs.

Also, if either input to "and" is non-negative, the output can't be negative, which is also useful to know. So if lhs->lower >= 0, you can set lower to 0, and likewise for rhs.
Yeah, it is a surprisingly complex operation.  The way other engines deal with it is they have two separate mechanisms in place, one for "here be ranges" and one for "here be masks".  I may try doing that, or I may spend an hour in front of a whiteboard until I have an exact algorithm mapped out.
I bet it would even be a fun interview question :-p
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla19
You need to log in before you can comment on or make changes to this bug.