From 8454975d2ccc5e3fe60e37f704ac7bf50cf0978b Mon Sep 17 00:00:00 2001 From: Jacob Bower Date: Tue, 31 Mar 2020 10:59:50 -0700 Subject: [PATCH] Save/restore IP when leaving the interpreter Summary: This diff implements the instruction pointer save/restore trick Tzvetan came up with; allowing us to observe and modify the IP from outside the interpreter loop with negligible overhead. From Tzvetan's internal post on the subject: > [Today] the interpreter IP is just a local variable in the interpreter function, so there is no way to get to its value from outside the function. It lives in a register and we don't want to make it a Runtime field since the overhead [of accessing it via memory in the interpeter loop] would kill us. > However, if you really think about it, it only lives in a register while the interpreter function is running. For the rest of the time, it is spilled by the C++ compiler onto the stack. So, precisely when we need it, it is actually stored in memory. The only problem is, we don't know where! Admittedly, that is an annoying problem, but it feels like it should be solvable. > What if, instead of relying on the compiler to spill the IP register, we manually spill it ourselves, to a known location? It works. Example: https://godbolt.org/z/ftSDnp This diff implements this approach across the whole interpreter loop: whenever we call out of the loop we capture/publish the IP and restore it again immediately after the external call returns. This means we can now see the IP outside the interpret loop and even change it. This is effectively "for free" as the compiler now skips spilling/restoring the IP behind the scenes. The immediate benefit of this is knowing the current IP allows us to have more accurate stack-traces during execution. In future this may enabled tricks like changing the IP before returning to the interpreter loop, allowing things outside the interpreter to affect program flow without adding logic to the interpreter loop. Reviewed By: tmikov Differential Revision: D20151091 fbshipit-source-id: 3814382639800208d8985a32ede31ba8f7ff7c80 --- ReactCommon/hermes/inspector/chrome/tests/ConnectionTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ReactCommon/hermes/inspector/chrome/tests/ConnectionTests.cpp b/ReactCommon/hermes/inspector/chrome/tests/ConnectionTests.cpp index b89030dc99d..18208192f67 100644 --- a/ReactCommon/hermes/inspector/chrome/tests/ConnectionTests.cpp +++ b/ReactCommon/hermes/inspector/chrome/tests/ConnectionTests.cpp @@ -1255,7 +1255,7 @@ TEST(ConnectionTests, testEvalOnCallFrameException) { // TODO: unsure why these frames are here, but they're in hdb tests // too. Ask Hermes about if they really should be there. FrameInfo("eval", 0, 0).setLineNumberMax(19), - FrameInfo("(native)", 0, 0), + FrameInfo("callme", 12, 2), FrameInfo("global", 0, 0).setLineNumberMax(19)}); expectEvalResponse(conn, msgId + 2, 5); msgId += 3;