lawrence_danna created this revision.
lawrence_danna added reviewers: JDevlieghere, jasonmolenda, labath.
Herald added a project: LLDB.

This patch replaces the FILE* python bindings for SBInstruction and
SBInstructionList and replaces them with the new, safe SBFile and FileSP
bindings.

I also re-enable `Test_Disassemble_VST1_64`, because now we can use
the file bindings as an additional test of the disassembler, and we
can use the disassembler test as a test of the file bindings.

The bugs referred to in the comments appear to have been fixed.   The
radar is closed now and the bugzilla bug does not reproduce with the
instructions given.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D68890

Files:
  lldb/include/lldb/API/SBInstruction.h
  lldb/include/lldb/API/SBInstructionList.h
  
lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py
  
lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py
  
lldb/packages/Python/lldbsuite/test/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
  lldb/scripts/interface/SBInstruction.i
  lldb/scripts/interface/SBInstructionList.i
  lldb/source/API/SBInstruction.cpp
  lldb/source/API/SBInstructionList.cpp

Index: lldb/source/API/SBInstructionList.cpp
===================================================================
--- lldb/source/API/SBInstructionList.cpp
+++ lldb/source/API/SBInstructionList.cpp
@@ -11,8 +11,10 @@
 #include "lldb/API/SBAddress.h"
 #include "lldb/API/SBInstruction.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/API/SBFile.h"
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Core/Module.h"
+#include "lldb/Core/StreamFile.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Utility/Stream.h"
 
@@ -118,21 +120,41 @@
 
 void SBInstructionList::Print(FILE *out) {
   LLDB_RECORD_METHOD(void, SBInstructionList, Print, (FILE *), out);
-
   if (out == nullptr)
     return;
+  StreamFile stream(out, false);
+  GetDescription(stream);
 }
 
-bool SBInstructionList::GetDescription(lldb::SBStream &description) {
+void SBInstructionList::Print(SBFile out) {
+  LLDB_RECORD_METHOD(void, SBInstructionList, Print, (SBFile), out);
+  if (!out.IsValid())
+    return;
+  StreamFile stream(out.GetFile());
+  GetDescription(stream);
+}
+
+void SBInstructionList::Print(FileSP out_sp) {
+  LLDB_RECORD_METHOD(void, SBInstructionList, Print, (FileSP), out_sp);
+  if (!out_sp || !out_sp->IsValid())
+    return;
+  StreamFile stream(out_sp);
+  GetDescription(stream);
+}
+
+bool SBInstructionList::GetDescription(lldb::SBStream &stream) {
   LLDB_RECORD_METHOD(bool, SBInstructionList, GetDescription,
-                     (lldb::SBStream &), description);
+                     (lldb::SBStream &), stream);
+  return GetDescription(stream.ref());
+}
+
+bool SBInstructionList::GetDescription(Stream &sref) {
 
   if (m_opaque_sp) {
     size_t num_instructions = GetSize();
     if (num_instructions) {
       // Call the ref() to make sure a stream is created if one deesn't exist
       // already inside description...
-      Stream &sref = description.ref();
       const uint32_t max_opcode_byte_size =
           m_opaque_sp->GetInstructionList().GetMaxOpcocdeByteSize();
       FormatEntity::Entry format;
@@ -200,6 +222,8 @@
   LLDB_REGISTER_METHOD(void, SBInstructionList, AppendInstruction,
                        (lldb::SBInstruction));
   LLDB_REGISTER_METHOD(void, SBInstructionList, Print, (FILE *));
+  LLDB_REGISTER_METHOD(void, SBInstructionList, Print, (SBFile));
+  LLDB_REGISTER_METHOD(void, SBInstructionList, Print, (FileSP));
   LLDB_REGISTER_METHOD(bool, SBInstructionList, GetDescription,
                        (lldb::SBStream &));
   LLDB_REGISTER_METHOD(bool, SBInstructionList,
Index: lldb/source/API/SBInstruction.cpp
===================================================================
--- lldb/source/API/SBInstruction.cpp
+++ lldb/source/API/SBInstruction.cpp
@@ -11,6 +11,7 @@
 
 #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"
@@ -255,10 +256,21 @@
   return false;
 }
 
-void SBInstruction::Print(FILE *out) {
-  LLDB_RECORD_METHOD(void, SBInstruction, Print, (FILE *), out);
+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);
+  Print(out.GetFile());
+}
+
+void SBInstruction::Print(FileSP out_sp) {
+  LLDB_RECORD_METHOD(void, SBInstruction, Print, (FileSP), out_sp);
 
-  if (out == nullptr)
+  if (!out_sp || !out_sp->IsValid())
     return;
 
   lldb::InstructionSP inst_sp(GetOpaque());
@@ -269,7 +281,7 @@
     if (module_sp)
       module_sp->ResolveSymbolContextForAddress(addr, eSymbolContextEverything,
                                                 sc);
-    StreamFile out_stream(out, false);
+    StreamFile out_stream(out_sp);
     FormatEntity::Entry format;
     FormatEntity::Parse("${addr}: ", format);
     inst_sp->Dump(&out_stream, 0, true, false, nullptr, &sc, nullptr, &format,
@@ -358,6 +370,8 @@
   LLDB_REGISTER_METHOD(bool, SBInstruction, GetDescription,
                        (lldb::SBStream &));
   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 *));
Index: lldb/scripts/interface/SBInstructionList.i
===================================================================
--- lldb/scripts/interface/SBInstructionList.i
+++ lldb/scripts/interface/SBInstructionList.i
@@ -55,7 +55,10 @@
     AppendInstruction (lldb::SBInstruction inst);
 
     void
-    Print (FILE *out);
+    Print (lldb::SBFile out);
+
+    void
+    Print (lldb::FileSP BORROWED);
 
     bool
     GetDescription (lldb::SBStream &description);
Index: lldb/scripts/interface/SBInstruction.i
===================================================================
--- lldb/scripts/interface/SBInstruction.i
+++ lldb/scripts/interface/SBInstruction.i
@@ -57,7 +57,10 @@
     CanSetBreakpoint ();
 
     void
-    Print (FILE *out);
+    Print (lldb::SBFile out);
+
+    void
+    Print (lldb::FileSP BORROWED);
 
     bool
     GetDescription (lldb::SBStream &description);
Index: lldb/packages/Python/lldbsuite/test/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
+++ lldb/packages/Python/lldbsuite/test/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py
@@ -4,6 +4,8 @@
 
 from __future__ import print_function
 
+from io import StringIO
+import sys
 
 import lldb
 from lldbsuite.test.decorators import *
@@ -15,9 +17,6 @@
 
     mydir = TestBase.compute_mydir(__file__)
 
-    @skipTestIfFn(
-        lambda: True,
-        "llvm.org/pr24575: all tests get ERRORs in dotest.py after this")
     @add_test_categories(['pyapi'])
     @no_debug_info_test
     @skipIf(triple='^mips')
@@ -33,19 +32,38 @@
                                0x24, 0xf0, 0x0f, 0x04,
                                0xa5, 0x46])
 
+        assembly = """
+        push   {r4, r5, r6, r7, lr}
+        add    r7, sp, #0xc
+        push.w {r8, r10, r11}
+        sub.w  r4, sp, #0x40
+        bic    r4, r4, #0xf
+        mov    sp, r4
+        """
+        def split(s):
+            return [x.strip() for x in s.strip().splitlines()]
+
         insts = target.GetInstructions(lldb.SBAddress(), raw_bytes)
 
+        if sys.version_info.major >= 3:
+            sio = StringIO()
+            insts.Print(sio)
+            self.assertEqual(split(assembly), split(sio.getvalue()))
+
+        self.assertEqual(insts.GetSize(), len(split(assembly)))
+
+        if sys.version_info.major >= 3:
+            for i,asm in enumerate(split(assembly)):
+                inst = insts.GetInstructionAtIndex(i)
+                sio = StringIO()
+                inst.Print(sio)
+                self.assertEqual(asm, sio.getvalue().strip())
+
         if self.TraceOn():
             print()
             for i in insts:
                 print("Disassembled%s" % str(i))
 
-        # Remove the following return statement when the radar is fixed.
-        return
-
-        # rdar://problem/11034702
-        # VST1 (multiple single elements) encoding?
-        # The disassembler should not crash!
         raw_bytes = bytearray([0x04, 0xf9, 0xed, 0x82])
 
         insts = target.GetInstructions(lldb.SBAddress(), raw_bytes)
Index: lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py
+++ lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py
@@ -9,7 +9,10 @@
     obj.GetSize()
     obj.GetInstructionAtIndex(0xffffffff)
     obj.AppendInstruction(lldb.SBInstruction())
-    obj.Print(None)
+    try:
+        obj.Print(None)
+    except Exception:
+        pass
     obj.GetDescription(lldb.SBStream())
     obj.DumpEmulationForAllInstructions("armv7")
     obj.Clear()
Index: lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py
===================================================================
--- lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py
+++ lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py
@@ -9,7 +9,10 @@
     obj.GetAddress()
     obj.GetByteSize()
     obj.DoesBranch()
-    obj.Print(None)
+    try:
+        obj.Print(None)
+    except Exception:
+        pass
     obj.GetDescription(lldb.SBStream())
     obj.EmulateWithFrame(lldb.SBFrame(), 0)
     obj.DumpEmulation("armv7")
Index: lldb/include/lldb/API/SBInstructionList.h
===================================================================
--- lldb/include/lldb/API/SBInstructionList.h
+++ lldb/include/lldb/API/SBInstructionList.h
@@ -46,6 +46,10 @@
 
   void Print(FILE *out);
 
+  void Print(SBFile out);
+
+  void Print(FileSP out);
+
   bool GetDescription(lldb::SBStream &description);
 
   bool DumpEmulationForAllInstructions(const char *triple);
@@ -56,6 +60,8 @@
   friend class SBTarget;
 
   void SetDisassembler(const lldb::DisassemblerSP &opaque_sp);
+  bool GetDescription(lldb_private::Stream &description);
+
 
 private:
   lldb::DisassemblerSP m_opaque_sp;
Index: lldb/include/lldb/API/SBInstruction.h
===================================================================
--- lldb/include/lldb/API/SBInstruction.h
+++ lldb/include/lldb/API/SBInstruction.h
@@ -55,6 +55,10 @@
 
   void Print(FILE *out);
 
+  void Print(SBFile out);
+
+  void Print(FileSP out);
+
   bool GetDescription(lldb::SBStream &description);
 
   bool EmulateWithFrame(lldb::SBFrame &frame, uint32_t evaluate_options);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] D6... Lawrence D'Anna via Phabricator via lldb-commits

Reply via email to