`Unsafe::compareAndSetFlatValue` calls `Unsafe::compareAndSetFlatValueAsBytes` 
which then calls `Unsafe::putFlatValue` on a flat array created by 
`Unsafe::newSpecialArray`.

https://github.com/openjdk/valhalla/blob/858be30119bd5bc37a69d16042523f53bea71a36/src/java.base/share/classes/jdk/internal/misc/Unsafe.java#L2870-L2875

`putFlatValue` can be intrinsified in

https://github.com/openjdk/valhalla/blob/858be30119bd5bc37a69d16042523f53bea71a36/src/hotspot/share/opto/library_call.cpp#L2727

that calls `cast_to_flat_array`

https://github.com/openjdk/valhalla/blob/858be30119bd5bc37a69d16042523f53bea71a36/src/hotspot/share/opto/library_call.cpp#L2831

which fails the assert

https://github.com/openjdk/valhalla/blob/858be30119bd5bc37a69d16042523f53bea71a36/src/hotspot/share/opto/graphKit.cpp#L1875-L1876

because of

https://github.com/openjdk/valhalla/blob/858be30119bd5bc37a69d16042523f53bea71a36/src/hotspot/share/oops/inlineKlass.cpp#L287-L290

Of course, if array flattening is disabled, one can't have flat arrays. And 
indeed, `Unsafe::newSpecialArray` will raise if called. So at runtime, the call 
to `Unsafe::compareAndSetFlatValue` should simply raise. But when compiled the 
assert is hit, crashing the VM, because it cannot tell that the code is dead, 
but can check that if it's not the array is not flat. That doesn't seem 
reasonable to me. I propose to insert a trap instead. Even in the case where 
this code wouldn't be dead (like if `Unsafe::newSpecialArray` is just undefined 
behavior instead of throwing), crashing the compiler doesn't seem like a good 
option.

The situation is actually more surprising than it seems: using 
`Unsafe::compareAndSetFlatValue` on a flat field with `-XX:-UseArrayFlattening` 
sounds reasonable, but doesn't actually work since the implementation will 
(attempt to) create flat arrays under the hood. It is not clear to me whether 
it's actually desirable, as `Unsafe::compareAndSetFlatValue` introduce some 
coupling of field and array flattening, but on the other hand, it's an unsafe 
API, so it's not that crazy to require more constrains to use.

Thanks,
Marc

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

Commit messages:
 - Add a trap and a test

Changes: https://git.openjdk.org/valhalla/pull/1549/files
  Webrev: https://webrevs.openjdk.org/?repo=valhalla&pr=1549&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8365978
  Stats: 98 lines in 2 files changed: 88 ins; 0 del; 10 mod
  Patch: https://git.openjdk.org/valhalla/pull/1549.diff
  Fetch: git fetch https://git.openjdk.org/valhalla.git pull/1549/head:pull/1549

PR: https://git.openjdk.org/valhalla/pull/1549

Reply via email to