Skip to content

Downstream aarch64 related fixes #154

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.

Sign up for GitHub

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

Merged
r1viollet merged 4 commits into from
Dec 5, 2024
Merged

Downstream aarch64 related fixes #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cbe3d13
Thread-safe lazy initialization pattern
jbachorik Nov 25, 2024
08c2e28
Add GHA aarch64 jobs
jbachorik Nov 12, 2024
729d9f9
#819: Extend AArch64 stack walking fix to cstack=dwarf
apangin Jan 16, 2024
dcc8730
#893: Fix [no_Java_frame] on ARM64
apangin Mar 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
275 changes: 221 additions & 54 deletions .github/workflows/test_workflow.yml
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 0 additions & 2 deletions ddprof-lib/src/main/cpp/libraries.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
#include "vmEntry.h"
#include "vmStructs.h"

Libraries* Libraries::_instance = new Libraries();

void Libraries::mangle(const char *name, char *buf, size_t size) {
char *buf_end = buf + size;
strcpy(buf, "_ZN");
Expand Down
11 changes: 8 additions & 3 deletions ddprof-lib/src/main/cpp/libraries.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@

class Libraries {
private:
static Libraries * _instance;

CodeCacheArray _native_libs;
CodeCache _runtime_stubs;

Expand All @@ -21,7 +19,14 @@ class Libraries {
CodeCache *findLibraryByName(const char *lib_name);
CodeCache *findLibraryByAddress(const void *address);

static Libraries *instance() { return _instance; }
static Libraries *instance() {
static Libraries instance;
return &instance;
}

// Delete copy constructor and assignment operator to prevent copies
Libraries(const Libraries&) = delete;
Libraries& operator=(const Libraries&) = delete;
};

#endif // _LIBRARIES_H
1 change: 0 additions & 1 deletion ddprof-lib/src/main/cpp/livenessTracker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
#include <jni.h>
#include <string.h>

LivenessTracker *const LivenessTracker::_instance = new LivenessTracker();
constexpr int LivenessTracker::MAX_TRACKING_TABLE_SIZE;
constexpr int LivenessTracker::MIN_SAMPLING_INTERVAL;

Expand Down
10 changes: 7 additions & 3 deletions ddprof-lib/src/main/cpp/livenessTracker.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,14 @@ class LivenessTracker {

jlong getMaxMemory(JNIEnv *env);

static LivenessTracker *const _instance;

public:
static LivenessTracker *instance() { return _instance; }
static LivenessTracker *instance() {
static LivenessTracker instance;
return &instance;
}
// Delete copy constructor and assignment operator to prevent copies
LivenessTracker(const LivenessTracker&) = delete;
LivenessTracker& operator=(const LivenessTracker&) = delete;

LivenessTracker()
: _initialized(false), _enabled(false), _stored_error(Error::OK),
Expand Down
3 changes: 2 additions & 1 deletion ddprof-lib/src/main/cpp/stackFrame.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,9 @@ class StackFrame {
bool unwindStub(instruction_t *entry, const char *name, uintptr_t &pc,
uintptr_t &sp, uintptr_t &fp);
bool unwindCompiled(NMethod *nm, uintptr_t &pc, uintptr_t &sp, uintptr_t &fp);
bool unwindAtomicStub(const void*& pc);

void adjustCompiled(NMethod *nm, const void *pc, uintptr_t &sp);
void adjustSP(const void *entry, const void *pc, uintptr_t &sp);

bool skipFaultInstruction();

Expand Down
16 changes: 14 additions & 2 deletions ddprof-lib/src/main/cpp/stackFrame_aarch64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

@MattAlp MattAlp Dec 5, 2024

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

const void* lr = (const void*)link();
if (VMStructs::libjvm()->contains(lr)) {
NMethod* nm = CodeHeap::findNMethod(pc);
if (nm != NULL && strncmp(nm->name(), "Stub", 4) == 0) {
pc = lr;
return true;
}
}
return false;
}

void StackFrame::adjustSP(const void *entry, const void *pc, uintptr_t &sp) {
instruction_t *ip = (instruction_t *)pc;
instruction_t *entry = (instruction_t *)nm->entry();
if (ip > entry && (ip[-1] == 0xa9bf27ff ||
(ip[-1] == 0xd63f0100 && ip[-2] == 0xa9bf27ff))) {
// When calling a leaf native from Java, JVM puts a dummy frame link onto
Expand Down
7 changes: 6 additions & 1 deletion ddprof-lib/src/main/cpp/stackFrame_arm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,15 @@ bool StackFrame::unwindCompiled(NMethod *nm, uintptr_t &pc, uintptr_t &sp,
return true;
}

void StackFrame::adjustCompiled(NMethod *nm, const void *pc, uintptr_t &sp) {
void StackFrame::adjustSP(const void *entry, const void *pc, uintptr_t &sp) {
// Not needed
}

bool StackFrame::unwindAtomicStub(const void*& pc) {
// Not needed
return false;
}

bool StackFrame::skipFaultInstruction() { return false; }

bool StackFrame::checkInterruptedSyscall() {
Expand Down
7 changes: 6 additions & 1 deletion ddprof-lib/src/main/cpp/stackFrame_i386.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,15 @@ bool StackFrame::unwindCompiled(NMethod *nm, uintptr_t &pc, uintptr_t &sp,
return false;
}

void StackFrame::adjustCompiled(NMethod *nm, const void *pc, uintptr_t &sp) {
void StackFrame::adjustSP(const void *entry, const void *pc, uintptr_t &sp) {
// Not needed
}

bool StackFrame::unwindAtomicStub(const void*& pc) {
// Not needed
return false;
}

bool StackFrame::skipFaultInstruction() { return false; }

bool StackFrame::checkInterruptedSyscall() {
Expand Down
7 changes: 6 additions & 1 deletion ddprof-lib/src/main/cpp/stackFrame_ppc64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,15 @@ bool StackFrame::unwindCompiled(NMethod *nm, uintptr_t &pc, uintptr_t &sp,
return true;
}

void StackFrame::adjustCompiled(NMethod *nm, const void *pc, uintptr_t &sp) {
void StackFrame::adjustSP(const void *entry, const void *pc, uintptr_t &sp) {
// Not needed
}

bool StackFrame::unwindAtomicStub(const void*& pc) {
// Not needed
return false;
}

bool StackFrame::skipFaultInstruction() { return false; }

bool StackFrame::checkInterruptedSyscall() {
Expand Down
7 changes: 6 additions & 1 deletion ddprof-lib/src/main/cpp/stackFrame_x64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,15 @@ bool StackFrame::unwindCompiled(NMethod *nm, uintptr_t &pc, uintptr_t &sp,
return false;
}

void StackFrame::adjustCompiled(NMethod *nm, const void *pc, uintptr_t &sp) {
void StackFrame::adjustSP(const void *entry, const void *pc, uintptr_t &sp) {
// Not needed
}

bool StackFrame::unwindAtomicStub(const void*& pc) {
// Not needed
return false;
}

// Skip failed MOV instruction by writing 0 to destination register
bool StackFrame::skipFaultInstruction() {
unsigned int insn = *(unsigned int *)pc();
Expand Down
9 changes: 6 additions & 3 deletions ddprof-lib/src/main/cpp/stackWalker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ int StackWalker::walkFP(void *ucontext, const void **callchain, int max_depth,
uintptr_t sp;
uintptr_t bottom = (uintptr_t)&sp + MAX_WALK_SIZE;

StackFrame frame(ucontext);
if (ucontext == NULL) {
pc = __builtin_return_address(0);
fp = (uintptr_t)__builtin_frame_address(1);
Expand All @@ -92,7 +93,7 @@ int StackWalker::walkFP(void *ucontext, const void **callchain, int max_depth,
*truncated = true;
break;
}
if (CodeHeap::contains(pc)) {
if (CodeHeap::contains(pc) && !(depth == 0 && frame.unwindAtomicStub(pc))) {
java_ctx->set(pc, sp, fp);
break;
}
Expand Down Expand Up @@ -154,7 +155,9 @@ int StackWalker::walkDwarf(void *ucontext, const void **callchain,
*truncated = true;
break;
}
if (CodeHeap::contains(pc)) {
if (CodeHeap::contains(pc) && !(depth == 0 && frame.unwindAtomicStub(pc))) {
const void* page_start = (const void*)((uintptr_t)pc & ~0xfffUL);
frame.adjustSP(page_start, pc, sp);
java_ctx->set(pc, sp, fp);
break;
}
Expand Down Expand Up @@ -302,7 +305,7 @@ int StackWalker::walkVM(void *ucontext, ASGCT_CallFrame *frames, int max_depth,

// Handle situations when sp is temporarily changed in the compiled
// code
frame.adjustCompiled(nm, pc, sp);
frame.adjustSP(nm->entry(), pc, sp);

sp += nm->frameSize() * sizeof(void *);
fp = ((uintptr_t *)sp)[-FRAME_PC_SLOT - 1];
Expand Down
3 changes: 3 additions & 0 deletions ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,9 +142,12 @@ protected boolean isPlatformSupported() {
return true;
}

protected void withTestAssumptions() {}

@BeforeEach
public void setupProfiler() throws Exception {
Assumptions.assumeTrue(isPlatformSupported());
withTestAssumptions();

jfrDump = Files.createTempFile(Paths.get("/tmp"), getClass().getName() + UUID.randomUUID(), ".jfr");
profiler = JavaProfiler.getInstance();
Expand Down
9 changes: 9 additions & 0 deletions ddprof-test/src/test/java/com/datadoghq/profiler/cpu/VMStructsBasedCpuTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.datadoghq.profiler.AbstractProfilerTest;
import com.datadoghq.profiler.Platform;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Test;
import org.openjdk.jmc.common.item.IItem;
import org.openjdk.jmc.common.item.IItemCollection;
Expand All @@ -18,8 +19,16 @@ public class VMStructsBasedCpuTest extends AbstractProfilerTest {

private ProfiledCode profiledCode;

@Override
protected void withTestAssumptions() {
// vmstructs based stackwalking will throw ISE on Java 23
Assumptions.assumeFalse(Platform.isJavaVersionAtLeast(23));
Copy link
Contributor

@r1viollet r1viollet Dec 5, 2024

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.

}

@Override
protected void before() {
// vmstructs based stackwalking will throw ISE on Java 23
Assumptions.assumeFalse(Platform.isJavaVersionAtLeast(23));
profiledCode = new ProfiledCode(profiler);
}

Expand Down
Loading