Closed Bug 1437970 Opened 7 years ago Closed 7 years ago

UBSan: signed integer overflow in powi

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox60 --- fixed

People

(Reporter: anba, Assigned: Waldo)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

https://searchfox.org/mozilla-central/rev/9d47b5fb14152865ad1375eb3ee2e571d81ecdb9/js/src/jsmath.cpp#664 Test case: --- js> 2**(-2147483648) /home/andre/hg/mozilla-inbound/js/src/jsmath.cpp:663:33: runtime error: negation of -2147483648 cannot be represented in type 'int'; cast to an unsigned type to negate this value to itself 0 ---
The int32_t signature changes are not quite necessary, just a useful cleanup in passing to align used type with the reality of all users (JIT and in-source both) actually passing int32_t specifically.
Attachment #8950689 - Flags: review?(andrebargull)
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
...I suppose this should have a test, since I didn't see it running tests locally. Bah. Guess I'll do a PR in test262 upstream or something.
https://github.com/tc39/test262/pull/1445 for upstream tests that would induce this signed integer overflow. Hopefully they won't kibitz too hard on tests that are arguably a little esoteric/focused on a (usually) unobservable foible known to be present only in one implementation.
Comment on attachment 8950689 [details] [diff] [review] Use mozilla::Abs in js::powi rather than unary negation Review of attachment 8950689 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Jeff Walden [:Waldo] from comment #1) > The int32_t signature changes are not quite necessary, just a useful cleanup > in passing to align used type with the reality of all users (JIT and > in-source both) actually passing int32_t specifically. U+1F44D (THUMBS UP SIGN) (In reply to Jeff Walden [:Waldo] from comment #3) > https://github.com/tc39/test262/pull/1445 for upstream tests that would > induce this signed integer overflow. Hopefully they won't kibitz too hard > on tests that are arguably a little esoteric/focused on a (usually) > unobservable foible known to be present only in one implementation. I've seen that implementation specific tests were accepted in the past, but it may depend on who shows up and reviews the patch.
Attachment #8950689 - Flags: review?(andrebargull) → review+
Pushed by jwalden@mit.edu: https://hg.mozilla.org/integration/mozilla-inbound/rev/75c054ccb1d5 Use mozilla::Abs in js::powi rather than unary negation, to avoid signed-integer-overflow issues with INT32_MIN. r=anba
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: