RFR: 8276563: Undefined Behaviour in class Assembler [v11]
Andrew Haley
aph at openjdk.java.net
Mon Jan 17 14:52:21 UTC 2022
> The HotSpot code base contains a number of instances of Undefined Behavior, which can cause all manner of unpleasant surprises.
> The UB to which this patch relates is in class `Assembler`, in which instances are pointers to (nonexistent) objects defined as, for example,
>
>
> typedef RegisterImpl *Register;
> const Register r10 = ((Register)10);
>
>
> Registers have accessors, e.g.:
>
> ` int RegisterImpl::encoding() const { return (intptr_t)this; }`
>
> This works by an accident of implementation: it is not legal C++.
>
> The most obvious way to this UB bug is to make instances of `Register` point to something, and to use pointer subtraction to find the encoding: (simplified for clarity)
>
>
> extern RegisterImpl all_Registers[num_Registers];
> int RegisterImpl::encoding() const { return this - all_Registers; }
>
>
> After this patch there is slightly more work to be done when assembling code but it's merely the subtraction of a constant in `encoding()` and the difference in execution time is so small (and the startup variance so large) that I have been unable to measure it, even after averaging 100 runs. It does lead to an increase of about 1% in the size of the stripped libjvm.so, but I think that can be recovered by a subsequent patch.
>
> An alternative way to implement this would be to make the encoding a byte-wide field in `RegisterImpl` and define encoding() this way:
>
> ` int RegisterImpl::encoding() const { return _encoding; }`
>
> This would result in smaller code, but I suspect slower.
>
> If this change is accepted, I propose that all instances of this pattern in HotSpot be treated similarly.
Andrew Haley 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 14 additional commits since the last revision:
- Merge https://git.openjdk.java.net/jdk into no-aarch64-asm-ub-rebase
- Delete has_byte_register().
- Tweako stuff.
- Back out incorrect change to x86.
- Whitespace
- Simplify and improve portability.
- More cleanups
- More cleanups
- More cleanups
- More cleanups
- ... and 4 more: https://git.openjdk.java.net/jdk/compare/b2a3fd1a...66f1d1a2
-------------
Changes:
- all: https://git.openjdk.java.net/jdk/pull/6280/files
- new: https://git.openjdk.java.net/jdk/pull/6280/files/8dfc54f5..66f1d1a2
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk&pr=6280&range=10
- incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=6280&range=09-10
Stats: 135271 lines in 2812 files changed: 92713 ins; 23624 del; 18934 mod
Patch: https://git.openjdk.java.net/jdk/pull/6280.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/6280/head:pull/6280
PR: https://git.openjdk.java.net/jdk/pull/6280
More information about the hotspot-compiler-dev
mailing list