Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent javacore generation from interfering with stack walker #18711

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

gacholio
Copy link
Contributor

@gacholio gacholio commented Jan 8, 2024

Add a flag to getOwnedObjectMonitors to prevent the stack walker from asserting and marking the thread stack corrupt.

Also remove the private flag for stack corruption - it has served no purpose for many releases.

Internal 150400, 150750.

Add a flag to getOwnedObjectMonitors to prevent the stack walker from
asserting and marking the thread stack corrupt.

Also remove the private flag for stack corruption - it has served no
purpose for many releases.

Signed-off-by: Graham Chapman <[email protected]>
@gacholio
Copy link
Contributor Author

gacholio commented Jan 8, 2024

@keithc-ca Please verify that my removal of the private flag won't break DDR.

@gacholio
Copy link
Contributor Author

gacholio commented Jan 8, 2024

This is related to RTC story 150400.

@keithc-ca
Copy link
Contributor

Please verify that my removal of the private flag won't break DDR.

I don't see any potential problems for DDR.

Please identify the assertion(s) that will be avoided by this change.

@gacholio
Copy link
Contributor Author

gacholio commented Jan 8, 2024

Please identify the assertion(s) that will be avoided by this change.

It avoids the "invalid JIT return address" assertion if the stack walker is called from getOwnedObjectMonitors with the reportErrors flag FALSE (at the moment, only javacore generation calls this way).

@keithc-ca keithc-ca self-requested a review January 8, 2024 22:35
@keithc-ca
Copy link
Contributor

I don't see any potential problems for DDR.

Actually, I take that back. We should keep J9_PRIVATE_FLAGS_STACK_CORRUPT (perhaps only as a (reverse) compatibility constant in CompatibilityConstants29.dat) so it is properly recognized in old core files.

Add to CompatibilityConstants29.dat:

J9Consts.J9_PRIVATE_FLAGS_STACK_CORRUPT = 0 /* no longer used */

and don't change DDR_VM/src/com/ibm/j9ddr/vm29/j9/stackwalker/StackWalker.java.

@gacholio
Copy link
Contributor Author

gacholio commented Jan 9, 2024

The tag bit is meaningless - stacks are never corrupt when running, and the tagging of foreign threads in the javacore writer is both unsafe an incorrect. DDR should not be processing this flag at all.

@keithc-ca
Copy link
Contributor

If the flag is set - even if it was in error - DDR should not ignore it.

@gacholio
Copy link
Contributor Author

gacholio commented Jan 9, 2024

No - the only possible result of the check is that the stack walk will fail in DDR (assuming any code in DDR even checks the return value), which serves no useful purpose. Hiding a possibly bad stack from the debug tools is the precise opposite of what we want.

@keithc-ca
Copy link
Contributor

Hiding a possibly bad stack from the debug tools is the precise opposite of what we want.

Agreed. One can use the !threads command to see privateFlags if necessary.

@keithc-ca
Copy link
Contributor

Jenkins test sanity alinux jdk17

@keithc-ca keithc-ca merged commit a14234e into eclipse-openj9:master Jan 9, 2024
5 checks passed
@gacholio gacholio deleted the corrupt branch January 9, 2024 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants