[lldb/Plugin] Add artificial stackframe loading in ScriptedThread

[mib]

Summary

This patch adds the ability for ScriptedThread to load artificial stack
frames. To do so, the interpreter instance can create a list that will
contain the frame index and its pc address.

Then, when the Scripted Process plugin stops, it will refresh its
Scripted Threads state by invalidating their register context and load
to list from the interpreter object and reconstruct each frame.

This patch also removes all of the default implementation for
get_stackframes from the derived ScriptedThread classes, and add the
interface code for the Scripted Thread Interface.

rdar://88721095

Signed-off-by: Med Ismail Bennani <[e-mail]>

[jingham]

Some nits. Mostly I think the "idx" field in the frame list is redundant and only allows you to get the order wrong w/o adding useful functionality.

I haven't studied Pavel's changes to the Python lifecycle management yet, so I can't comment on whether that part of the patch is done correctly.

I don't think it's necessary at this point, but at some point you are going to have to make ScriptedStackFrame be a thing or there won't be a natural way to produce variables from the frame. The other way to program it would be to fake a CFA, and then fake memory reads so that the regular Variable printing code will produce the ValueObjectVariable's. I can see that being a useful method in some cases, but super-labor intensive in others...

lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

151

The GetStackFrames call should take a Status parameter, in case it has some good reason why it couldn't fetch the stack frames. It will make debugging these things a lot easier if you can pass the real error back to yourself.

171

As I say below, I don't think the specified frame_idx is necessary. But it's also odd that you initialize it to the total number of frames, then immediately overwrite is with the value from the dictionary. If you don't get a value, you exit from this closure, so this seems wasted work.

172

It seems awkward to have the producer of artificial stack frames number the frames. After all, they are already putting them into an array, and the natural way to do that is in stack frame order. Unless you see some really good use case for putting them in scrambled and using the "idx" key to unscramble them, the redundant idx seems like something you could get wrong w/o adding much value.

286

LoadArtificialStackFrames returns a bool for success/failure. It always bugs me when returns signifying errors get dropped on the floor like this.

lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py

78

newline

[mib]

lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

151

For now the Scripted{Process,Thread}Interface will log the error to the appropriate channel, but I can see having all the interface methods return an llvm::Expected<T> object with an error message instead. I'll do it in a follow-up patch.

172

Makes sense.

286

Thread::RefreshStateAfterStop doesn't return anything, and the failure case is already handled in LoadArtificialStackFrames by logging an error message in the appropriate channel.

Do you have any other suggestion on how I could improve this ?

[mib]

Address @jingham's comments

[mib]

Reformat and update error messages.

[jingham]

LGTM

[shafik]

lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

159

const

195

const

196

const

286

If we are never going to check the return value, we should not have one, it is meaningless.

lldb/examples/python/scripted_process/scripted_process.py

def get_stackframes(self):
                 self.symbol_ctx = symbol_ctx
         ```
 
         Returns:
             List[ScriptedFrame]: A list of `ScriptedStackFrame`
                 containing for each entry, the frame index, the canonical
                 frame address, the program counter value for that frame
                 and a symbol context.
-                None if the list is empty.
+                The list can be empty.
         """
-        return 0
+        return self.frames
 
     def get_register_info(self):
         if self.register_info is None:
             self.register_info = dict()
             if self.scripted_process.arch == 'x86_64':
                 self.register_info['sets'] = ['General Purpose Registers']
                 self.register_info['registers'] = INTEL64_GPR
             elif 'arm64' in self.scripted_process.arch:

lldb/include/lldb/Target/StackFrameList.h

 #include <memory>
 #include <mutex>
 #include <vector>
 
 #include "lldb/Target/StackFrame.h"
 
 namespace lldb_private {
 
+class ScriptedThread;
+
 class StackFrameList {
 public:
   // Constructors and Destructors
   StackFrameList(Thread &thread, const lldb::StackFrameListSP &prev_frames_sp,
                  bool show_inline_frames);
 
   ~StackFrameList();
 
public:
 
   size_t GetStatus(Stream &strm, uint32_t first_frame, uint32_t num_frames,
                    bool show_frame_info, uint32_t num_frames_with_source,
                    bool show_unique = false,
                    const char *frame_marker = nullptr);
 
 protected:
   friend class Thread;
+  friend class ScriptedThread;
 
   bool SetFrameAtIndex(uint32_t idx, lldb::StackFrameSP &frame_sp);
 
   void GetFramesUpTo(uint32_t end_idx);
 
   void GetOnlyConcreteFramesUpTo(uint32_t end_idx, Unwind &unwinder);
 
   void SynthesizeTailCallFrames(StackFrame &next_frame);

lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp

bool ScriptedProcess::DoUpdateThreadList(ThreadList &old_thread_list,
   thread_info_sp->ForEach(create_scripted_thread);
 
   return new_thread_list.GetSize(false) > 0;
 }
 
 void ScriptedProcess::RefreshStateAfterStop() {
   // Let all threads recover from stopping and do any clean up based on the
   // previous thread state (if any).
+  m_thread_list.RefreshStateAfterStop();
 }
 
 bool ScriptedProcess::GetProcessInfo(ProcessInstanceInfo &info) {
   info.Clear();
   info.SetProcessID(GetID());
   info.SetArchitecture(GetArchitecture());
   lldb::ModuleSP module_sp = GetTarget().GetExecutableModule();
   if (module_sp) {

lldb/source/Plugins/Process/scripted/ScriptedThread.h

public:
   Create(ScriptedProcess &process,
          StructuredData::Generic *script_object = nullptr);
 
   lldb::RegisterContextSP GetRegisterContext() override;
 
   lldb::RegisterContextSP
   CreateRegisterContextForFrame(lldb_private::StackFrame *frame) override;
 
+  bool LoadArtificialStackFrames();
+
   bool CalculateStopInfo() override;
 
   const char *GetInfo() override { return nullptr; }
 
   const char *GetName() override;
 
   const char *GetQueueName() override;
 

lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

return GetInterface()->ErrorWithMessage<lldb::RegisterContextSP>(
         LLDBLog::Thread);
 
   reg_ctx_memory->SetAllRegisterData(data_sp);
   m_reg_context_sp = reg_ctx_memory;
 
   return m_reg_context_sp;
 }
 
+bool ScriptedThread::LoadArtificialStackFrames() {
+  StructuredData::ArraySP arr_sp = GetInterface()->GetStackFrames();
+
+  Status error;
+  if (!arr_sp)
+    return GetInterface()->ErrorWithMessage<bool>(
+        LLVM_PRETTY_FUNCTION, "Failed to get scripted thread stackframes.",
+        error, LLDBLog::Thread);
+
+  size_t arr_size = arr_sp->GetSize();
+  if (arr_size > std::numeric_limits<uint32_t>::max())
+    return GetInterface()->ErrorWithMessage<bool>(
+        LLVM_PRETTY_FUNCTION,
+        llvm::Twine(
+            "StackFrame array size (" + llvm::Twine(arr_size) +
+            llvm::Twine(
+                ") is greater than maximum autorized for a StackFrameList."))
+            .str(),
+        error, LLDBLog::Thread);
+
+  StackFrameListSP frames = GetStackFrameList();
+
+  for (size_t idx = 0; idx < arr_size; idx++) {
+
+    StructuredData::Dictionary *dict;
+
+    if (!arr_sp->GetItemAtIndexAsDictionary(idx, dict) || !dict)
+      return GetInterface()->ErrorWithMessage<bool>(
+          LLVM_PRETTY_FUNCTION,
+          llvm::Twine(
+              "Couldn't get artificial stackframe dictionary at index (" +
+              llvm::Twine(idx) + llvm::Twine(") from stackframe array."))
+              .str(),
+          error, LLDBLog::Thread);
+
+    lldb::addr_t pc;
+    if (!dict->GetValueForKeyAsInteger("pc", pc))
+      return ScriptedInterface::ErrorWithMessage<bool>(
+          LLVM_PRETTY_FUNCTION,
+          "Couldn't find value for key 'pc' in stackframe dictionary.", error,
+          LLDBLog::Thread);
+
+    Address symbol_addr;
+    symbol_addr.SetLoadAddress(pc, &this->GetProcess()->GetTarget());
+
+    lldb::addr_t cfa = LLDB_INVALID_ADDRESS;
+    bool cfa_is_valid = false;
+    const bool behaves_like_zeroth_frame = false;
+    SymbolContext sc;
+    symbol_addr.CalculateSymbolContext(&sc);
+
+    StackFrameSP synth_frame_sp = std::make_shared<StackFrame>(
+        this->shared_from_this(), idx, idx, cfa, cfa_is_valid, pc,
+        StackFrame::Kind::Artificial, behaves_like_zeroth_frame, &sc);
+
+    if (!frames->SetFrameAtIndex(static_cast<uint32_t>(idx), synth_frame_sp))
+      return GetInterface()->ErrorWithMessage<bool>(
+          LLVM_PRETTY_FUNCTION,
+          llvm::Twine("Couldn't add frame (" + llvm::Twine(idx) +
+                      llvm::Twine(") to ScriptedThread StackFrameList."))
+              .str(),
+          error, LLDBLog::Thread);
+  }
+
+  return true;
+}
+
 bool ScriptedThread::CalculateStopInfo() {
   StructuredData::DictionarySP dict_sp = GetInterface()->GetStopReason();
 
   Status error;
   if (!dict_sp)
     return GetInterface()->ErrorWithMessage<bool>(
         LLVM_PRETTY_FUNCTION, "Failed to get scripted thread stop info.", error,
         LLDBLog::Thread);
if (!stop_info_sp)
     return false;
 
   SetStopInfo(stop_info_sp);
   return true;
 }
 
 void ScriptedThread::RefreshStateAfterStop() {
   GetRegisterContext()->InvalidateIfNeeded(/*force=*/false);
+  LoadArtificialStackFrames();
 }
 
 lldb::ScriptedThreadInterfaceSP ScriptedThread::GetInterface() const {
   return m_scripted_thread_interface_sp;
 }
 
 std::shared_ptr<DynamicRegisterInfo> ScriptedThread::GetDynamicRegisterInfo() {
   CheckInterpreterAndScriptObject();

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.h

protected:
 
   void FormatArgs(std::string &fmt) const {}
 
   // The lifetime is managed by the ScriptInterpreter
   ScriptInterpreterPythonImpl &m_interpreter;
 };
 
 template <>
+StructuredData::ArraySP
+ScriptedPythonInterface::ExtractValueFromPythonObject<StructuredData::ArraySP>(
+    python::PythonObject &p, Status &error);
+
+template <>
 StructuredData::DictionarySP
 ScriptedPythonInterface::ExtractValueFromPythonObject<
     StructuredData::DictionarySP>(python::PythonObject &p, Status &error);
 
 template <>
 Status ScriptedPythonInterface::ExtractValueFromPythonObject<Status>(
     python::PythonObject &p, Status &error);
 

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedPythonInterface.cpp

 ScriptedPythonInterface::GetStatusFromMethod(llvm::StringRef method_name) {
   Status error;
   Dispatch<Status>(method_name, error);
 
   return error;
 }
 
 template <>
+StructuredData::ArraySP
+ScriptedPythonInterface::ExtractValueFromPythonObject<StructuredData::ArraySP>(
+    python::PythonObject &p, Status &error) {
+  python::PythonList result_list(python::PyRefType::Borrowed, p.get());
+  return result_list.CreateStructuredArray();
+}
+
+template <>
 StructuredData::DictionarySP
 ScriptedPythonInterface::ExtractValueFromPythonObject<
     StructuredData::DictionarySP>(python::PythonObject &p, Status &error) {
   python::PythonDictionary result_dict(python::PyRefType::Borrowed, p.get());
   return result_dict.CreateStructuredDictionary();
 }
 
 template <>

lldb/source/Plugins/ScriptInterpreter/Python/ScriptedThreadPythonInterface.cpp

StructuredData::DictionarySP ScriptedThreadPythonInterface::GetStopReason() {
 
   if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, dict, error))
     return {};
 
   return dict;
 }
 
 StructuredData::ArraySP ScriptedThreadPythonInterface::GetStackFrames() {
-  return nullptr;
+  Status error;
+  StructuredData::ArraySP arr =
+      Dispatch<StructuredData::ArraySP>("get_stackframes", error);
+
+  if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, arr, error))
+    return {};
+
+  return arr;
 }
 
 StructuredData::DictionarySP ScriptedThreadPythonInterface::GetRegisterInfo() {
   Status error;
   StructuredData::DictionarySP dict =
       Dispatch<StructuredData::DictionarySP>("get_register_info", error);
 
   if (!CheckStructuredDataObject(LLVM_PRETTY_FUNCTION, dict, error))

lldb/test/API/functionalities/scripted_process/TestScriptedProcess.py

def test_scripted_process_and_scripted_thread(self):
 
         self.assertTrue(GPRs, "Invalid General Purpose Registers Set")
         self.assertGreater(GPRs.GetNumChildren(), 0)
         for idx, reg in enumerate(GPRs, start=1):
             if idx > 21:
                 break
             self.assertEqual(idx, int(reg.value, 16))
 
+        self.assertTrue(frame.IsArtificial(), "Frame is not artificial")
+        pc = frame.GetPCAddress().GetLoadAddress(target)
+        self.assertEqual(pc, 0x0100001b00)

lldb/test/API/functionalities/scripted_process/dummy_scripted_process.py

class DummyScriptedProcess(ScriptedProcess):
 
     def get_scripted_thread_plugin(self):
         return DummyScriptedThread.__module__ + "." + DummyScriptedThread.__name__
 
 
 class DummyScriptedThread(ScriptedThread):
     def __init__(self, process, args):
         super().__init__(process, args)
+        self.frames.append({"pc": 0x0100001b00 })
 
     def get_thread_id(self) -> int:
         return 0x19
 
     def get_name(self) -> str:
         return DummyScriptedThread.__name__ + ".thread-1"
 
     def get_state(self) -> int:
         return lldb.eStateStopped
 
     def get_stop_reason(self) -> Dict[str, Any]:
         return { "type": lldb.eStopReasonSignal, "data": {
             "signal": signal.SIGINT
         } }
 
-    def get_stackframes(self):
-        class ScriptedStackFrame:
-            def __init__(idx, cfa, pc, symbol_ctx):
-                self.idx = idx
-                self.cfa = cfa
-                self.pc = pc
-                self.symbol_ctx = symbol_ctx
-
-
-        symbol_ctx = lldb.SBSymbolContext()
-        frame_zero = ScriptedStackFrame(0, 0x42424242, 0x5000000, symbol_ctx)
-        self.frames.append(frame_zero)
-
-        return self.frame_zero[0:0]
-
     def get_register_context(self) -> str:
         return struct.pack(
                 '21Q', 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21)
 
 
 def __lldb_init_module(debugger, dict):
     if not 'SKIP_SCRIPTED_PROCESS_LAUNCH' in os.environ:
         debugger.HandleCommand(
             "process launch -C %s.%s" % (__name__,
                                      DummyScriptedProcess.__name__))
     else:
         print("Name of the class that will manage the scripted process: '%s.%s'"
-                % (__name__, DummyScriptedProcess.__name__))
+                % (__name__, DummyScriptedProcess.__name__))
 

lldb/test/API/functionalities/scripted_process/invalid_scripted_process.py

class InvalidScriptedThread(ScriptedThread):
     def get_state(self) -> int:
         return lldb.eStateInvalid
 
     def get_stop_reason(self) -> Dict[str, Any]:
         return { "type": lldb.eStopReasonSignal, "data": {
             "signal": signal.SIGTRAP
         } }
 
-    def get_stackframes(self):
-        class ScriptedStackFrame:
-            def __init__(idx, cfa, pc, symbol_ctx):
-                self.idx = idx
-                self.cfa = cfa
-                self.pc = pc
-                self.symbol_ctx = symbol_ctx
-
-
-        symbol_ctx = lldb.SBSymbolContext()
-        frame_zero = ScriptedStackFrame(0, 0x42424242, 0x5000000, symbol_ctx)
-        self.frames.append(frame_zero)
-
-        return self.frame_zero[0:0]
-
     def get_register_context(self) -> str:
         return None
 
 def __lldb_init_module(debugger, dict):
     if not 'SKIP_SCRIPTED_PROCESS_LAUNCH' in os.environ:
         debugger.HandleCommand(
             "process launch -C %s.%s" % (__name__,
                                      InvalidScriptedProcess.__name__))
     else:
         print("Name of the class that will manage the scripted process: '%s.%s'"
                 % (__name__, InvalidScriptedProcess.__name__))

lldb/test/API/functionalities/scripted_process/stack_core_scripted_process.py

def get_stop_reason(self) -> Dict[str, Any]:
                 elif self.scripted_process.arch == 'x86_64':
                     stop_reason["type"] = lldb.eStopReasonSignal
                     stop_reason["data"]["signal"] = signal.SIGTRAP
                 else:
                     stop_reason["type"] = self.corefile_thread.GetStopReason()
 
         return stop_reason
 
-    def get_stackframes(self):
-        class ScriptedStackFrame:
-            def __init__(idx, cfa, pc, symbol_ctx):
-                self.idx = idx
-                self.cfa = cfa
-                self.pc = pc
-                self.symbol_ctx = symbol_ctx
-
-
-        symbol_ctx = lldb.SBSymbolContext()
-        frame_zero = ScriptedStackFrame(0, 0x42424242, 0x5000000, symbol_ctx)
-        self.frames.append(frame_zero)
-
-        return self.frame_zero[0:0]
-
     def get_register_context(self) -> str:
         if not self.corefile_thread or self.corefile_thread.GetNumFrames() == 0:
             return None
         frame = self.corefile_thread.GetFrameAtIndex(0)
 
         GPRs = None
         registerSet = frame.registers # Returns an SBValueList.
         for regs in registerSet: