RFR: JDK-8294902: Undefined Behavior in C2 regalloc with null references [v4]

[Vladimir Ivanov]

On Wed, 30 Nov 2022 18:35:37 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Sorry, missed a couple of null checks.  The following patch on top of the previous one passes hs-tier1/2:
>> 
>> diff --git a/src/hotspot/share/opto/postaloc.cpp b/src/hotspot/share/opto/postaloc.cpp
>> index 10c9d1f90ae..b39a78eef48 100644
>> --- a/src/hotspot/share/opto/postaloc.cpp
>> +++ b/src/hotspot/share/opto/postaloc.cpp
>> @@ -87,7 +87,8 @@ int PhaseChaitin::yank(Node *old, Block *current_block, Node_List *value, Node_L
>>    }
>>    _cfg.unmap_node_from_block(old);
>>    OptoReg::Name old_reg = lrgs(_lrg_map.live_range_id(old)).reg();
>> -  if (regnd != NULL && regnd->at(old_reg) == old) { // Instruction is currently available?
>> +  assert(value != NULL || regnd == NULL, "sanity");
>> +  if (value != NULL && regnd != NULL && regnd->at(old_reg) == old) { // Instruction is currently available?
>>      value->map(old_reg, NULL); // Yank from value/regnd maps
>>      regnd->map(old_reg, NULL); // This register's value is now unknown
>>    }
>> @@ -257,7 +258,8 @@ int PhaseChaitin::elide_copy( Node *n, int k, Block *current_block, Node_List *v
>>      return blk_adjust;          // Only check stupid copies!
>>    }
>>    // Loop backedges won't have a value-mapping yet
>> -  if (value == NULL) {
>> +  assert(regnd != NULL || value == NULL, "sanity");
>> +  if (value == NULL || regnd == NULL) {
>>      return blk_adjust;
>>    }
>>    // Skip through all copies to the _value_ being used.  Do not change from
>
> Done. If you're happy with this I'll push after tests. Thanks!

Thanks. Is the null check in `Node_Array` constructor still needed?

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

PR: https://git.openjdk.org/jdk/pull/10920