[lldb][Target] RunThreadPlan to save/restore the ExecutionContext's frame if one exists

[Michael137]

When using SBFrame::EvaluateExpression on a frame that's not the currently selected frame, we would sometimes run into errors such as:

error: error: The context has changed before we could JIT the expression!
error: errored out in DoExecute, couldn't PrepareToExecuteJITExpression

During expression parsing, we call RunStaticInitializers. On our internal fork this happens quite frequently because any usage of, e.g., function pointers, will inject ptrauth fixup code into the expression. The static initializers are run using RunThreadPlan. The ExecutionContext::m_frame_sp going into the RunThreadPlan is the SBFrame that we called EvaluateExpression on. LLDB then tries to save this frame to restore it after the thread-plan ran (the restore occurs by unconditionally overwriting whatever is in ExecutionContext::m_frame_sp). However, if the selected_frame_sp is not the same as the SBFrame, then RunThreadPlan would set the ExecutionContext's frame to a different frame than what we started with. When we PrepareToExecuteJITExpression, LLDB checks whether the ExecutionContext frame changed from when we initially EvaluateExpression, and if did, bails out with the error above.

One such test-case is attached. This currently passes regardless of the fix because our ptrauth static initializers code isn't upstream yet. But the plan is to upstream it soon.

This patch addresses the issue by saving/restoring the frame of the incoming ExecutionContext, if such frame exists. Otherwise, fall back to using the selected frame.

rdar://147456589

[adrian-prantl]

Should this be a C file instead?

[jimingham]

LGTM. Even the variable name made it clear this wasn't being set correctly. We just usually do evaluate expressions in the selected frame context.
This should cause a failure in the current code if you call SBThread.SetSelectedFrame for frame A and then get the SBFrame for another frame and call EvaluateExpression on it? Maybe that would allow you to write a test that fails more directly?

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

When using SBFrame::EvaluateExpression on a frame that's not the currently selected frame, we would sometimes run into errors such as:

error: error: The context has changed before we could JIT the expression!
error: errored out in DoExecute, couldn't PrepareToExecuteJITExpression

During expression parsing, we call RunStaticInitializers. On our internal fork this happens quite frequently because any usage of, e.g., function pointers, will inject ptrauth fixup code into the expression. The static initializers are run using RunThreadPlan. The ExecutionContext::m_frame_sp going into the RunThreadPlan is the SBFrame that we called EvaluateExpression on. LLDB then tries to save this frame to restore it after the thread-plan ran (the restore occurs by unconditionally overwriting whatever is in ExecutionContext::m_frame_sp). However, if the selected_frame_sp is not the same as the SBFrame, then RunThreadPlan would set the ExecutionContext's frame to a different frame than what we started with. When we PrepareToExecuteJITExpression, LLDB checks whether the ExecutionContext frame changed from when we initially EvaluateExpression, and if did, bails out with the error above.

One such test-case is attached. This currently passes regardless of the fix because our ptrauth static initializers code isn't upstream yet. But the plan is to upstream it soon.

This patch addresses the issue by saving/restoring the frame of the incoming ExecutionContext, if such frame exists. Otherwise, fall back to using the selected frame.

rdar://147456589

---

Full diff: https://github.com/llvm/llvm-project/pull/134097.diff

4 Files Affected:

• (modified) lldb/source/Target/Process.cpp (+7-1)
• (added) lldb/test/API/commands/expression/expr-from-non-zero-frame/Makefile (+3)
• (added) lldb/test/API/commands/expression/expr-from-non-zero-frame/TestExprFromNonZeroFrame.py (+28)
• (added) lldb/test/API/commands/expression/expr-from-non-zero-frame/main.cpp (+6)

diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 369933234ccca..f4ecb0b7ea307 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -5080,7 +5080,13 @@ Process::RunThreadPlan(ExecutionContext &exe_ctx,
     return eExpressionSetupError;
   }
 
-  StackID ctx_frame_id = selected_frame_sp->GetStackID();
+  // If the ExecutionContext has a frame, we want to make sure to save/restore
+  // that frame into exe_ctx. This can happen when we run expressions from a
+  // non-selected SBFrame, in which case we don't want some thread-plan
+  // to overwrite the ExecutionContext frame.
+  StackID ctx_frame_id = exe_ctx.HasFrameScope()
+                             ? exe_ctx.GetFrameRef().GetStackID()
+                             : selected_frame_sp->GetStackID();
 
   // N.B. Running the target may unset the currently selected thread and frame.
   // We don't want to do that either, so we should arrange to reset them as
diff --git a/lldb/test/API/commands/expression/expr-from-non-zero-frame/Makefile b/lldb/test/API/commands/expression/expr-from-non-zero-frame/Makefile
new file mode 100644
index 0000000000000..99998b20bcb05
--- /dev/null
+++ b/lldb/test/API/commands/expression/expr-from-non-zero-frame/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/commands/expression/expr-from-non-zero-frame/TestExprFromNonZeroFrame.py b/lldb/test/API/commands/expression/expr-from-non-zero-frame/TestExprFromNonZeroFrame.py
new file mode 100644
index 0000000000000..692d24fd2c332
--- /dev/null
+++ b/lldb/test/API/commands/expression/expr-from-non-zero-frame/TestExprFromNonZeroFrame.py
@@ -0,0 +1,28 @@
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class ExprFromNonZeroFrame(TestBase):
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def test(self):
+        """
+        Tests that we can use SBFrame::EvaluateExpression on a frame
+        that we're not stopped in, even if thread-plans run as part of
+        parsing the expression (e.g., when running static initializers).
+        """
+        self.build()
+
+        (_, _, thread, _) = lldbutil.run_to_source_breakpoint(
+            self, "Break here", lldb.SBFileSpec("main.cpp")
+        )
+        frame = thread.GetFrameAtIndex(1)
+
+        # Using a function pointer inside the expression ensures we
+        # emit a ptrauth static initializer on arm64e into the JITted
+        # expression. The thread-plan that runs for this static
+        # initializer should save/restore the current execution context
+        # frame (which in this test is frame #1).
+        frame.EvaluateExpression("void (*fptr)() = &func; fptr()")
diff --git a/lldb/test/API/commands/expression/expr-from-non-zero-frame/main.cpp b/lldb/test/API/commands/expression/expr-from-non-zero-frame/main.cpp
new file mode 100644
index 0000000000000..27105c339f3e5
--- /dev/null
+++ b/lldb/test/API/commands/expression/expr-from-non-zero-frame/main.cpp
@@ -0,0 +1,6 @@
+void func() { __builtin_printf("Break here"); }
+
+int main(int argc) {
+  func();
+  return 0;
+}

[Michael137]

LGTM. Even the variable name made it clear this wasn't being set correctly. We just usually do evaluate expressions in the selected frame context. This should cause a failure in the current code if you call SBThread.SetSelectedFrame for frame A and then get the SBFrame for another frame and call EvaluateExpression on it? Maybe that would allow you to write a test that fails more directly?

We still need the function pointer stuff (and the ptrauth static initializer code) because that's the only case where we RunThreadPlan as part of parsing (before JIT execution)

[DavidSpickett]

This fails to compile on Windows:
https://lab.llvm.org/buildbot/#/builders/141/builds/7573

lld-link: error: undefined symbol: printf

>>> referenced by main.o:(func)

clang: error: linker command failed with exit code 1 (use -v to see invocation)

No idea why it would complain, perhaps on some platforms the builtin use here is compiled to puts and on windows it isn't?

[Michael137]

Oh I fixed this in a follow-up commit. Is the test still failing?

Yea not exactly sure about why the builtin doesnt link. Your theory sounds plausible

[DavidSpickett]

Sorry yes it is building now but failing:

FAIL: test (TestExprFromNonZeroFrame.ExprFromNonZeroFrame.test)

   Tests that we can use SBFrame::EvaluateExpression on a frame

----------------------------------------------------------------------

Traceback (most recent call last):

  File "C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\test\API\commands\expression\expr-from-non-zero-frame\TestExprFromNonZeroFrame.py", line 29, in test

    self.assertTrue(result.GetError().Success())

AssertionError: False is not true

False is indeed not true, thank you test framework /s

[DavidSpickett]

Skipped for now: d4002b4

It could be another case where we link with link.exe which can't handle DWARF information, and that's needed for the test. We will try to confirm or deny that.

[Michael137]

Thanks!