Skip to content

fix(harness): pass magic index in r10 for arm32 - #394

Draft
Wenzel wants to merge 2 commits into
intel:mainfrom
Wenzel:fix/arm32-harness-index-245
Draft

Wenzel wants to merge 2 commits into
intel:mainfrom
Wenzel:fix/arm32-harness-index-245

Conversation

@Wenzel

@Wenzel Wenzel commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Fixes #245.

Summary

src/arch/arm.rs reads the magic index from r10 and arguments from r9/r8/r7. The arm32 harness put the testcase buffer in r10 and never set an index, so every start/stop was rejected with index is not configured (index 2129111884 = 0x7ee7ab4c, the reporter's stack buffer; see mov sl, r7 before orr r1,r1,r1 in their disassembly). #217 fixed the aarch64 registers but not arm32.

 HARNESS_START(buf, size_ptr)
-  __orr_extended2(1, buf, size_ptr)                 // r10=buf, r9=size_ptr
+  __orr_extended3(1, DEFAULT_INDEX, buf, size_ptr)  // r10=0,   r9=buf, r8=size_ptr
 HARNESS_STOP()
-  __orr(4)                                          // r10 = whatever was left
+  __orr_extended1(4, DEFAULT_INDEX)
+HARNESS_{START,START_WITH_MAXIMUM_SIZE,START_WITH_MAXIMUM_SIZE_AND_PTR,STOP,ASSERT}_INDEX  // new on arm32
  • harness/tsffs-gcc-arm32.h: ported from the aarch64 header (same macro set and register order). __orr_extended4 saves/restores r7 instead of clobbering it (Thumb frame pointer).
  • harness/tsffs-gcc-aarch64.h: add missing x8/x7 clobbers to __orr_extended3/__orr_extended4.
  • harness/tsffs.h: regenerated with build.sh; harness/test.c no longer skips the *_INDEX tests on arm.
  • tests/arm32_harness_abi_latest.rs: new test. Public Simics has no ARM CPU model, so it compiles every HARNESS_* macro with clang -target arm-unknown-linux-gnueabi -O2, decodes each function with goblin + yaxpeax-arm (existing deps), constant-propagates up to the magic orr rN,rN,rN, and asserts the magic number, r10 index, and r9/r8/r7 arguments.

Evidence

  • Before (old header, non-_INDEX macros only):
    assertion `left == right` failed: start: index register r10
      left: Some(4096)   // buffer pointer 0x1000
     right: Some(0)
    
    Old header with the full test: clang fails, HARNESS_START_INDEX etc. undeclared.
  • After: test test_arm32_harness_abi_latest ... ok

Also: cargo build -r, cargo clippy --test arm32_harness_abi_latest -- -D warnings, and harness/build.sh pass (Simics 7.70.0). Thumb -O0 build of harness/test.c emits no warnings.

Merge Danger

Door: two-way

Blast Radius: arm32 harness users

Targets must be rebuilt with the new header; the TSFFS module is unchanged. Not run end to end in Simics on an ARM target (no public ARM package). Thumb-mode encoding of the orr magic instruction was not verified against Simics.

The arm32 header put the testcase buffer in r10, which TSFFS reads as the
index selector, so HARNESS_START was ignored with 'index is not configured'.
HARNESS_STOP left r10 untouched. Align arm32 with aarch64: index in r10,
args in r9/r8/r7, add *_INDEX macros, and add missing aarch64 clobbers.
@Wenzel
Wenzel force-pushed the fix/arm32-harness-index-245 branch from a426002 to dbe4e00 Compare October 28, 2026 09:10
@Wenzel
Wenzel requested a balanced review from Copilot October 28, 2026 09:10

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

6 open issues
What changed in this PR

Fixes the ARM32 harness ABI by ensuring the magic index is passed in r10 (and arguments in r9/r8/r7) to match what src/arch/arm.rs expects, and adds an integration test that verifies the generated ARM32 harness code without requiring an ARM Simics model.

Changes:

  • Update ARM32 harness macros to pass DEFAULT_INDEX in r10 and add *_INDEX macro variants.
  • Fix AArch64 harness inline-asm clobbers for __orr_extended3/__orr_extended4.
  • Add a new Rust test that cross-compiles harness macros for ARM32 and validates register values via disassembly and constant propagation.
File Description
tests/​arm32_harness_abi_latest.rs Adds a cross-compile + disassembly-based ABI test for ARM32 harness macros/register placement.
harness/​tsffs.h Regenerates umbrella header to reflect ARM32 macro ABI fixes and AArch64 clobber updates.
harness/​tsffs-gcc-arm32.h Fixes ARM32 register assignment (index in r10) and introduces *_INDEX harness macros.
harness/​tsffs-gcc-aarch64.h Adds missing x8/x7 clobbers for correctness under optimization.
harness/​test.c Stops skipping _INDEX harness tests on ARM now that ARM32 supports them.

Review effort: Lite

Comment thread harness/tsffs-gcc-arm32.h
Comment on lines 105 to +113
#define __orr_extended4(value, arg0, arg1, arg2, arg3) \
__asm__ __volatile__( \
"mov r10, %0; mov r9, %1; mov r8, %2; mov r7, %3; " \
"push {r7}; mov r10, %0; mov r9, %1; mov r8, %2; mov r7, %3; " \
"orr r" __tostring(value) ", r" __tostring(value) ", r" __tostring( \
value) \
value) "; pop {r7}" \
: \
: "r"(arg0), "r"(arg1), "r"(arg2), "r"(arg3) \
: "r10", "r9", "r8", "r7");

/// Magic value defined by SIMICS as the "leaf" value of a CPUID instruction
/// that is treated as a magic instruction.
#define MAGIC (0x4711U)
: "r"((unsigned int)arg0), "r"((unsigned int)arg1), \
"r"((unsigned int)arg2), "r"((unsigned int)arg3) \
: "r10", "r9", "r8");
Comment on lines +77 to +78
let manifest_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
let out_dir = PathBuf::from(env!("CARGO_TARGET_TMPDIR")).join("arm32_harness_abi");
Comment on lines +83 to +90
let status = Command::new("clang")
.args([
"-target",
"arm-unknown-linux-gnueabi",
"-marm",
"-mfloat-abi=soft",
"-O2",
])
Comment on lines +107 to +113
let name = elf
.strtab
.get_at(sym.st_name)
.ok_or_else(|| anyhow!("bad symbol name"))?;
let section = &elf.section_headers[sym.st_shndx];
let start = (section.sh_offset + sym.st_value) as usize;
functions.insert(name, &bytes[start..start + sym.st_size as usize]);
Comment on lines +111 to +113
let section = &elf.section_headers[sym.st_shndx];
let start = (section.sh_offset + sym.st_value) as usize;
functions.insert(name, &bytes[start..start + sym.st_size as usize]);
Comment thread harness/tsffs-gcc-arm32.h
/// support multiple start and stop indices, which defaults to 0 if not
/// specified.
#define DEFAULT_INDEX (0x0000U)
#define DEFAULT_INDEX (0x0000)

This branch has not been deployed

No deployments
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.

Getting "index is not configured" error while fuzzing an ARM arch.

2 participants