remove FILE* bindings from SBInstruction.

[lawrence_danna]

Summary

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.

lldb/include/lldb/API/SBInstruction.h

public:
   bool DoesBranch();
 
   bool HasDelaySlot();
 
   bool CanSetBreakpoint();
 
   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);
 
   bool DumpEmulation(const char *triple); // triple is to specify the
                                           // architecture, e.g. 'armv6' or
                                           // 'armv7-apple-ios'
 

lldb/include/lldb/API/SBInstructionList.h

size_t GetInstructionsCount(const SBAddress &start,
                               bool canSetBreakpoint = false);                                   
 
   void Clear();
 
   void AppendInstruction(lldb::SBInstruction inst);
 
   void Print(FILE *out);
 
+  void Print(SBFile out);
+
+  void Print(FileSP out);
+
   bool GetDescription(lldb::SBStream &description);
 
   bool DumpEmulationForAllInstructions(const char *triple);
 
 protected:
   friend class SBFunction;
   friend class SBSymbol;
   friend class SBTarget;
 
   void SetDisassembler(const lldb::DisassemblerSP &opaque_sp);
+  bool GetDescription(lldb_private::Stream &description);
+
 
 private:
   lldb::DisassemblerSP m_opaque_sp;
 };
 
 } // namespace lldb
 
 #endif // LLDB_SBInstructionList_h_

lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py

 """
 Fuzz tests an object after the default construction to make sure it does not crash lldb.
 """
 
 import lldb
 
 
 def fuzz_obj(obj):
     obj.GetAddress()
     obj.GetByteSize()
     obj.DoesBranch()
+    try:
         obj.Print(None)
+    except Exception:
+        pass
     obj.GetDescription(lldb.SBStream())
     obj.EmulateWithFrame(lldb.SBFrame(), 0)
     obj.DumpEmulation("armv7")
     obj.TestEmulation(lldb.SBStream(), "my-file")

lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py

 """
 Fuzz tests an object after the default construction to make sure it does not crash lldb.
 """
 
 import lldb
 
 
 def fuzz_obj(obj):
     obj.GetSize()
     obj.GetInstructionAtIndex(0xffffffff)
     obj.AppendInstruction(lldb.SBInstruction())
+    try:
         obj.Print(None)
+    except Exception:
+        pass
     obj.GetDescription(lldb.SBStream())
     obj.DumpEmulationForAllInstructions("armv7")
     obj.Clear()
     for inst in obj:
         s = str(inst)

lldb/packages/Python/lldbsuite/test/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py

 """
 Use lldb Python API to disassemble raw machine code bytes
 """
 
 from __future__ import print_function
 
+from io import StringIO
+import sys
 
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 
 class Disassemble_VST1_64(TestBase):
 
     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')
     def test_disassemble_invalid_vst_1_64_raw_data(self):
         """Test disassembling invalid vst1.64 raw bytes with the API."""
         # Create a target from the debugger.
         target = self.dbg.CreateTargetWithFileAndTargetTriple("", "thumbv7")
         self.assertTrue(target, VALID_TARGET)
 
         raw_bytes = bytearray([0xf0, 0xb5, 0x03, 0xaf,
                                0x2d, 0xe9, 0x00, 0x0d,
                                0xad, 0xf1, 0x40, 0x04,
                                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)
 
         inst = insts.GetInstructionAtIndex(0)
 
         if self.TraceOn():
             print()
             print("Raw bytes:    ", [hex(x) for x in raw_bytes])
             print("Disassembled%s" % str(inst))
 
         self.assertTrue(inst.GetMnemonic(target) == "vst1.64")

lldb/scripts/interface/SBInstruction.i

public:
 
     bool
     HasDelaySlot ();
 
     bool
     CanSetBreakpoint ();
 
     void
-    Print (FILE *out);
+    Print (lldb::SBFile out);
+
+    void
+    Print (lldb::FileSP BORROWED);
 
     bool
     GetDescription (lldb::SBStream &description);
 
     bool
     EmulateWithFrame (lldb::SBFrame &frame, uint32_t evaluate_options);
 
     bool

lldb/scripts/interface/SBInstructionList.i

public:
 
     void
     Clear ();
 
     void
     AppendInstruction (lldb::SBInstruction inst);
 
     void
-    Print (FILE *out);
+    Print (lldb::SBFile out);
+
+    void
+    Print (lldb::FileSP BORROWED);
 
     bool
     GetDescription (lldb::SBStream &description);
 
     bool
     DumpEmulationForAllInstructions (const char *triple);
 
     %pythoncode %{

lldb/source/API/SBInstruction.cpp

 //===-- SBInstruction.cpp ---------------------------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 
 #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"
 #include "lldb/API/SBTarget.h"
 #include "lldb/Core/Disassembler.h"
 #include "lldb/Core/EmulateInstruction.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/StreamFile.h"
if (inst_sp) {
     FormatEntity::Entry format;
     FormatEntity::Parse("${addr}: ", format);
     inst_sp->Dump(&s.ref(), 0, true, false, nullptr, &sc, nullptr, &format, 0);
     return true;
   }
   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());
   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);
-    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,
                   0);
   }
 }
 
 bool SBInstruction::EmulateWithFrame(lldb::SBFrame &frame,
void RegisterMethods<SBInstruction>(Registry &R) {
   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(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/source/API/SBInstructionList.cpp

 //===-- SBInstructionList.cpp -----------------------------------*- 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
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/API/SBInstructionList.h"
 #include "SBReproducerPrivate.h"
 #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"
 
 using namespace lldb;
 using namespace lldb_private;
 
 SBInstructionList::SBInstructionList() : m_opaque_sp() {
   LLDB_RECORD_CONSTRUCTOR_NO_ARGS(SBInstructionList);

 }
 
 void SBInstructionList::SetDisassembler(const lldb::DisassemblerSP &opaque_sp) {
   m_opaque_sp = opaque_sp;
 }
 
 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;
       FormatEntity::Parse("${addr}: ", format);
       SymbolContext sc;
       SymbolContext prev_sc;
       for (size_t i = 0; i < num_instructions; ++i) {
         Instruction *inst =
LLDB_REGISTER_METHOD(lldb::SBInstruction, SBInstructionList,
                        GetInstructionAtIndex, (uint32_t));
   LLDB_REGISTER_METHOD(
       size_t, SBInstructionList, GetInstructionsCount,
       (const lldb::SBAddress &, const lldb::SBAddress &, bool));
   LLDB_REGISTER_METHOD(void, SBInstructionList, Clear, ());
   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,
                        DumpEmulationForAllInstructions, (const char *));
 }
 
 }
 }