[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.
labath added a comment. In https://reviews.llvm.org/D46588#1102363, @aprantl wrote: > > The advantage of the second one is that we will have the ability to > > inject commands which depend on the results of previous commands (something > > that I think we will need, sooner or later). > > That is worth considering. To write good tests that depend on previous > results, we probably want to have SBAPI-like Python scripting for lldb-mi > available. To make that work we would need to introduce an > lldbmi.HandleCommand("-mycmd...") interface that runs one command, blocks on > it, and returns the output in a string. And then we would need to export that > to Python. Since the interface is string-in-string-out, we could probably do > it via C without even involving SWIG. As an escape hatch to access features > outside of the mi command set, we could implement the -interpreter-exec mi > command that passes its input to SBAPI's HandleCommand() if need be. > Alternatively we could use gtest to write unittest-style lldb-mi tests > directly in C++, if going via Python is too messy. Again, we would do that in > a blocking or synchronized I/O mode. I agree with your analysis. I don't want to give an opinion on the direction, because: a) it's not clear to me which way to go, each possibility has different tradeoffs involved b) I don't see myself doing anything related to lldb-mi any time soon What I can offer as an additional data point is the lldb-server test suite. lldb-server very similar to lldb-mi in a way, as it has an almost-text-based protocol as its main form of interaction with the world. We also have two ways of testing it: a python one (`test/testcases/tools/lldb-server` and a gtest one (`unittests/tools/lldb-server`). the python one is older, but a bit messy (IMO). The gtest one was started because we had a person interested in improving lldb-server testing situation. I supported starting a fresh sub-test suite, because I saw it as a way to solve a couple some fundamental issues in the old one: - code duplication: the python test suite needs a complete reimplementation of the gdb-remote protocol. the c++ test can just hook it to the existing functionality at an appropriate layer. - remote testing mess: a very convoluted setup is required to run tests on a remote (android) device. It's weird that you have to build a host lldb and run the tests from there, even though the lldb-server and the executables it debugs run on a different host/arch. The python part of the test needs to run on the host (there's no easily available python for android), so there's a lot of flakyness and complexity coming from the port forwarding/network communication). The c++ based approach could solve these, as the gtest executable could run on the same device as the lldb-server under test, and it would be possible to run the tests via check-lldb(-server?) from the cross-build tree instead of copying things around between two build trees. Sadly, the person who was supposed to do this left the team. I have tried to carry this idea on slowly, but haven't yet gotten around to implementing the most important part (running tests remotely), so I couldn't start migrating the tests. In retrospect, I am less excited about this idea than I was a year ago, though I still think it would offer a better testing story for lldb-server. Overall, I am not sure how much of this relates to lldb-mi (for example, you probably aren't interested in running it in such a constrained environment), but I thought it might be interesting for you to look at these before deciding on a direction. I also don't want to hold up Alexander's internship over this, as I'm not sure this is what he signed up for. I wanted to bring up the testing story, because I think there are some issues there, but I'll leave it up to you (for some definition of "you") to decide on what to do about that. Repository: rL LLVM https://reviews.llvm.org/D46588 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.
labath accepted this revision. labath added a comment. This revision is now accepted and ready to land. It would still be interesting to align remove_dots with the "standard practice", but it looks like that would end up being a big project. I think we can live with tweak on our side in this case. If someone ever comes around to changing remote_dots behavior, at least it we will be ready. Thank you for trying though. https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46934: Make ObjectFileMachO work on non-darwin platforms
labath added a comment. In https://reviews.llvm.org/D46934#1101963, @aprantl wrote: > Does that mean we can now also remove the #ifdef __APPLE__ from the > objectfile unit tests? Which ones do you mean? I wasn't aware we had any. The thing I know of, which would be interesting to enable is the MachO core file tests. This can't be done yet because the respective process plugin is apple-only, but I haven't looked at what it would take to enable it everywhere. https://reviews.llvm.org/D46934 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46934: Make ObjectFileMachO work on non-darwin platforms
labath updated this revision to Diff 147286. labath added a comment. This is the version with new symbolic constants. I put them in a new file, as I couldn't think of a better place for them. I didn't want to put them in a too generic place as I don't think we should encourage their use (we should convert error to one of our standard errors types as soon as possible). On the other hand, they are used in two files, so they can't be internal to a compilation unit. There wasn't anything reasonable in the overlap of the existing includes of the two files, so I created a new file instead. https://reviews.llvm.org/D46934 Files: lit/Modules/lc_version_min.yaml source/Initialization/CMakeLists.txt source/Initialization/SystemInitializerCommon.cpp source/Plugins/Process/Utility/RegisterContextDarwinConstants.h source/Plugins/Process/Utility/RegisterContextDarwin_arm.cpp source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp Index: source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp === --- source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp +++ source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp @@ -8,14 +8,8 @@ // //===--===// -#if defined(__APPLE__) - #include "RegisterContextDarwin_arm64.h" - -// C Includes -#include -#include -#include +#include "RegisterContextDarwinConstants.h" // C++ Includes // Other libraries and framework includes @@ -221,7 +215,7 @@ int set = GPRRegSet; if (!RegisterSetIsCached(set)) { SetError(set, Write, -1); -return KERN_INVALID_ARGUMENT; +return KERNEL_INVALID_ARGUMENT; } SetError(set, Write, DoWriteGPR(GetThreadID(), set, gpr)); SetError(set, Read, -1); @@ -232,7 +226,7 @@ int set = FPURegSet; if (!RegisterSetIsCached(set)) { SetError(set, Write, -1); -return KERN_INVALID_ARGUMENT; +return KERNEL_INVALID_ARGUMENT; } SetError(set, Write, DoWriteFPU(GetThreadID(), set, fpu)); SetError(set, Read, -1); @@ -243,7 +237,7 @@ int set = EXCRegSet; if (!RegisterSetIsCached(set)) { SetError(set, Write, -1); -return KERN_INVALID_ARGUMENT; +return KERNEL_INVALID_ARGUMENT; } SetError(set, Write, DoWriteEXC(GetThreadID(), set, exc)); SetError(set, Read, -1); @@ -254,7 +248,7 @@ int set = DBGRegSet; if (!RegisterSetIsCached(set)) { SetError(set, Write, -1); -return KERN_INVALID_ARGUMENT; +return KERNEL_INVALID_ARGUMENT; } SetError(set, Write, DoWriteDBG(GetThreadID(), set, dbg)); SetError(set, Read, -1); @@ -274,7 +268,7 @@ default: break; } - return KERN_INVALID_ARGUMENT; + return KERNEL_INVALID_ARGUMENT; } int RegisterContextDarwin_arm64::WriteRegisterSet(uint32_t set) { @@ -293,7 +287,7 @@ break; } } - return KERN_INVALID_ARGUMENT; + return KERNEL_INVALID_ARGUMENT; } void RegisterContextDarwin_arm64::LogDBGRegisters(Log *log, const DBG &dbg) { @@ -313,7 +307,7 @@ if (set == -1) return false; - if (ReadRegisterSet(set, false) != KERN_SUCCESS) + if (ReadRegisterSet(set, false) != KERNEL_SUCCESS) return false; switch (reg) { @@ -545,7 +539,7 @@ if (set == -1) return false; - if (ReadRegisterSet(set, false) != KERN_SUCCESS) + if (ReadRegisterSet(set, false) != KERNEL_SUCCESS) return false; switch (reg) { @@ -642,14 +636,14 @@ default: return false; } - return WriteRegisterSet(set) == KERN_SUCCESS; + return WriteRegisterSet(set) == KERNEL_SUCCESS; } bool RegisterContextDarwin_arm64::ReadAllRegisterValues( lldb::DataBufferSP &data_sp) { data_sp.reset(new DataBufferHeap(REG_CONTEXT_SIZE, 0)); - if (data_sp && ReadGPR(false) == KERN_SUCCESS && - ReadFPU(false) == KERN_SUCCESS && ReadEXC(false) == KERN_SUCCESS) { + if (data_sp && ReadGPR(false) == KERNEL_SUCCESS && + ReadFPU(false) == KERNEL_SUCCESS && ReadEXC(false) == KERNEL_SUCCESS) { uint8_t *dst = data_sp->GetBytes(); ::memcpy(dst, &gpr, sizeof(gpr)); dst += sizeof(gpr); @@ -675,11 +669,11 @@ ::memcpy(&exc, src, sizeof(exc)); uint32_t success_count = 0; -if (WriteGPR() == KERN_SUCCESS) +if (WriteGPR() == KERNEL_SUCCESS) ++success_count; -if (WriteFPU() == KERN_SUCCESS) +if (WriteFPU() == KERNEL_SUCCESS) ++success_count; -if (WriteEXC() == KERN_SUCCESS) +if (WriteEXC() == KERNEL_SUCCESS) ++success_count; return success_count == 3; } @@ -980,7 +974,7 @@ // Read the debug state int kret = ReadDBG(false); - if (kret == KERN_SUCCESS) { + if (kret == KERNEL_SUCCESS) { // Check to make sure we have the needed hardware support uint32_t i = 0; @@ -1007,7 +1001,7 @@ //("RegisterContextDarwin_arm64::EnableHardwareWatchpoint() //WriteDBG() => 0x%8.8x.", kret); - if (kret == KERN_SUCCESS) +
[Lldb-commits] [lldb] r332596 - [DWARF] Have HashedNameToDIE store a DataExtractor by value
Author: labath Date: Thu May 17 04:36:08 2018 New Revision: 332596 URL: http://llvm.org/viewvc/llvm-project?rev=332596&view=rev Log: [DWARF] Have HashedNameToDIE store a DataExtractor by value Summary: The DataExtractors are cheap to copy so there is no reason to store them by reference. Also, in my upcoming indexing refactor I am planning to remove the apple tables data extractor members from the SymbolFileDWARF class, so there will not be a DataExtractor with a suitable lifetime to refer to. Reviewers: clayborg, JDevlieghere Subscribers: aprantl, lldb-commits Differential Revision: https://reviews.llvm.org/D46888 Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h?rev=332596&r1=332595&r2=332596&view=diff == --- lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h (original) +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h Thu May 17 04:36:08 2018 @@ -156,8 +156,8 @@ public: lldb::offset_t *hash_data_offset_ptr, Pair &pair) const override; -const lldb_private::DWARFDataExtractor &m_data; -const lldb_private::DWARFDataExtractor &m_string_table; +lldb_private::DWARFDataExtractor m_data; +lldb_private::DWARFDataExtractor m_string_table; std::string m_name; }; ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46888: [DWARF] Have HashedNameToDIE store a DataExtractor by value
This revision was automatically updated to reflect the committed changes. Closed by commit rL332596: [DWARF] Have HashedNameToDIE store a DataExtractor by value (authored by labath, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D46888 Files: lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h === --- lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h @@ -156,8 +156,8 @@ lldb::offset_t *hash_data_offset_ptr, Pair &pair) const override; -const lldb_private::DWARFDataExtractor &m_data; -const lldb_private::DWARFDataExtractor &m_string_table; +lldb_private::DWARFDataExtractor m_data; +lldb_private::DWARFDataExtractor m_string_table; std::string m_name; }; Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h === --- lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h @@ -156,8 +156,8 @@ lldb::offset_t *hash_data_offset_ptr, Pair &pair) const override; -const lldb_private::DWARFDataExtractor &m_data; -const lldb_private::DWARFDataExtractor &m_string_table; +lldb_private::DWARFDataExtractor m_data; +lldb_private::DWARFDataExtractor m_string_table; std::string m_name; }; ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy
JDevlieghere added a comment. Looks good to me, but I'll defer to Greg as I've never actually touched this code myself. https://reviews.llvm.org/D46889 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy
clayborg added a comment. Looks fine. Just one question on keeping the DWARFIndex::Create() functions so they all have the same signature. Comment at: source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp:40 + +std::unique_ptr AppleIndex::Create( +Module &module, DWARFDataExtractor apple_names, Move all AppleIndex stuff to a dedicated .cpp file? Comment at: source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp:321 + +void ManualIndex::Index() { + if (!m_debug_info) Move all ManualIndex stuff to a dedicated .cpp file? Comment at: source/Plugins/SymbolFile/DWARF/DWARFIndex.h:71 + +class AppleIndex : public DWARFIndex { +public: Rename to AppleDWARFIndex and move to AppleDWARFIndex.h? Comment at: source/Plugins/SymbolFile/DWARF/DWARFIndex.h:127 + +class ManualIndex : public DWARFIndex { +public: Rename to ManualDWARFIndex and move to ManualDWARFIndex.h? https://reviews.llvm.org/D46889 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy
clayborg added inline comments. Comment at: source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp:40 + +std::unique_ptr AppleIndex::Create( +Module &module, DWARFDataExtractor apple_names, clayborg wrote: > Move all AppleIndex stuff to a dedicated .cpp file? Do we want all DWARFIndex::Create(...) signatures to take a SymbolFileDWARF only? Then module can be extracted from that and all sections can be fetched as well? https://reviews.llvm.org/D46889 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47014: Fix _NSCFBoolean data formatter.
JDevlieghere created this revision. JDevlieghere added reviewers: jingham, clayborg, davide. In r265181 the test for the NSCFBoolean data formatter was removed. Later, in r279353 and r279446 a new implementation was provided for the formatter, which I believe never worked (and this wasn't caught because the test was never re-enabled). This commit fixes the bug and re-enables the old test case. Repository: rL LLVM https://reviews.llvm.org/D47014 Files: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp Index: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp === --- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp +++ source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp @@ -2525,14 +2525,14 @@ if (m_CFBoolean_values) return true; - static ConstString g_kCFBooleanFalse("kCFBooleanFalse"); - static ConstString g_kCFBooleanTrue("kCFBooleanTrue"); + static ConstString g_kCFBooleanFalse("__kCFBooleanFalse"); + static ConstString g_kCFBooleanTrue("__kCFBooleanTrue"); std::function get_symbol = [this](ConstString sym) -> lldb::addr_t { SymbolContextList sc_list; if (GetProcess()->GetTarget().GetImages().FindSymbolsWithNameAndType( -g_kCFBooleanFalse, lldb::eSymbolTypeData, sc_list) == 1) { +sym, lldb::eSymbolTypeData, sc_list) == 1) { SymbolContext sc; sc_list.GetContextAtIndex(0, sc); if (sc.symbol) Index: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py === --- packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py +++ packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py @@ -186,16 +186,18 @@ def nsnumber_data_formatter_commands(self): # Now enable AppKit and check we are displaying Cocoa classes correctly -self.expect('frame variable num1 num2 num3 num5 num6 num7 num9', +self.expect('frame variable num1 num2 num3 num5 num6 num7 num8_Y num8_N num9', substrs=['(NSNumber *) num1 = ', ' (int)5', '(NSNumber *) num2 = ', ' (float)3.1', '(NSNumber *) num3 = ', ' (double)3.14', '(NSNumber *) num5 = ', ' (char)65', '(NSNumber *) num6 = ', ' (long)255', '(NSNumber *) num7 = ', '200', + '(NSNumber *) num8_Y = ', 'YES', + '(NSNumber *) num8_N = ', 'NO', '(NSNumber *) num9 = ', ' (short)-31616']) - + self.runCmd('frame variable num4', check=True) output = self.res.GetOutput() i128_handled_correctly = False Index: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp === --- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp +++ source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp @@ -2525,14 +2525,14 @@ if (m_CFBoolean_values) return true; - static ConstString g_kCFBooleanFalse("kCFBooleanFalse"); - static ConstString g_kCFBooleanTrue("kCFBooleanTrue"); + static ConstString g_kCFBooleanFalse("__kCFBooleanFalse"); + static ConstString g_kCFBooleanTrue("__kCFBooleanTrue"); std::function get_symbol = [this](ConstString sym) -> lldb::addr_t { SymbolContextList sc_list; if (GetProcess()->GetTarget().GetImages().FindSymbolsWithNameAndType( -g_kCFBooleanFalse, lldb::eSymbolTypeData, sc_list) == 1) { +sym, lldb::eSymbolTypeData, sc_list) == 1) { SymbolContext sc; sc_list.GetContextAtIndex(0, sc); if (sc.symbol) Index: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py === --- packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py +++ packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-objc/TestDataFormatterObjC.py @@ -186,16 +186,18 @@ def nsnumber_data_formatter_commands(self): # Now enable AppKit and check we are displaying Cocoa classes correctly -self.expect('frame variable num1 num2 num3 num5 num6 num7 num9', +self.expect('frame variable num1 num2 num3 num5 num6 num7 num8_Y num8_N num9', substrs=['(NSNumber *) num1 = ', ' (int)5',
[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy
labath added a comment. Thank you for the review. I'll have the updated diff shortly. In the mean time, here are my responses. Comment at: source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp:40 + +std::unique_ptr AppleIndex::Create( +Module &module, DWARFDataExtractor apple_names, clayborg wrote: > clayborg wrote: > > Move all AppleIndex stuff to a dedicated .cpp file? > Do we want all DWARFIndex::Create(...) signatures to take a SymbolFileDWARF > only? Then module can be extracted from that and all sections can be fetched > as well? SymbolFileDWARF does not provide public accessors for individual sections. I would have to make `LoadSectionData` or some other get-me-a-section api available. I like how this is explicit about what kind of data a particular accelerator table depends on. Comment at: source/Plugins/SymbolFile/DWARF/DWARFIndex.h:71 + +class AppleIndex : public DWARFIndex { +public: clayborg wrote: > Rename to AppleDWARFIndex and move to AppleDWARFIndex.h? The classes seemed small enough to keep in one file, but that works for me too. A more canonical name here would be DWARFAppleIndex.h, but i hate how everything in this folder begins with DWARF, so this is a place I'll happily diverge from the norm. :P https://reviews.llvm.org/D46889 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy
labath updated this revision to Diff 147326. labath added a comment. - s/AppleIndex/AppleDWARFIndex - move things to separate files https://reviews.llvm.org/D46889 Files: source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h source/Plugins/SymbolFile/DWARF/CMakeLists.txt source/Plugins/SymbolFile/DWARF/DWARFAbbreviationDeclaration.cpp source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp source/Plugins/SymbolFile/DWARF/DWARFIndex.h source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h === --- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -38,8 +38,7 @@ // Project includes #include "DWARFDataExtractor.h" #include "DWARFDefines.h" -#include "HashedNameToDIE.h" -#include "NameToDIE.h" +#include "DWARFIndex.h" #include "UniqueDWARFASTType.h" //-- @@ -313,6 +312,10 @@ // the method returns a pointer to the base compile unit. virtual DWARFUnit *GetBaseCompileUnit(); + static bool + DIEInDeclContext(const lldb_private::CompilerDeclContext *parent_decl_ctx, + const DWARFDIE &die); + protected: typedef llvm::DenseMap DIEToTypePtr; @@ -340,10 +343,6 @@ bool DeclContextMatchesThisSymbolFile( const lldb_private::CompilerDeclContext *decl_ctx); - bool - DIEInDeclContext(const lldb_private::CompilerDeclContext *parent_decl_ctx, - const DWARFDIE &die); - virtual DWARFUnit * GetDWARFCompileUnit(lldb_private::CompileUnit *comp_unit); @@ -390,19 +389,6 @@ bool ResolveFunction(const DWARFDIE &die, bool include_inlines, lldb_private::SymbolContextList &sc_list); - void FindFunctions(const lldb_private::ConstString &name, - const NameToDIE &name_to_die, bool include_inlines, - lldb_private::SymbolContextList &sc_list); - - void FindFunctions(const lldb_private::RegularExpression ®ex, - const NameToDIE &name_to_die, bool include_inlines, - lldb_private::SymbolContextList &sc_list); - - void FindFunctions(const lldb_private::RegularExpression ®ex, - const DWARFMappedHash::MemoryTable &memory_table, - bool include_inlines, - lldb_private::SymbolContextList &sc_list); - virtual lldb::TypeSP FindDefinitionTypeForDWARFDeclContext(const DWARFDeclContext &die_decl_ctx); @@ -413,14 +399,9 @@ lldb_private::Symbol * GetObjCClassSymbol(const lldb_private::ConstString &objc_class_name); - void ParseFunctions(const DIEArray &die_offsets, bool include_inlines, - lldb_private::SymbolContextList &sc_list); - lldb::TypeSP GetTypeForDIE(const DWARFDIE &die, bool resolve_function_context = false); - void Index(); - void DumpIndexes(); void SetDebugMapModule(const lldb::ModuleSP &module_sp) { @@ -506,28 +487,15 @@ std::unique_ptr m_abbr; std::unique_ptr m_info; std::unique_ptr m_line; - std::unique_ptr m_apple_names_ap; - std::unique_ptr m_apple_types_ap; - std::unique_ptr m_apple_namespaces_ap; - std::unique_ptr m_apple_objc_ap; std::unique_ptr m_global_aranges_ap; typedef std::unordered_map DebugMacrosMap; DebugMacrosMap m_debug_macros_map; ExternalTypeModuleMap m_external_type_modules; - NameToDIE m_function_basename_index; // All concrete functions - NameToDIE m_function_fullname_index; // All concrete functions - NameToDIE m_function_method_index; // All inlined functions - NameToDIE - m_function_selector_index; // All method names for functions of classes - NameToDIE m_objc_class_selectors_index; // Given a class name, find all - // selectors for the class - NameToDIE m_global_index; // Global and static variables - NameToDIE m_type_index; // All type DIE offsets - NameToDIE m_namespace_index;// All type DIE offsets - bool m_indexed : 1, m_using_apple_tables : 1, m_fetched_external_modules : 1; + std::unique_ptr m_index; + bool m_fetched_external_modules : 1; lldb_private::LazyBool m_supports_DW_AT_APPLE_objc_complete_type; typedef std::shared_ptr> DIERefSetSP; Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp === --- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -49,16
[Lldb-commits] [PATCH] D46889: [DWARF] Extract indexing code into a separate class hierarchy
clayborg accepted this revision. clayborg added inline comments. This revision is now accepted and ready to land. Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1984 GetObjectFile()->GetModule()->GetMutex()); - Index(); -} Yikes, who was calling PreloadSymbols before? Was this causing all Apple systems to manually index the DWARF as well??? That is bad. Fix now with you changes to the right, but scary. https://reviews.llvm.org/D46889 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.
aprantl added a comment. My overall mental image of lldb-mi is very incomplete, but I'm imagining lldb-mi as having one thread that wakes up every n milliseconds checks for command input and then calls into the SBAPI to handle the commands. If that is how it works, then one very simple thing we can try is to call SBDebugger::SetAsync(false), thus forcing all the SBAPI calls to block until completion. Alexander, can you see what happens to your lit test when you enable synchronous mode in the SBAPI? Does it block now as expected, or does it still finish early? If that experiment works, we could add a --async command line option to lldb-mi to enable this behavior for running the test suite. Repository: rL LLVM https://reviews.llvm.org/D46588 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46934: Make ObjectFileMachO work on non-darwin platforms
aprantl added inline comments. Comment at: source/Plugins/Process/Utility/RegisterContextDarwinConstants.h:18 + KERNEL_SUCCESS = 0, + KERNEL_INVALID_ARGUMENT = 4, +}; I think I would prefer #ifndef KERN_INVALID_ARGUMENT #define KERN_INVALID_ARGUMENT 4 #endif over renaming the constant. I think that that is less confusing to read and we use that pattern in other places in LLDB, too. https://reviews.llvm.org/D46934 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46934: Make ObjectFileMachO work on non-darwin platforms
aprantl added a comment. In https://reviews.llvm.org/D46934#1102867, @labath wrote: > In https://reviews.llvm.org/D46934#1101963, @aprantl wrote: > > > Does that mean we can now also remove the #ifdef __APPLE__ from the > > objectfile unit tests? > > > Which ones do you mean? I wasn't aware we had any. The thing I know of, > which would be interesting to enable is the MachO core file tests. This can't > be done yet because the respective process plugin is apple-only, but I > haven't looked at what it would take to enable it everywhere. Sorry, I was confused. I was thinking about the HostInfoMacOSX class unit test which I added at the same time and somehow the two got conflated in my memory. https://reviews.llvm.org/D46934 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r332618 - FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.
Author: gclayton Date: Thu May 17 09:12:38 2018 New Revision: 332618 URL: http://llvm.org/viewvc/llvm-project?rev=332618&view=rev Log: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty. After switching to LLVM normalization, if we init FileSpec with "." we would end up with m_directory being NULL and m_filename being "". This patch fixes this by allowing the path to be normalized and if it normalized to nothing, set it to m_filename. Differential Revision: https://reviews.llvm.org/D46783 Modified: lldb/trunk/source/Utility/FileSpec.cpp lldb/trunk/unittests/Utility/FileSpecTest.cpp Modified: lldb/trunk/source/Utility/FileSpec.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/FileSpec.cpp?rev=332618&r1=332617&r2=332618&view=diff == --- lldb/trunk/source/Utility/FileSpec.cpp (original) +++ lldb/trunk/source/Utility/FileSpec.cpp Thu May 17 09:12:38 2018 @@ -258,6 +258,14 @@ void FileSpec::SetFile(llvm::StringRef p if (m_style == Style::windows) std::replace(resolved.begin(), resolved.end(), '\\', '/'); + if (resolved.empty()) { +// If we have no path after normalization set the path to the current +// directory. This matches what python does and also a few other path +// utilities. +m_filename.SetString("."); +return; + } + m_filename.SetString(llvm::sys::path::filename(resolved, m_style)); llvm::StringRef dir = llvm::sys::path::parent_path(resolved, m_style); if (!dir.empty()) Modified: lldb/trunk/unittests/Utility/FileSpecTest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/FileSpecTest.cpp?rev=332618&r1=332617&r2=332618&view=diff == --- lldb/trunk/unittests/Utility/FileSpecTest.cpp (original) +++ lldb/trunk/unittests/Utility/FileSpecTest.cpp Thu May 17 09:12:38 2018 @@ -199,9 +199,10 @@ TEST(FileSpecTest, GetNormalizedPath) { {"/..", "/"}, {"/.", "/"}, {"..", ".."}, - {".", ""}, + {".", "."}, + {"", "."}, {"../..", "../.."}, - {"foo/..", ""}, + {"foo/..", "."}, {"foo/../bar", "bar"}, {"../foo/..", ".."}, {"./foo", "foo"}, @@ -230,17 +231,18 @@ TEST(FileSpecTest, GetNormalizedPath) { {R"(\..)", R"(\..)"}, // {R"(c:..)", R"(c:..)"}, {R"(..)", R"(..)"}, - {R"(.)", R"()"}, + {R"(.)", R"(.)"}, // TODO: fix llvm::sys::path::remove_dots() to return "c:\" below. {R"(c:..\..)", R"(c:\..\..)"}, {R"(..\..)", R"(..\..)"}, - {R"(foo\..)", R"()"}, + {R"(foo\..)", R"(.)"}, {R"(foo\..\bar)", R"(bar)"}, {R"(..\foo\..)", R"(..)"}, {R"(.\foo)", R"(foo)"}, {R"(.\.\foo)", R"(foo)"}, {R"(..\foo)", R"(..\foo)"}, {R"(..\..\foo)", R"(..\..\foo)"}, + {"", "."}, }; for (auto test : windows_tests) { EXPECT_EQ(test.second, ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.
This revision was automatically updated to reflect the committed changes. Closed by commit rL332618: FileSpec objects that resolve to "." should have "." in m_filename and… (authored by gclayton, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D46783?vs=146434&id=147329#toc Repository: rL LLVM https://reviews.llvm.org/D46783 Files: lldb/trunk/source/Utility/FileSpec.cpp lldb/trunk/unittests/Utility/FileSpecTest.cpp Index: lldb/trunk/unittests/Utility/FileSpecTest.cpp === --- lldb/trunk/unittests/Utility/FileSpecTest.cpp +++ lldb/trunk/unittests/Utility/FileSpecTest.cpp @@ -199,9 +199,10 @@ {"/..", "/"}, {"/.", "/"}, {"..", ".."}, - {".", ""}, + {".", "."}, + {"", "."}, {"../..", "../.."}, - {"foo/..", ""}, + {"foo/..", "."}, {"foo/../bar", "bar"}, {"../foo/..", ".."}, {"./foo", "foo"}, @@ -230,17 +231,18 @@ {R"(\..)", R"(\..)"}, // {R"(c:..)", R"(c:..)"}, {R"(..)", R"(..)"}, - {R"(.)", R"()"}, + {R"(.)", R"(.)"}, // TODO: fix llvm::sys::path::remove_dots() to return "c:\" below. {R"(c:..\..)", R"(c:\..\..)"}, {R"(..\..)", R"(..\..)"}, - {R"(foo\..)", R"()"}, + {R"(foo\..)", R"(.)"}, {R"(foo\..\bar)", R"(bar)"}, {R"(..\foo\..)", R"(..)"}, {R"(.\foo)", R"(foo)"}, {R"(.\.\foo)", R"(foo)"}, {R"(..\foo)", R"(..\foo)"}, {R"(..\..\foo)", R"(..\..\foo)"}, + {"", "."}, }; for (auto test : windows_tests) { EXPECT_EQ(test.second, Index: lldb/trunk/source/Utility/FileSpec.cpp === --- lldb/trunk/source/Utility/FileSpec.cpp +++ lldb/trunk/source/Utility/FileSpec.cpp @@ -258,6 +258,14 @@ if (m_style == Style::windows) std::replace(resolved.begin(), resolved.end(), '\\', '/'); + if (resolved.empty()) { +// If we have no path after normalization set the path to the current +// directory. This matches what python does and also a few other path +// utilities. +m_filename.SetString("."); +return; + } + m_filename.SetString(llvm::sys::path::filename(resolved, m_style)); llvm::StringRef dir = llvm::sys::path::parent_path(resolved, m_style); if (!dir.empty()) Index: lldb/trunk/unittests/Utility/FileSpecTest.cpp === --- lldb/trunk/unittests/Utility/FileSpecTest.cpp +++ lldb/trunk/unittests/Utility/FileSpecTest.cpp @@ -199,9 +199,10 @@ {"/..", "/"}, {"/.", "/"}, {"..", ".."}, - {".", ""}, + {".", "."}, + {"", "."}, {"../..", "../.."}, - {"foo/..", ""}, + {"foo/..", "."}, {"foo/../bar", "bar"}, {"../foo/..", ".."}, {"./foo", "foo"}, @@ -230,17 +231,18 @@ {R"(\..)", R"(\..)"}, // {R"(c:..)", R"(c:..)"}, {R"(..)", R"(..)"}, - {R"(.)", R"()"}, + {R"(.)", R"(.)"}, // TODO: fix llvm::sys::path::remove_dots() to return "c:\" below. {R"(c:..\..)", R"(c:\..\..)"}, {R"(..\..)", R"(..\..)"}, - {R"(foo\..)", R"()"}, + {R"(foo\..)", R"(.)"}, {R"(foo\..\bar)", R"(bar)"}, {R"(..\foo\..)", R"(..)"}, {R"(.\foo)", R"(foo)"}, {R"(.\.\foo)", R"(foo)"}, {R"(..\foo)", R"(..\foo)"}, {R"(..\..\foo)", R"(..\..\foo)"}, + {"", "."}, }; for (auto test : windows_tests) { EXPECT_EQ(test.second, Index: lldb/trunk/source/Utility/FileSpec.cpp === --- lldb/trunk/source/Utility/FileSpec.cpp +++ lldb/trunk/source/Utility/FileSpec.cpp @@ -258,6 +258,14 @@ if (m_style == Style::windows) std::replace(resolved.begin(), resolved.end(), '\\', '/'); + if (resolved.empty()) { +// If we have no path after normalization set the path to the current +// directory. This matches what python does and also a few other path +// utilities. +m_filename.SetString("."); +return; + } + m_filename.SetString(llvm::sys::path::filename(resolved, m_style)); llvm::StringRef dir = llvm::sys::path::parent_path(resolved, m_style); if (!dir.empty()) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.
polyakov.alex added a comment. In https://reviews.llvm.org/D46588#1103180, @aprantl wrote: > My overall mental image of lldb-mi is very incomplete, but I'm imagining > lldb-mi as having one thread that wakes up every n milliseconds checks for > command input and then calls into the SBAPI to handle the commands. If that > is how it works, then one very simple thing we can try is to call > SBDebugger::SetAsync(false), thus forcing all the SBAPI calls to block until > completion. > > Alexander, can you see what happens to your lit test when you enable > synchronous mode in the SBAPI? Does it block now as expected, or does it > still finish early? If that experiment works, we could add a --async command > line option to lldb-mi to enable this behavior for running the test suite. Where is the right place to call SBDebugger::SetAsync(true) to enable asynchronous mode? Repository: rL LLVM https://reviews.llvm.org/D46588 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47014: Fix _NSCFBoolean data formatter.
davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM Repository: rL LLVM https://reviews.llvm.org/D47014 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.
aprantl added a comment. For the experiment you can probably just stick it into `CMICmnLLDBDebugger::InitSBDebugger()`. Repository: rL LLVM https://reviews.llvm.org/D46588 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r332629 - [lit, lldbsuite] Disable tests that are failing because of pr21765 and pr24489
Author: stella.stamenova Date: Thu May 17 09:58:00 2018 New Revision: 332629 URL: http://llvm.org/viewvc/llvm-project?rev=332629&view=rev Log: [lit, lldbsuite] Disable tests that are failing because of pr21765 and pr24489 Summary: These three tests are failing on Windows and looking into the failures, they could be mapped to pr21765 and pr24489 Reviewers: asmith, labath, zturner Reviewed By: zturner Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D47018 Modified: lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py lldb/trunk/packages/Python/lldbsuite/test/expression_command/pr35310/TestExprsBug35310.py lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py Modified: lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py?rev=332629&r1=332628&r2=332629&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/expression_command/call-overridden-method/TestCallOverriddenMethod.py Thu May 17 09:58:00 2018 @@ -26,6 +26,7 @@ class ExprCommandCallOverriddenMethod(Te # Find the line number to break for main.c. self.line = line_number('main.cpp', '// Set breakpoint here') +@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr21765") def test(self): """Test calls to overridden methods in derived classes.""" self.build() Modified: lldb/trunk/packages/Python/lldbsuite/test/expression_command/pr35310/TestExprsBug35310.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/pr35310/TestExprsBug35310.py?rev=332629&r1=332628&r2=332629&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/expression_command/pr35310/TestExprsBug35310.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/expression_command/pr35310/TestExprsBug35310.py Thu May 17 09:58:00 2018 @@ -16,6 +16,7 @@ class ExprBug35310(TestBase): self.main_source = "main.cpp" self.main_source_spec = lldb.SBFileSpec(self.main_source) +@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr21765") def test_issue35310(self): """Test invoking functions with non-standard linkage names. Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py?rev=332629&r1=332628&r2=332629&view=diff == --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/completion/TestCompletion.py Thu May 17 09:58:00 2018 @@ -235,6 +235,7 @@ class CommandLineCompletionTestCase(Test self.complete_from_to("watchpoint set variable foo --watch w", "watchpoint set variable foo --watch write") self.complete_from_to('watchpoint set variable foo -w read_', 'watchpoint set variable foo -w read_write') +@expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24489") def test_symbol_name(self): self.build() self.dbg.CreateTarget(self.getBuildArtifact("a.out")) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46934: Make ObjectFileMachO work on non-darwin platforms
labath updated this revision to Diff 147341. labath added a comment. - Use #ifndef approach for handling the constants. https://reviews.llvm.org/D46934 Files: lit/Modules/lc_version_min.yaml source/Initialization/CMakeLists.txt source/Initialization/SystemInitializerCommon.cpp source/Plugins/Process/Utility/RegisterContextDarwinConstants.h source/Plugins/Process/Utility/RegisterContextDarwin_arm.cpp source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp Index: source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp === --- source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp +++ source/Plugins/Process/Utility/RegisterContextDarwin_arm64.cpp @@ -8,14 +8,8 @@ // //===--===// -#if defined(__APPLE__) - #include "RegisterContextDarwin_arm64.h" - -// C Includes -#include -#include -#include +#include "RegisterContextDarwinConstants.h" // C++ Includes // Other libraries and framework includes @@ -1043,5 +1037,3 @@ } return false; } - -#endif Index: source/Plugins/Process/Utility/RegisterContextDarwin_arm.cpp === --- source/Plugins/Process/Utility/RegisterContextDarwin_arm.cpp +++ source/Plugins/Process/Utility/RegisterContextDarwin_arm.cpp @@ -7,13 +7,8 @@ // //===--===// -#if defined(__APPLE__) - #include "RegisterContextDarwin_arm.h" - -// C Includes -#include -#include +#include "RegisterContextDarwinConstants.h" // C++ Includes // Other libraries and framework includes @@ -1766,5 +1761,3 @@ } return false; } - -#endif Index: source/Plugins/Process/Utility/RegisterContextDarwinConstants.h === --- /dev/null +++ source/Plugins/Process/Utility/RegisterContextDarwinConstants.h @@ -0,0 +1,26 @@ +//===-- RegisterContextDarwinConstants.h *- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#ifndef LLDB_REGISTERCONTEXTDARWINCONSTANTS_H +#define LLDB_REGISTERCONTEXTDARWINCONSTANTS_H + +namespace lldb_private { + +/// Constants returned by various RegisterContextDarwin_*** functions. +#ifndef KERN_SUCCESS +#define KERN_SUCCESS 0 +#endif + +#ifndef KERN_INVALID_ARGUMENT +#define KERN_INVALID_ARGUMENT 4 +#endif + +} // namespace lldb_private + +#endif // LLDB_REGISTERCONTEXTDARWINCONSTANTS_H Index: source/Initialization/SystemInitializerCommon.cpp === --- source/Initialization/SystemInitializerCommon.cpp +++ source/Initialization/SystemInitializerCommon.cpp @@ -21,10 +21,7 @@ #include "lldb/Host/HostInfo.h" #include "lldb/Utility/Log.h" #include "lldb/Utility/Timer.h" - -#if defined(__APPLE__) #include "Plugins/ObjectFile/Mach-O/ObjectFileMachO.h" -#endif #if defined(__linux__) || defined(__FreeBSD__) || defined(__NetBSD__) #include "Plugins/Process/POSIX/ProcessPOSIXLog.h" @@ -82,6 +79,7 @@ // Initialize plug-ins ObjectContainerBSDArchive::Initialize(); ObjectFileELF::Initialize(); + ObjectFileMachO::Initialize(); ObjectFilePECOFF::Initialize(); EmulateInstructionARM::Initialize(); @@ -93,9 +91,6 @@ //-- ObjectContainerUniversalMachO::Initialize(); -#if defined(__APPLE__) - ObjectFileMachO::Initialize(); -#endif #if defined(__linux__) || defined(__FreeBSD__) || defined(__NetBSD__) ProcessPOSIXLog::Initialize(); #endif @@ -109,16 +104,14 @@ Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION); ObjectContainerBSDArchive::Terminate(); ObjectFileELF::Terminate(); + ObjectFileMachO::Terminate(); ObjectFilePECOFF::Terminate(); EmulateInstructionARM::Terminate(); EmulateInstructionMIPS::Terminate(); EmulateInstructionMIPS64::Terminate(); ObjectContainerUniversalMachO::Terminate(); -#if defined(__APPLE__) - ObjectFileMachO::Terminate(); -#endif #if defined(_MSC_VER) ProcessWindowsLog::Terminate(); Index: source/Initialization/CMakeLists.txt === --- source/Initialization/CMakeLists.txt +++ source/Initialization/CMakeLists.txt @@ -1,7 +1,3 @@ -if ( CMAKE_SYSTEM_NAME MATCHES "Darwin" ) - list(APPEND EXTRA_PLUGINS lldbPluginObjectFileMachO) -endif() - if ( CMAKE_SYSTEM_NAME MATCHES "Linux|Android|FreeBSD|NetBSD" ) list(APPEND EXTRA_PLUGINS lldbPluginProcessPOSIX) endif() @@ -24,6 +20,7 @@ lldbPluginObjectContainerBSDArchive lldbPluginObjectContainerMachOArchive lldbPluginObjectFileELF +lldbPl
[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.
clayborg added a comment. In https://reviews.llvm.org/D46588#1103237, @aprantl wrote: > For the experiment you can probably just stick it into > `CMICmnLLDBDebugger::InitSBDebugger()`. But don't do it here permanently... Repository: rL LLVM https://reviews.llvm.org/D46588 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.
polyakov.alex added a comment. In https://reviews.llvm.org/D46588#1103237, @aprantl wrote: > For the experiment you can probably just stick it into > `CMICmnLLDBDebugger::InitSBDebugger()`. Yep, after that, test is passing with only `CHECK` directives. Repository: rL LLVM https://reviews.llvm.org/D46588 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.
This has broken the unit tests. Looks like a bad merge that did not take into account the refactoring in r332088. On Thu, 17 May 2018 at 17:16, Phabricator via Phabricator < revi...@reviews.llvm.org> wrote: > This revision was automatically updated to reflect the committed changes. > Closed by commit rL332618: FileSpec objects that resolve to "." should have "." in m_filename and… (authored by gclayton, committed by ). > Herald added a subscriber: llvm-commits. > Changed prior to commit: > https://reviews.llvm.org/D46783?vs=146434&id=147329#toc > Repository: > rL LLVM > https://reviews.llvm.org/D46783 > Files: > lldb/trunk/source/Utility/FileSpec.cpp > lldb/trunk/unittests/Utility/FileSpecTest.cpp > Index: lldb/trunk/unittests/Utility/FileSpecTest.cpp > === > --- lldb/trunk/unittests/Utility/FileSpecTest.cpp > +++ lldb/trunk/unittests/Utility/FileSpecTest.cpp > @@ -199,9 +199,10 @@ > {"/..", "/"}, > {"/.", "/"}, > {"..", ".."}, > - {".", ""}, > + {".", "."}, > + {"", "."}, > {"../..", "../.."}, > - {"foo/..", ""}, > + {"foo/..", "."}, > {"foo/../bar", "bar"}, > {"../foo/..", ".."}, > {"./foo", "foo"}, > @@ -230,17 +231,18 @@ > {R"(\..)", R"(\..)"}, > // {R"(c:..)", R"(c:..)"}, > {R"(..)", R"(..)"}, > - {R"(.)", R"()"}, > + {R"(.)", R"(.)"}, > // TODO: fix llvm::sys::path::remove_dots() to return "c:\" below. > {R"(c:..\..)", R"(c:\..\..)"}, > {R"(..\..)", R"(..\..)"}, > - {R"(foo\..)", R"()"}, > + {R"(foo\..)", R"(.)"}, > {R"(foo\..\bar)", R"(bar)"}, > {R"(..\foo\..)", R"(..)"}, > {R"(.\foo)", R"(foo)"}, > {R"(.\.\foo)", R"(foo)"}, > {R"(..\foo)", R"(..\foo)"}, > {R"(..\..\foo)", R"(..\..\foo)"}, > + {"", "."}, > }; > for (auto test : windows_tests) { >EXPECT_EQ(test.second, > Index: lldb/trunk/source/Utility/FileSpec.cpp > === > --- lldb/trunk/source/Utility/FileSpec.cpp > +++ lldb/trunk/source/Utility/FileSpec.cpp > @@ -258,6 +258,14 @@ > if (m_style == Style::windows) >std::replace(resolved.begin(), resolved.end(), '\\', '/'); > + if (resolved.empty()) { > +// If we have no path after normalization set the path to the current > +// directory. This matches what python does and also a few other path > +// utilities. > +m_filename.SetString("."); > +return; > + } > + > m_filename.SetString(llvm::sys::path::filename(resolved, m_style)); > llvm::StringRef dir = llvm::sys::path::parent_path(resolved, m_style); > if (!dir.empty()) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.
labath added subscribers: clayborg, davide, jingham, zturner. labath added a comment. This has broken the unit tests. Looks like a bad merge that did not take into account the refactoring in r332088. Repository: rL LLVM https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.
The patch in the differential is off, this is the fixed version of this patch? > On May 17, 2018, at 10:09 AM, Pavel Labath wrote: > > This has broken the unit tests. Looks like a bad merge that did not take > into account the refactoring in r332088. > On Thu, 17 May 2018 at 17:16, Phabricator via Phabricator < > revi...@reviews.llvm.org> wrote: > >> This revision was automatically updated to reflect the committed changes. >> Closed by commit rL332618: FileSpec objects that resolve to "." > should have "." in m_filename and… (authored by gclayton, > committed by ). >> Herald added a subscriber: llvm-commits. > >> Changed prior to commit: >>https://reviews.llvm.org/D46783?vs=146434&id=147329#toc > >> Repository: >>rL LLVM > >> https://reviews.llvm.org/D46783 > >> Files: >>lldb/trunk/source/Utility/FileSpec.cpp >>lldb/trunk/unittests/Utility/FileSpecTest.cpp > > >> Index: lldb/trunk/unittests/Utility/FileSpecTest.cpp >> === >> --- lldb/trunk/unittests/Utility/FileSpecTest.cpp >> +++ lldb/trunk/unittests/Utility/FileSpecTest.cpp >> @@ -199,9 +199,10 @@ >> {"/..", "/"}, >> {"/.", "/"}, >> {"..", ".."}, >> - {".", ""}, >> + {".", "."}, >> + {"", "."}, >> {"../..", "../.."}, >> - {"foo/..", ""}, >> + {"foo/..", "."}, >> {"foo/../bar", "bar"}, >> {"../foo/..", ".."}, >> {"./foo", "foo"}, >> @@ -230,17 +231,18 @@ >> {R"(\..)", R"(\..)"}, >> // {R"(c:..)", R"(c:..)"}, >> {R"(..)", R"(..)"}, >> - {R"(.)", R"()"}, >> + {R"(.)", R"(.)"}, >> // TODO: fix llvm::sys::path::remove_dots() to return "c:\" below. >> {R"(c:..\..)", R"(c:\..\..)"}, >> {R"(..\..)", R"(..\..)"}, >> - {R"(foo\..)", R"()"}, >> + {R"(foo\..)", R"(.)"}, >> {R"(foo\..\bar)", R"(bar)"}, >> {R"(..\foo\..)", R"(..)"}, >> {R"(.\foo)", R"(foo)"}, >> {R"(.\.\foo)", R"(foo)"}, >> {R"(..\foo)", R"(..\foo)"}, >> {R"(..\..\foo)", R"(..\..\foo)"}, >> + {"", "."}, >> }; >> for (auto test : windows_tests) { >> EXPECT_EQ(test.second, >> Index: lldb/trunk/source/Utility/FileSpec.cpp >> === >> --- lldb/trunk/source/Utility/FileSpec.cpp >> +++ lldb/trunk/source/Utility/FileSpec.cpp >> @@ -258,6 +258,14 @@ >> if (m_style == Style::windows) >> std::replace(resolved.begin(), resolved.end(), '\\', '/'); > >> + if (resolved.empty()) { >> +// If we have no path after normalization set the path to the current >> +// directory. This matches what python does and also a few other path >> +// utilities. >> +m_filename.SetString("."); >> +return; >> + } >> + >> m_filename.SetString(llvm::sys::path::filename(resolved, m_style)); >> llvm::StringRef dir = llvm::sys::path::parent_path(resolved, m_style); >> if (!dir.empty()) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.
clayborg updated this revision to Diff 147345. clayborg added a comment. Updated to what was committed. https://reviews.llvm.org/D46783 Files: source/Utility/FileSpec.cpp unittests/Utility/FileSpecTest.cpp Index: unittests/Utility/FileSpecTest.cpp === --- unittests/Utility/FileSpecTest.cpp +++ unittests/Utility/FileSpecTest.cpp @@ -199,9 +199,10 @@ {"/..", "/"}, {"/.", "/"}, {"..", ".."}, - {".", ""}, + {".", "."}, + {"", "."}, {"../..", "../.."}, - {"foo/..", ""}, + {"foo/..", "."}, {"foo/../bar", "bar"}, {"../foo/..", ".."}, {"./foo", "foo"}, @@ -230,17 +231,18 @@ {R"(\..)", R"(\..)"}, // {R"(c:..)", R"(c:..)"}, {R"(..)", R"(..)"}, - {R"(.)", R"()"}, + {R"(.)", R"(.)"}, // TODO: fix llvm::sys::path::remove_dots() to return "c:\" below. {R"(c:..\..)", R"(c:\..\..)"}, {R"(..\..)", R"(..\..)"}, - {R"(foo\..)", R"()"}, + {R"(foo\..)", R"(.)"}, {R"(foo\..\bar)", R"(bar)"}, {R"(..\foo\..)", R"(..)"}, {R"(.\foo)", R"(foo)"}, {R"(.\.\foo)", R"(foo)"}, {R"(..\foo)", R"(..\foo)"}, {R"(..\..\foo)", R"(..\..\foo)"}, + {"", "."}, }; for (auto test : windows_tests) { EXPECT_EQ(test.second, Index: source/Utility/FileSpec.cpp === --- source/Utility/FileSpec.cpp +++ source/Utility/FileSpec.cpp @@ -258,6 +258,14 @@ if (m_style == Style::windows) std::replace(resolved.begin(), resolved.end(), '\\', '/'); + if (resolved.empty()) { +// If we have no path after normalization set the path to the current +// directory. This matches what python does and also a few other path +// utilities. +m_filename.SetString("."); +return; + } + m_filename.SetString(llvm::sys::path::filename(resolved, m_style)); llvm::StringRef dir = llvm::sys::path::parent_path(resolved, m_style); if (!dir.empty()) Index: unittests/Utility/FileSpecTest.cpp === --- unittests/Utility/FileSpecTest.cpp +++ unittests/Utility/FileSpecTest.cpp @@ -199,9 +199,10 @@ {"/..", "/"}, {"/.", "/"}, {"..", ".."}, - {".", ""}, + {".", "."}, + {"", "."}, {"../..", "../.."}, - {"foo/..", ""}, + {"foo/..", "."}, {"foo/../bar", "bar"}, {"../foo/..", ".."}, {"./foo", "foo"}, @@ -230,17 +231,18 @@ {R"(\..)", R"(\..)"}, // {R"(c:..)", R"(c:..)"}, {R"(..)", R"(..)"}, - {R"(.)", R"()"}, + {R"(.)", R"(.)"}, // TODO: fix llvm::sys::path::remove_dots() to return "c:\" below. {R"(c:..\..)", R"(c:\..\..)"}, {R"(..\..)", R"(..\..)"}, - {R"(foo\..)", R"()"}, + {R"(foo\..)", R"(.)"}, {R"(foo\..\bar)", R"(bar)"}, {R"(..\foo\..)", R"(..)"}, {R"(.\foo)", R"(foo)"}, {R"(.\.\foo)", R"(foo)"}, {R"(..\foo)", R"(..\foo)"}, {R"(..\..\foo)", R"(..\..\foo)"}, + {"", "."}, }; for (auto test : windows_tests) { EXPECT_EQ(test.second, Index: source/Utility/FileSpec.cpp === --- source/Utility/FileSpec.cpp +++ source/Utility/FileSpec.cpp @@ -258,6 +258,14 @@ if (m_style == Style::windows) std::replace(resolved.begin(), resolved.end(), '\\', '/'); + if (resolved.empty()) { +// If we have no path after normalization set the path to the current +// directory. This matches what python does and also a few other path +// utilities. +m_filename.SetString("."); +return; + } + m_filename.SetString(llvm::sys::path::filename(resolved, m_style)); llvm::StringRef dir = llvm::sys::path::parent_path(resolved, m_style); if (!dir.empty()) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.
I just updated the differential with the actual patch. If you tried to apply the old one, then try it again with the latest diff I just uploaded > On May 17, 2018, at 10:09 AM, Pavel Labath wrote: > > This has broken the unit tests. Looks like a bad merge that did not take > into account the refactoring in r332088. > On Thu, 17 May 2018 at 17:16, Phabricator via Phabricator < > revi...@reviews.llvm.org> wrote: > >> This revision was automatically updated to reflect the committed changes. >> Closed by commit rL332618: FileSpec objects that resolve to "." > should have "." in m_filename and… (authored by gclayton, > committed by ). >> Herald added a subscriber: llvm-commits. > >> Changed prior to commit: >>https://reviews.llvm.org/D46783?vs=146434&id=147329#toc > >> Repository: >>rL LLVM > >> https://reviews.llvm.org/D46783 > >> Files: >>lldb/trunk/source/Utility/FileSpec.cpp >>lldb/trunk/unittests/Utility/FileSpecTest.cpp > > >> Index: lldb/trunk/unittests/Utility/FileSpecTest.cpp >> === >> --- lldb/trunk/unittests/Utility/FileSpecTest.cpp >> +++ lldb/trunk/unittests/Utility/FileSpecTest.cpp >> @@ -199,9 +199,10 @@ >> {"/..", "/"}, >> {"/.", "/"}, >> {"..", ".."}, >> - {".", ""}, >> + {".", "."}, >> + {"", "."}, >> {"../..", "../.."}, >> - {"foo/..", ""}, >> + {"foo/..", "."}, >> {"foo/../bar", "bar"}, >> {"../foo/..", ".."}, >> {"./foo", "foo"}, >> @@ -230,17 +231,18 @@ >> {R"(\..)", R"(\..)"}, >> // {R"(c:..)", R"(c:..)"}, >> {R"(..)", R"(..)"}, >> - {R"(.)", R"()"}, >> + {R"(.)", R"(.)"}, >> // TODO: fix llvm::sys::path::remove_dots() to return "c:\" below. >> {R"(c:..\..)", R"(c:\..\..)"}, >> {R"(..\..)", R"(..\..)"}, >> - {R"(foo\..)", R"()"}, >> + {R"(foo\..)", R"(.)"}, >> {R"(foo\..\bar)", R"(bar)"}, >> {R"(..\foo\..)", R"(..)"}, >> {R"(.\foo)", R"(foo)"}, >> {R"(.\.\foo)", R"(foo)"}, >> {R"(..\foo)", R"(..\foo)"}, >> {R"(..\..\foo)", R"(..\..\foo)"}, >> + {"", "."}, >> }; >> for (auto test : windows_tests) { >> EXPECT_EQ(test.second, >> Index: lldb/trunk/source/Utility/FileSpec.cpp >> === >> --- lldb/trunk/source/Utility/FileSpec.cpp >> +++ lldb/trunk/source/Utility/FileSpec.cpp >> @@ -258,6 +258,14 @@ >> if (m_style == Style::windows) >> std::replace(resolved.begin(), resolved.end(), '\\', '/'); > >> + if (resolved.empty()) { >> +// If we have no path after normalization set the path to the current >> +// directory. This matches what python does and also a few other path >> +// utilities. >> +m_filename.SetString("."); >> +return; >> + } >> + >> m_filename.SetString(llvm::sys::path::filename(resolved, m_style)); >> llvm::StringRef dir = llvm::sys::path::parent_path(resolved, m_style); >> if (!dir.empty()) ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.
clayborg added a comment. I did run unit tests and they all passed here? https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.
labath added a comment. Sorry, I was wrong about the cause, but the bots are red nonetheless. My bet is it's the last `{"", "."},` test, which is not working because of an early return in SetFile. TBH, I am not sure we would want that to work anyway, as we too treat an empty filespec specially in a lot of places. https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.
labath added a comment. http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/23447 https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r332633 - Fix buildbots after it 332618
Author: gclayton Date: Thu May 17 10:18:11 2018 New Revision: 332633 URL: http://llvm.org/viewvc/llvm-project?rev=332633&view=rev Log: Fix buildbots after it 332618 Modified: lldb/trunk/unittests/Utility/FileSpecTest.cpp Modified: lldb/trunk/unittests/Utility/FileSpecTest.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/FileSpecTest.cpp?rev=332633&r1=332632&r2=332633&view=diff == --- lldb/trunk/unittests/Utility/FileSpecTest.cpp (original) +++ lldb/trunk/unittests/Utility/FileSpecTest.cpp Thu May 17 10:18:11 2018 @@ -200,7 +200,6 @@ TEST(FileSpecTest, GetNormalizedPath) { {"/.", "/"}, {"..", ".."}, {".", "."}, - {"", "."}, {"../..", "../.."}, {"foo/..", "."}, {"foo/../bar", "bar"}, @@ -242,7 +241,6 @@ TEST(FileSpecTest, GetNormalizedPath) { {R"(.\.\foo)", R"(foo)"}, {R"(..\foo)", R"(..\foo)"}, {R"(..\..\foo)", R"(..\..\foo)"}, - {"", "."}, }; for (auto test : windows_tests) { EXPECT_EQ(test.second, ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.
Fixed with svn commit unittests/Utility/FileSpecTest.cpp Sendingunittests/Utility/FileSpecTest.cpp Transmitting file data .done Committing transaction... Committed revision 332633. > On May 17, 2018, at 10:18 AM, Pavel Labath via Phabricator > wrote: > > labath added a comment. > > http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/23447 > > > https://reviews.llvm.org/D46783 > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.
clayborg added a subscriber: labath. clayborg added a comment. Fixed with svn commit unittests/Utility/FileSpecTest.cpp Sendingunittests/Utility/FileSpecTest.cpp Transmitting file data .done Committing transaction... Committed revision 332633. https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46783: FileSpec objects that resolve to "." should have "." in m_filename and m_directory empty.
clayborg added a comment. Fixed with: svn commit unittests/Utility/FileSpecTest.cpp Sendingunittests/Utility/FileSpecTest.cpp Transmitting file data .done Committing transaction... Committed revision 332633. https://reviews.llvm.org/D46783 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D46588: [LLDB][lldb-mi] Add possibility to set breakpoints without selecting a target.
aprantl added a comment. Okay, that sounds promising! Then let's proceed this way: - Add a new command line option to lldb-mi that is called `--synchronous` with a help text "Block until each command has finished executing. Used for testing only." and use it in this test - continue writing as many lldb-mi tests as possible (everything where the commands don't depend on replies) using the lit/FileCheck approach - gradually convert existing pexpect tests over to lit/FileCheck - convert the the remaining tests to use synchronous mode, which might improve the reliability a tiny bit. We will still run into situations where on very loaded systems the pexpect timeout is triggered before a reply comes back from lldb-mi, but it will be less likely. Repository: rL LLVM https://reviews.llvm.org/D46588 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47021: Fix PathMappingList for relative and empty paths after recent FileSpec normalization changes
clayborg created this revision. clayborg added reviewers: labath, zturner, davide. Herald added subscribers: JDevlieghere, aprantl, mgorny. PathMappingList was broken for relative and empty paths after normalization changes in FileSpec. There were also no tests for PathMappingList so I added those. Changes include: - Change PathMappingList::ReverseRemapPath() to take FileSpec objects instead of ConstString. The only client of this was doing work to convert to and from ConstString objects for no reason. - Normalize all paths prefix and replacements that are added to the PathMappingList vector so they match the paths that have been already normalized in the debug info - Unify code in the two forms of PathMappingList::RemapPath() so only one contains the actual functionality. Prior to this, there were two versions of this code. - Use FileSpec::AppendPathComponent() and remove a long standing TODO so paths are correctly appended to each other. - Correctly handle the case where someone maps "" to something else. This allows all paths to be prefixed with the replacement. - Added tests for absolute, relative and empty paths. https://reviews.llvm.org/D47021 Files: include/lldb/Target/PathMappingList.h lldb.xcodeproj/project.pbxproj source/Target/PathMappingList.cpp source/Target/Target.cpp unittests/Utility/CMakeLists.txt unittests/Utility/PathMappingListTest.cpp Index: unittests/Utility/PathMappingListTest.cpp === --- unittests/Utility/PathMappingListTest.cpp +++ unittests/Utility/PathMappingListTest.cpp @@ -0,0 +1,101 @@ +//===-- NameMatchesTest.cpp -*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "lldb/Target/PathMappingList.h" +#include "lldb/Utility/FileSpec.h" +#include "gtest/gtest.h" +#include + +using namespace lldb_private; + +namespace { + struct Matches { +ConstString original; +ConstString remapped; +Matches(const char *o, const char *r) : original(o), +remapped(r) {} + }; +} + +TEST(PathMappingListTest, RelativeTests) { + PathMappingList map; + map.Append(ConstString("."), ConstString("/tmp"), false); + Matches tests[] = { +{".", "/tmp"}, +{"./", "/tmp"}, +{"./", "/tmp"}, +{"./foo.c", "/tmp/foo.c"}, +{"./bar/foo.c", "/tmp/bar/foo.c"} + }; + ConstString actual_remapped; + EXPECT_FALSE(map.RemapPath(ConstString("/foo"), actual_remapped)); + for (const auto &m: tests) { +FileSpec orig_spec(m.original.GetCString(), false); +std::string orig_normalized = orig_spec.GetPath(); +EXPECT_TRUE(map.RemapPath(m.original, actual_remapped)); +EXPECT_TRUE(actual_remapped == m.remapped); +FileSpec remapped_spec(m.remapped.GetCString(), false); +FileSpec unmapped_spec; +EXPECT_TRUE(map.ReverseRemapPath(remapped_spec, unmapped_spec)); +std::string unmapped_path = unmapped_spec.GetPath(); +EXPECT_TRUE(unmapped_path == orig_normalized); + } +} + +TEST(PathMappingListTest, AbsoluteTests) { + PathMappingList map; + map.Append(ConstString("/old"), ConstString("/new"), false); + Matches tests[] = { +{"/old", "/new"}, +{"/old/", "/new"}, +{"/old/foo/.", "/new/foo"}, +{"/old/foo.c", "/new/foo.c"}, +{"/old/foo.c/.", "/new/foo.c"}, +{"/old/./foo.c", "/new/foo.c"}, + }; + ConstString actual_remapped; + // Make sure that the path + for (const auto &m: tests) { +FileSpec orig_spec(m.original.GetCString(), false); +std::string orig_normalized = orig_spec.GetPath(); +EXPECT_TRUE(map.RemapPath(m.original, actual_remapped)); +EXPECT_TRUE(actual_remapped == m.remapped); +FileSpec remapped_spec(m.remapped.GetCString(), false); +FileSpec unmapped_spec; +EXPECT_TRUE(map.ReverseRemapPath(remapped_spec, unmapped_spec)); +std::string unmapped_path = unmapped_spec.GetPath(); +EXPECT_TRUE(unmapped_path == orig_normalized); + } +} + +TEST(PathMappingListTest, RemapEverything) { + PathMappingList map; + map.Append(ConstString(""), ConstString("/new"), false); + Matches tests[] = { +{"/old", "/new/old"}, +{"/old/", "/new/old"}, +{"/old/foo/.", "/new/old/foo"}, +{"/old/foo.c", "/new/old/foo.c"}, +{"/old/foo.c/.", "/new/old/foo.c"}, +{"/old/./foo.c", "/new/old/foo.c"}, + }; + ConstString actual_remapped; + // Make sure that the path + for (const auto &m: tests) { +FileSpec orig_spec(m.original.GetCString(), false); +std::string orig_normalized = orig_spec.GetPath(); +EXPECT_TRUE(map.RemapPath(m.original, actual_remapped)); +EXPECT_TRUE(actual_remapped == m.remapped); +FileSpec remapped_spec(m.remapped.GetCString(), false); +FileSpec unmapped_spec; +EXPECT_TRUE(map.Re
[Lldb-commits] [PATCH] D47021: Fix PathMappingList for relative and empty paths after recent FileSpec normalization changes
zturner added inline comments. Comment at: source/Target/PathMappingList.cpp:76 ++m_mod_id; - m_pairs.push_back(pair(path, replacement)); + m_pairs.push_back(pair(NormalizePath(path), NormalizePath(replacement))); if (notify && m_callback) Slightly more idiomatic to say `emplace_back(NormalizePath(path), NormalizePath(replacement));` Comment at: source/Target/PathMappingList.cpp:178 +remapped.AppendPathComponent(path); +new_path = std::move(remapped.GetPath()); return true; I don't think the `std::move` is necessary here. Since the result of `GetPath()` is already a temporary (rvalue), the move assignment operator will already be invoked even without `std::move`. Comment at: source/Target/PathMappingList.cpp:186 + std::string path = file.GetPath(); + llvm::StringRef path_ref(path.data(), path.size()); for (const auto &it : m_pairs) { `llvm::StringRef path_ref(path);` should be fine, no need to explicitly specify the pointer and size. Comment at: source/Target/PathMappingList.cpp:188 for (const auto &it : m_pairs) { -// FIXME: This should be using FileSpec API's to do the path appending. -const size_t prefixLen = it.second.GetLength(); -if (::strncmp(it.second.GetCString(), path_cstr, prefixLen) == 0) { - std::string new_path_str(it.first.GetCString()); - new_path_str.append(path.GetCString() + prefixLen); - new_path.SetCString(new_path_str.c_str()); +llvm::StringRef second(it.second.GetCString(), it.second.GetLength()); +if (path_ref.startswith(second)) { How about `llvm::StringRef second = it.second.GetStringRef();` Comment at: source/Target/PathMappingList.cpp:189 +llvm::StringRef second(it.second.GetCString(), it.second.GetLength()); +if (path_ref.startswith(second)) { + llvm::StringRef first(it.first.GetCString(), it.first.GetLength()); Can you invert this conditional and `continue;` if it's false? Comment at: source/Target/PathMappingList.cpp:189 +llvm::StringRef second(it.second.GetCString(), it.second.GetLength()); +if (path_ref.startswith(second)) { + llvm::StringRef first(it.first.GetCString(), it.first.GetLength()); zturner wrote: > Can you invert this conditional and `continue;` if it's false? It looks like after this check, the first `second.size()` characters of `path_ref` are ignored. So it sounds like you can write this as: ``` if (!path_ref.consume_front(second)) continue; ``` Then further down you can delete the line that calls `substr`. Comment at: source/Target/PathMappingList.cpp:190 +if (path_ref.startswith(second)) { + llvm::StringRef first(it.first.GetCString(), it.first.GetLength()); + llvm::StringRef suffix = path_ref.substr(second.size()); `llvm::StringRef first = it.first.GetStringRef();` https://reviews.llvm.org/D47021 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47021: Fix PathMappingList for relative and empty paths after recent FileSpec normalization changes
clayborg updated this revision to Diff 147354. clayborg added a comment. Fix issues found by Zach. https://reviews.llvm.org/D47021 Files: include/lldb/Target/PathMappingList.h lldb.xcodeproj/project.pbxproj source/Target/PathMappingList.cpp source/Target/Target.cpp unittests/Utility/CMakeLists.txt unittests/Utility/PathMappingListTest.cpp Index: unittests/Utility/PathMappingListTest.cpp === --- unittests/Utility/PathMappingListTest.cpp +++ unittests/Utility/PathMappingListTest.cpp @@ -0,0 +1,101 @@ +//===-- NameMatchesTest.cpp -*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===--===// + +#include "lldb/Target/PathMappingList.h" +#include "lldb/Utility/FileSpec.h" +#include "gtest/gtest.h" +#include + +using namespace lldb_private; + +namespace { + struct Matches { +ConstString original; +ConstString remapped; +Matches(const char *o, const char *r) : original(o), +remapped(r) {} + }; +} + +TEST(PathMappingListTest, RelativeTests) { + PathMappingList map; + map.Append(ConstString("."), ConstString("/tmp"), false); + Matches tests[] = { +{".", "/tmp"}, +{"./", "/tmp"}, +{"./", "/tmp"}, +{"./foo.c", "/tmp/foo.c"}, +{"./bar/foo.c", "/tmp/bar/foo.c"} + }; + ConstString actual_remapped; + EXPECT_FALSE(map.RemapPath(ConstString("/foo"), actual_remapped)); + for (const auto &m: tests) { +FileSpec orig_spec(m.original.GetCString(), false); +std::string orig_normalized = orig_spec.GetPath(); +EXPECT_TRUE(map.RemapPath(m.original, actual_remapped)); +EXPECT_TRUE(actual_remapped == m.remapped); +FileSpec remapped_spec(m.remapped.GetCString(), false); +FileSpec unmapped_spec; +EXPECT_TRUE(map.ReverseRemapPath(remapped_spec, unmapped_spec)); +std::string unmapped_path = unmapped_spec.GetPath(); +EXPECT_TRUE(unmapped_path == orig_normalized); + } +} + +TEST(PathMappingListTest, AbsoluteTests) { + PathMappingList map; + map.Append(ConstString("/old"), ConstString("/new"), false); + Matches tests[] = { +{"/old", "/new"}, +{"/old/", "/new"}, +{"/old/foo/.", "/new/foo"}, +{"/old/foo.c", "/new/foo.c"}, +{"/old/foo.c/.", "/new/foo.c"}, +{"/old/./foo.c", "/new/foo.c"}, + }; + ConstString actual_remapped; + // Make sure that the path + for (const auto &m: tests) { +FileSpec orig_spec(m.original.GetCString(), false); +std::string orig_normalized = orig_spec.GetPath(); +EXPECT_TRUE(map.RemapPath(m.original, actual_remapped)); +EXPECT_TRUE(actual_remapped == m.remapped); +FileSpec remapped_spec(m.remapped.GetCString(), false); +FileSpec unmapped_spec; +EXPECT_TRUE(map.ReverseRemapPath(remapped_spec, unmapped_spec)); +std::string unmapped_path = unmapped_spec.GetPath(); +EXPECT_TRUE(unmapped_path == orig_normalized); + } +} + +TEST(PathMappingListTest, RemapEverything) { + PathMappingList map; + map.Append(ConstString(""), ConstString("/new"), false); + Matches tests[] = { +{"/old", "/new/old"}, +{"/old/", "/new/old"}, +{"/old/foo/.", "/new/old/foo"}, +{"/old/foo.c", "/new/old/foo.c"}, +{"/old/foo.c/.", "/new/old/foo.c"}, +{"/old/./foo.c", "/new/old/foo.c"}, + }; + ConstString actual_remapped; + // Make sure that the path + for (const auto &m: tests) { +FileSpec orig_spec(m.original.GetCString(), false); +std::string orig_normalized = orig_spec.GetPath(); +EXPECT_TRUE(map.RemapPath(m.original, actual_remapped)); +EXPECT_TRUE(actual_remapped == m.remapped); +FileSpec remapped_spec(m.remapped.GetCString(), false); +FileSpec unmapped_spec; +EXPECT_TRUE(map.ReverseRemapPath(remapped_spec, unmapped_spec)); +std::string unmapped_path = unmapped_spec.GetPath(); +EXPECT_TRUE(unmapped_path == orig_normalized); + } +} Index: unittests/Utility/CMakeLists.txt === --- unittests/Utility/CMakeLists.txt +++ unittests/Utility/CMakeLists.txt @@ -8,6 +8,7 @@ JSONTest.cpp LogTest.cpp NameMatchesTest.cpp + PathMappingListTest.cpp StatusTest.cpp StringExtractorTest.cpp StructuredDataTest.cpp Index: source/Target/Target.cpp === --- source/Target/Target.cpp +++ source/Target/Target.cpp @@ -328,11 +328,7 @@ bool hardware, LazyBool move_to_nearest_code) { FileSpec remapped_file; - ConstString remapped_path; - if (GetSourcePathMap().ReverseRemapPath(ConstString(file.GetPath().c_str()), - remapped_path)) -remapped_file.SetFile(remapp
[Lldb-commits] [PATCH] D47021: Fix PathMappingList for relative and empty paths after recent FileSpec normalization changes
clayborg marked 7 inline comments as done. clayborg added a comment. Marked things as done. https://reviews.llvm.org/D47021 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r332670 - [Windows, Process] Fix an issue in windows thread handling that was causing LLDB to hang
Author: stella.stamenova Date: Thu May 17 14:34:24 2018 New Revision: 332670 URL: http://llvm.org/viewvc/llvm-project?rev=332670&view=rev Log: [Windows, Process] Fix an issue in windows thread handling that was causing LLDB to hang Summary: The function ResumeThread on Windows returns a DWORD which is an unsigned int. In TargetThreadWindows::DoResume, there's code that determines how many times to call ResumeThread based on whether the return value is greater than 0. Since the function returns -1 (as an unsigned int) on failure, this was getting stuck in an infinite loop if ResumeThread failed for any reason. The correct thing to do is check whether the return value is -1 and then return the appropriate error instead of ignoring the return value. Reviewers: asmith, zturner, labath Reviewed By: zturner Subscribers: llvm-commits Differential Revision: https://reviews.llvm.org/D47020 Modified: lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h Modified: lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp?rev=332670&r1=332669&r2=332670&view=diff == --- lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp (original) +++ lldb/trunk/source/Plugins/Process/Windows/Common/ProcessWindows.cpp Thu May 17 14:34:24 2018 @@ -349,13 +349,23 @@ Status ProcessWindows::DoResume() { LLDB_LOG(log, "resuming {0} threads.", m_thread_list.GetSize()); +bool failed = false; for (uint32_t i = 0; i < m_thread_list.GetSize(); ++i) { auto thread = std::static_pointer_cast( m_thread_list.GetThreadAtIndex(i)); - thread->DoResume(); + Status result = thread->DoResume(); + if (result.Fail()) { +failed = true; +LLDB_LOG(log, "Trying to resume thread at index {0}, but failed with error {1}.", i, result); + } } -SetPrivateState(eStateRunning); +if (failed) { + error.SetErrorString("ProcessWindows::DoResume failed"); + return error; +} else { + SetPrivateState(eStateRunning); +} } else { LLDB_LOG(log, "error: process %I64u is in state %u. Returning...", m_session_data->m_debugger->GetProcess().GetProcessId(), Modified: lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp?rev=332670&r1=332669&r2=332670&view=diff == --- lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp (original) +++ lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.cpp Thu May 17 14:34:24 2018 @@ -98,11 +98,11 @@ Unwind *TargetThreadWindows::GetUnwinder return m_unwinder_ap.get(); } -bool TargetThreadWindows::DoResume() { +Status TargetThreadWindows::DoResume() { StateType resume_state = GetTemporaryResumeState(); StateType current_state = GetState(); if (resume_state == current_state) -return true; +return Status(); if (resume_state == eStateStepping) { uint32_t flags_index = @@ -118,8 +118,16 @@ bool TargetThreadWindows::DoResume() { DWORD previous_suspend_count = 0; HANDLE thread_handle = m_host_thread.GetNativeThread().GetSystemHandle(); do { + // ResumeThread returns -1 on error, or the thread's *previous* suspend count on success. + // This means that the return value is 1 when the thread was restarted. + // Note that DWORD is an unsigned int, so we need to explicitly compare with -1. previous_suspend_count = ::ResumeThread(thread_handle); -} while (previous_suspend_count > 0); + + if (previous_suspend_count == (DWORD)-1) +return Status(::GetLastError(), eErrorTypeWin32); + +} while (previous_suspend_count > 1); } - return true; + + return Status(); } Modified: lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h?rev=332670&r1=332669&r2=332670&view=diff == --- lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h (original) +++ lldb/trunk/source/Plugins/Process/Windows/Common/TargetThreadWindows.h Thu May 17 14:34:24 2018 @@ -37,7 +37,7 @@ public: bool CalculateStopInfo() override; Unwind *GetUnwinder() override; - bool DoResume(); + Status DoResume(); HostThread GetHostThread() const { return m_host_thread; } _
[Lldb-commits] [lldb] r332671 - [Windows, Process] LLDB reads wrong registers on 64bit Windows
Author: stella.stamenova Date: Thu May 17 14:42:37 2018 New Revision: 332671 URL: http://llvm.org/viewvc/llvm-project?rev=332671&view=rev Log: [Windows, Process] LLDB reads wrong registers on 64bit Windows Summary: LLDB reads wrong registers on 64bit Windows because RegisterContextWindows_x64::GetRegisterInfoAtIndex returns wrong reference. I encountered broken backtrace when the program stopped at function which does not have prologue code, such as compiled with '-fomit-frame-pointer'. In this situation, CFA is equal to rsp but LLDB reads r9. RegisterContextWindows_x64::GetRegisterInfoAtIndex depends the order of lldb_XXX_x86_64 values, but RegisterIndex/g_register_infos/g_gpr_reg_indices does not follow order. In source/Plugins/Process/Utility/lldb-x86-register-enums.h The order of GPRs is rax, rbx, rcx, rdx, rdi, rsi, rbp, rsp, r8, ... In source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp The order of GPRs is rax, rbx, rcx, rdx, rdi, rsi, r8, r9, r10, ... Patch by Kenji Koyanagi Modified: lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp Modified: lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp?rev=332671&r1=332670&r2=332671&view=diff == --- lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp (original) +++ lldb/trunk/source/Plugins/Process/Windows/Common/x64/RegisterContextWindows_x64.cpp Thu May 17 14:42:37 2018 @@ -41,6 +41,8 @@ enum RegisterIndex { eRegisterIndexRdx, eRegisterIndexRdi, eRegisterIndexRsi, + eRegisterIndexRbp, + eRegisterIndexRsp, eRegisterIndexR8, eRegisterIndexR9, eRegisterIndexR10, @@ -49,8 +51,6 @@ enum RegisterIndex { eRegisterIndexR13, eRegisterIndexR14, eRegisterIndexR15, - eRegisterIndexRbp, - eRegisterIndexRsp, eRegisterIndexRip, eRegisterIndexRflags }; @@ -93,6 +93,16 @@ RegisterInfo g_register_infos[] = { LLDB_INVALID_REGNUM, lldb_rsi_x86_64}, nullptr, nullptr}, +{DEFINE_GPR(rbp, "fp"), + {dwarf_rbp_x86_64, dwarf_rbp_x86_64, LLDB_REGNUM_GENERIC_FP, + LLDB_INVALID_REGNUM, lldb_rbp_x86_64}, + nullptr, + nullptr}, +{DEFINE_GPR(rsp, "sp"), + {dwarf_rsp_x86_64, dwarf_rsp_x86_64, LLDB_REGNUM_GENERIC_SP, + LLDB_INVALID_REGNUM, lldb_rsp_x86_64}, + nullptr, + nullptr}, {DEFINE_GPR(r8, nullptr), {dwarf_r8_x86_64, dwarf_r8_x86_64, LLDB_INVALID_REGNUM, LLDB_INVALID_REGNUM, lldb_r8_x86_64}, @@ -133,16 +143,6 @@ RegisterInfo g_register_infos[] = { LLDB_INVALID_REGNUM, lldb_r15_x86_64}, nullptr, nullptr}, -{DEFINE_GPR(rbp, "fp"), - {dwarf_rbp_x86_64, dwarf_rbp_x86_64, LLDB_REGNUM_GENERIC_FP, - LLDB_INVALID_REGNUM, lldb_rbp_x86_64}, - nullptr, - nullptr}, -{DEFINE_GPR(rsp, "sp"), - {dwarf_rsp_x86_64, dwarf_rsp_x86_64, LLDB_REGNUM_GENERIC_SP, - LLDB_INVALID_REGNUM, lldb_rsp_x86_64}, - nullptr, - nullptr}, {DEFINE_GPR(rip, "pc"), {dwarf_rip_x86_64, dwarf_rip_x86_64, LLDB_REGNUM_GENERIC_PC, LLDB_INVALID_REGNUM, lldb_rip_x86_64}, @@ -162,10 +162,10 @@ static size_t k_num_register_infos = llv uint32_t g_gpr_reg_indices[] = { eRegisterIndexRax, eRegisterIndexRbx, eRegisterIndexRcx, eRegisterIndexRdx, eRegisterIndexRdi, eRegisterIndexRsi, -eRegisterIndexR8, eRegisterIndexR9, eRegisterIndexR10, -eRegisterIndexR11, eRegisterIndexR12, eRegisterIndexR13, -eRegisterIndexR14, eRegisterIndexR15, eRegisterIndexRbp, -eRegisterIndexRsp, eRegisterIndexRip, eRegisterIndexRflags}; +eRegisterIndexRbp, eRegisterIndexRsp, eRegisterIndexR8, +eRegisterIndexR9, eRegisterIndexR10, eRegisterIndexR11, +eRegisterIndexR12, eRegisterIndexR13, eRegisterIndexR14, +eRegisterIndexR15, eRegisterIndexRip, eRegisterIndexRflags}; RegisterSet g_register_sets[] = { {"General Purpose Registers", "gpr", ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits