[9] RFR (XXS): 8062258: compiler/debug/TraceIterativeGVN.java segfaults in trace_PhaseIterGVN

[Vladimir Kozlov]

Unfortunately not only MemNode has Type::MEMORY type. PhiNode, 
SCMemProjNode and others. It looks like you need to fix all places.
Change in memnode.cpp is fine but in node.cpp you need:

     } else if (t == Type::MEMORY && (!is_Mem() || is_LoadStore())) {
       st->print("  Memory:");
       MemNode::dump_adr_type(this, adr_type(), st);

Make sure that other Type::MEMORY nodes don't crash in adr_type().
Don't do check under NOT_PRODUCT as in your first fix - just return NULL 
in all cases.

Thanks,
Vladimir

On 10/30/14 1:48 PM, Vladimir Ivanov wrote:
> Vladimir, nice observation!
>
> http://cr.openjdk.java.net/~vlivanov/8062258/webrev.01/
>
> Best regards,
> Vladimir Ivanov
>
> On 10/30/14, 11:25 PM, Vladimir Kozlov wrote:
>> Hmm, next code in PhaseIterGVN::optimize() worries me. We dump dead
>> nodes before removing them from graph:
>>
>>      DEBUG_ONLY(trace_PhaseIterGVN_verbose(n, num_processed++);)
>>      if (n->outcnt() != 0) {
>>        NOT_PRODUCT(const Type* oldtype = type_or_null(n));
>>        // Do the transformation
>>        Node* nn = transform_old(n);
>>        NOT_PRODUCT(trace_PhaseIterGVN(n, nn, oldtype);)
>>      } else if (!n->is_top()) {
>>        remove_dead_node(n);
>>      }
>>
>> I am changing my suggestion: fix  Node::dump() which calls  adr_type()
>> without check. It should not call MemNode::dump_adr_type() because
>> MemNode::dump_spec(), which is called before, already does that and it
>> has NULL check! If you want, you can add "  Memory:" output in
>> MemNode::dump_spec() to be similar to Node::dump() output.
>>
>> Thanks,
>> Vladimir
>>
>> On 10/30/14 10:26 AM, Vladimir Ivanov wrote:
>>> Vladimir, do you suggest to simply skip printing info about dead nodes
>>> w/ -XX:+TraceIterativeGVN?
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> On 10/29/14, 7:00 PM, Vladimir Kozlov wrote:
>>>> I would suggest to dead node check in trace_PhaseIterGVN() instead of
>>>> in  MemNode::adr_type().
>>>> So the fix for your JDK-8061995 should fix this problem too.
>>>>
>>>> Thanks,
>>>> Vladimir
>>>>
>>>> On 10/29/14 4:57 AM, Vladimir Ivanov wrote:
>>>>> http://cr.openjdk.java.net/~vlivanov/8062258/webrev.00/
>>>>> https://bugs.openjdk.java.net/browse/JDK-8062258
>>>>>
>>>>> Trivial fix to avoid NULL dereference w/ -XX:+TraceIterativeGVN while
>>>>> querying adr_type() on dead MemNode.
>>>>>
>>>>> Testing: failing test.
>>>>>
>>>>> Best regards,
>>>>> Vladimir Ivanov