8358725: RunThese30M: assert(nm->insts_contains_inclusive(original_pc)) failed: original PC must be in the main code section of the compiled method (or must be immediately following it)

[dean-long]

The problem is code called from a signal handler, like SharedRuntime::handle_unsafe_access(), can call os::malloc(), and when NMT is enabled, we try to get a stack backtrace. But os::get_native_stack() does not know how to walk through signal handler frames.

This fix introduces FirstNativeFrameMark to be used by the POSIX version of os::get_native_stack() to set a frame to stop at in the POSIX signal handler.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8358725: RunThese30M: assert(nm->insts_contains_inclusive(original_pc)) failed: original PC must be in the main code section of the compiled method (or must be immediately following it) (Bug - P3)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27985/head:pull/27985
$ git checkout pull/27985

Update a local copy of the PR:
$ git checkout pull/27985
$ git pull https://git.openjdk.org/jdk.git pull/27985/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 27985

View PR using the GUI difftool:
$ git pr show -t 27985

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27985.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back dlong! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[openjdk]

@dean-long The following label will be automatically applied to this pull request:

• hotspot-runtime

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (d5a0b53e)

[dholmes-ora]

If we only the print the stack up to the signal handler, and not all the way to the allocation, then won't the resulting stack trace be confusing for the reader?

The problem is code called from a signal handler, like SharedRuntime::handle_unsafe_access(), can call os::malloc(),

Yeah and it really should not do that. :(

[dean-long]

If we only the print the stack up to the signal handler, and not all the way to the allocation, then won't the resulting stack trace be confusing for the reader?

The stack trace starts at the most recent stack frames (the allocation) and works backwards through the callers (signal handler). So the signal handler frame looks like a special entry or "first" frame, similar to a thread start function or libc init code.

[dean-long]

Yeah and it really should not do that. :(

I agree, but I am not addressing that in this fix. There might be other legitimate reasons for wanting a stack trace during a signal handler, other than NMT allocation tracking, though off the top of my head I can't think of any :-)

[dholmes-ora]

If we only the print the stack up to the signal handler, and not all the way to the allocation, then won't the resulting stack trace be confusing for the reader?

The stack trace starts at the most recent stack frames (the allocation) and works backwards through the callers (signal handler). So the signal handler frame looks like a special entry or "first" frame, similar to a thread start function or libc init code.

Sorry I'm confused about which part of the stack - that preceding the signal handler, or that after - will be printed after this fix.

[dean-long]

Sorry I'm confused about which part of the stack - that preceding the signal handler, or that after - will be printed after this fix.

After, temporally. The history before the signal handler happened is erased. This should match the meaning of os::is_first_C_frame().