diff --git a/.claude/skills/debug-test-failures/SKILL.md b/.claude/skills/debug-test-failures/SKILL.md new file mode 100644 index 0000000..be3950f --- /dev/null +++ b/.claude/skills/debug-test-failures/SKILL.md @@ -0,0 +1,214 @@ +--- +name: debug-test-failures +version: "1.0.0" +description: > + This skill should be used when debugging test failures, build errors, runtime + crashes, or assertion failures in the Sleigh/Ghidra decompiler project. It + covers building with libc++ hardening assertions (ci-head-assertions preset), + running ctest, obtaining stack traces from SIGABRT or crash signals, narrowing + down which datatest triggers a failure, creating patch files for upstream Ghidra + source in src/patches/, and recognizing common C++ bug patterns such as + strict-weak ordering violations in comparators. Relevant when encountering + "the build is failing", "ctest fails", decompiler crashes, or libc++ assertion + errors. +--- + +# Debugging Test Failures in the Sleigh Project + +This project builds the Ghidra decompiler as a standalone C++ library. Ghidra source +is fetched via CMake FetchContent and patches are applied via `git am`. The key +challenge is that bugs live in upstream Ghidra code and fixes must be packaged as +patch files. + +## Quick Reference + +```bash +# Build with libc++ assertions (catches strict-weak ordering, container misuse, etc.) +cmake --preset ci-head-assertions +cmake --build build/ci-head-assertions +ctest --test-dir build/ci-head-assertions --output-on-failure + +# Clean cached Ghidra source (needed after adding/changing patches) +rm -rf build/ci-head-assertions/_deps/ghidrasource-* + +# Where the Ghidra C++ source lives after fetch +build/ci-head-assertions/_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/ +``` + +## 0. Prerequisites + +### Create `CMakeUserPresets.json` + +The `ci-head-assertions` preset used throughout this workflow lives in +`CMakeUserPresets.json`, which is not version-controlled and does **not** exist on +a fresh checkout. You must create it in the repository root before any build +commands will work. + +Create `/workspaces/sleigh/CMakeUserPresets.json` with this content: + +```json +{ + "version": 2, + "cmakeMinimumRequired": { "major": 3, "minor": 18, "patch": 0 }, + "configurePresets": [ + { + "name": "libcxx", + "hidden": true, + "cacheVariables": { + "CMAKE_CXX_COMPILER": "clang++", + "CMAKE_CXX_FLAGS": "-stdlib=libc++ -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=3 -fstrict-flex-arrays=3 -fstack-clash-protection -fstack-protector-strong", + "CMAKE_EXE_LINKER_FLAGS": "-stdlib=libc++ -lc++abi", + "CMAKE_SHARED_LINKER_FLAGS": "-stdlib=libc++ -lc++abi" + } + }, + { + "name": "ci-head", + "hidden": true, + "inherits": ["ci-ubuntu"], + "cacheVariables": { "sleigh_RELEASE_TYPE": "HEAD" } + }, + { + "name": "ci-head-assertions", + "binaryDir": "${sourceDir}/build/${presetName}", + "inherits": ["libcxx", "ci-head"], + "generator": "Ninja", + "cacheVariables": { "CMAKE_BUILD_TYPE": "Debug" } + } + ] +} +``` + +This file inherits from `ci-ubuntu` (defined in `CMakePresets.json`), so the +repo's version-controlled presets must also be present (they always are in a +normal checkout). + +## 1. Build Presets + +### The assertions preset + +The `ci-head-assertions` preset (created in Section 0 above) links against libc++ +with `_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG`. This enables runtime +checks that catch undefined behavior in STL usage — most importantly, it validates +that comparators passed to `std::sort` satisfy strict-weak ordering. + +### Other useful presets + +| Preset | Source | Purpose | +|--------|--------|---------| +| `ci-head-assertions` | `CMakeUserPresets.json` | HEAD build with libc++ debug assertions (primary debugging preset) | +| `ci-head` | `CMakeUserPresets.json` | HEAD build without assertions (faster, hidden/base preset) | +| `ci-sanitize` | `CMakePresets.json` | Stable build with ASan + UBSan (repo-controlled) | + +Note: `ci-head-assertions` and `ci-head` are user-local presets created in +Section 0. `ci-sanitize` is the only sanitizer preset version-controlled in the +repo. Add your own sanitizer presets to `CMakeUserPresets.json` as needed. + +## 2. Running Tests + +```bash +# Run all tests +ctest --test-dir build/ci-head-assertions --output-on-failure + +# Run a specific test +ctest --test-dir build/ci-head-assertions -R decomp_datatest --output-on-failure +``` + +There are typically 3 tests: + +| Test | What it does | +|------|--------------| +| `sleigh_decomp_unittest` | Unit tests (fast) | +| `sleigh_decomp_datatest` | Decompiler data tests — runs XML test files from the `datatests/` directory | +| `sleigh_namespace_std_test` | Namespace compilation test | + +The datatest exercises the most code paths and is the most likely to trigger +assertion failures from comparator bugs. + +### How datatest works + +The test binary loads XML files from the datatests directory in alphabetical order. +Each XML file contains binary data + expected decompiler output patterns. The test +decompiles each function and checks the output against `` patterns. + +The exact command ctest uses (check `tests/CTestTestfile.cmake` for the authoritative version): +```bash +./tests/sleigh_decomp_test \ + -sleighpath \ + -path /_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/datatests \ + datatests +``` + +## 3. Getting a Stack Trace from a Crash + +When a test crashes (e.g., SIGABRT from an assertion), you need a stack trace to +identify which comparator or code path is the culprit. The test binaries may not +have full debug symbols, so multiple techniques may be needed. + +See `references/stack-trace-techniques.md` for the full details on each method. +Here's the summary: + +1. **gdb**: Use `gdb -batch` for full demangled backtraces — the simplest method +2. **lldb**: Use `lldb -b` with `-k` for crash-time backtrace commands +3. **LD_PRELOAD backtrace**: Build a small shared library that catches SIGABRT and + prints a backtrace — works when neither debugger can attach (containers) +4. **addr2line**: Resolve raw hex addresses from the backtrace to function names + and source lines + +## 4. Narrowing Down Which Test Triggers a Crash + +> **Tip**: Often the stack trace from Section 3 identifies the exact function and +> source line, making this step unnecessary. Use this binary search technique only +> when the stack trace is unclear or you need to isolate which specific test input +> triggers the crash. + +When you know the test crashes but can't immediately identify the cause from the +stack trace, use binary search on the test XML files. + +The idea: copy subsets of test files to a temporary directory and run the test +binary against that subset. Keep halving until you isolate the triggering file. + +```bash +cd build/ci-head-assertions + +# Copy a subset of test files to a temporary directory +mkdir -p /tmp/test_subset +SRC=_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/datatests +for f in $(ls $SRC/*.xml | sort | sed -n '1,20p'); do cp "$f" /tmp/test_subset/; done + +# Run just those tests +./tests/sleigh_decomp_test -sleighpath . -path /tmp/test_subset datatests +``` + +Important: some crashes only reproduce when specific files are processed together +(accumulated state from earlier tests), so start by splitting the full set in half +rather than testing individual files. If an individual file doesn't crash alone, try +combining it with files that ran before it in alphabetical order. + +## 5. Creating Patches for Ghidra Source + +Patches live in `src/patches/HEAD/` (for HEAD builds) and `src/patches/stable/` +(for stable builds). They are applied via `git am` during the CMake FetchContent +step. See `references/patching-workflow.md` for the full step-by-step. + +### Quick workflow + +1. **Edit the bug** in `build//_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/` +2. **Commit**: `cd` into the ghidrasource-src dir, `git add` + `git commit` +3. **Generate patches**: `git format-patch -o /workspaces/sleigh/src/patches/HEAD/ ..HEAD` +4. **Register** the new patch in `src/setup-ghidra-source.cmake` +5. **Clean and rebuild**: `rm -rf build//_deps/ghidrasource-*` then reconfigure + +The base commit for HEAD is the `ghidra_head_git_tag` value in `src/setup-ghidra-source.cmake`. + +## 6. Common Bug Patterns + +The most common bugs are **strict-weak ordering violations** in comparators used +by `std::sort` or `std::set`. These are caught by libc++ assertions at runtime. +Key patterns include null pointer sentinels that don't check self-comparison, +special index early-returns without checking the other operand, and unsafe casts +in `compareDependency` methods of TypePartial* classes. + +See `references/common-bug-patterns.md` for detailed before/after code examples, +real-world instances from the codebase (e.g., `PullRecord::operator<` in +`bitfield.cc`, `FlowBlock::compareFinalOrder` in `block.cc`), and a systematic +audit checklist for finding new comparator bugs. diff --git a/.claude/skills/debug-test-failures/references/common-bug-patterns.md b/.claude/skills/debug-test-failures/references/common-bug-patterns.md new file mode 100644 index 0000000..3d54004 --- /dev/null +++ b/.claude/skills/debug-test-failures/references/common-bug-patterns.md @@ -0,0 +1,72 @@ +# Common Bug Patterns in Ghidra Decompiler Source + +## Strict-weak ordering violations + +The most common class of bugs caught by libc++ assertions. A comparator for +`std::sort` or `std::set` must satisfy: + +- **Irreflexivity**: `comp(a, a)` must return `false` +- **Asymmetry**: if `comp(a, b)` is true then `comp(b, a)` must be false +- **Transitivity**: if `comp(a, b)` and `comp(b, c)` then `comp(a, c)` + +The libc++ assertion message looks like: +``` +strict_weak_ordering_check.h:50: libc++ Hardening assertion + !__comp(*(__first + __a), *(__first + __b)) failed: + Your comparator is not a valid strict-weak ordering +``` + +### Pattern 1: Null pointer sentinel without self-check + +```cpp +// BUG: when both are null, returns true (violates irreflexivity) +if (ptr == nullptr) + return true; + +// FIX: only return true when the other is non-null +if (ptr == nullptr) + return (other.ptr != nullptr); +``` + +Real example: `PullRecord::operator<` in `bitfield.cc` — when both `readOp` +pointers were null, the comparator returned `true`. + +### Pattern 2: Special index without self-check + +```cpp +// BUG: when comparing the entry point with itself, returns true +if (bl1->getIndex() == 0) return true; + +// FIX: check that the other is different +if (bl1->getIndex() == 0) return (bl2->getIndex() != 0); +``` + +Real example: `FlowBlock::compareFinalOrder` in `block.cc`. + +### Pattern 3: Unsafe cast in compareDependency (TypePartial* classes) + +The `TypePartialEnum`, `TypePartialStruct`, and `TypePartialUnion` classes have +`compareDependency` methods that cast `op` to their own type. If `op` is actually +the parent container type (not a partial), the cast accesses invalid memory. + +```cpp +// BUG: casts op to TypePartialFoo without checking if op IS the container +TypePartialFoo *tp = (TypePartialFoo *) &op; + +// FIX: add a guard before the cast +if (container == &op) return 1; // op is our container +TypePartialFoo *tp = (TypePartialFoo *) &op; +``` + +Real example: `TypePartialEnum::compareDependency` in `type.cc` (patched in 0005). + +## How to audit comparators systematically + +1. Search for all `operator<`, `::compare`, and `::compareDependency` functions +2. Search for all `sort(`, `stable_sort(`, and `std::set` usage to find which + comparators are actually used in sorting contexts +3. For each comparator, mentally trace the case where both arguments are identical + — does it return false? +4. Check for early-return branches that don't verify the other operand differs +5. For `compareDependency` methods in TypePartial* classes, check for the + container-guard pattern before unsafe casts diff --git a/.claude/skills/debug-test-failures/references/patching-workflow.md b/.claude/skills/debug-test-failures/references/patching-workflow.md new file mode 100644 index 0000000..defb8bd --- /dev/null +++ b/.claude/skills/debug-test-failures/references/patching-workflow.md @@ -0,0 +1,153 @@ +# Patching Workflow for Ghidra Source + +## Overview + +Ghidra source is fetched via CMake FetchContent from GitHub. Patches are applied +automatically during the configure step using `git am`. The patch files live in +the Sleigh repo and are version-controlled. + +## Directory Structure + +``` +src/patches/ +├── HEAD/ # Patches for the HEAD (development) Ghidra commit +│ ├── 0001-Fix-UBSAN-errors-in-decompiler.patch +│ ├── 0002-Use-stroull-instead-of-stroul-to-parse-address-offse.patch +│ └── ... +└── stable/ # Patches for the stable Ghidra release + ├── 0001-Fix-UBSAN-errors-in-decompiler.patch + └── ... +``` + +## Key Configuration + +The base commit and patch list are defined in `src/setup-ghidra-source.cmake`: + +- `ghidra_head_git_tag`: The pinned HEAD commit hash +- `ghidra_version` / `ghidra_git_tag`: The stable version tag +- `ghidra_patches`: The ordered list of patch files to apply + +## Step-by-Step: Creating a New Patch + +### 1. Identify the build preset + +Determine which build you're fixing. The Ghidra source checkout location depends +on the preset: + +``` +build//_deps/ghidrasource-src/ +``` + +For example: +``` +build/ci-head-assertions/_deps/ghidrasource-src/ +``` + +The C++ source files are in: +``` +build//_deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/cpp/ +``` + +### 2. Make the fix + +Edit the file directly in the Ghidra source checkout. The file already has all +existing patches applied, so you're working on top of the current patch stack. + +### 3. Commit in the Ghidra source checkout + +```bash +cd build//_deps/ghidrasource-src + +# In a fresh container, git may require identity configuration before committing: +# git config user.name "Your Name" && git config user.email "your@email.com" + +git add Ghidra/Features/Decompiler/src/decompile/cpp/.cc +git commit -m "decompiler: Short description + +Longer explanation of the bug and fix. Mention what assertion or test +was failing and why." +``` + +Commit message conventions: +- Prefix with `decompiler:` for decompiler fixes +- First line should be concise (under 72 chars) +- Body explains the *why*, not just the *what* + +### 4. Generate patch files + +Use `git format-patch` to regenerate ALL patches from the base commit: + +```bash +# Get the base commit from src/setup-ghidra-source.cmake +# For HEAD: it's the ghidra_head_git_tag value +# For stable: it's the Ghidra_X.Y.Z_build tag + +git format-patch -o /workspaces/sleigh/src/patches/HEAD/ ..HEAD +``` + +This regenerates patches 0001 through 000N. Existing patches will be regenerated +with updated numbering in the `[PATCH N/M]` subject line, but the content stays +the same. The new patch gets the next number. + +### 5. Register in CMake + +Edit `src/setup-ghidra-source.cmake` and add the new patch file to the appropriate +`ghidra_patches` list: + +```cmake +"${CMAKE_CURRENT_LIST_DIR}/patches/HEAD/0006-your-new-patch-name.patch" +``` + +### 6. Verify from scratch + +Clean the cached source so FetchContent re-fetches and applies all patches fresh: + +```bash +rm -rf build//_deps/ghidrasource-* +cmake --preset +cmake --build build/ +ctest --test-dir build/ --output-on-failure +``` + +Watch the configure output for: +``` +Applying: +``` + +If a patch fails to apply, you'll see `Patch failed at NNNN ...` during configure. + +## HEAD vs Stable + +- **HEAD patches** (`src/patches/HEAD/`): Applied to the development commit. May + include fixes for newly added files (e.g., `bitfield.cc` is HEAD-only). +- **Stable patches** (`src/patches/stable/`): Applied to the release tag. These + are typically a subset of HEAD patches that also apply to stable. + +Check `src/setup-ghidra-source.cmake` to see which files are HEAD-only (look for +`if("${sleigh_RELEASE_TYPE}" STREQUAL "HEAD")` blocks). + +## Troubleshooting + +### Patch fails to apply + +If `git am` fails during configure, the Ghidra source checkout may be in a dirty +state. Clean it: + +```bash +rm -rf build//_deps/ghidrasource-* +``` + +Then reconfigure. If the patch itself is wrong, fix it and try again. + +### Patches conflict with new Ghidra commit + +When updating the pinned Ghidra HEAD commit, existing patches may not apply cleanly. +See the [wiki](https://github.com/lifting-bits/sleigh/wiki/Patching-and-Updating) +for the full rebasing procedure. + +### Source already patched from manual edits + +If you edited files directly in the checkout and then try to reconfigure, CMake +may try to apply patches on top of already-patched code. Always clean the +`_deps/ghidrasource-*` directories before reconfiguring after generating new +patches. diff --git a/.claude/skills/debug-test-failures/references/stack-trace-techniques.md b/.claude/skills/debug-test-failures/references/stack-trace-techniques.md new file mode 100644 index 0000000..acc4b21 --- /dev/null +++ b/.claude/skills/debug-test-failures/references/stack-trace-techniques.md @@ -0,0 +1,149 @@ +# Stack Trace Techniques + +When a test binary crashes (typically SIGABRT from a libc++ hardening assertion), +these techniques can be used to identify the exact function and source line. + +## Method 1: gdb (simplest) + +gdb in batch mode produces full demangled backtraces and works reliably in most +environments, including containers with default seccomp profiles. + +```bash +cd build/ci-head-assertions + +gdb -batch \ + -ex "run -sleighpath . -path _deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/datatests datatests" \ + -ex "bt" \ + ./tests/sleigh_decomp_test +``` + +gdb will run the program, and when it crashes (e.g., SIGABRT), it automatically +stops and prints the backtrace. The output includes fully demangled C++ function +names and source locations. + +If gdb is unavailable, try lldb (Method 2). If neither debugger can attach +(restricted ptrace), fall back to Method 3. + +## Method 2: lldb + +Use lldb in batch mode with the full test arguments. The key is to handle SIGABRT +so it stops instead of terminating. + +**Important**: Use `-k` (one-line-on-crash) for the backtrace command, not `-o`. +Commands passed via `-o` run sequentially after file load, but when lldb hits a +signal stop, it enters its event loop and won't execute subsequent `-o` commands. +The `-k` flag queues commands to run specifically when the target crashes. + +```bash +cd build/ci-head-assertions + +lldb -b \ + -o "process handle SIGABRT --stop true --pass false" \ + -o "run -sleighpath . -path _deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/datatests datatests" \ + -k "thread backtrace" \ + -k "quit" \ + -- ./tests/sleigh_decomp_test +``` + +If lldb shows full symbol names (function + file + line), you're done. + +If lldb fails with "personality set failed: Operation not permitted", this is +common in Docker containers where ptrace is restricted. Use Method 3 instead. + +## Method 3: LD_PRELOAD Backtrace Library + +Build a small shared library that intercepts SIGABRT and prints a backtrace: + +```bash +cat > /tmp/bt_preload.c << 'BTEOF' +#include +#include +#include +#include +#include + +void bt_handler(int sig) { + void *array[50]; + int size = backtrace(array, 50); + fprintf(stderr, "=== BACKTRACE (signal %d) ===\n", sig); + backtrace_symbols_fd(array, size, STDERR_FILENO); + fprintf(stderr, "=== END BACKTRACE ===\n"); + signal(sig, SIG_DFL); + raise(sig); +} + +__attribute__((constructor)) +void install_handler(void) { + signal(SIGABRT, bt_handler); +} +BTEOF +gcc -shared -fPIC -o /tmp/bt_preload.so /tmp/bt_preload.c -rdynamic +``` + +Run the test with the preloaded library: + +```bash +cd build/ci-head-assertions + +LD_PRELOAD=/tmp/bt_preload.so ./tests/sleigh_decomp_test \ + -sleighpath . \ + -path _deps/ghidrasource-src/Ghidra/Features/Decompiler/src/decompile/datatests \ + datatests +``` + +The output will look like: + +``` +=== BACKTRACE (signal 6) === +./tests/sleigh_decomp_test(+0xf1a64)[0x55c312f02a64] +./tests/sleigh_decomp_test(+0xf17ea)[0x55c312f027ea] +./tests/sleigh_decomp_test(+0xef732)[0x55c312f00732] +... +=== END BACKTRACE === +``` + +The addresses in parentheses (like `+0xf1a64`) are the key — these are offsets +from the binary's load address. + +## Method 4: addr2line + +Resolve the hex offsets from the backtrace to function names and source lines: + +```bash +for addr in 0xf1a64 0xf17ea 0xef732 0x31fbd9 0x31d536; do + echo -n "$addr: " + addr2line -e ./tests/sleigh_decomp_test -f -C "$addr" + echo "---" +done +``` + +Flags: +- `-e` specifies the executable +- `-f` prints function names +- `-C` demangles C++ names + +Example output: +``` +0xf1a64: void std::__1::__check_strict_weak_ordering_sorted<...>(...) +/usr/lib/llvm-21/bin/../include/c++/v1/__debug_utils/strict_weak_ordering_check.h:51 +--- +0x31fbd9: ghidra::BlockGraph::orderBlocks() +/path/to/block.hh:431 +--- +``` + +This tells you exactly which sort call and which comparator function triggered +the assertion. + +## Putting It All Together + +Typical debugging flow: + +1. Run the test: `ctest --test-dir build/ci-head-assertions -R decomp_datatest --output-on-failure` +2. See assertion failure → need stack trace +3. Try gdb first (Method 1) — it typically gives the best output with full demangled symbols +4. If gdb is unavailable, try lldb (Method 2) +5. If neither debugger can attach → build LD_PRELOAD library (Method 3) +6. Resolve addresses with addr2line (Method 4) if the backtrace only has hex offsets +7. Now you know which comparator function and sort call is the problem +8. Read the comparator code and check for strict-weak ordering violations