Skip to content

Increase native test coverage and add fuzzing #221

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .github/scripts/prepare_reports.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ cp ddprof-test/build/hs_err* reports/ || true
cp -r ddprof-lib/build/tmp reports/native_build || true
cp -r ddprof-test/build/reports/tests reports/tests || true
cp -r /tmp/recordings reports/recordings || true
cp -r ddprof-lib/gtest/build/tmp reports/gtest || true
find ddprof-lib/build -name 'libjavaProfiler.*' -exec cp {} reports/ \; || true
3 changes: 3 additions & 0 deletions ddprof-lib/bin/test/native-libs/reladyn-lib/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
TARGET_DIR = ../build/test/resources/native-libs/reladyn-lib
all:
g++ -fPIC -shared -o $(TARGET_DIR)/libreladyn.so reladyn.c
28 changes: 28 additions & 0 deletions ddprof-lib/bin/test/native-libs/reladyn-lib/reladyn.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright The async-profiler authors
* SPDX-License-Identifier: Apache-2.0
*/
#include <pthread.h>
#include <stdio.h>
// Force pthread_setspecific into .rela.dyn with R_X86_64_GLOB_DAT.
int (*indirect_pthread_setspecific)(pthread_key_t, const void*);
// Force pthread_exit into .rela.dyn with R_X86_64_64.
void (*static_pthread_exit)(void*) = pthread_exit;
void* thread_function(void* arg) {
printf("Thread running\n");
return NULL;
}
// Not indended to be executed.
int reladyn() {
pthread_t thread;
pthread_key_t key;
pthread_key_create(&key, NULL);
// Direct call, forces into .rela.plt.
pthread_create(&thread, NULL, thread_function, NULL);
// Assign to a function pointer at runtime, forces into .rela.dyn as R_X86_64_GLOB_DAT.
indirect_pthread_setspecific = pthread_setspecific;
indirect_pthread_setspecific(key, "Thread-specific value");
// Use pthread_exit via the static pointer, forces into .rela.dyn as R_X86_64_64.
static_pthread_exit(NULL);
return 0;
}
3 changes: 3 additions & 0 deletions ddprof-lib/bin/test/native-libs/small-lib/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
TARGET_DIR = ../build/test/resources/native-libs/small-lib
all:
g++ -fPIC -shared -o $(TARGET_DIR)/libsmall-lib.so small_lib.cpp
6 changes: 6 additions & 0 deletions ddprof-lib/bin/test/native-libs/small-lib/small_lib.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#include <iostream>
#include "small_lib.h"

extern "C" void hello() {
std::cout << "Hello, World from shared library!" << std::endl;
}
5 changes: 5 additions & 0 deletions ddprof-lib/bin/test/native-libs/small-lib/small_lib.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#pragma once

extern "C" {
void hello();
}
4 changes: 4 additions & 0 deletions ddprof-lib/bin/test/native-libs/unresolved-functions/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
TARGET_DIR = ../build/test/resources/unresolved-functions
all:
gcc -c main.c -o $(TARGET_DIR)/main.o
gcc -o $(TARGET_DIR)/main $(TARGET_DIR)/main.o -T linker.ld
43 changes: 43 additions & 0 deletions ddprof-lib/bin/test/native-libs/unresolved-functions/linker.ld
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
PHDRS
{
headers PT_PHDR PHDRS ;
interp PT_INTERP ;
text PT_LOAD FILEHDR PHDRS ;
data PT_LOAD ;
}

SECTIONS
{
. = 0x10000;
.text : {
*(.text)
} :text

. = 0x20000;
.data : {
*(.data)
} :data

.bss : {
*(.bss)
}

. = 0x30000;
unresolved_symbol = .;
. = 0xffffffffffffffff;
unresolved_function = .;

/* Add the .init_array section */
.init_array : {
__init_array_start = .;
KEEP(*(.init_array))
__init_array_end = .;
}

/* Add the .fini_array section */
.fini_array : {
__fini_array_start = .;
KEEP(*(.fini_array))
__fini_array_end = .;
}
}
10 changes: 10 additions & 0 deletions ddprof-lib/bin/test/native-libs/unresolved-functions/main.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
#include <stdio.h>

extern int unresolved_symbol;
extern int unresolved_function();

int main() {
printf("Value of unresolved_symbol: %p\n", &unresolved_symbol);
printf("Value of unresolved_function: %p\n", &unresolved_function);
return 0;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Description

This binary tests that we are able to parse symbols even when they point to unresolved functions.
The function is set to point to a 0xffffffffffffffff address.
4 changes: 4 additions & 0 deletions ddprof-lib/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,10 @@ library {
privateHeaders.from file('src/main/cpp')
privateHeaders.from file('src/main/cpp-external')

// Include JDK headers
privateHeaders.from System.getenv("JAVA_HOME") + "/include"
privateHeaders.from System.getenv("JAVA_HOME") + "/include/" + osIdentifier()

// aarch64 support is still incubating
// for the time being an aarch64 linux machine will match 'machines.linux.x86_64'
targetMachines = [machines.macOS, machines.linux.x86_64]
Expand Down
9 changes: 8 additions & 1 deletion ddprof-lib/gtest/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,10 @@ tasks.whenTaskAdded { task ->
buildConfigurations.each { config ->
if (config.os == osIdentifier() && config.arch == archIdentifier()) {
project(':ddprof-lib').file("src/test/cpp/").eachFile {
// Only process .cpp files, skip header files
if (!it.name.endsWith('.cpp')) {
return
}
def testFile = it
def testName = it.name.substring(0, it.name.lastIndexOf('.'))
def gtestCompileTask = tasks.register("compileGtest${config.name.capitalize()}_${testName}", CppCompile) {
Expand Down Expand Up @@ -155,7 +159,10 @@ tasks.whenTaskAdded { task ->
description = "Run all Google Tests for the ${config.name} build of the library"
}
project(':ddprof-lib').file("src/test/cpp/").eachFile {

// Only process .cpp files, skip header files
if (!it.name.endsWith('.cpp')) {
return
}
def testFile = it
def testName = it.name.substring(0, it.name.lastIndexOf('.'))
def binary = file("$buildDir/bin/gtest/${config.name}_${testName}/${testName}")
Expand Down
2 changes: 1 addition & 1 deletion ddprof-lib/src/main/cpp/arguments.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ const char *Arguments::file() {
// Should match statically computed HASH(arg)
long long Arguments::hash(const char *arg) {
long long h = 0;
for (int shift = 0; *arg != 0; shift += 5) {
for (int shift = 0; *arg != 0 && shift <= 55; shift += 5) {
h |= (*arg++ & 31LL) << shift;
}
return h;
Expand Down
1 change: 1 addition & 0 deletions ddprof-lib/src/main/cpp/buffers.h
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ class Buffer {
// the trickery of RecordingBuffer extending Buffer::_data array may trip off asan on aarch64
__attribute__((no_sanitize("bounds")))
#endif
__attribute__((no_sanitize("undefined")))
void put16(short v) {
assert(_offset + 2 < limit());
*(short *)(_data + _offset) = htons(v);
Expand Down
30 changes: 29 additions & 1 deletion ddprof-lib/src/main/cpp/callTraceStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,12 @@ u64 CallTraceStorage::calcHash(int num_frames, ASGCT_CallFrame *frames,
const u64 M = 0xc6a4a7935bd1e995ULL;
const int R = 47;

// Safety check for NULL frames or invalid frame count
if (frames == NULL || num_frames <= 0) {
// Return a consistent hash for empty/invalid frames
return truncated ? 0x1234567890ABCDEFULL : 0xFEDCBA0987654321ULL;
}

int len = num_frames * sizeof(ASGCT_CallFrame);
u64 h = len * M * (truncated ? 1 : 2);

Expand Down Expand Up @@ -157,6 +163,11 @@ u64 CallTraceStorage::calcHash(int num_frames, ASGCT_CallFrame *frames,
CallTrace *CallTraceStorage::storeCallTrace(int num_frames,
ASGCT_CallFrame *frames,
bool truncated) {
// Safety check for NULL frames or invalid frame count
if (frames == NULL || num_frames <= 0) {
return NULL;
}

const size_t header_size = sizeof(CallTrace) - sizeof(ASGCT_CallFrame);
const size_t total_size = header_size + num_frames * sizeof(ASGCT_CallFrame);
CallTrace *buf = (CallTrace *)_allocator.alloc(total_size);
Expand Down Expand Up @@ -194,6 +205,11 @@ CallTrace *CallTraceStorage::findCallTrace(LongHashTable *table, u64 hash) {

u32 CallTraceStorage::put(int num_frames, ASGCT_CallFrame *frames,
bool truncated, u64 weight) {
// Safety check for invalid input
if (num_frames <= 0) {
return 0;
}

// Currently, CallTraceStorage is a singleton used globally in Profiler and
// therefore start-stop operation requires data structures cleanup. This
// cleanup may and will race this method and the racing can cause all sorts of
Expand All @@ -204,9 +220,15 @@ u32 CallTraceStorage::put(int num_frames, ASGCT_CallFrame *frames,
return 0;
}

// Note: calcHash now handles NULL frames safely
u64 hash = calcHash(num_frames, frames, truncated);

LongHashTable *table = _current_table;
if (table == NULL) {
_lock.unlockShared();
return 0;
}

u64 *keys = table->keys();
u32 capacity = table->capacity();
u32 slot = hash & (capacity - 1);
Expand Down Expand Up @@ -235,7 +257,13 @@ u32 CallTraceStorage::put(int num_frames, ASGCT_CallFrame *frames,
if (trace == NULL) {
trace = storeCallTrace(num_frames, frames, truncated);
}
table->values()[slot].setTrace(trace);
if (trace != NULL) {
table->values()[slot].setTrace(trace);
} else {
// If we couldn't store the trace, use the overflow trace
table->values()[slot].setTrace(&_overflow_trace);
atomicInc(_overflow);
}

// clear the slot in the prev table such it is not written out to constant
// pool multiple times
Expand Down
7 changes: 6 additions & 1 deletion ddprof-lib/src/main/cpp/dictionary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@ static inline char *allocateKey(const char *key, size_t length) {

static inline bool keyEquals(const char *candidate, const char *key,
size_t length) {
return strncmp(candidate, key, length) == 0 && candidate[length] == 0;
if (strncmp(candidate, key, length) != 0) {
return false;
}

size_t candidate_len = strlen(candidate);
return candidate_len == length;
}

Dictionary::~Dictionary() {
Expand Down
5 changes: 5 additions & 0 deletions ddprof-lib/src/main/cpp/linearAllocator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ void LinearAllocator::clear() {
}

void *LinearAllocator::alloc(size_t size) {
if (size > _chunk_size) {
// If the requested size is larger than the chunk size, return NULL
return NULL;
}

Chunk *chunk = __atomic_load_n(&_tail, __ATOMIC_ACQUIRE);
do {
// Fast path: bump a pointer with CAS
Expand Down
39 changes: 28 additions & 11 deletions ddprof-lib/src/main/cpp/rustDemangler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,27 @@ bool is_probably_rust_legacy(const std::string &str) {
}
return ptr[2] == '$' || ptr[3] == '$' || ptr[4] == '$';
}
if (*ptr == '.') {
return '.' != ptr[1] ||
'.' != ptr[2]; // '.' and '..' are fine, '...' is not
if (*ptr == '.' && (ptr + 2 < end)) {
return '.' != ptr[1] || '.' != ptr[2];
}
}
return true;
}

void append_checked(std::string &str, const std::string &append) {
if (str.size() + append.size() < MAX_DEMANGLE_OUTPUT_SIZE) {
str += append;
} else {
str += "...[truncated]";
}
}

// Demangles a Rust string by building a copy piece-by-piece
std::string demangle(const std::string &str) {
if (!has_hash(str)) {
return str;
}

std::string ret;
ret.reserve(str.size() - hash_eg.size() - hash_pre.size());

Expand All @@ -115,7 +126,6 @@ std::string demangle(const std::string &str) {
}

for (; i < str.size() - hash_pre.size() - hash_eg.size(); ++i) {

// Fast sieve for pattern-matching, since we know first chars
if (str[i] == '.' || str[i] == '$') {
bool replaced = false;
Expand All @@ -125,7 +135,7 @@ std::string demangle(const std::string &str) {
const std::string &pattern = pair.first;
const std::string &replacement = pair.second;
if (!str.compare(i, pattern.size(), pattern)) {
ret += replacement;
append_checked(ret, replacement);
i += pattern.size() - 1; // -1 because iterator inc
replaced = true;
break;
Expand All @@ -137,26 +147,33 @@ std::string demangle(const std::string &str) {
// implementations treat many individual points as patterns to search on)
if (!replaced && str[i] == '.') {
// Special-case for '.'
ret += '-';
} else if (!replaced && !str.compare(i, 2, "$u") && str[i + 4] == '$') {
append_checked(ret, "-");
} else if (!replaced && i + 4 < str.size() && !str.compare(i, 2, "$u") && str[i + 4] == '$') {
const size_t k_nb_read_chars = 5;
const int hexa_base = 16;
const int hi = hex_to_int(str[i + 2]);
const int lo = hex_to_int(str[i + 3]);
if (hi != -1 && lo != -1) {
ret += static_cast<unsigned char>(lo + hexa_base * hi);
append_checked(ret, std::string(1, static_cast<unsigned char>(lo + hexa_base * hi)));

i += k_nb_read_chars - 1; // - 1 because iterator inc
} else {
// We didn't have valid unicode values. No further processing is
// done, reinsert the `$u...$` sequence into the output string.
ret += str.substr(i, k_nb_read_chars);
if (i + k_nb_read_chars <= str.size()) {
append_checked(ret, str.substr(i, k_nb_read_chars));
i += k_nb_read_chars - 1;
} else {
append_checked(ret, str.substr(i));
break; // cannot continue safely
}
i += k_nb_read_chars - 1; // -1 because iterator inc
}
} else if (!replaced) {
ret += str[i];
append_checked(ret, std::string(1, str[i]));
}
} else {
ret += str[i];
append_checked(ret, std::string(1, str[i]));
}
}

Expand Down
4 changes: 4 additions & 0 deletions ddprof-lib/src/main/cpp/rustDemangler.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@
#include <string>

namespace RustDemangler {
constexpr size_t MAX_DEMANGLE_OUTPUT_SIZE = 4096;

bool is_probably_rust_legacy(const std::string &str);
std::string demangle(const std::string &str);
void append_checked(std::string &str, const std::string &append);

}; // namespace RustDemangler
Loading
Loading