-
Notifications
You must be signed in to change notification settings - Fork 7
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
Downstream aarch64 related fixes #154
Conversation
🔧 Report generated by pr-comment-scanbuild Scan-Build Report
Bug Summary
Reports
|
702725a
to
a0d07f9
Compare
76c46d3
to
dcc8730
Compare
@Override | ||
protected void withTestAssumptions() { | ||
// vmstructs based stackwalking will throw ISE on Java 23 | ||
Assumptions.assumeFalse(Platform.isJavaVersionAtLeast(23)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume we do not use vmstructs for now on 23. Happy to talk more about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I'm especially happy about the arm tests we are adding
@@ -136,9 +136,21 @@ bool StackFrame::unwindCompiled(NMethod *nm, uintptr_t &pc, uintptr_t &sp, | |||
return true; | |||
} | |||
|
|||
void StackFrame::adjustCompiled(NMethod *nm, const void *pc, uintptr_t &sp) { | |||
bool StackFrame::unwindAtomicStub(const void*& pc) { | |||
// VM threads may call generated atomic stubs, which are not normally walkable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, leaving comment here to refer back later
What does this PR do?:
It downstreams two aarch64 related fixes
Motivation:
Improving the aarch64 profiling stability
Additional Notes:
Additional aarch64 GHA runners were added. This also required some minor adjustments in the code to avoid crashes on those aarch64 runners (these things are most probably setup specific as I could not reproduce the problems on an aarch64 ec2 instance)
How to test the change?:
For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance
.Unsure? Have a question? Request a review!