fix: x86-64 JIT SIGSEGV handler crash with comp_catchfault on Linux

[midwan]

Summary

• Fix silent crash when starting A4000 config with 68040 + JIT + A4091 SCSI + comp_catchfault=true on Linux x86-64
• Add comp_catchfault fallback to POSIX SIGSEGV handler (matching Windows __except behavior) with jit_exception_pending mechanism
• Fix handle_access() rejecting valid Amiga addresses when host fault address exceeds 32 bits
• Add dummy_bank bypass in all SIGSEGV handlers (x86-64, ARM64, 32-bit ARM) to prevent signal-unsafe THROW(2) from gary_nonrange()
• Fix SIB byte handling in x86-64 decode_instruction()

Fixes #1815

Test plan

• A4000 config (68040 + JIT + A4091 + comp_catchfault=true) boots to Workbench on Linux x86-64
• Standard A500/A1200 config with JIT works normally (regression test)
• No JIT-related crashes or exception loops in log

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0473b32b5d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[midwan]

@claude give me a code review on this PR

[claude]

Claude finished @midwan's task in 4m 6s —— View job

---

Code Review: PR #1816 — x86-64 JIT SIGSEGV handler crash with comp_catchfault

Tasks:

• Gather context and read changed files
• Review src/jit/compemu.h
• Review src/jit/x86/exception_handler.cpp
• Review src/newcpu.cpp
• Review src/osdep/sigsegv_handler.cpp
• Address Codex review comments
• Post final review

---

Overall this is a solid, well-reasoned fix to a real crash. The root cause diagnosis is correct and the multi-layer approach (fix address math → add dummy_bank bypass → add comp_catchfault fallback) is the right strategy. Below are my findings ordered by severity.

---

Correctness Issues

1. SIB byte fix does not handle mod=0x00 + SIB.base=0b101 case

src/jit/x86/exception_handler.cpp, lines 359–361

if (*r != -1 && (pc[1] & 0x07) == 0x04) {
    *len += 1;
}

When mod=0b00 and rm=0b100 (SIB present), if the SIB byte's base field is 0b101, the encoding means "disp32-only base" — adding 4 more bytes beyond the SIB byte. The fix only adds 1 for the SIB itself. For the specific JIT-generated pattern (MOV r32, [R15+reg*1+disp32], which uses mod=0x80) this is never exercised, so it's not a practical problem today. But it makes the decoder silently wrong for some valid x86-64 encodings. Consider documenting this as a known limitation, or add a guard:

// Note: does not handle mod=00, SIB.base=101 (disp32-only base case)
// safe because JIT only generates mod=0x80 patterns

2. uae_p32 vs. corrected formula inconsistency in comp_catchfault path

src/jit/x86/exception_handler.cpp, line 846

uae_u32 amiga_addr = uae_p32(address) - uae_p32(NATMEM_OFFSET);

On x86-64, uae_p32(x) is (uintptr)(x) (see compemu_x86.h:568) — so this subtracts two uintptr values and truncates to uae_u32, which is equivalent to the corrected formula used in handle_access(). It's not a bug, but it's inconsistent with the (uae_u32)(fault_addr - (uintptr_t) NATMEM_OFFSET) pattern just introduced 200 lines above. Fix this →

---

Minor Issues

3. jit_exception_pending checks are not guarded for ARM64

src/newcpu.cpp, lines 5753 and 5808

if (jit_exception_pending) { ... Exception(exc); }

jit_exception_pending is only ever set inside exception_handler.cpp (x86-64-only code). On ARM64 builds, the variable exists (defined in newcpu.cpp under #ifdef JIT, declared in compemu.h) but will always be 0 — so the check is dead code. The Codex P1 concern about a compile error is incorrect (both declaration and definition are visible), but wrapping these checks in #if defined(CPU_x86_64) || defined(_M_AMD64) would be cleaner and self-documenting. Fix this →

4. Indentation inconsistency in sigsegv_handler.cpp

src/osdep/sigsegv_handler.cpp, lines 437–441

			/* dummy_bank handler is NOT signal-safe: it calls gary_nonrange()
		 * which can trigger hardware_exception2() -> THROW(2) (longjmp from

The comment body continuation uses fewer tabs than the opener. Cosmetic, but inconsistent with the surrounding code style. Fix this →

5. write_log calls in signal handler context

src/jit/x86/exception_handler.cpp, lines 823–834, 847–848, 863

The new diagnostic write_log calls inside sigsegv_handler() (outside handle_access()) are not async-signal-safe — write_log may internally use printf/malloc. This is consistent with the existing pattern in this file so it's not a regression, but worth noting: these log calls are only reached on the error path (before exit() or before comp_catchfault redirect), so the risk is limited to the diagnostic path.

---

Correctness Confirmed

Addressing the Codex automated review comments:

• Codex P1 ("guard jit_exception_pending to x86 JIT builds"): The claim of a compile error is incorrect. compemu.h provides the extern declaration visible to all JIT builds; newcpu.cpp provides the definition under #ifdef JIT. ARM64 JIT will compile fine — the check is simply dead code. See minor issue Keyboard LEDs assigned to drives don't work in DietPi (work in Raspbian) #3 above.
• Codex P2 ("Handle pending JIT faults in threaded CPU mode"): This was already addressed in commit 2a5b594 which added the identical jit_exception_pending check to cpu_thread_run_jit().

---

What's Well Done

• Address range fix in handle_access(): The fault_addr - natmem_offset > 0xffffffff check correctly handles the case where natmem_offset + amiga_addr wraps past 4 GB — the previous fault_addr > 0xffffffff would incorrectly reject valid Amiga addresses when natmem is placed in the upper 2 GB of 32-bit virtual space.
• i <= current_compile_p upper-bound check: Tightens the JIT range test, preventing the SIGSEGV handler from trying to decode non-JIT instructions.
• dummy_bank bypass: All three handler paths (x86-64 handle_access, ARM64, 32-bit ARM) correctly handle dummy_bank without calling the signal-unsafe gary_nonrange(). On ARM64/32-bit ARM, the bypass falls through to the shared PC-advance and handled = HANDLE_EXCEPTION_OK block — structurally correct.
• comp_catchfault fallback: The mechanism (exception2_setup + jit_exception_pending + popall_do_nothing redirect) cleanly separates the signal handler's responsibility (capture fault info, unwind JIT) from exception processing (raise Exception(2) in the CPU loop) — matching the spirit of the Windows __except path.
• SIB byte fix: Correctly adds 1 to instruction length when ModR/M rm=0b100, covering the JIT-generated [R15+reg+disp32] pattern.

---

The two critical fixes (address range math and dummy_bank bypass) are correct. The comp_catchfault mechanism is a reasonable and well-contained design. The minor issues above don't block the fix and can be addressed in follow-up commits.