[PATCH] 8202414: Unsafe crash in C2

[Vladimir Kozlov]

Thank you, Andy

Unfortunately your change may leave uninitialized (not zeroed) bytes in object.
Instead unaligned stores should be treated as subword stores:

diff -r b9f6a4427da9 src/hotspot/share/opto/memnode.cpp
--- a/src/hotspot/share/opto/memnode.cpp
+++ b/src/hotspot/share/opto/memnode.cpp
@@ -4095,10 +4095,11 @@
        // See if this store needs a zero before it or under it.
        intptr_t zeroes_needed = st_off;

-      if (st_size < BytesPerInt) {
+      if (st_size < BytesPerInt || (zeroes_needed % BytesPerInt) != 0) {
          // Look for subword stores which only partially initialize words.
          // If we find some, we must lay down some word-level zeroes first,
          // underneath the subword stores.
+        // Do the same for unaligned stores.
          //
          // Examples:
          //   byte[] a = { p,q,r,s }  =>  a[0]=p,a[1]=q,a[2]=r,a[3]=s

Rahul, the bug is assigned to you. Please, test this solution.

Thanks,
Vladimir

On 9/10/18 4:10 AM, Andy Law wrote:
> This change is only about:
> Disabling the un-aligned C2 `clean_memory()` optimization when using Unsafe to write to an unaligned address.
> 
> ```
> java -version
> openjdk version "1.8.0-internal-debug"
> OpenJDK Runtime Environment (build 1.8.0-internal-debug-***_2018_09_03_19_31-b00)
> OpenJDK 64-Bit Server VM (build 25.71-b00-debug, mixed mode)
> ```
> 
> This issue 8202414 is about:
> ArrayObjects of -XX:+UseCompressedOops on 64-bit has a 12 bits header and a 4 bits length. So the length address is from 12th to 16th bytes.
> If we use Unsafe.putInt() to write at the 17th bit, the C2 `clean_memory()` will mistakenly do `done_offset -= BytesPerInt;`, then `done_offset` will become 13.  And then it will clear the address from the 13th bit, make the array length changes to a different value. When a GC happens, it will crash.
> 
> I didn’t find the unaligned memory support of `clear_memory()`, so I only do a small fix to make the affect be the least:
> When Unsafe.put*() writes to an aligned memory as above, it will cause the assert fail. So when it fails, we don’t do any optimizations instead, and the problem solves.
> 
> I don’t know if it is a good solution? It is only 3 lines of code, so please have a look:) Thank you!
> 
> 
> Andy
>