[15] RFR(S): 8246203: Segmentation fault in verification due to stack overflow with -XX:+VerifyIterativeGVN

[Vladimir Kozlov]

Good.

Thanks,
Vladimir

On 6/9/20 10:32 AM, Christian Hagedorn wrote:
> Hi Vladimir
> 
> On 09.06.20 18:54, Vladimir Kozlov wrote:
>> I think the check should be 'verify_depth > 0' because verify_depth can be negative:
>>
>> +  verify_depth--; // Visiting the first node on depth 1
>> +  bool add_to_worklist = verify_depth != 0;
>>
>> Or it is intentional for negative value to visit all nodes? Then it needs comment.
> 
> Yes, with negative values it visit all nodes. There is a comment about it above:
> 
> 2155 // Verify all nodes if verify_depth is negative
> 2156 void Node::verify(Node* n, int verify_depth) {
> 
> But maybe I should add another comment for add_to_worklist as well to make it more clear.
> 
>> In such case you need restore verify_depth == 0 check to return otherwise with 0 the code will work as with negative 
>> value:
>>    if (verify_depth == 0) {
>>      return;
>>    }
>>    bool add_to_worklist = true;
>>
>> Or may be use assert(verify_depth != 0, "sanity") instead of check.
> 
> I like the solution of an assert. I added it at the start of the method together with the additional comment in a new 
> webrev. It only initializes add_to_worklist with false if verify() is called with verify_depth = 1.
> 
> http://cr.openjdk.java.net/~chagedorn/8246203/webrev.01/
> 
> Best regards,
> Christian
> 
> 
>> On 6/9/20 8:26 AM, Christian Hagedorn wrote:
>>> Hi
>>>
>>> Please review the following patch:
>>> https://bugs.openjdk.java.net/browse/JDK-8246203
>>> http://cr.openjdk.java.net/~chagedorn/8246203/webrev.00/
>>>
>>> The testcase creates a deep graph with a lot of nodes on a chain. When running with -XX:+VerifyIterativeGVN, it 
>>> recursively calls Node::verify_recur() for each input node discovered which eventually results in a segmentation 
>>> fault due to a stack overflow (around 10000 recursive 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.
>>>
>>> Thank you!
>>>
>>> Best regards,
>>> Christian