RFR: 8331194: NPE in ArrayCreationTree.java with -XX:-UseCompressedOops [v3]
Vladimir Kozlov
kvn at openjdk.org
Thu Jul 11 19:49:55 UTC 2024
On Thu, 11 Jul 2024 19:18:36 GMT, Cesar Soares Lucas <cslucas at openjdk.org> wrote:
>> Please, review this PR to fix issue in debug information serialization encountered when RAM reduces a Phi for which one of the inputs is an object already scalar replaced.
>>
>> **Details:**
>>
>> Consider class `Picture` that has two reference fields, `first` and `second` of type `Point`. In a random method in the application an object `obj` of this class is created, and the fields of this object are initialized such that `first` is assigned a new object whereas `second` receives the output of a `Phi` node merging the object assigned to `first` and some other allocation. Also, assumes `obj` is used as debug information in an `uncommon_trap` and none of these objects escapes. I.e., we have a scenario like this:
>>
>>
>> Picture obj = new Picture(); // allocation A
>> obj.first = new Point(); // allocation B
>>
>> Point p2 = obj.first;
>> if (<cond>) p2 = new Point(); // allocation C
>>
>> obj.second = p2;
>>
>> <trap>
>>
>>
>> After one iteration of EA+SR, Allocation `A` will be scalar replaced and debug information in `<Trap>` adjusted accordingly. The description of field `second` in the debug information on `<Trap>` will, however, still involve a `Phi` node between allocation `B` and `C`. In the next iteration of EA+SR the `Phi` node for field `second` will be reduced by RAM and debug information will be adjusted accordingly. So far nothing is wrong.
>>
>> The issue happens because the existing code in `Process_OopMap_Node` to serialize debug information was missing a check. Simply, the check for setting `is_root` of an `ObjectValue` wasn't checking that the `ObjectValue` might be a description of a field of a scalar replaced object. It was only checking whether `ObjectValue` was a local, stack or monitor. Consequently, the allocation assigned to `obj.first` (yes, `first`) was **not** being marked as _root_.
>>
>> But the issue only manifested if the `<trap>` was exercised **AND** the result of `<cond>` was _true_. If the result of `<cond>` was `false` when the trap was exercised, then no problem would happen. The reason is, when `<cond>` is _true_ the `select` method in [ObjectMergeValue](https://github.com/openjdk/jdk/blob/1472124489c841642996ae984e21c533ffec8091/src/hotspot/share/code/debugInfo.cpp#L237) would flag, correctly, that the allocation inside the `if` needs to be rematerialized and the other input of the ObjectMergeValue shouldn't be rematerialized because `_is_root == false`, meaning it's just a candidate for rematerialization....
>
> Cesar Soares Lucas has updated the pull request incrementally with one additional commit since the last revision:
>
> Address PR feedback. Move test files to correct folder. Renames & comments.
Please, reverse movement of old tests. We need to backport this fix into JDK 23 and for that we need only related test.
You can move them in separate RFE in JDK 24 (current mainline).
-------------
Changes requested by kvn (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/20087#pullrequestreview-2172866104
More information about the hotspot-compiler-dev
mailing list