InvalidateCacheRange does not update fast dispatch table
comex opened this issue · 4 comments
I'm not sure how easy to reproduce this is, but while running Super Mario Maker 2 in yuzu 6c72b7f01109 / dynarmic 6.0.1 on Linux x86-64, using the new GDB stub, I ran into a situation where, for certain instruction addresses:
- Consistently, either Yuzu fails to stop on a breakpoint I have set there (even though I know the game is repeatedly executing that address), or Yuzu stops on the breakpoint despite me having previously disabled the breakpoint (causing gdb to print
Thread 1 "MainThread" received signal SIGTRAP, Trace/breakpoint trap
). - Repeatedly enabling and disabling the breakpoint does not help.
- I verified that
Jit::Impl::InvalidateCacheRange
is being called properly after I enable or disable the breakpoint. - It turns out that I can temporarily fix the situation, using a debugger attached to yuzu, by either:
- Setting
invalidate_entire_cache
to true in theJit::Impl
object; or - Manually calling
ClearFastDispatchTable()
.
By "temporarily", I mean that it prevents the misbehavior until the next time I enable or disable the breakpoint, at which point Yuzu again fails to recognize the change.
- Setting
From a quick code inspection, it seems like the entry in the fast dispatch table for the old JIT block is not being cleared. A64EmitX64::Unpatch
would do so, but nothing ever calls that. EmitX64::InvalidateBasicBlocks
may call Unpatch
, but since Unpatch
is not virtual, it only calls the base EmitX64::Unpatch
. However, I'm not sure whether Unpatch
is the right place to do this anyway, since EmitX64::InvalidateBasicBlocks
only calls it if the address has entries in patch_information
, which seems unrelated to the fast dispatch table. (However, I could easily be missing something as I haven't spent too much time looking at this code).
cc @liamwhite
Thanks for digging into this.
Unpatch
is not virtual
Unpatch
is virtual.
since
EmitX64::InvalidateBasicBlocks
only calls it if the address has entries inpatch_information
This is the logic error here.
Will push a fix.
Reproduced in a small test case, unfortunately the bug seems to be more complex than above. Looking into this.
TEST_CASE("ensure fast dispatch entry is cleared even when a block does not have any patching requirements", "[a64]") {
A64TestEnv env;
A64::UserConfig conf{&env};
A64::Jit jit{conf};
REQUIRE(conf.HasOptimization(OptimizationFlag::FastDispatch));
env.code_mem_start_address = 100;
env.code_mem.clear();
env.code_mem.emplace_back(0xd2800d80); // MOV X0, 108
env.code_mem.emplace_back(0xd61f0000); // BR X0
env.code_mem.emplace_back(0xd2800540); // MOV X0, 42
env.code_mem.emplace_back(0x14000000); // B .
jit.SetPC(100);
env.ticks_left = 4;
jit.Run();
REQUIRE(jit.GetRegister(0) == 42);
jit.SetPC(100);
env.ticks_left = 4;
jit.Run();
REQUIRE(jit.GetRegister(0) == 42);
jit.InvalidateCacheRange(108, 4);
jit.SetPC(100);
env.ticks_left = 4;
jit.Run();
REQUIRE(jit.GetRegister(0) == 42);
env.code_mem[2] = 0xd28008a0; // MOV X0, 69
jit.SetPC(100);
env.ticks_left = 4;
jit.Run();
REQUIRE(jit.GetRegister(0) == 42);
jit.InvalidateCacheRange(108, 4);
jit.SetPC(100);
env.ticks_left = 4;
jit.Run();
REQUIRE(jit.GetRegister(0) == 69);
jit.SetPC(100);
env.ticks_left = 4;
jit.Run();
REQUIRE(jit.GetRegister(0) == 69);
}
Found a second long-standing bug in the fast-dispatch lookup code.