RFR: 8235385: AArch64: Crash on aarch64 JDK due to long offset
Andrew Haley
aph at redhat.com
Thu Dec 12 19:14:53 UTC 2019
This assertion failure happens because load/store patterns in
aarch64.ad are incorrect.
The operands immIOffset and operand immLoffset() are only correct for
load/store *byte* offsets. Offsets of sizes greater than a byte should
be shifted by the operand size, and misaligned offsets are only
allowed for a small range. We get this wrong, so we try to use
misaligned byte addresses for sizes greater than byte. This fails
at compile time.
We've never noticed this before because Java code doesn't generate
misaligned offsets, so we can only test this fix by using Unsafe (or a
customer of Unsafe such as ByteBuffer.)
Wang Zhuo(Zhuoren) <zhuoren.wz at alibaba-inc.com> wrote a patch for this
bug but it was incomplete; the problem reached deeper than we at first
realized. Zhuoren's approach was to fix up the code generation after
pattern matching, but this masks an efficiency problem. In many cases
where we could use offset addresses, we don't because the pattern
matcher incorrectly decides offsets are out of range.
This patch fixes both problems by using memory types that are correct
for all operand sizes. These are memory1, memory2, memory4, etc; this
naming fits in with the existing types used by vectors.
It does lead to a rather large patch, but it's not quite as bad as it
looks because much of the code is auto-generated by the script
ad_encode.m4. Unfortunately, in the time since it was written some
developers have edited that auto-generated section of aarch64.ad by
hand, so I had to move these hand edits out of the way. I also
I have also added big scary
// DO NOT EDIT ANYTHING IN THIS SECTION OF THE FILE
comments so this doesn't happen again.
In a rather belt-and-braces way I've also added some code that fixes
up illegal addresses. I did consider removing it, but I left it in
because it doesn't hurt. If we ever do generate similar illegal
addresses, debug builds will assert. I'm not sure whether to keep this
or not.
Andrew Dinn will probably have a cow when he sees this patch. :-)
OK for HEAD?
--
Andrew Haley (he/him)
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
https://keybase.io/andrewhaley
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
More information about the hotspot-compiler-dev
mailing list