RFR: 8365604: Null pointer dereference in src/hotspot/share/adlc/output_h.cpp ArchDesc::declareClasses() [v2]

[Dean Long]

On Thu, 21 Aug 2025 09:01:12 GMT, Artem Semenov <asemenov at openjdk.org> wrote:

>> The defect has been detected and confirmed in the function ArchDesc::declareClasses() located in the file src/hotspot/share/adlc/output_h.cpp with static code analysis. This defect can potentially lead to a null pointer dereference.
>> 
>> The pointer instr->_matrule is dereferenced in line 1952 without checking for nullptr, although earlier in line 1858 the same pointer is checked for nullptr, which indicates that it can be null.
>> 
>> According to [this](https://github.com/openjdk/jdk/pull/26002#issuecomment-3023050372) comment, this PR contains fixes for similar cases in other places.
>
> Artem Semenov has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Update src/hotspot/share/c1/c1_LinearScan.cpp
>    
>    Co-authored-by: David Holmes <62092539+dholmes-ora at users.noreply.github.com>
>  - Update src/hotspot/share/adlc/output_h.cpp
>    
>    Co-authored-by: David Holmes <62092539+dholmes-ora at users.noreply.github.com>

Most of these mitigations to guard against a possible null pointer dereference are inside `if` expressions, which means if there was a null pointer, then we will now end up in the `else` clause, changing the behavior of the code to something that was perhaps unintended, and we still don't know what caused the null pointer.  So this is just silently masking potential problems, and in my experience is usually not the correct fix.  Most of the time the correct fix is to tell the static analyzer that it is a false positive and move on.  Sometimes it is appropriate to add an assert or guarantee, and yes sometimes it is appropriate to do something different if there is a null, for example if it is a result of an allocation that can fail.

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

PR Comment: https://git.openjdk.org/jdk/pull/26798#issuecomment-3212300703