From e7a9115680e22fac3dc996800deaec773becfac0 Mon Sep 17 00:00:00 2001 From: Lawrence D'Anna Date: Mon, 14 Oct 2019 20:59:57 +0000 Subject: [PATCH] remove FILE* bindings from SBInstruction. 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. Reviewers: JDevlieghere, jasonmolenda, labath Reviewed By: labath Subscribers: lldb-commits Tags: #lldb Differential Revision: https://reviews.llvm.org/D68890 llvm-svn: 374820 --- lldb/include/lldb/API/SBInstruction.h | 4 +++ lldb/include/lldb/API/SBInstructionList.h | 6 ++++ .../default-constructor/sb_instruction.py | 5 ++- .../default-constructor/sb_instructionlist.py | 5 ++- .../TestDisassemble_VST1_64.py | 36 ++++++++++++++++------ lldb/scripts/interface/SBInstruction.i | 5 ++- lldb/scripts/interface/SBInstructionList.i | 5 ++- lldb/source/API/SBInstruction.cpp | 22 ++++++++++--- lldb/source/API/SBInstructionList.cpp | 32 ++++++++++++++++--- 9 files changed, 99 insertions(+), 21 deletions(-) diff --git a/lldb/include/lldb/API/SBInstruction.h b/lldb/include/lldb/API/SBInstruction.h index 0bb5877..7d83a37 100644 --- a/lldb/include/lldb/API/SBInstruction.h +++ b/lldb/include/lldb/API/SBInstruction.h @@ -55,6 +55,10 @@ public: 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); diff --git a/lldb/include/lldb/API/SBInstructionList.h b/lldb/include/lldb/API/SBInstructionList.h index 4434d50..e203d0b 100644 --- a/lldb/include/lldb/API/SBInstructionList.h +++ b/lldb/include/lldb/API/SBInstructionList.h @@ -46,6 +46,10 @@ public: 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 @@ protected: friend class SBTarget; void SetDisassembler(const lldb::DisassemblerSP &opaque_sp); + bool GetDescription(lldb_private::Stream &description); + private: lldb::DisassemblerSP m_opaque_sp; diff --git a/lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py b/lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py index aca1d8a..fbfb231 100644 --- a/lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py +++ b/lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instruction.py @@ -9,7 +9,10 @@ def fuzz_obj(obj): 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") diff --git a/lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py b/lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py index 88fbc06..9e7ebf9 100644 --- a/lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py +++ b/lldb/packages/Python/lldbsuite/test/python_api/default-constructor/sb_instructionlist.py @@ -9,7 +9,10 @@ def fuzz_obj(obj): 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() diff --git a/lldb/packages/Python/lldbsuite/test/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py b/lldb/packages/Python/lldbsuite/test/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py index d171b02..1b627c0 100644 --- a/lldb/packages/Python/lldbsuite/test/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py +++ b/lldb/packages/Python/lldbsuite/test/python_api/disassemble-raw-data/TestDisassemble_VST1_64.py @@ -4,6 +4,8 @@ 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 * @@ -15,9 +17,6 @@ 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') @@ -33,19 +32,38 @@ class Disassemble_VST1_64(TestBase): 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) diff --git a/lldb/scripts/interface/SBInstruction.i b/lldb/scripts/interface/SBInstruction.i index 82b31fd..4586978 100644 --- a/lldb/scripts/interface/SBInstruction.i +++ b/lldb/scripts/interface/SBInstruction.i @@ -57,7 +57,10 @@ public: CanSetBreakpoint (); void - Print (FILE *out); + Print (lldb::SBFile out); + + void + Print (lldb::FileSP BORROWED); bool GetDescription (lldb::SBStream &description); diff --git a/lldb/scripts/interface/SBInstructionList.i b/lldb/scripts/interface/SBInstructionList.i index edcceea..1e5aa94 100644 --- a/lldb/scripts/interface/SBInstructionList.i +++ b/lldb/scripts/interface/SBInstructionList.i @@ -55,7 +55,10 @@ public: AppendInstruction (lldb::SBInstruction inst); void - Print (FILE *out); + Print (lldb::SBFile out); + + void + Print (lldb::FileSP BORROWED); bool GetDescription (lldb::SBStream &description); diff --git a/lldb/source/API/SBInstruction.cpp b/lldb/source/API/SBInstruction.cpp index fcf66fd..3114028 100644 --- a/lldb/source/API/SBInstruction.cpp +++ b/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 @@ bool SBInstruction::GetDescription(lldb::SBStream &s) { 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(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 @@ void SBInstruction::Print(FILE *out) { 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 @@ void RegisterMethods(Registry &R) { 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 *)); diff --git a/lldb/source/API/SBInstructionList.cpp b/lldb/source/API/SBInstructionList.cpp index cce923b..403ccaf 100644 --- a/lldb/source/API/SBInstructionList.cpp +++ b/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::SetDisassembler(const lldb::DisassemblerSP &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; @@ -200,6 +222,8 @@ void RegisterMethods(Registry &R) { 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, -- 2.7.4