[jdk8u-dev] RFR: 8250825: C2 crashes with assert(field != __null) failed: missing field [v2]

[Andrew John Hughes]

On Thu, 25 Jul 2024 03:21:08 GMT, SendaoYan <syan at openjdk.org> wrote:

>> Hi all,
>> This is backport of [JDK-8250825](https://bugs.openjdk.org/browse/JDK-8250825). It's prefixed PR for [JDK-8255466](https://bugs.openjdk.org/browse/JDK-8255466) [backport](https://github.com/openjdk/jdk8u-dev/pull/536).
>> New test fails without the patch, passes with it.
>> 
>> There are two parts make this backport not clean:
>> 
>> 1. Patch does not apply cleanly due to different context, before [JDK-8250825](https://bugs.openjdk.org/browse/JDK-8250825)  the  PR [JDK-8230505](https://bugs.openjdk.org/browse/JDK-8230505) change releated lines, and I think [JDK-8230505](https://bugs.openjdk.org/browse/JDK-8230505) do not needed backport to jdk8u-dev.
>> 2. The added testcase can't backport to jdk8u directly, `import jdk.internal.misc.Unsafe` should instead of `import sun.misc.Unsafe` in jdk8u.
>> 
>> 
>> 
>> Additional testing:
>> 
>> - [x] linux x64 tier1/2/3 jtreg test
>> - [x] inux aarch64 tier1/2/3 jtreg test
>
> SendaoYan has updated the pull request incrementally with one additional commit since the last revision:
> 
>   implement Unsafe.getUnsafe() in jdk8u

The change to the test for `Unsafe` looks fine and matches other 8u tests that use `Unsafe`.

I'm not sure what patch you're backporting from, but the 11u version is already adjusted for the absence of JDK-8230505. If you fix up the indentation to match 11u, this should be good to go.

hotspot/src/share/vm/opto/type.cpp line 2561:

> 2559:             BasicType basic_elem_type = field->layout_type();
> 2560:             _is_ptr_to_narrowoop = UseCompressedOops && (basic_elem_type == T_OBJECT ||
> 2561:                                                        basic_elem_type == T_ARRAY);

This line hasn't been adjusted to line up with the one above.

Are you not backporting from the 11u patch? That also doesn't have JDK-8230505 and would match your patch exactly if not for this misaligned line.

This is the 11u version:

~~~
-          assert(field != NULL, "missing field");
-          BasicType basic_elem_type = field->layout_type();
-          _is_ptr_to_narrowoop = UseCompressedOops && (basic_elem_type == T_OBJECT ||
-                                                       basic_elem_type == T_ARRAY);
+          if (field != NULL) {
+            BasicType basic_elem_type = field->layout_type();
+            _is_ptr_to_narrowoop = UseCompressedOops && (basic_elem_type == T_OBJECT ||
+                                                         basic_elem_type == T_ARRAY);
+          } else {
+            // unsafe access
+            _is_ptr_to_narrowoop = UseCompressedOops;
+          }
~~~

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

PR Review: https://git.openjdk.org/jdk8u-dev/pull/552#pullrequestreview-2264717131
PR Review Comment: https://git.openjdk.org/jdk8u-dev/pull/552#discussion_r1733694929