RFR: 8338449: ubsan: division by zero in sharedRuntimeTrans.cpp

[Joe Darcy]

On Tue, 22 Oct 2024 03:48:15 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> `ATTRIBUTE_NO_UBSAN` suppresses conditions found by ubsan instrumentation. If
>> ubsan is enabled, the compiler's optimizer presumably doesn't take advantage
>> of the no-UB assumption, as that would defeat the point of ubsan. But in a
>> non-ubsan-enabled build, I think that attribute does nothing and the compiler
>> may take advantage of the no-UB assumption. So that part of the change may not
>> do anything useful for non-ubsan builds.
>> 
>> cppreference is inconsistent about this. It first states that division by zero
>> is UB, as per the standard. But then it says if the type `is_iec599`, then
>> division by zero returns the IEEE defined value (correctly signed infinity if
>> lhs is non-zero, NaN if lhs is zero) and raises the appropriate floating point
>> trap. I think this claim makes sense for a C compiler that supports Annex F.
>> But C++ isn't C, and I don't see anything comparable in a C++ standard.
>> 
>> I can't find any text in any version of either the C or C++ standard to
>> support the claimed `is_iec599` behavior though.
>> 
>> I did find this, which has some interesting bits.
>> https://stackoverflow.com/questions/42926763/the-behaviour-of-floating-point-division-by-zero
>> 
>> For example, it is suggested that claiming `is_iec599` implicity defines the
>> behavior for floating point division by zero (among other things). It is also
>> mentioned that gcc does that, but (some version of?) clang-the-compiler
>> doesn't implement iec599, even though the standard library one is using might
>> claim so. Hm...
>> 
>> It also mentions that the default for `-fsanitize=undefined` for clang includes
>> `float-divide-by-zero`, while gcc does not.  Any chance the reported issue is
>> arising with clang as the compiler?
>
> Never mind my question about whether this was happening with clang as the compiler rather than gcc.
> Our ubsan configuration explicitly includes `-fsantize=float-divide-by-zero`.
> https://github.com/openjdk/jdk/blame/8bcd4920f1b03d0ef8e295e53557c629f05ceaa4/make/autoconf/jdk-options.m4#L516

> I'm not sure what the implications of this actually are. Do we really need it? Maybe @jddarcy can comment?

Catching up on email, whatever the appropriate C/C++ idiom is, it is appropriate for this file to assert "use IEEE 754 semantics for floating-point operations." The divide by zero behavior is well-defined by IEEE 754/IEC 559.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21500#discussion_r1809827572