8268673: Stack walk across optimized entry frame on fresh native thread fails #558
Conversation
| @@ -2306,6 +2306,7 @@ WB_ENTRY(void, WB_VerifyFrames(JNIEnv* env, jobject wb, jboolean log, jboolean u | |||
| tty_token = ttyLocker::hold_tty(); | |||
| tty->print_cr("[WhiteBox::VerifyFrames] Walking Frames"); | |||
| } | |||
| ResourceMark rm; // for verify | |||
There was a problem hiding this comment.
Ran into an assert when testing because of a missing resource mark here, so I've added that as well.
|
👋 Welcome back jvernee! A progress list of the required criteria for merging this PR into |
Webrevs
|
|
@JornVernee This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be: You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed. At the time when this comment was updated there had been no new commits pushed to the ➡️ To integrate this PR with the above commit message to the |
|
/integrate |
|
Going to push as commit 2287ca5. |
openjdk
bot
added
integrated
|
@JornVernee Pushed as commit 2287ca5. 💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored. |
|
Mailing list message from Sebastian Stenzel on panama-dev: Hi, I have pulled this fix (foreign-jextract commit e4f89d7) and can confirm that the crashes are gone. However, copying bytes from heap to native buffers still is a little odd. Take this code: [1]; [2] If I run the project with `-Djdk.internal.foreign.ProgrammableUpcallHandler.USE_INTRINSICS=false`, it copies the contents of HELLO_STR to the segment. Without the flag it copies 13 null-bytes, though. 13 being the correct length of the source buffer. I guess this is not related to the stack walking problem. The only thing I can tell is that said flag has some influence on the behaviour. Cheers! [1] my upcall method: https://github.com/skymatic/fuse-panama/blob/3e5dc43ec7a6ba7e2c39fcce0db48d4350fdc0b3/src/main/java/de/skymatic/fusepanama/FuseOperations.java#L222-L227 <https://github.com/skymatic/fuse-panama/blob/3e5dc43ec7a6ba7e2c39fcce0db48d4350fdc0b3/src/main/java/de/skymatic/fusepanama/FuseOperations.java#L222-L227> |
1 similar comment
|
Mailing list message from Sebastian Stenzel on panama-dev: Hi, I have pulled this fix (foreign-jextract commit e4f89d7) and can confirm that the crashes are gone. However, copying bytes from heap to native buffers still is a little odd. Take this code: [1]; [2] If I run the project with `-Djdk.internal.foreign.ProgrammableUpcallHandler.USE_INTRINSICS=false`, it copies the contents of HELLO_STR to the segment. Without the flag it copies 13 null-bytes, though. 13 being the correct length of the source buffer. I guess this is not related to the stack walking problem. The only thing I can tell is that said flag has some influence on the behaviour. Cheers! [1] my upcall method: https://github.com/skymatic/fuse-panama/blob/3e5dc43ec7a6ba7e2c39fcce0db48d4350fdc0b3/src/main/java/de/skymatic/fusepanama/FuseOperations.java#L222-L227 <https://github.com/skymatic/fuse-panama/blob/3e5dc43ec7a6ba7e2c39fcce0db48d4350fdc0b3/src/main/java/de/skymatic/fusepanama/FuseOperations.java#L222-L227> |
|
Mailing list message from Jorn Vernee on panama-dev: Hi Sebastian, This sounds very odd, as the code you link to doesn't seem to use May I ask how exactly you're diagnosing this problem? Jorn On 15/06/2021 15:48, Sebastian Stenzel wrote:
|
1 similar comment
|
Mailing list message from Jorn Vernee on panama-dev: Hi Sebastian, This sounds very odd, as the code you link to doesn't seem to use May I ask how exactly you're diagnosing this problem? Jorn On 15/06/2021 15:48, Sebastian Stenzel wrote:
|
|
Mailing list message from Sebastian Stenzel on panama-dev: A reference to the first mentioned method is stored in a struct [1] (defined in fuse_operations.h) right here [2], which then gets passed to FUSE. FUSE then decides to call some of these methods when a corresponding file system access happens. [1]: https://github.com/skymatic/fuse-panama/blob/3e5dc43ec7a6ba7e2c39fcce0db48d4350fdc0b3/src/main/java/de/skymatic/fusepanama/lowlevel/fuse_operations.java#L28 <https://github.com/skymatic/fuse-panama/blob/3e5dc43ec7a6ba7e2c39fcce0db48d4350fdc0b3/src/main/java/de/skymatic/fusepanama/lowlevel/fuse_operations.java#L28>
|
|
Mailing list message from Jorn Vernee on panama-dev: Ok thanks, If upcalls are involved it makes sense then that the Thanks for testing, I'll try and reproduce the issue here as well. Jorn On 15/06/2021 17:10, Sebastian Stenzel wrote:
|
|
Mailing list message from Jorn Vernee on panama-dev: Hi Sebastian, I was not able to reproduce your specific problem, but I did find I've just integrated a patch that fixes the issue: If you have an opportunity to test out the fix, that would be greatly Thanks, On 15/06/2021 17:46, Jorn Vernee wrote:
|
1 similar comment
|
Mailing list message from Jorn Vernee on panama-dev: Hi Sebastian, I was not able to reproduce your specific problem, but I did find I've just integrated a patch that fixes the issue: If you have an opportunity to test out the fix, that would be greatly Thanks, On 15/06/2021 17:46, Jorn Vernee wrote:
|
|
Mailing list message from Sebastian Stenzel on panama-dev: Hi Jorn, yes, I can confirm this fixes the issue. I tested on foreign-jextract, where the issue was still present on 6f8f9e2 and fixed on 42e03fd. Cheers!
|
1 similar comment
|
Mailing list message from Sebastian Stenzel on panama-dev: Hi Jorn, yes, I can confirm this fixes the issue. I tested on foreign-jextract, where the issue was still present on 6f8f9e2 and fixed on 42e03fd. Cheers!
|
|
Mailing list message from Jorn Vernee on panama-dev: Great! Thanks for testing, On 09/07/2021 09:58, Sebastian Stenzel wrote:
|
|
Mailing list message from Sebastian Stenzel on panama-dev: Btw: Any chance this will get backported toJDK-17?
|
1 similar comment
|
Mailing list message from Sebastian Stenzel on panama-dev: Btw: Any chance this will get backported toJDK-17?
|
|
Mailing list message from Jorn Vernee on panama-dev: Yes, we are in the process of doing that, but currently a lot of the Jorn On 09/07/2021 14:27, Sebastian Stenzel wrote:
|
1 similar comment
|
Mailing list message from Jorn Vernee on panama-dev: Yes, we are in the process of doing that, but currently a lot of the Jorn On 09/07/2021 14:27, Sebastian Stenzel wrote:
|
Hi,
When native code creates a new thread and calls a Panama upcall, and during that upcall a stack walk is triggered, getting the sender frame for the entry frame is not possible, and should not be attempted.
For JNI this case is handled already by indicating the end of the stack frame stream, but for Panama upcalls it is not, and the VM will either hit an assert or crash when trying to find the last Java frame before the entry frame (which does not exist in this case).
This patch adds handling for panama upcalls frames to
frame::is_first_frame, which is used by the stack walking code to determine when to stop walking.Thanks,
Jorn
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.java.net/panama-foreign pull/558/head:pull/558$ git checkout pull/558Update a local copy of the PR:
$ git checkout pull/558$ git pull https://git.openjdk.java.net/panama-foreign pull/558/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 558View PR using the GUI difftool:
$ git pr show -t 558Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/panama-foreign/pull/558.diff