Skip to content

Fix critical correctness bugs in SASS rewriter and interceptor#1

Open
Zorg33 wants to merge 1 commit into
jonpry:masterfrom
Zorg33:fix/rewriter-correctness
Open

Fix critical correctness bugs in SASS rewriter and interceptor#1
Zorg33 wants to merge 1 commit into
jonpry:masterfrom
Zorg33:fix/rewriter-correctness

Conversation

@Zorg33

@Zorg33 Zorg33 commented Apr 4, 2026

Copy link
Copy Markdown

Summary

  • ELF header parsing: e_phnum/e_shnum were read from offset 48/50 (inside e_flags field) instead of the correct 56/60, causing the rewriter to process zero sections and produce unmodified output
  • Multi-kernel ELF splice: cum_growth double-counted offsets already shifted by the inner loop, corrupting binaries with 2+ .text sections
  • Predicated FFMA split: FMUL was forced to always-execute (0xFF), clobbering r_dest when the predicate is false and temp_reg == r_dest
  • Opcode matching: "FMA" in op incorrectly matched DFMA (double) and HFMA2 (half), splitting them with float FMUL/FADD opcodes
  • Jump table safety: verified_jump_tables was computed via BRX/LDC tracing but never consulted — .nv.constant* values matching PCs were blindly patched, risking corruption of float/integer literals
  • Per-kernel REGCOUNT: global max_reg set every kernel's register count to the max across all kernels, reducing occupancy
  • Program headers: p_filesz/p_memsz/p_offset were not updated after .text expansion
  • Interceptor get_elf_size: computed size from header positions + 8192 padding instead of walking section data extents
  • Thread safety: declared g_mutex was never locked in patch_and_load
  • Minor: write() return check, file-open guards in tests, R255 filtering, symtab bounds checks, SHT_DYNSYM support, unused deps removed

Builds on c500a4c (Fix elf processing) with additional correctness fixes.

Test plan

  • Build with ./build.sh on sm_70 hardware
  • Run ./run_test.sh — all 4 suites (basic, comprehensive, stress, runtime JIT) should pass
  • Verify rewriter actually transforms FFMAs: check [Rewriter] Patch Complete output shows Sections: N where N > 0
  • Test with a multi-kernel cubin (comprehensive test) to validate the splice fix
  • Test with predicated FFMA kernels to validate predicate preservation

🤖 Generated with Claude Code

Resolves multiple correctness issues that made the rewriter either
a no-op or produce corrupt output for non-trivial cubins:

- Fix ELF header parsing: e_phnum/e_shnum were read from offset 48/50
  (inside e_flags) instead of 56/60, causing zero sections to be processed
- Fix multi-kernel ELF splice: remove cum_growth double-counting that
  corrupted binaries with 2+ .text sections
- Fix predicated FFMA split: preserve original predicate on FMUL to
  avoid clobbering r_dest when predicate is false
- Fix opcode matching: use startswith('FFMA') to avoid matching DFMA/HFMA2
- Use verified_jump_tables to filter .nv.constant patching, preventing
  corruption of float/integer literals
- Track per-kernel max register for REGCOUNT instead of global max
- Update program headers (p_filesz/p_memsz/p_offset) after .text expansion
- Fix get_elf_size to walk section data extents instead of header+8192 guess
- Use declared g_mutex in patch_and_load for actual thread safety
- Add write() return value check, file-open guards, R255 filtering,
  symtab bounds checks, SHT_DYNSYM support

Incorporates and extends c500a4c (Fix elf processing) with additional
correctness fixes.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant