Closed
Bug 1437970
Opened 7 years ago
Closed 7 years ago
UBSan: signed integer overflow in powi
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla60
Tracking | Status | |
---|---|---|
firefox60 | --- | fixed |
People
(Reporter: anba, Assigned: Waldo)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
anba
:
review+
|
Details | Diff | Splinter Review |
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
---
Assignee | ||
Comment 1•7 years ago
|
||
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 | ||
Updated•7 years ago
|
Assignee: nobody → jwalden+bmo
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•7 years ago
|
||
...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.
Assignee | ||
Comment 3•7 years ago
|
||
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.
Reporter | ||
Comment 4•7 years ago
|
||
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
Comment 6•7 years ago
|
||
bugherder |
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.
Description
•