From eaf88aca2af2b8ad7a277c09d0b45f6e2de8ac4a Mon Sep 17 00:00:00 2001 From: Aurelio Bernal Date: Sun, 11 Oct 2026 18:42:08 -0600 Subject: [PATCH] test(ghidra): require pending multi-HI16 coverage in the real MIPS relocatable lane (#1974) * test(ghidra): require pending multi-HI16 coverage in the real MIPS relocatable lane Post-merge review on #1950: the relocation oracle exposes coverage.multiple_hi16, but the real-Ghidra lane never required it, and Clang's C output never produces a LO16 that resolves several pending HI16s. Ghidra's loader could therefore mishandle that stream while the lane still passed, although most GCC-built kernel modules checked contain it. The relocatable fixture adds an assembly block with two lui %hi(rea_mips_rel_carry); LLVM's relocation sort then leaves one HI16 pending until a later LO16 that resolves it together with another HI16, including the carry. The lane now requires multiple_hi16: false on the previous fixture, true for all three variants (LE, BE, soft-float module) on real Ghidra 12.1.4. * test(ghidra): make the MIPS multi-HI16 fixture independent of relocation sorting Review on #1974: the block used two %lo of rea_mips_rel_carry, a symbol with other %lo uses, so the relocation stream depended on the assembler's HI16/LO16 sort. Clang 21 happened to leave a HI16 pending; Clang 17 (per review) and GNU as 2.45 (checked here: HI@4 LO@8 HI@0 LO@10) pair each HI16 with its own LO16, so multiple_hi16 stays false and the lane fails. Use a dedicated symbol with two %hi and exactly one %lo. Any assembler that orders each HI16 before a matching LO16 must then emit both HI16s ahead of that single LO16: Clang 21 gives HI@0 HI@4 LO@8, GNU as 2.45 gives HI@4 HI@0 LO@8. The real-Ghidra lane passes for LE, BE and the soft-float module variant with multiple_hi16 true. --- docs/ghidra-mips.md | 6 +++-- .../verify-real-ghidra-mips-relocatable.mjs | 1 + tests/conformance/c/mips-relocatable.c | 25 ++++++++++++++++++- 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/docs/ghidra-mips.md b/docs/ghidra-mips.md index a3e77f7a0..59d04a8b0 100644 --- a/docs/ghidra-mips.md +++ b/docs/ghidra-mips.md @@ -244,8 +244,10 @@ from the original file bytes. HI16/LO16 are checked as the effective address each LO16 forms with each HI16 it resolves, including several HI16s before one LO16, so a missing signed-low carry fails and a HI16 without a following LO16 fails. The lane also requires matching Ghidra relocation entries and statuses, -section file offsets and sizes, coverage of a shared HI16, a LO16 carry, -external code and data and `R_MIPS_32` words, direct calls from the entry +section file offsets and sizes, coverage of a shared HI16, a LO16 resolving +several pending HI16s (from an assembly block, since Clang does not emit that +from C), a LO16 carry, external code and data and `R_MIPS_32` words, direct +calls from the entry function, the relocatable support lane in the snapshot profile, the relocatable limitation in Evidence and an unchanged source object. The fixture also calls `powf`, an external whose float prototype comes from Ghidra's C diff --git a/scripts/verify-real-ghidra-mips-relocatable.mjs b/scripts/verify-real-ghidra-mips-relocatable.mjs index ab26f0699..da8a821b1 100755 --- a/scripts/verify-real-ghidra-mips-relocatable.mjs +++ b/scripts/verify-real-ghidra-mips-relocatable.mjs @@ -163,6 +163,7 @@ async function verifyFixture(byteOrder, kernelModule) { }); assert.equal(coverage.unplaced, 0, "Fixture references an empty section"); assert.ok(coverage.shared_hi16, "No HI16 shared by several LO16s"); + assert.ok(coverage.multiple_hi16, "No LO16 resolved several pending HI16s"); assert.ok(coverage.lo16_carry, "No LO16 >= 0x8000 forced a HI16 carry"); assert.ok(coverage.external_code, "No external code relocation"); assert.ok(coverage.external_data, "No external data relocation"); diff --git a/tests/conformance/c/mips-relocatable.c b/tests/conformance/c/mips-relocatable.c index 7552a918c..12eb4da5c 100644 --- a/tests/conformance/c/mips-relocatable.c +++ b/tests/conformance/c/mips-relocatable.c @@ -1,7 +1,8 @@ /* Relocatable (ET_REL) static-analysis fixture, built with -c -G0 and never * executed or linked by REA. It exercises the relocations Linux MIPS kernel * modules use: R_MIPS_26 calls, HI16/LO16 address pairs (one HI16 shared by - * several LO16s, and a LO16 >= 0x8000 that forces a HI16 carry), R_MIPS_32 + * several LO16s, two HI16s resolved by one LO16, and a LO16 >= 0x8000 that + * forces a HI16 carry), R_MIPS_32 * data words, and undefined external code and data symbols. powf is an * external whose float prototype Ghidra's C library archive supplies by name, * so soft-float builds must give it the soft-float prototype too. @@ -19,6 +20,8 @@ struct rea_mips_rel_pair rea_mips_rel_pair = { 0x1111, 0x2222 }; /* Places rea_mips_rel_carry past a signed 16-bit low-half boundary. */ char rea_mips_rel_padding[0x8000] = { 1 }; int rea_mips_rel_carry = 0x3333; +/* Referenced only by rea_mips_rel_highs, so its single LO16 stays unique. */ +int rea_mips_rel_highs_target = 0x4444; static const char rea_mips_rel_marker[] = "rea-mips-relocatable-fixture"; __attribute__((noinline)) int rea_mips_rel_leaf(int value) @@ -45,6 +48,26 @@ __attribute__((noinline)) int rea_mips_rel_scaled(int value) return value ^ *(int *)&scaled; } +/* Most GCC-built kernel modules checked contain LO16s that resolve several + * pending HI16s; Clang does not produce that from C. Two %hi and exactly one + * %lo of a symbol nothing else references: any assembler that orders each + * HI16 before a matching LO16 (GNU as, every LLVM version) must emit both + * HI16s ahead of that single LO16, which therefore resolves both. + */ +__asm__(".text\n" + ".globl rea_mips_rel_highs\n" + ".type rea_mips_rel_highs, @function\n" + ".set push\n" + ".set noreorder\n" + "rea_mips_rel_highs:\n" + " lui $2, %hi(rea_mips_rel_highs_target)\n" + " lui $3, %hi(rea_mips_rel_highs_target)\n" + " addiu $2, $2, %lo(rea_mips_rel_highs_target)\n" + " jr $31\n" + " move $3, $2\n" + ".set pop\n" + ".size rea_mips_rel_highs, .-rea_mips_rel_highs\n"); + int rea_mips_rel_entry(int value) { return rea_mips_rel_leaf(value) + rea_mips_rel_far(value) +