RFR: 8331194: NPE in ArrayCreationTree.java with -XX:-UseCompressedOops [v5]

[Emanuel Peter]

On Fri, 12 Jul 2024 17:22:12 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:
> 
>   Fix test package path.

test/hotspot/jtreg/compiler/escapeAnalysis/TestReduceAllocationAndNestedScalarized.java line 42:

> 40:  *                   -server
> 41:  *                   compiler.escapeAnalysis.TestReduceAllocationAndNestedScalarized
> 42:  * @run main/othervm compiler.escapeAnalysis.TestReduceAllocationAndNestedScalarized

Suggestion:

 * @run main compiler.escapeAnalysis.TestReduceAllocationAndNestedScalarized

You can run it directly in the main vm, with the external flags applied. othervm creates an extra vm which is unnecessary if you do not have additional arguments. So your first run needs othervm, but not your second ;)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20087#discussion_r1678970105