RFR: 8324833: Signed integer overflows in ABS [v3]

Dean Long dlong at openjdk.org
Sat Feb 3 00:07:03 UTC 2024


On Fri, 2 Feb 2024 09:47:28 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> See the details in the bug. I think current `ABS` implementation is beyond repair, and we should just switch to `uabs`.
>> 
>> Additional testing:
>>  - [x] Linux x86_64 fastdebug, `all` with `-ftrapv` (now fully passes!)
>>  - [x] Linux x86_64 fastdebug, `all`
>
> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Touchups

I think it's confusing and error-prone to use uabs() for signed values.  Using `checked_cast` silences warnings but doesn't handle the undefined behavior caused by values such as min_jint and min_jlong.  I suggest fixing ABS so that it first calls uabs and then safely converts to signed, checking for negative values caused by MIN INT inputs.  However, that will cause some new asserts where we are relying on undefine behavior, like in exact_limit() described in JBS, so it seems like that caller needs to be fixed.
Fixing all the -Wsigned-conversion warnings in these files, or even in the changed functions, seems out of scope for this fix.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/17617#issuecomment-1924922608


More information about the graal-dev mailing list