[16] RFR(S): 8247743: Segmentation fault in debug builds due to stack overflow in find_recur with deep graphs

[Christian Hagedorn]

Hi Vladimir

On 13.07.20 19:43, Vladimir Kozlov wrote:
> Node::find_ctrl() is used during debugging when you want to print and 
> look on only control nodes.
> We have several such methods which are only used in debugger.

I see, I restored this method and changed Node::find() accordingly. I 
additionally added two find_ctrl() methods to make it easier to call it 
from a debugger (as already present for find_node()).

> I suggest to store old_arena() in local var and pass into 
> add_to_worklist().
> 
> You can make add_to_worklist() static since you pass node as argument.

Okay. I updated this and the change above in a new webrev:
http://cr.openjdk.java.net/~chagedorn/8247743/webrev.01/

Best regards,
Christian

> Thanks,
> Vladimir
> 
> On 7/13/20 3:09 AM, Christian Hagedorn wrote:
>> Ping - could anyone review it, please? Thanks!
>>
>> Best regards,
>> Christian
>>
>> On 02.07.20 09:33, Christian Hagedorn wrote:
>>> Hi
>>>
>>> Please review the following patch:
>>> https://bugs.openjdk.java.net/browse/JDK-8247743
>>> http://cr.openjdk.java.net/~chagedorn/8247743/webrev.00/
>>>
>>> The testcase creates a deep graph with a lot of nodes on a chain. 
>>> When running with the specified test flags, it recursively calls 
>>> Node::find_recur() for each node discovered which eventually results 
>>> in a segmentation fault due to a stack overflow (around 10000 calls 
>>> due to such a long chain of nodes). The fix just converts the 
>>> recursive algorithm into an iterative one to avoid a segmentation 
>>> fault. This is similar to JDK-8246203 [1].
>>>
>>> I additionally removed Node::find_ctrl() and its special handling in 
>>> the algorithm since it is not used.
>>>
>>> There is actually another problem with the recursive version. When 
>>> running the testcase without 
>>> -XX:CompileOnly=compiler/c2/TestFindNode, it will spin forever inside 
>>> [2] because there is a debug_orig node cycle and the loop does not 
>>> break based on the debug_orig nodes being visited. This is also fixed 
>>> in the patch.
>>>
>>> Thank you!
>>>
>>> Best regards,
>>> Christian
>>>
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8246203
>>> [2] 
>>> http://hg.openjdk.java.net/jdk/jdk/file/e2622818f0bd/src/hotspot/share/opto/node.cpp#l1589 
>>>