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

[Christian Hagedorn]

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