RFR: 8344013: "bad tag in log" assert with +LogCompilation +CITimeVerbose

[Christian Hagedorn]

On Fri, 22 Nov 2024 18:53:28 GMT, Sonia Zaldana Calles <szaldana at openjdk.org> wrote:

> Hi folks, 
> 
> This PR addresses [8344013](https://bugs.openjdk.org/browse/JDK-8344013).
> 
> Sometimes the writing to xmlStream is mixed from several threads, and therefore the xmlStream tag stack can end up in a bad state. When this occurs, the VM crashes in `xmlStream::pop_tag` with `assert(false) failed: bad tag in log`. 
> 
> In this case, running `java -XX:+LogCompilation -XX:CompileCommand="log,*.*" -XX:+CITimeVerbose -Xcomp -Xbatch -version` , `xmlStream::pop_tag` is expecting to pop the tag `task` but finds `phase` instead. 
> 
> I found the issue stems from [8330157](https://bugs.openjdk.org/browse/JDK-8330157). The problematic code is in the destructor for [TracePhase](https://github.com/openjdk/jdk/blame/master/src/hotspot/share/opto/compile.cpp#L4337).
> 
> Note how the constructor adds the [phase tag](https://github.com/openjdk/jdk/blame/master/src/hotspot/share/opto/compile.cpp#L4327).
> 
> However, in the destructor, if we return early, we don’t pop that tag, leading to the xmlStream tag stack to end up in a bad state. With this patch, I made sure we pop the tag even if we return early. 
> 
> Cheers, 
> Sonia

Looks good to me. Can you also add a regression test for it? Since this already triggers with `--version`, you can just create a hello world like test and run with the mentioned flags.

Just a side note:

> Note how the constructor adds the [phase tag](https://github.com/openjdk/jdk/blame/master/src/hotspot/share/opto/compile.cpp#L4327).

You can directly insert the permalink (in the non-blame view) such that the code is inlined in the PR for easier reading :-)

https://github.com/openjdk/jdk/blob/6f622da7fbae67d8c1cd9e795127adac58a246a9/src/hotspot/share/opto/compile.cpp#L4327

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

PR Review: https://git.openjdk.org/jdk/pull/22331#pullrequestreview-2457422083