RFR: 8279143: Undefined behaviours in globalDefinitions.hpp [v4]

[Kim Barrett]

On Tue, 4 Jan 2022 08:37:55 GMT, Quan Anh Mai <duke at openjdk.java.net> wrote:

>> Hi,
>> 
>> This patch replaces undefined behaviours in globalDefinitions.hpp by proper well-defined ones.
>> 
>> Thank you very much.
>
> Quan Anh Mai has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits since the last revision:
> 
>  - Merge branch 'master' into undefinedBehaviour
>  - update copyright
>  - typo
>  - clean
>  - Merge branch 'master' into undefinedBehaviour
>  - Merge branch 'master' of github.com:MeryKitty/jdk into undefinedBehaviour
>  - implementation limits
>  - const reference
>  - words not need to be initialized
>  - undefined behaviour in globalDefinitions.hpp

Changes requested by kbarrett (Reviewer).

src/hotspot/share/utilities/globalDefinitions.hpp line 617:

> 615:                 std::is_trivially_copy_assignable<T>(), "implementation limits");
> 616:   T to;
> 617:   memcpy(&to, &from, sizeof(T));

During the review of JDK-8145096 it was found that some compilers produce
wretched code for these kinds of memcpy uses, even at fairly high optimization
levels.  (I don't know if we still care about those compilers.  Unfortunately I
don't remember which ones they were, other than gcc/clang/VS all being good.)

While using the so-called "union trick" is technically undefined behavior, it
is a technique that is known to be widely and well supported and produces good
code, at least for the cases where it is being used in HotSpot. In some cases,
such as gcc (and I think Visual Studio, though can't find a reference right
now), this behavior is documented.

Rather than adding a partial bit_cast (or moving it from elsewhere), we should
be using our existing PrimitiveConversions::cast
(metaprogramming/primitiveConversions.hpp). That has the small difficulty of a
circular include dependency with globalDefintions.hpp. That can be fixed by
moving the various jfoo_cast functions elsewhere (either to
primitiveConversions.hpp or to a new file; I might prefer the latter (along
with the Translate<jfloat/jdouble> specializations in primitiveConversions),
moving these relatively infrequently used utilities to their own dedicated
location). That also reduces the content of globalDefinitions.hpp, which IMO
is too much of a random dumping ground.

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

PR: https://git.openjdk.java.net/jdk/pull/6930