Skip to content

[WIP] Add early frame pointer reliability detection in walkFP() #351

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

Draft
jbachorik wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

[WIP] Add early frame pointer reliability detection in walkFP() #351

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
ee79344
Add immediate FP chain validation in walkFP()
jbachorik Feb 3, 2026
387f36d
Add global native code bounds for PC validation in walkFP()
jbachorik Feb 3, 2026
9a7e6ee
Fix FPChainProgression_ValidChain test flakiness
jbachorik Feb 3, 2026
3582d28
Merge branch 'main' into jb/fp_sanitation
jbachorik Feb 3, 2026
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
4 changes: 4 additions & 0 deletions ddprof-lib/src/main/cpp/codeCache.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
* Copyright The async-profiler authors
* Copyright 2026, Datadog, Inc
* SPDX-License-Identifier: Apache-2.0
*/

Expand Down Expand Up @@ -423,3 +424,6 @@ void CodeCache::setBuildId(const char* build_id, size_t build_id_len) {
}
}

// NativeCodeBounds static member initialization
const void* NativeCodeBounds::_min = (const void*)UINTPTR_MAX;
const void* NativeCodeBounds::_max = (const void*)0;
40 changes: 40 additions & 0 deletions ddprof-lib/src/main/cpp/codeCache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
* Copyright The async-profiler authors
* Copyright 2026, Datadog, Inc
* SPDX-License-Identifier: Apache-2.0
*/

Expand Down Expand Up @@ -298,4 +299,43 @@ class CodeCacheArray {
}
};

/**
* Global bounds tracking for all native code regions.
* Used by stack walker to quickly validate PC values point to actual code.
*
* Thread-safety: updateBounds() uses atomic CAS for concurrent updates.
* Signal-safety: contains() only reads two pointers (no locks).
*/
class NativeCodeBounds {
private:
static const void* _min;
static const void* _max;

public:
/**
* Update bounds when a new library is loaded.
* Uses atomic CAS loop to handle concurrent updates safely.
*/
static void updateBounds(const void* start, const void* end) {
for (const void* m = _min; start < m &&
!__sync_bool_compare_and_swap(&_min, m, start); m = _min);
for (const void* m = _max; end > m &&
!__sync_bool_compare_and_swap(&_max, m, end); m = _max);
}

/**
* Check if address falls within any known native code region.
* Signal-safe: only two pointer comparisons, no locks.
* Returns true if bounds haven't been initialized yet (fail-open).
*/
static bool contains(const void* pc) {
// If bounds haven't been initialized yet (_max <= _min), return true
// to avoid rejecting valid PCs before libraries are parsed.
if (_max <= _min) {
return true;
}
return pc >= _min && pc < _max;
}
};

#endif // _CODECACHE_H
27 changes: 26 additions & 1 deletion ddprof-lib/src/main/cpp/stackWalker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

#include <setjmp.h>
#include "stackWalker.h"
#include "codeCache.h"
#include "dwarf.h"
#include "profiler.h"
#include "safeAccess.h"
Expand Down Expand Up @@ -111,8 +112,32 @@ int StackWalker::walkFP(void* ucontext, const void** callchain, int max_depth, S
break;
}

// Verify PC points to actual code, not data.
// When FP wanders into locals, the value at FRAME_PC_SLOT is a local variable
// (object pointer, integer, etc.) that rarely falls within code address ranges.
if (!CodeHeap::contains(pc) && !NativeCodeBounds::contains(pc)) {
break;
}

sp = fp + (FRAME_PC_SLOT + 1) * sizeof(void*);
fp = *(uintptr_t*)fp;
uintptr_t prev_fp = fp;
fp = (uintptr_t)SafeAccess::load((void**)fp);

// Early detection: validate immediately instead of waiting for next iteration.
// When FP wanders into local variables (due to -fomit-frame-pointer or
// -momit-leaf-frame-pointer), the value read as "next FP" is typically a local
// variable that won't form a valid upward chain toward older stack frames.
if (fp != 0) {
// FP chain must progress toward higher addresses (older frames on stack).
// Stack grows downward, so caller's FP must be > current FP.
if (fp <= prev_fp) {
break;
}
// Check alignment and upper bound immediately rather than next iteration
if (!aligned(fp) || fp >= bottom) {
break;
}
}
}

if (truncated && depth > max_depth) {
Expand Down
6 changes: 6 additions & 0 deletions ddprof-lib/src/main/cpp/symbols_linux.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
* Copyright The async-profiler authors
* Copyright 2026, Datadog, Inc
* SPDX-License-Identifier: Apache-2.0
*/

Expand Down Expand Up @@ -958,6 +959,8 @@ void Symbols::parseLibraries(CodeCacheArray* array, bool kernel_symbols) {
if (haveKernelSymbols()) {
cc->sort();
array->add(cc);
// Update global native code bounds for PC validation in stack walker
NativeCodeBounds::updateBounds(cc->minAddress(), cc->maxAddress());
} else {
delete cc;
}
Expand Down Expand Up @@ -996,6 +999,9 @@ void Symbols::parseLibraries(CodeCacheArray* array, bool kernel_symbols) {
cc->sort();
applyPatch(cc);
array->add(cc);

// Update global native code bounds for PC validation in stack walker
NativeCodeBounds::updateBounds(cc->minAddress(), cc->maxAddress());
}

if (array->count() >= MAX_NATIVE_LIBS && !_libs_limit_reported) {
Expand Down
3 changes: 3 additions & 0 deletions ddprof-lib/src/main/cpp/symbols_macos.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*
* Copyright The async-profiler authors
* Copyright 2026, Datadog, Inc
* SPDX-License-Identifier: Apache-2.0
*/

Expand Down Expand Up @@ -214,6 +215,8 @@ void Symbols::parseLibraries(CodeCacheArray* array, bool kernel_symbols) {
}
cc->sort();
array->add(cc);
// Update global native code bounds for PC validation in stack walker
NativeCodeBounds::updateBounds(cc->minAddress(), cc->maxAddress());
} else {
delete cc;
}
Expand Down
186 changes: 186 additions & 0 deletions ddprof-lib/src/test/cpp/stackWalkValidation_ut.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
/*
* Copyright 2026, Datadog, Inc
* SPDX-License-Identifier: Apache-2.0
*/

#include <gtest/gtest.h>
#include <cstdint>

#include "stackWalker.h"

using namespace StackWalkValidation;

class StackWalkValidationTest : public ::testing::Test {
protected:
// Simulated stack frame structure for testing FP chain validation logic
struct MockFrame {
uintptr_t saved_fp; // Saved frame pointer (points to caller's frame)
void* return_addr; // Return address
};
};

// Test inDeadZone helper
TEST_F(StackWalkValidationTest, inDeadZone_NullPointer) {
EXPECT_TRUE(inDeadZone(nullptr));
}

TEST_F(StackWalkValidationTest, inDeadZone_LowAddress) {
// Addresses below DEAD_ZONE (0x1000) should be in dead zone
EXPECT_TRUE(inDeadZone((void*)0x100));
EXPECT_TRUE(inDeadZone((void*)0xFFF));
}

TEST_F(StackWalkValidationTest, inDeadZone_ValidAddress) {
// Addresses above DEAD_ZONE should not be in dead zone
EXPECT_FALSE(inDeadZone((void*)0x1000));
EXPECT_FALSE(inDeadZone((void*)0x10000));
}

TEST_F(StackWalkValidationTest, inDeadZone_HighAddress) {
// Very high addresses (near -DEAD_ZONE) should be in dead zone
EXPECT_TRUE(inDeadZone((void*)(~(uintptr_t)0)));
EXPECT_TRUE(inDeadZone((void*)(~(uintptr_t)0 - 0x100)));
}

// Test aligned helper
TEST_F(StackWalkValidationTest, aligned_WordAligned) {
// Word-aligned addresses (8-byte on 64-bit)
EXPECT_TRUE(aligned(0x1000));
EXPECT_TRUE(aligned(0x1008));
EXPECT_TRUE(aligned(0x7fff00000000));
}

TEST_F(StackWalkValidationTest, aligned_NotAligned) {
// Non-word-aligned addresses
EXPECT_FALSE(aligned(0x1001));
EXPECT_FALSE(aligned(0x1002));
EXPECT_FALSE(aligned(0x1004)); // 4-byte aligned but not 8-byte on 64-bit
EXPECT_FALSE(aligned(0x1007));
}

// Test sameStack helper
TEST_F(StackWalkValidationTest, sameStack_SameLocation) {
int x;
EXPECT_TRUE(sameStack(&x, &x));
}

TEST_F(StackWalkValidationTest, sameStack_NearbyLocations) {
char buffer[4096];
EXPECT_TRUE(sameStack(&buffer[4095], &buffer[0]));
}

TEST_F(StackWalkValidationTest, sameStack_FarApart) {
// Addresses more than SAME_STACK_DISTANCE apart
void* hi = (void*)0x7fff00010000;
void* lo = (void*)0x7fff00000000;
EXPECT_FALSE(sameStack(hi, lo));
}

/**
* Test the FP chain progression validation logic.
*
* In a valid FP chain:
* - Stack grows downward (toward lower addresses)
* - Saved FP points to caller's frame (higher address)
* - Therefore: next_fp > current_fp
*
* When FP wanders into local variables:
* - The value read as "saved FP" is actually a local variable
* - Locals are typically at lower or similar addresses
* - This violates the fp > prev_fp invariant
*/
TEST_F(StackWalkValidationTest, FPChainProgression_ValidChain) {
// Allocate frames in a buffer with controlled layout
// Frame layout must simulate: frame1 (low addr) -> frame2 -> frame3 (high addr)
// where each frame's saved_fp points to the next higher frame
alignas(sizeof(uintptr_t)) char buffer[3 * sizeof(MockFrame)];

MockFrame* frame1 = (MockFrame*)(buffer);
MockFrame* frame2 = (MockFrame*)(buffer + sizeof(MockFrame));
MockFrame* frame3 = (MockFrame*)(buffer + 2 * sizeof(MockFrame));

// Verify ordering: frame1 < frame2 < frame3
ASSERT_LT((uintptr_t)frame1, (uintptr_t)frame2);
ASSERT_LT((uintptr_t)frame2, (uintptr_t)frame3);

// Setup chain: frame1 -> frame2 -> frame3 -> null
frame3->saved_fp = 0; // Oldest frame (null FP = end)
frame3->return_addr = nullptr;

frame2->saved_fp = (uintptr_t)frame3;
frame2->return_addr = (void*)0x400000;

frame1->saved_fp = (uintptr_t)frame2;
frame1->return_addr = (void*)0x400100;

uintptr_t fp = (uintptr_t)frame1;

// Walk the chain and verify progression
uintptr_t prev_fp = fp;
fp = *(uintptr_t*)fp; // Read saved FP from frame1

// Valid: next FP (frame2) should be greater than current (frame1)
EXPECT_GT(fp, prev_fp);
EXPECT_TRUE(aligned(fp));

prev_fp = fp;
fp = *(uintptr_t*)fp; // Read saved FP from frame2

// Valid: next FP (frame3) should be greater than current (frame2)
EXPECT_GT(fp, prev_fp);

// Final frame has null FP (chain terminator)
prev_fp = fp;
fp = *(uintptr_t*)fp; // Read saved FP from frame3
EXPECT_EQ(fp, 0u);
}

TEST_F(StackWalkValidationTest, FPChainProgression_BackwardsChain) {
// Simulate an invalid FP chain where FP goes backward
// This happens when FP points to a local variable
MockFrame frame1;
uintptr_t fake_local = 0x12345678; // Simulated local variable value

// Point FP to a location that contains a "backwards" pointer
frame1.saved_fp = (uintptr_t)&fake_local - 0x1000; // Lower address

uintptr_t fp = (uintptr_t)&frame1;
uintptr_t prev_fp = fp;
fp = *(uintptr_t*)fp;

// Invalid: next FP is less than current (backwards)
// This is the condition our validation catches
EXPECT_LE(fp, prev_fp);
}

TEST_F(StackWalkValidationTest, FPChainProgression_SameAddress) {
// FP pointing to itself (no progress) is invalid
MockFrame frame1;
frame1.saved_fp = (uintptr_t)&frame1; // Points to itself

uintptr_t fp = (uintptr_t)&frame1;
uintptr_t prev_fp = fp;
fp = *(uintptr_t*)fp;

// Invalid: FP equals prev_fp (no progress)
EXPECT_EQ(fp, prev_fp);
}

/**
* Test that validates the relationship between alignment and FP validity.
* Pointer-sized local variables are word-aligned by the compiler, so alignment
* alone cannot distinguish valid FPs from locals. The progression check
* (fp > prev_fp) is the key discriminator.
*/
TEST_F(StackWalkValidationTest, PointerSizedLocalsAreAligned) {
// Demonstrate that pointer-sized local variables are word-aligned.
// This is why alignment alone isn't sufficient to validate FP chains.
// Note: smaller types like int may only be 4-byte aligned on 64-bit systems.
uintptr_t local_uintptr;
void* local_ptr;
size_t local_size;

EXPECT_TRUE(aligned((uintptr_t)&local_uintptr));
EXPECT_TRUE(aligned((uintptr_t)&local_ptr));
EXPECT_TRUE(aligned((uintptr_t)&local_size));
}
Loading