target/mips: fix spurious EXCP_RI under UC_TLB_VIRTUAL after a delay-slot TLB fill

[retrocpugeek]

Fixes #2272.

Symptom

Under UC_TLB_VIRTUAL, a memory access in a branch delay slot raises a spurious
EXCP_RI ("Reserved Instruction / branch in delay-forbidden slot") on the branch
target, aborting emulation. The MIPS repro is glibc's __libc_setup_tls PT_TLS
program-header scan, whose loop body ends in a branch whose delay-slot load forces a
TLB fill.

Root cause

unicorn_fill_tlb() called cpu_restore_state(cs, retaddr, false) unconditionally.
cpu_restore_state() → restore_state_to_opc() rolls env back to the instruction
boundary of the faulting access, and on MIPS that re-applies the per-insn start
hflags. For a delay-slot instruction those include the branch hflags
(MIPS_HFLAG_B / MIPS_HFLAG_BDS32 / …), which are only ever meant to exist during
translation, never in the live runtime hflags.

That rollback is fine on the paths cpu_restore_state() was designed for — the CPU is
about to leave the run loop (raise an exception, or read the exact PC then exit), so
the transient hflags are re-derived on the next entry and never observed. But on a
successful TLB fill the faulting access is resumed in place inside the same TB.
Nothing clears the leaked bits, so the next TB — the branch target — is translated as
if it sat in a delay slot, and MIPS raises EXCP_RI.

Fix

Remove the unconditional cpu_restore_state() from the fill path. The exception path
still restores via cpu_loop_exit_restore() (in raise_mmu_exception()), mirroring the
CPUs' own *_cpu_tlb_fill handlers, which only restore on the exception path. The
UC_HOOK_TLB_FILL callback is invoked with the faulting page/type and the entry to
fill; it does not consume rolled-back register state.

This also removes a per-fill cpu_restore_state() from the hot path, which is exactly
the change requested for performance reasons in #2342.

Note on the earlier wording: I originally justified this as "the TLB_FILL callback only
needs the faulting address." That over-generalized — per the FAQ and #2342/#2049, hooks
are normally expected to see a synced PC. The accurate framing is the one above: the
restore is unconditional on a resume-in-place path, where it corrupts live env state
regardless of whether any hook is registered.

Note on #2273: it also targets #2272 but changes only the no-hook fallback path's
permissions; it does not fix the case where a UC_HOOK_TLB_FILL hook is registered
(verified — the RI still fires). This fix covers both paths and is orthogonal to #2273.

Answer to "can a UC_HOOK_MEM_*_UNMAPPED hook trigger a similar bug?"

Yes — and it is not specific to the virtual TLB. The MEM hook paths in cputlb.c
(UC_HOOK_MEM_READ/WRITE and the *_UNMAPPED variants) also call
cpu_restore_state(uc->cpu, retaddr, false) before invoking the hook, and after a
*_UNMAPPED hook maps the page and returns true the faulting access likewise resumes
in place. So the same delay-slot hflag leak occurs.

I reproduced it in the ordinary soft-MMU (no UC_TLB_VIRTUAL): a delay-slot load
that faults, serviced by a UC_HOOK_MEM_READ_UNMAPPED callback that maps the page and
returns true, resumes with the leaked hflags and raises EXCP_RI (intno 20 via
UC_HOOK_INTR; UC_ERR_EXCEPTION otherwise). Pre-mapping the page so no hook fires
makes the same program complete correctly — isolating the cause to the hook-triggered
cpu_restore_state(). Standalone failing repro (kept out of this PR): #2352.

The important difference: those MEM paths call cpu_restore_state() deliberately, to
honour the "synced PC in hooks" contract, and are already gated (!synced && !skip_sync_pc_on_exit). They can't simply drop the call the way the vtlb fill can.
The proper fix for that class is lower-level — clearing the transient branch hflags
after a resume-in-place restore (in restore_state_to_opc() / its MIPS callers) so the
contract and correctness both hold. That's a broader change than this regression fix, so
I've kept it out of this PR; happy to open a follow-up if you agree with the direction.

Test

Adds test_mips_virtual_tlb_branch_in_delay_slot_2272 in tests/unit/test_mips.c,
exercising the UC_HOOK_TLB_FILL path (how a soft-MMU front-end such as Qiling drives
the virtual TLB).

---

Downstream tracking: qilingframework/qiling#1645 (Qiling MIPS64 real-ELF/glibc support gates its CI tests on this fix).

[PhilippTakacs]

Thanks for the detailed explanation of the bug.

The UC_HOOK_TLB_FILL callbacks only need the faulting address, which is passed explicitly

This is not correct, see #2342 and #2049. The FAQ also says following:

We should always have valid PC all the time. Please report an issue for your case.

So general it's assumed that the PC is synced (and all other registers) when a hook is called. I see that the vtlb hook is a candidate for an exception and would also like to remove the cpu_restore_state() call (for performance reasons).

By the way: Is it possible to trigger similar bug with an UC_HOOK_MEM_*_UNMAPPED hook?

[retrocpugeek]

You're right — thanks, I've corrected the PR description. "Hooks only need the faulting address" over-generalized the contract; the accurate framing is that the restore here is unconditional on a resume-in-place path, so it corrupts live env regardless of whether a hook is registered. And since you'd like this call gone for perf (#2342) anyway, this fix and that goal line up.

On the UC_HOOK_MEM_*_UNMAPPED question: yes, the same bug is reachable — and it's not vtlb-specific. The MEM hook paths in cputlb.c also cpu_restore_state() before the callback and then resume the access in place after a *_UNMAPPED hook maps the page. I reproduced it in the plain soft-MMU (no UC_TLB_VIRTUAL): a delay-slot lw that faults, serviced by a UC_HOOK_MEM_READ_UNMAPPED hook that maps the page and returns true, raises EXCP_RI (intno 20). Pre-mapping the page so no hook fires makes the same program complete correctly. Standalone failing repro on a separate branch so it doesn't muddy this PR: #2352.

The real root cause is that MIPS restore_state_to_opc() re-applies the delay-slot branch hflags into the live env, which is only safe when the CPU is about to leave the run loop. The vtlb fill can simply not restore (this PR); the MEM paths restore on purpose for the synced-PC contract, so they'd need the transient branch hflags cleared after a resume-in-place restore instead. Happy to open a follow-up PR for that if you agree with the direction.