[lldb] Add Getdescription function for SBInstruction.

[rdhindsa]

Summary

Add another Getdescription function for SBInstruction.

When trying to print the instruction, existing GetDescription function doesn't have access to Execution Context. Hence, it prints instructions of the following form for the added test case:
libc.so.6[0x3bce1]: movq 0x108(%rsp), %rax

When a target is passed with this new API call, it is able to extract execution context and hence it prints in the following form:
0x7ffff7af6ce1:movq 0x108(%rsp), %rax

[JDevlieghere]

lldb/source/API/SBInstruction.cpp

260

In addition to the LLDB_RECORD_METHOD, you also need the corresponding LLDB_REGISTER_METHOD macro. If you remove the macro, lldb-instr can generate both for you if you pass it the cpp file.

[rdhindsa]

Added LLDB_REGISTER_METHOD for GetDescription.

[labath]

I'm not sure what's the exact use case here, but I /think/ that passing just the target part of the execution context will is not sufficient to enable all the disassembler bells and whistles. For example the regular "disassemble" command will also print a "-> " next to the instruction that references the current PC.

Even if that is not required for your use case, I think that a better (more generic) API would be for this function to take a SBExecutionContext argument, and let the user choose how it wants to populate it.

lldb/source/API/SBInstruction.cpp

241–256

It seems like this function could now just delegate to the new version, passing a null/empty target or execution context

[jingham]

Would it be better to make an API that takes an SBExecutionContext? If you knew the frame that held

In D109249#2988808, @labath wrote:

I'm not sure what's the exact use case here, but I /think/ that passing just the target part of the execution context will is not sufficient to enable all the disassembler bells and whistles. For example the regular "disassemble" command will also print a "-> " next to the instruction that references the current PC.

Even if that is not required for your use case, I think that a better (more generic) API would be for this function to take a SBExecutionContext argument, and let the user choose how it wants to populate it.

I thought the same thing, but then I got to wondering what you should do when the SBExecutionContext you were passed in has a frame with a PC that doesn't point to the instruction you were passed. Is that an error? Does it matter?

I still think the SBExecutionContext is a better choice, and will prevent us from having to add another override later on when we find we need the thread or frame. But we should decide up front whether we want there to be any linkage between the exact ExecutionContext and the instruction you are dumping.

lldb/bindings/interface/SBInstruction.i

public:
 
     void
     Print (lldb::FileSP BORROWED);
 
     bool
     GetDescription (lldb::SBStream &description);
 
     bool
+    GetDescription (lldb::SBStream &description, lldb::SBTarget target);
+
+    bool
     EmulateWithFrame (lldb::SBFrame &frame, uint32_t evaluate_options);
 
     bool
     DumpEmulation (const char * triple); // triple is to specify the architecture, e.g. 'armv6' or 'armv7-apple-ios'
 
     bool
     TestEmulation (lldb::SBStream &output_stream, const char *test_file);
 

lldb/include/lldb/API/SBInstruction.h

 //===-- SBInstruction.h -----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_API_SBINSTRUCTION_H
 #define LLDB_API_SBINSTRUCTION_H
 
 #include "lldb/API/SBData.h"
 #include "lldb/API/SBDefines.h"
 
 #include <cstdio>
 
 // There's a lot to be fixed here, but need to wait for underlying insn
 // implementation to be revised & settle down first.
 
 class InstructionImpl;
public:
   void Print(FILE *out);
 
   void Print(SBFile out);
 
   void Print(FileSP out);
 
   bool GetDescription(lldb::SBStream &description);
 
+  bool GetDescription(lldb::SBStream &s, lldb::SBTarget target);
+
   bool EmulateWithFrame(lldb::SBFrame &frame, uint32_t evaluate_options);
 
   bool DumpEmulation(const char *triple); // triple is to specify the
                                           // architecture, e.g. 'armv6' or
                                           // 'armv7-apple-ios'
 
   bool TestEmulation(lldb::SBStream &output_stream, const char *test_file);
 

lldb/source/API/SBInstruction.cpp

 //===-- SBInstruction.cpp -------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/API/SBInstruction.h"
 #include "SBReproducerPrivate.h"
 
 #include "lldb/API/SBAddress.h"
 #include "lldb/API/SBFrame.h"
 #include "lldb/API/SBFile.h"
 
 #include "lldb/API/SBInstruction.h"
 #include "lldb/API/SBStream.h"
void SBInstruction::SetOpaque(const lldb::DisassemblerSP &disasm_sp,
                               const lldb::InstructionSP &inst_sp) {
   m_opaque_sp = std::make_shared<InstructionImpl>(disasm_sp, inst_sp);
 }
 
 bool SBInstruction::GetDescription(lldb::SBStream &s) {
   LLDB_RECORD_METHOD(bool, SBInstruction, GetDescription, (lldb::SBStream &),
                      s);
 
   lldb::InstructionSP inst_sp(GetOpaque());
   if (inst_sp) {
     SymbolContext sc;
     const Address &addr = inst_sp->GetAddress();
     ModuleSP module_sp(addr.GetModule());
     if (module_sp)
       module_sp->ResolveSymbolContextForAddress(addr, eSymbolContextEverything,
                                                 sc);
     // Use the "ref()" instead of the "get()" accessor in case the SBStream
     // didn't have a stream already created, one will get created...
     FormatEntity::Entry format;
     FormatEntity::Parse("${addr}: ", format);
     inst_sp->Dump(&s.ref(), 0, true, false, nullptr, &sc, nullptr, &format, 0);
     return true;
   }
   return false;
 }
 
+bool SBInstruction::GetDescription(lldb::SBStream &s, SBTarget target) {
+  LLDB_RECORD_METHOD(bool, SBInstruction, GetDescription,
+                     (lldb::SBStream &, lldb::SBTarget), s, target);
+
+  lldb::InstructionSP inst_sp(GetOpaque());
+  if (inst_sp) {
+    ExecutionContext exe_ctx;
+    TargetSP target_sp(target.GetSP());
+    std::unique_lock<std::recursive_mutex> lock;
+    if (target_sp) {
+      lock = std::unique_lock<std::recursive_mutex>(target_sp->GetAPIMutex());
+      target_sp->CalculateExecutionContext(exe_ctx);
+    }
+    SymbolContext sc;
+    const Address &addr = inst_sp->GetAddress();
+    ModuleSP module_sp(addr.GetModule());
+    if (module_sp)
+      module_sp->ResolveSymbolContextForAddress(addr, eSymbolContextEverything,
+                                                sc);
+    FormatEntity::Entry format;
+    FormatEntity::Parse("${addr}:", format);
+    inst_sp->Dump(&s.ref(), 0, true, false, &exe_ctx, &sc, nullptr, &format, 0);
+    return true;
+  }
+  return false;
+}
+
 void SBInstruction::Print(FILE *outp) {
   LLDB_RECORD_METHOD(void, SBInstruction, Print, (FILE *), outp);
   FileSP out = std::make_shared<NativeFile>(outp, /*take_ownership=*/false);
   Print(out);
 }
 
 void SBInstruction::Print(SBFile out) {
   LLDB_RECORD_METHOD(void, SBInstruction, Print, (SBFile), out);
void RegisterMethods<SBInstruction>(Registry &R) {
   LLDB_REGISTER_METHOD(size_t, SBInstruction, GetByteSize, ());
   LLDB_REGISTER_METHOD(lldb::SBData, SBInstruction, GetData,
                        (lldb::SBTarget));
   LLDB_REGISTER_METHOD(bool, SBInstruction, DoesBranch, ());
   LLDB_REGISTER_METHOD(bool, SBInstruction, HasDelaySlot, ());
   LLDB_REGISTER_METHOD(bool, SBInstruction, CanSetBreakpoint, ());
   LLDB_REGISTER_METHOD(bool, SBInstruction, GetDescription,
                        (lldb::SBStream &));
+  LLDB_REGISTER_METHOD(bool, SBInstruction, GetDescription,
+                       (lldb::SBStream &, lldb::SBTarget));
   LLDB_REGISTER_METHOD(void, SBInstruction, Print, (FILE *));
   LLDB_REGISTER_METHOD(void, SBInstruction, Print, (SBFile));
   LLDB_REGISTER_METHOD(void, SBInstruction, Print, (FileSP));
   LLDB_REGISTER_METHOD(bool, SBInstruction, EmulateWithFrame,
                        (lldb::SBFrame &, uint32_t));
   LLDB_REGISTER_METHOD(bool, SBInstruction, DumpEmulation, (const char *));
   LLDB_REGISTER_METHOD(bool, SBInstruction, TestEmulation,
                        (lldb::SBStream &, const char *));
 }
 
 }
 }

lldb/test/API/functionalities/disassemble/Makefile

+CXX_SOURCES := main.cpp
+include Makefile.rules

lldb/test/API/functionalities/disassemble/TestDisassemble.py

+"""
+Test that description for instruction can print address if target is provided.
+"""
+
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class TestDisassembledInstructionPrint(TestBase):
+  mydir = TestBase.compute_mydir(__file__)
+
+  def test(self):
+
+    self.build()
+    exe = self.getBuildArtifact("a.out")
+    target = self.dbg.CreateTarget(exe)
+    self.assertTrue(target, VALID_TARGET)
+    self.runCmd("run", RUN_SUCCEEDED)
+
+    thread = target.GetProcess().GetSelectedThread()
+    instructions = target.ReadInstructions(thread.GetFrameAtIndex(0).GetPCAddress(),1)
+    instr = instructions.GetInstructionAtIndex(0)
+    strm_instr = lldb.SBStream()
+    instr.GetDescription(strm_instr, target)
+    disasm_instr = strm_instr.GetData()
+    self.assertTrue(disasm_instr.startswith("0x"))

lldb/test/API/functionalities/disassemble/main.cpp

+#include <signal.h>
+int main() {
+  // Break here
+  raise(SIGSEGV);
+  return 0;
+}