Skip to content

feat(sframe): complete SFrame V2 parser — replace stub with full implementation #471

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
+957 −30
Draft

feat(sframe): complete SFrame V2 parser — replace stub with full implementation #471

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
6981841
feat(sframe): complete SFrame V2 parser with review fixes
jbachorik Jun 10, 2026
bf0a5b3
fix(sframe): update DwarfParser call to 6-arg constructor
jbachorik Jun 10, 2026
6416341
fix(sframe): use main's symbols_linux.cpp as base, add SFrame fast-path
jbachorik Jun 10, 2026
649718f
fix(sframe): correct RA/FP read order, rollback on partial FDE, u32 g…
jbachorik Jun 10, 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
230 changes: 213 additions & 17 deletions ddprof-lib/src/main/cpp/sframe.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*/

#include "sframe.h"
#include "log.h"
#include <climits>
#include <cstdlib>
#include <cstring>

Expand All @@ -16,7 +18,8 @@ SFrameParser::SFrameParser(const char* name, const char* section_base,
_capacity(128),
_count(0),
_table(static_cast<FrameDesc*>(malloc(128 * sizeof(FrameDesc)))),
_linked_frame_size(-1) {}
_linked_frame_size(-1),
_oom(false) {}

SFrameParser::~SFrameParser() {
free(_table); // safe: free(nullptr) is a no-op; table() nulls _table on success
Expand All @@ -37,10 +40,15 @@ const FrameDesc& SFrameParser::detectedDefaultFrame() const {
}

FrameDesc* SFrameParser::addRecord(u32 loc, u32 cfa, int fp_off, int pc_off) {
if (!_table) return nullptr; // constructor malloc failed
if (_count >= _capacity) {
if (_capacity > (INT_MAX / 2)) return nullptr; // overflow guard
FrameDesc* resized = static_cast<FrameDesc*>(
realloc(_table, _capacity * 2 * sizeof(FrameDesc)));
if (!resized) return nullptr;
if (!resized) {
_oom = true;
return nullptr;
}
_capacity *= 2;
_table = resized;
}
Expand All @@ -52,54 +60,242 @@ FrameDesc* SFrameParser::addRecord(u32 loc, u32 cfa, int fp_off, int pc_off) {
return fd;
}

bool SFrameParser::parseFDE(const SFrameHeader* /*hdr*/, const SFrameFDE* /*fde*/,
const char* /*fre_section*/, const char* /*fre_end*/) {
return true; // stub — implemented in Task 4
bool SFrameParser::parseFDE(const SFrameHeader* hdr, const SFrameFDE* fde,
const char* fre_section, const char* fre_end) {
// Determine FRE start address size
int addr_size;
switch (SFRAME_FUNC_FRE_TYPE(fde->info)) {
case SFRAME_FRE_TYPE_ADDR1: addr_size = 1; break;
case SFRAME_FRE_TYPE_ADDR2: addr_size = 2; break;
case SFRAME_FRE_TYPE_ADDR4: addr_size = 4; break;
Comment thread
jbachorik marked this conversation as resolved.
default: return false;
}

// Bounds-check fre_off before pointer arithmetic
size_t fre_section_len = static_cast<size_t>(fre_end - fre_section);
if (fde->fre_off >= fre_section_len) return false;

const char* fre_ptr = fre_section + fde->fre_off;

for (uint32_t j = 0; j < fde->fre_num; j++) {
// (a) Entry-level bounds check
if (fre_ptr >= fre_end) return false;

// (b) Read FRE start address offset (unsigned)
if (fre_ptr + addr_size > fre_end) return false;
uint32_t fre_start = 0;
if (addr_size == 1) {
fre_start = *reinterpret_cast<const uint8_t*>(fre_ptr);
} else if (addr_size == 2) {
uint16_t v; memcpy(&v, fre_ptr, 2); fre_start = v;
} else {
memcpy(&fre_start, fre_ptr, 4);
}
fre_ptr += addr_size;

// (c) Read FRE info byte
if (fre_ptr + 1 > fre_end) return false;
uint8_t fre_info = *reinterpret_cast<const uint8_t*>(fre_ptr);
fre_ptr += 1;

// (d) Determine offset encoding size
int off_size;
switch (SFRAME_FRE_OFFSET_SIZE(fre_info)) {
case SFRAME_FRE_OFFSET_1B: off_size = 1; break;
case SFRAME_FRE_OFFSET_2B: off_size = 2; break;
case SFRAME_FRE_OFFSET_4B: off_size = 4; break;
default: return false;
}

// Decide what to read from the stream (governed by FRE info byte alone)
bool fp_tracked = SFRAME_FRE_FP_TRACKED(fre_info);
bool ra_in_fre = SFRAME_FRE_RA_TRACKED(fre_info);

// (e) Bounds check all remaining reads for this FRE
int n_offsets = 1 + (fp_tracked ? 1 : 0) + (ra_in_fre ? 1 : 0);
if (fre_ptr + n_offsets * off_size > fre_end) return false;

// (f) Read CFA offset (signed)
int32_t cfa_offset = 0;
if (off_size == 1) {
cfa_offset = *reinterpret_cast<const int8_t*>(fre_ptr);
} else if (off_size == 2) {
int16_t v; memcpy(&v, fre_ptr, 2); cfa_offset = v;
} else {
memcpy(&cfa_offset, fre_ptr, 4);
}
fre_ptr += off_size;

// Guard: CFA offset must fit in the 24-bit field packed into FrameDesc::cfa
if (cfa_offset < -8388608 || cfa_offset > 8388607) return false;

// (g) Read RA offset if tracked
int32_t ra_offset = 0;
if (ra_in_fre) {
if (off_size == 1) {
ra_offset = *reinterpret_cast<const int8_t*>(fre_ptr);
} else if (off_size == 2) {
int16_t v; memcpy(&v, fre_ptr, 2); ra_offset = v;
} else {
memcpy(&ra_offset, fre_ptr, 4);
}
fre_ptr += off_size;
}

// (h) Read FP offset if tracked
int32_t fp_offset = 0;
if (fp_tracked) {
if (off_size == 1) {
fp_offset = *reinterpret_cast<const int8_t*>(fre_ptr);
} else if (off_size == 2) {
int16_t v; memcpy(&v, fre_ptr, 2); fp_offset = v;
} else {
memcpy(&fp_offset, fre_ptr, 4);
}
fre_ptr += off_size;
}

// (i) Translate to FrameDesc fields
// Use unsigned arithmetic to avoid implementation-defined signed cast for large offsets
u32 loc = _section_offset + static_cast<u32>(fde->start_addr) + fre_start;

u32 cfa_reg = SFRAME_FRE_BASE_REG(fre_info) ? static_cast<u32>(DW_REG_FP)
: static_cast<u32>(DW_REG_SP);
u32 cfa = (static_cast<u32>(cfa_offset) << 8) | cfa_reg;

// aarch64 GCC vs Clang detection: first FP-based entry with cfa_offset > 0
if (_linked_frame_size < 0 && cfa_reg == static_cast<u32>(DW_REG_FP) && cfa_offset > 0) {
_linked_frame_size = cfa_offset;
}

// Determine fp_off: per-FRE value takes priority; fall back to header fixed offset
int fp_off;
if (fp_tracked) {
fp_off = static_cast<int>(fp_offset);
} else if (hdr->cfa_fixed_fp_offset != 0) {
fp_off = static_cast<int>(hdr->cfa_fixed_fp_offset);
} else {
fp_off = DW_SAME_FP;
}

// Header fixed RA offset takes priority over per-FRE value
int pc_off;
if (hdr->cfa_fixed_ra_offset != 0) {
pc_off = static_cast<int>(hdr->cfa_fixed_ra_offset);
} else if (ra_in_fre) {
pc_off = static_cast<int>(ra_offset);
} else {
pc_off = DW_LINK_REGISTER;
}

// (j) Append record
if (!addRecord(loc, cfa, fp_off, pc_off)) return false;
}

return true;
}

bool SFrameParser::parse() {
// 1. Size check
if (_section_size < sizeof(SFrameHeader)) return false;
if (_section_size < sizeof(SFrameHeader)) {
Log::warn("SFrame section too small in %s", _name);
return false;
}

const SFrameHeader* hdr = reinterpret_cast<const SFrameHeader*>(_section_base);

// 2-4. Header field validation
if (hdr->magic != SFRAME_MAGIC) return false;
if (hdr->version != SFRAME_VERSION_2) return false;
if (hdr->magic != SFRAME_MAGIC) {
Log::warn("SFrame bad magic in %s", _name);
return false;
}
if (hdr->version != SFRAME_VERSION_2) {
Log::warn("SFrame unsupported version %d in %s", (int)hdr->version, _name);
return false;
}

#if defined(__x86_64__)
if (hdr->abi_arch != SFRAME_ABI_AMD64_ENDIAN_LITTLE) return false;
if (hdr->abi_arch != SFRAME_ABI_AMD64_ENDIAN_LITTLE) {
Log::warn("SFrame wrong ABI 0x%x in %s", (int)hdr->abi_arch, _name);
return false;
}
#elif defined(__aarch64__)
if (hdr->abi_arch != SFRAME_ABI_AARCH64_ENDIAN_LITTLE) return false;
if (hdr->abi_arch != SFRAME_ABI_AARCH64_ENDIAN_LITTLE) {
Log::warn("SFrame wrong ABI 0x%x in %s", (int)hdr->abi_arch, _name);
return false;
}
#else
return false;
#endif

// 5. Bounds check auxhdr_len before computing data_start
if (sizeof(SFrameHeader) + hdr->auxhdr_len > _section_size) return false;
if (sizeof(SFrameHeader) + hdr->auxhdr_len > _section_size) {
Log::warn("SFrame auxhdr_len overflows section in %s", _name);
return false;
}

const char* data_start = _section_base + sizeof(SFrameHeader) + hdr->auxhdr_len;
const char* section_end = _section_base + _section_size;

// Bounds-check fdeoff, freoff, and fre_len before pointer arithmetic
size_t data_len = static_cast<size_t>(section_end - data_start);
if (hdr->fdeoff > data_len) {
Log::warn("SFrame fdeoff out of bounds in %s", _name);
return false;
}
if (hdr->freoff > data_len) {
Log::warn("SFrame freoff out of bounds in %s", _name);
return false;
}
if (hdr->fre_len > data_len - hdr->freoff) {
Log::warn("SFrame fre_len overflows section in %s", _name);
return false;
}

const SFrameFDE* fde_array = reinterpret_cast<const SFrameFDE*>(data_start + hdr->fdeoff);
const char* fre_section = data_start + hdr->freoff;
const char* fre_end = fre_section + hdr->fre_len;

// 6-7. Bounds checks for FDE array and FRE section
if (reinterpret_cast<const char*>(fde_array) +
(size_t)hdr->num_fdes * sizeof(SFrameFDE) > section_end) return false;
if (fre_end > section_end) return false;
(size_t)hdr->num_fdes * sizeof(SFrameFDE) > section_end) {
Log::warn("SFrame FDE array overflows section in %s", _name);
return false;
}
if (fre_end > section_end) {
Log::warn("SFrame FRE section overflows in %s", _name);
return false;
}

// 8. FDE array / FRE section overlap check
if (hdr->num_fdes > 0 && hdr->fre_len > 0) {
const char* fde_start_ptr = reinterpret_cast<const char*>(fde_array);
const char* fde_end_ptr = fde_start_ptr + (size_t)hdr->num_fdes * sizeof(SFrameFDE);
if (fde_end_ptr > fre_section && fde_start_ptr < fre_end) {
Log::warn("SFrame FDE array overlaps FRE section in %s", _name);
Comment thread
jbachorik marked this conversation as resolved.
return false;
}
}

// 8. Iterate FDEs
// 9. Iterate FDEs
for (uint32_t i = 0; i < hdr->num_fdes; i++) {
const SFrameFDE* fde = &fde_array[i];
if (SFRAME_FUNC_FDE_TYPE(fde->info) != 0) continue; // skip PCMASK
if (fde->fre_num == 0) continue; // empty FDE
parseFDE(hdr, fde, fre_section, fre_end); // ignore return; skip corrupt FDE
int saved_count = _count;
int saved_linked_frame_size = _linked_frame_size;
if (!parseFDE(hdr, fde, fre_section, fre_end)) {
if (_oom) return false; // OOM: partial table is not safe to use
// Corrupt FDE: roll back any partially-added records.
_count = saved_count;
_linked_frame_size = saved_linked_frame_size;
}
Comment thread
jbachorik marked this conversation as resolved.
}

// 9. Sort
qsort(_table, _count, sizeof(FrameDesc), FrameDesc::comparator);
// 10. Sort (skip if the section header guarantees sorted order)
if (_count > 0 && !(hdr->flags & SFRAME_F_FDE_SORTED)) {
qsort(_table, _count, sizeof(FrameDesc), FrameDesc::comparator);
}

return _count > 0;
}
5 changes: 3 additions & 2 deletions ddprof-lib/src/main/cpp/sframe.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const int SFRAME_FRE_TYPE_ADDR1 = 0;
const int SFRAME_FRE_TYPE_ADDR2 = 1;
const int SFRAME_FRE_TYPE_ADDR4 = 2;

struct __attribute__((packed)) SFrameHeader { // 28 bytes
struct __attribute__((packed, may_alias)) SFrameHeader { // 28 bytes
uint16_t magic;
uint8_t version;
uint8_t flags;
Expand All @@ -57,7 +57,7 @@ struct __attribute__((packed)) SFrameHeader { // 28 bytes
uint32_t freoff;
};

struct __attribute__((packed)) SFrameFDE { // 20 bytes
struct __attribute__((packed, may_alias)) SFrameFDE { // 20 bytes
int32_t start_addr; // signed, relative to .sframe section start (V2)
uint32_t func_size;
uint32_t fre_off; // byte offset into FRE sub-section
Expand All @@ -78,6 +78,7 @@ class SFrameParser {
int _count;
FrameDesc* _table;
int _linked_frame_size; // for aarch64 GCC vs Clang detection; -1 = undetected
bool _oom; // set by addRecord on realloc failure

bool parseFDE(const SFrameHeader* hdr, const SFrameFDE* fde,
const char* fre_section, const char* fre_end);
Expand Down
22 changes: 22 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
Expand Up @@ -25,6 +25,7 @@
#include "common.h"
#include "symbols.h"
#include "dwarf.h"
#include "sframe.h"
#include "fdtransferClient.h"
#include "log.h"
#include "os.h"
Expand Down Expand Up @@ -702,6 +703,27 @@ void ElfParser::parseDynamicSection() {
void ElfParser::parseDwarfInfo() {
if (!DWARF_SUPPORTED) return;

// Try SFrame first (simpler format, faster parsing, no opcode interpretation).
ElfProgramHeader* sframe_phdr = findProgramHeader(PT_GNU_SFRAME);
if (sframe_phdr != NULL && sframe_phdr->p_vaddr != 0) {
const char* section_base = at(sframe_phdr);
uintptr_t section_offset_full = static_cast<uintptr_t>(section_base - _base);
if (section_offset_full > static_cast<uintptr_t>(UINT32_MAX)) {
Log::warn("SFrame section offset too large for u32 in %s; falling back to DWARF", _cc->name());
goto dwarf_fallback;
}
u32 section_offset = static_cast<u32>(section_offset_full);
SFrameParser sframe(_cc->name(), section_base,
static_cast<size_t>(sframe_phdr->p_filesz), section_offset);
if (sframe.parse()) {
_cc->setDwarfTable(sframe.table(), sframe.count(),
sframe.detectedDefaultFrame());
return;
}
// SFrame parse failed; fall through to DWARF
}

dwarf_fallback:
ElfProgramHeader* eh_frame_hdr = findProgramHeader(PT_GNU_EH_FRAME);
if (eh_frame_hdr != NULL) {
if (eh_frame_hdr->p_vaddr != 0) {
Expand Down
Loading
Loading