tomoaki/hakmem
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Larson double-free investigation: Enhanced diagnostics + Remove buggy drain pushback

Browse Source 
**Problem**: Larson benchmark crashes with TLS_SLL_DUP (double-free), 100% crash rate in debug

**Root Cause**: TLS drain pushback code (commit c2f104618) created duplicates by
pushing pointers back to TLS SLL while they were still in the linked list chain.

**Diagnostic Enhancements** (ChatGPT + Claude collaboration):
1. **Callsite Tracking**: Track file:line for each TLS SLL push (debug only)
   - Arrays: g_tls_sll_push_file[], g_tls_sll_push_line[]
   - Macro: tls_sll_push() auto-records __FILE__, __LINE__

2. **Enhanced Duplicate Detection**:
   - Scan depth: 64 → 256 nodes (deep duplicate detection)
   - Error message shows BOTH current and previous push locations
   - Calls ptr_trace_dump_now() for detailed analysis

3. **Evidence Captured**:
   - Both duplicate pushes from same line (221)
   - Pointer at position 11 in TLS SLL (count=18, scanned=11)
   - Confirms pointer allocated without being popped from TLS SLL

**Fix**:
- **core/box/tls_sll_drain_box.h**: Remove pushback code entirely
  - Old: Push back to TLS SLL on validation failure → duplicates!
  - New: Skip pointer (accept rare leak) to avoid duplicates
  - Rationale: SuperSlab lookup failures are transient/rare

**Status**: Fix implemented, ready for testing

**Updated**:
- LARSON_DOUBLE_FREE_INVESTIGATION.md: Root cause confirmed
This commit is contained in:
Moe Charm (CI)
2025-11-27 07:30:32 +09:00
parent c2f104618f
commit 8553894171
3 changed files with 83 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
core/box/tls_sll_box.h
View File

@ -52,6 +52,12 @@ extern __thread uint64_t g_tls_canary_after_sll;
extern __thread const char* g_tls_sll_last_writer[TINY_NUM_CLASSES];
extern int g_tls_sll_class_mask; // bit i=1 → SLL allowed for class i
#if !HAKMEM_BUILD_RELEASE
// Global callsite record (debug only; zero overhead in release)
static const char* g_tls_sll_push_file[TINY_NUM_CLASSES] = {0};
static int g_tls_sll_push_line[TINY_NUM_CLASSES] = {0};
#endif
// ========== Debug guard ==========
#if !HAKMEM_BUILD_RELEASE
@ -669,12 +675,51 @@ static inline uint32_t tls_sll_splice(int class_idx,
// ========== Macro Wrappers ==========
//
// Box Theory: Callers use tls_sll_push/pop() macros which auto-insert __func__.
// No changes required to 20+ call sites.
// Box Theory: Callers use tls_sll_push/pop() macros which auto-insert callsite info (debug only).
// No changes required to call sites.
#if !HAKMEM_BUILD_RELEASE
static inline bool tls_sll_push_guarded(int class_idx, void* ptr, uint32_t capacity,
const char* where, const char* file, int line) {
// Enhanced duplicate guard (scan up to 256 nodes for deep duplicates)
uint32_t scanned = 0;
void* cur = g_tls_sll[class_idx].head;
const uint32_t limit = (g_tls_sll[class_idx].count < 256) ? g_tls_sll[class_idx].count : 256;
while (cur && scanned < limit) {
if (cur == ptr) {
// Enhanced error message with both old and new callsite info
const char* last_file = g_tls_sll_push_file[class_idx] ? g_tls_sll_push_file[class_idx] : "(null)";
fprintf(stderr,
"[TLS_SLL_DUP] cls=%d ptr=%p head=%p count=%u scanned=%u\n"
" Current push: where=%s at %s:%d\n"
" Previous push: %s:%d\n",
class_idx, ptr, g_tls_sll[class_idx].head, g_tls_sll[class_idx].count, scanned,
where, file, line,
last_file, g_tls_sll_push_line[class_idx]);
// Dump pointer trace for detailed analysis
ptr_trace_dump_now("tls_sll_dup");
abort();
}
void* next = NULL;
PTR_NEXT_READ("tls_sll_dupcheck", class_idx, cur, 0, next);
cur = next;
scanned++;
}
// Call impl (duplicate check in impl will be skipped since we already checked above and would abort)
// Note: impl has its own duplicate check, but we'll never reach it because we abort above
bool ok = tls_sll_push_impl(class_idx, ptr, capacity, where);
if (ok) {
g_tls_sll_push_file[class_idx] = file;
g_tls_sll_push_line[class_idx] = line;
}
return ok;
}
# define tls_sll_push(cls, ptr, cap) \
tls_sll_push_impl((cls), (ptr), (cap), __func__)
tls_sll_push_guarded((cls), (ptr), (cap), __func__, __FILE__, __LINE__)
# define tls_sll_pop(cls, out) \
tls_sll_pop_impl((cls), (out), __func__)
#else

31
core/box/tls_sll_drain_box.h
View File

@ -146,35 +146,30 @@ static inline uint32_t tiny_tls_sll_drain(int class_idx, uint32_t batch_size) {
// Resolve SuperSlab/Slab (like slow path does)
SuperSlab* ss = hak_super_lookup(base);
if (!ss || ss->magic != SUPERSLAB_MAGIC) {
// CRITICAL FIX: Don't leak pointers when SuperSlab lookup fails!
// Problem: Pointer was popped from TLS SLL but not returned anywhere → leak + potential double-alloc
// Solution: Push back to TLS SLL (will retry on next drain cycle)
// CRITICAL FIX (2025-11-27): Don't push back - causes duplicates!
// Problem: Pushback bypasses duplicate checking and creates cycles
// Old buggy approach: Push back to TLS SLL → pointer at BOTH position 0 and position N
// New approach: Skip this pointer (accept rare leak) to avoid duplicates
// Leak is acceptable because SuperSlab lookup failure is transient/rare
if (g_debug) {
fprintf(stderr, "[TLS_SLL_DRAIN] PUSHBACK: class=%d base=%p (invalid SuperSlab, will retry)\n",
fprintf(stderr, "[TLS_SLL_DRAIN] SKIP: class=%d base=%p (invalid SuperSlab, pointer leaked)\n",
class_idx, base);
}
// Push back to TLS SLL head (retry later when SS becomes valid)
extern __thread TinyTLSSLL g_tls_sll[TINY_NUM_CLASSES];
tiny_next_write(class_idx, base, g_tls_sll[class_idx].head);
g_tls_sll[class_idx].head = base;
g_tls_sll[class_idx].count++; // Restore count
break; // Stop draining this class for now (avoid infinite retry loop)
// DO NOT push back - would create duplicate!
// Just continue to next pointer
continue;
}
// Get slab index
int slab_idx = slab_index_for(ss, base);
if (slab_idx < 0 || slab_idx >= ss_slabs_capacity(ss)) {
// CRITICAL FIX: Don't leak pointers when slab index is invalid!
// CRITICAL FIX (2025-11-27): Don't push back - causes duplicates!
if (g_debug) {
fprintf(stderr, "[TLS_SLL_DRAIN] PUSHBACK: class=%d base=%p (invalid slab_idx=%d, will retry)\n",
fprintf(stderr, "[TLS_SLL_DRAIN] SKIP: class=%d base=%p (invalid slab_idx=%d, pointer leaked)\n",
class_idx, base, slab_idx);
}
// Push back to TLS SLL head (retry later)
extern __thread TinyTLSSLL g_tls_sll[TINY_NUM_CLASSES];
tiny_next_write(class_idx, base, g_tls_sll[class_idx].head);
g_tls_sll[class_idx].head = base;
g_tls_sll[class_idx].count++;
break;
// DO NOT push back - would create duplicate!
continue;
}
// Get slab metadata

46
docs/analysis/LARSON_DOUBLE_FREE_INVESTIGATION.md
View File

@ -85,40 +85,38 @@ File: `core/box/tls_sll_box.h:381`
**Impact**: Enables precise root cause identification.
## Next Steps
## Root Cause CONFIRMED (2025-11-27)
### Priority 1: Verify Metadata Consistency
### TLS Drain Pushback Bug Creates Duplicates!
Add assertions to check:
1. Pointer is in ONLY ONE location at a time:
- TLS SLL
- Slab freelist
- Not both!
**File**: `core/box/tls_sll_drain_box.h:148-162`
2. `meta->used` count matches reality
### Priority 2: Fix TLS Drain Leak
File: `core/box/tls_sll_drain_box.h:154`
**Current (buggy)**:
**Buggy Fix (commit c2f104618)**:
```c
if (!ss || ss->magic != SUPERSLAB_MAGIC) {
continue; // ← LEAK!
// CRITICAL BUG: Creates duplicates!
tiny_next_write(class_idx, base, g_tls_sll[class_idx].head);
g_tls_sll[class_idx].head = base; // ← Pushes to position 0
g_tls_sll[class_idx].count++; // ← But pointer ALREADY at position 11!
break;
}
```
**Proposed Fix**:
```c
if (!ss || ss->magic != SUPERSLAB_MAGIC) {
// Option A: Push back to TLS SLL (retry later)
tls_sll_push(class_idx, base, UINT32_MAX);
break; // Stop draining this class for now
**Scenario**:
1. TLS SLL has pointer at position 11 (count=18)
2. Drain loop pops pointer from TLS SLL (now count=17, but pointer still in chain at position 10)
3. SuperSlab lookup fails (transient state)
4. Pushback adds pointer to position 0 → **NOW AT TWO POSITIONS** (0 and 10)
5. Allocation pops from position 0
6. User frees → tries to push → **duplicate detected at position 10**
// Option B: Force to remote queue (if lookup fails, assume remote)
// (requires more analysis)
}
**Evidence**:
```
[TLS_SLL_DUP] cls=1 ptr=0x... count=18 scanned=11
```
Pointer found at position 11 during duplicate scan!
**Correct Fix**: DON'T push back when already in TLS SLL. Just **stop draining** when validation fails.
### Priority 3: Enhanced Tracing