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

[Andrew Haley]

On Tue, 1 Nov 2022 23:51:08 GMT, Vladimir Ivanov <vlivanov at openjdk.org> wrote:

>> This patch fixes the remaining null pointer dereference bugs that I know of.
>> 
>> For the main bug, C2 was using a null reference to indicate an uninitialized `Node_List`. I replaced the null reference with a static sentinel.
>> 
>> I also turned on `-fsanitize=null` and found and fixed a bunch of other null pointer dereferences. With this,I have run a full bootstrap and tier1 tests with `-fsanitize=null` enabled.
>> 
>> I have checked that the code generated by GCC is not worse in any significant way, so I don't expect to see any performance regressions.
>> 
>> I'd like to enable `-fsanitize=null` in debug builds to prevent regressions in this area. What do you think?
>
> src/hotspot/share/oops/instanceKlass.cpp line 390:
> 
>> 388:   // Record dependency to keep nest host from being unloaded before this class.
>> 389:   ClassLoaderData* this_key = class_loader_data();
>> 390:   if (this_key != NULL) {
> 
> The code assumes `this_key != NULL`. Do we need an assert/guarantee here?

I did see this one trigger, otherwise I wouldn't have known about it, but I can't reproduce it today. Whether it's an assert or a guarantee depends on how serious the problem would be.

> src/hotspot/share/opto/node.hpp line 1528:
> 
>> 1526: public:
>> 1527:   Node_Array(Arena* a, uint max = OptoNodeListSize) : _a(a), _max(max) {
>> 1528:     if (a != NULL) {
> 
> Add `assert(a != NULL, "...")` here?

This change is to allow the creation of a null sentinel. See `Node_List::_empty_list((Arena*)NULL)`

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

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