Skip to content

Commit 1392de7

Browse files
zhengyu123zhengyu123
authored
Safer type casting (#363)
1 parent 76205bd commit 1392de7

19 files changed

Lines changed: 277 additions & 185 deletions

‎ddprof-lib/src/main/cpp/javaApi.cpp‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ Java_com_datadoghq_profiler_JVMAccess_findStringJVMFlag0(JNIEnv *env,
348348
jobject unused,
349349
jstring flagName) {
350350
JniString flag_str(env, flagName);
351-
JVMFlag *f = JVMFlag::find(flag_str.c_str(), {JVMFlag::Type::String, JVMFlag::Type::Stringlist});
351+
VMFlag *f = VMFlag::find(flag_str.c_str(), {VMFlag::Type::String, VMFlag::Type::Stringlist});
352352
if (f) {
353353
char** value = static_cast<char**>(f->addr());
354354
if (value != NULL && *value != NULL) {
@@ -365,7 +365,7 @@ Java_com_datadoghq_profiler_JVMAccess_setStringJVMFlag0(JNIEnv *env,
365365
jstring flagValue) {
366366
JniString flag_str(env, flagName);
367367
JniString value_str(env, flagValue);
368-
JVMFlag *f = JVMFlag::find(flag_str.c_str(), {JVMFlag::Type::String, JVMFlag::Type::Stringlist});
368+
VMFlag *f = VMFlag::find(flag_str.c_str(), {VMFlag::Type::String, VMFlag::Type::Stringlist});
369369
if (f) {
370370
char** value = static_cast<char**>(f->addr());
371371
if (value != NULL) {
@@ -379,7 +379,7 @@ Java_com_datadoghq_profiler_JVMAccess_findBooleanJVMFlag0(JNIEnv *env,
379379
jobject unused,
380380
jstring flagName) {
381381
JniString flag_str(env, flagName);
382-
JVMFlag *f = JVMFlag::find(flag_str.c_str(), {JVMFlag::Type::Bool});
382+
VMFlag *f = VMFlag::find(flag_str.c_str(), {VMFlag::Type::Bool});
383383
if (f) {
384384
char* value = static_cast<char*>(f->addr());
385385
if (value != NULL) {
@@ -395,7 +395,7 @@ Java_com_datadoghq_profiler_JVMAccess_setBooleanJVMFlag0(JNIEnv *env,
395395
jstring flagName,
396396
jboolean flagValue) {
397397
JniString flag_str(env, flagName);
398-
JVMFlag *f = JVMFlag::find(flag_str.c_str(), {JVMFlag::Type::Bool});
398+
VMFlag *f = VMFlag::find(flag_str.c_str(), {VMFlag::Type::Bool});
399399
if (f) {
400400
char* value = static_cast<char*>(f->addr());
401401
if (value != NULL) {
@@ -409,7 +409,7 @@ Java_com_datadoghq_profiler_JVMAccess_findIntJVMFlag0(JNIEnv *env,
409409
jobject unused,
410410
jstring flagName) {
411411
JniString flag_str(env, flagName);
412-
JVMFlag *f = JVMFlag::find(flag_str.c_str(), {JVMFlag::Type::Int, JVMFlag::Type::Uint, JVMFlag::Type::Intx, JVMFlag::Type::Uintx, JVMFlag::Type::Uint64_t, JVMFlag::Type::Size_t});
412+
VMFlag *f = VMFlag::find(flag_str.c_str(), {VMFlag::Type::Int, VMFlag::Type::Uint, VMFlag::Type::Intx, VMFlag::Type::Uintx, VMFlag::Type::Uint64_t, VMFlag::Type::Size_t});
413413
if (f) {
414414
long* value = static_cast<long*>(f->addr());
415415
if (value != NULL) {
@@ -424,7 +424,7 @@ Java_com_datadoghq_profiler_JVMAccess_findFloatJVMFlag0(JNIEnv *env,
424424
jobject unused,
425425
jstring flagName) {
426426
JniString flag_str(env, flagName);
427-
JVMFlag *f = JVMFlag::find(flag_str.c_str(),{ JVMFlag::Type::Double});
427+
VMFlag *f = VMFlag::find(flag_str.c_str(),{ VMFlag::Type::Double});
428428
if (f) {
429429
double* value = static_cast<double*>(f->addr());
430430
if (value != NULL) {

‎ddprof-lib/src/main/cpp/profiler.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ int Profiler::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames,
587587
VM::_asyncGetCallTrace(&trace, max_depth, ucontext);
588588
}
589589
} else if (VMStructs::hasMethodStructs()) {
590-
NMethod *nmethod = CodeHeap::findNMethod((const void *)frame.pc());
590+
VMNMethod *nmethod = CodeHeap::findNMethod((const void *)frame.pc());
591591
if (nmethod != NULL && nmethod->isNMethod() && nmethod->isAlive()) {
592592
VMMethod *method = nmethod->method();
593593
if (method != NULL) {
@@ -623,7 +623,7 @@ int Profiler::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames,
623623
}
624624
} else if (trace.num_frames == ticks_unknown_not_Java &&
625625
!(_safe_mode & LAST_JAVA_PC)) {
626-
JavaFrameAnchor* anchor = vm_thread->anchor();
626+
VMJavaFrameAnchor* anchor = vm_thread->anchor();
627627
uintptr_t sp = anchor->lastJavaSP();
628628
const void* pc = anchor->lastJavaPC();
629629
if (sp != 0 && pc == NULL) {
@@ -632,7 +632,7 @@ int Profiler::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames,
632632
pc = ((const void**)sp)[-1];
633633
anchor->setLastJavaPC(pc);
634634

635-
NMethod *m = CodeHeap::findNMethod(pc);
635+
VMNMethod *m = CodeHeap::findNMethod(pc);
636636
if (m != NULL) {
637637
// AGCT fails if the last Java frame is a Runtime Stub with an invalid
638638
// _frame_complete_offset. In this case we patch _frame_complete_offset
@@ -650,13 +650,13 @@ int Profiler::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames,
650650
}
651651
} else if (trace.num_frames == ticks_not_walkable_not_Java &&
652652
!(_safe_mode & LAST_JAVA_PC)) {
653-
JavaFrameAnchor* anchor = vm_thread->anchor();
653+
VMJavaFrameAnchor* anchor = vm_thread->anchor();
654654
uintptr_t sp = anchor->lastJavaSP();
655655
const void* pc = anchor->lastJavaPC();
656656
if (sp != 0 && pc != NULL) {
657657
// Similar to the above: last Java frame is set,
658658
// but points to a Runtime Stub with an invalid _frame_complete_offset
659-
NMethod *m = CodeHeap::findNMethod(pc);
659+
VMNMethod *m = CodeHeap::findNMethod(pc);
660660
if (m != NULL && !m->isNMethod() && m->frameSize() > 0 &&
661661
m->frameCompleteOffset() == -1) {
662662
m->setFrameCompleteOffset(0);
@@ -691,7 +691,7 @@ int Profiler::getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames,
691691
}
692692

693693
void Profiler::fillFrameTypes(ASGCT_CallFrame *frames, int num_frames,
694-
NMethod *nmethod) {
694+
VMNMethod *nmethod) {
695695
if (nmethod->isNMethod() && nmethod->isAlive()) {
696696
VMMethod *method = nmethod->method();
697697
if (method == NULL) {
@@ -850,7 +850,7 @@ void Profiler::recordSample(void *ucontext, u64 counter, int tid,
850850
if (mutex.acquired()) {
851851
java_frames = getJavaTraceAsync(ucontext, frames + num_frames, max_remaining, &java_ctx, &truncated);
852852
if (java_frames > 0 && java_ctx.pc != NULL && VMStructs::hasMethodStructs()) {
853-
NMethod* nmethod = CodeHeap::findNMethod(java_ctx.pc);
853+
VMNMethod* nmethod = CodeHeap::findNMethod(java_ctx.pc);
854854
if (nmethod != NULL) {
855855
fillFrameTypes(frames + num_frames, java_frames, nmethod);
856856
}

‎ddprof-lib/src/main/cpp/profiler.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ union CallTraceBuffer {
5252
};
5353

5454
class FrameName;
55-
class NMethod;
55+
class VMNMethod;
5656
class StackContext;
5757
class VM;
5858

@@ -178,7 +178,7 @@ class alignas(alignof(SpinLock)) Profiler {
178178
int getJavaTraceAsync(void *ucontext, ASGCT_CallFrame *frames, int max_depth,
179179
StackContext *java_ctx, bool *truncated);
180180
void fillFrameTypes(ASGCT_CallFrame *frames, int num_frames,
181-
NMethod *nmethod);
181+
VMNMethod *nmethod);
182182
void updateThreadName(jvmtiEnv *jvmti, JNIEnv *jni, jthread thread,
183183
bool self = false);
184184
void updateJavaThreadNames();

‎ddprof-lib/src/main/cpp/safeAccess.h‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
#include "arch.h"
2121
#include "codeCache.h"
22+
#include "os.h"
2223
#include <cassert>
2324
#include <stdint.h>
2425

@@ -74,10 +75,24 @@ class SafeAccess {
7475
NOINLINE __attribute__((aligned(16)))
7576
static void *loadPtr(void** ptr, void* default_value);
7677

77-
static inline bool isReadable(void* ptr) {
78+
static inline bool isReadable(const void* ptr) {
7879
return load32((int32_t*)ptr, 1) != 1 ||
7980
load32((int32_t*)ptr, -1) != -1;
8081
}
82+
83+
static inline bool isReadableRange(const void* start, size_t size) {
84+
assert(size > 0);
85+
void* start_page = (void*)align_down((uintptr_t)start, OS::page_size);
86+
void* end_page = (void*)align_down((uintptr_t)start + size - 1, OS::page_size);
87+
// Memory readability is determined at the page level, so we check each page in the range for readability.
88+
// This is more efficient than checking each byte.
89+
for (void* page = start_page; page <= end_page; page = (void*)((uintptr_t)page + OS::page_size)) {
90+
if (!isReadable(page)) {
91+
return false;
92+
}
93+
}
94+
return true;
95+
}
8196
};
8297

8398
#endif // _SAFEACCESS_H

‎ddprof-lib/src/main/cpp/stackFrame.h‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
#include "arch.h"
1313

1414

15-
class NMethod;
15+
class VMNMethod;
1616

1717
class StackFrame {
1818
private:
@@ -61,23 +61,23 @@ class StackFrame {
6161
return unwindStub(entry, name, pc(), sp(), fp());
6262
}
6363

64-
bool unwindCompiled(NMethod* nm) {
64+
bool unwindCompiled(VMNMethod* nm) {
6565
return unwindCompiled(nm, pc(), sp(), fp());
6666
}
6767

6868
bool unwindStub(instruction_t* entry, const char* name, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp);
6969
bool unwindAtomicStub(const void*& pc);
7070

7171
// TODO: this function will be removed once `vm` becomes the default stack walking mode
72-
bool unwindCompiled(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp);
72+
bool unwindCompiled(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp);
7373

74-
bool unwindPrologue(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp);
75-
bool unwindEpilogue(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp);
74+
bool unwindPrologue(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp);
75+
bool unwindEpilogue(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp);
7676

7777
void adjustSP(const void* entry, const void* pc, uintptr_t& sp);
7878

7979
// SP baseline helpers for compiled frame unwinding
80-
uintptr_t sender_sp_baseline(const NMethod* nm, uintptr_t sp, uintptr_t fp, const void* pc);
80+
uintptr_t sender_sp_baseline(const VMNMethod* nm, uintptr_t sp, uintptr_t fp, const void* pc);
8181
const void* read_caller_pc_from_sp(uintptr_t sp_base);
8282
uintptr_t read_saved_fp_from_sp(uintptr_t sp_base);
8383

‎ddprof-lib/src/main/cpp/stackFrame_aarch64.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ static inline bool isEntryBarrier(instruction_t* ip) {
197197
return ip[0] == 0xb9402389 && ip[1] == 0xeb09011f;
198198
}
199199

200-
bool StackFrame::unwindCompiled(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
200+
bool StackFrame::unwindCompiled(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
201201
instruction_t* ip = (instruction_t*)pc;
202202
instruction_t* entry = (instruction_t*)nm->entry();
203203
if ((*ip & 0xffe07fff) == 0xa9007bfd) {
@@ -236,7 +236,7 @@ static inline bool isFrameComplete(instruction_t* entry, instruction_t* ip) {
236236
return false;
237237
}
238238

239-
bool StackFrame::unwindPrologue(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
239+
bool StackFrame::unwindPrologue(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
240240
// C1/C2 methods:
241241
// {stack_bang}
242242
// sub sp, sp, #0x40
@@ -312,7 +312,7 @@ static inline bool isPollReturn(instruction_t* ip) {
312312
return false;
313313
}
314314

315-
bool StackFrame::unwindEpilogue(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
315+
bool StackFrame::unwindEpilogue(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
316316
// ldp x29, x30, [sp, #32]
317317
// add sp, sp, #0x30
318318
// {poll_return}
@@ -329,7 +329,7 @@ bool StackFrame::unwindAtomicStub(const void*& pc) {
329329
// VM threads may call generated atomic stubs, which are not normally walkable
330330
const void* lr = (const void*)link();
331331
if (VMStructs::libjvm()->contains(lr)) {
332-
NMethod* nm = CodeHeap::findNMethod(pc);
332+
VMNMethod* nm = CodeHeap::findNMethod(pc);
333333
if (nm != NULL && strncmp(nm->name(), "Stub", 4) == 0) {
334334
pc = lr;
335335
return true;

‎ddprof-lib/src/main/cpp/stackFrame_arm.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ bool StackFrame::unwindStub(instruction_t* entry, const char* name, uintptr_t& p
7878
return false;
7979
}
8080

81-
bool StackFrame::unwindCompiled(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
81+
bool StackFrame::unwindCompiled(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
8282
instruction_t* ip = (instruction_t*)pc;
8383
instruction_t* entry = (instruction_t*)nm->entry();
8484
if (ip > entry && ip <= entry + 4 && (*ip & 0xffffff00) == 0xe24dd000) {
@@ -101,7 +101,7 @@ bool StackFrame::unwindCompiled(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintp
101101
return true;
102102
}
103103

104-
bool StackFrame::unwindPrologue(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
104+
bool StackFrame::unwindPrologue(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
105105
instruction_t* ip = (instruction_t*)pc;
106106
instruction_t* entry = (instruction_t*)nm->entry();
107107
if (ip <= entry) {
@@ -111,7 +111,7 @@ bool StackFrame::unwindPrologue(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintp
111111
return false;
112112
}
113113

114-
bool StackFrame::unwindEpilogue(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
114+
bool StackFrame::unwindEpilogue(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
115115
// Not yet implemented
116116
return false;
117117
}

‎ddprof-lib/src/main/cpp/stackFrame_i386.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ bool StackFrame::unwindStub(instruction_t* entry, const char* name, uintptr_t& p
9595
return false;
9696
}
9797

98-
bool StackFrame::unwindCompiled(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
98+
bool StackFrame::unwindCompiled(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
9999
instruction_t* ip = (instruction_t*)pc;
100100
instruction_t* entry = (instruction_t*)nm->entry();
101101
if (ip <= entry
@@ -116,7 +116,7 @@ bool StackFrame::unwindCompiled(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintp
116116
return false;
117117
}
118118

119-
bool StackFrame::unwindPrologue(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
119+
bool StackFrame::unwindPrologue(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
120120
instruction_t* ip = (instruction_t*)pc;
121121
instruction_t* entry = (instruction_t*)nm->entry();
122122
if (ip <= entry || *ip == 0x55) { // push ebp
@@ -127,7 +127,7 @@ bool StackFrame::unwindPrologue(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintp
127127
return false;
128128
}
129129

130-
bool StackFrame::unwindEpilogue(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
130+
bool StackFrame::unwindEpilogue(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
131131
instruction_t* ip = (instruction_t*)pc;
132132
if (*ip == 0xc3) { // ret
133133
pc = *(uintptr_t*)sp;

‎ddprof-lib/src/main/cpp/stackFrame_loongarch64.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,17 +77,17 @@ bool StackFrame::unwindStub(instruction_t* entry, const char* name, uintptr_t& p
7777
return false;
7878
}
7979

80-
bool StackFrame::unwindCompiled(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
80+
bool StackFrame::unwindCompiled(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
8181
// Not yet implemented
8282
return false;
8383
}
8484

85-
bool StackFrame::unwindPrologue(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
85+
bool StackFrame::unwindPrologue(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
8686
// Not yet implemented
8787
return false;
8888
}
8989

90-
bool StackFrame::unwindEpilogue(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
90+
bool StackFrame::unwindEpilogue(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
9191
// Not yet implemented
9292
return false;
9393
}

‎ddprof-lib/src/main/cpp/stackFrame_ppc64.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ bool StackFrame::unwindStub(instruction_t* entry, const char* name, uintptr_t& p
105105
return true;
106106
}
107107

108-
bool StackFrame::unwindCompiled(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
108+
bool StackFrame::unwindCompiled(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
109109
// On PPC there is a valid back link to the previous frame at all times. The callee stores
110110
// the return address in the caller's frame before it constructs its own frame. After it
111111
// has destroyed its frame it restores the link register and returns. A problematic sequence
@@ -127,12 +127,12 @@ bool StackFrame::unwindCompiled(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintp
127127
return true;
128128
}
129129

130-
bool StackFrame::unwindPrologue(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
130+
bool StackFrame::unwindPrologue(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
131131
// Not yet implemented
132132
return false;
133133
}
134134

135-
bool StackFrame::unwindEpilogue(NMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
135+
bool StackFrame::unwindEpilogue(VMNMethod* nm, uintptr_t& pc, uintptr_t& sp, uintptr_t& fp) {
136136
// Not yet implemented
137137
return false;
138138
}

0 commit comments

Comments
 (0)