[Lldb-commits] [PATCH] D53140: [LLDB] - Add support for DW_RLE_base_address and DW_RLE_offset_pair entries (.debug_rnglists)
This revision was automatically updated to reflect the committed changes. Closed by commit rL345127: [LLDB] - Add support for DW_RLE_base_address and DW_RLE_offset_pair entries (. (authored by grimar, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D53140?vs=170050&id=170833#toc Repository: rL LLVM https://reviews.llvm.org/D53140 Files: lldb/trunk/lit/Breakpoint/Inputs/debug_rnglist_offset_pair.yaml lldb/trunk/lit/Breakpoint/debug_rnglist_offset_pair.test lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.h lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp === --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp @@ -456,20 +456,15 @@ break; case DW_AT_ranges: { - const DWARFDebugRanges *debug_ranges = dwarf2Data->DebugRanges(); - if (debug_ranges) { -debug_ranges->FindRanges(cu->GetRangesBase(), form_value.Unsigned(), ranges); -// All DW_AT_ranges are relative to the base address of the compile -// unit. We add the compile unit base address to make sure all the -// addresses are properly fixed up. -ranges.Slide(cu->GetBaseAddress()); - } else { + const DWARFDebugRangesBase *debug_ranges = dwarf2Data->DebugRanges(); + if (debug_ranges) +debug_ranges->FindRanges(cu, form_value.Unsigned(), ranges); + else cu->GetSymbolFileDWARF()->GetObjectFile()->GetModule()->ReportError( "{0x%8.8x}: DIE has DW_AT_ranges(0x%" PRIx64 ") attribute yet DWARF has no .debug_ranges, please file a bug " "and attach the file at the start of this error message", m_offset, form_value.Unsigned()); - } } break; case DW_AT_name: @@ -1065,10 +1060,8 @@ dwarf2Data, cu, DW_AT_ranges, DW_INVALID_OFFSET, check_specification_or_abstract_origin); if (debug_ranges_offset != DW_INVALID_OFFSET) { -if (DWARFDebugRanges *debug_ranges = dwarf2Data->DebugRanges()) - debug_ranges->FindRanges(cu->GetRangesBase(), debug_ranges_offset, - ranges); -ranges.Slide(cu->GetBaseAddress()); +if (DWARFDebugRangesBase *debug_ranges = dwarf2Data->DebugRanges()) + debug_ranges->FindRanges(cu, debug_ranges_offset, ranges); } else if (check_hi_lo_pc) { dw_addr_t lo_pc = LLDB_INVALID_ADDRESS; dw_addr_t hi_pc = LLDB_INVALID_ADDRESS; @@ -1723,12 +1716,9 @@ dwarf2Data, cu, DW_AT_ranges, DW_INVALID_OFFSET); if (debug_ranges_offset != DW_INVALID_OFFSET) { DWARFRangeList ranges; - DWARFDebugRanges *debug_ranges = dwarf2Data->DebugRanges(); - debug_ranges->FindRanges(cu->GetRangesBase(), debug_ranges_offset, ranges); - // All DW_AT_ranges are relative to the base address of the compile - // unit. We add the compile unit base address to make sure all the - // addresses are properly fixed up. - ranges.Slide(cu->GetBaseAddress()); + DWARFDebugRangesBase *debug_ranges = dwarf2Data->DebugRanges(); + debug_ranges->FindRanges(cu, debug_ranges_offset, ranges); + if (ranges.FindEntryThatContains(address)) { found_address = true; // puts("***MATCH***"); Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h === --- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -53,8 +53,7 @@ class DWARFDebugInfo; class DWARFDebugInfoEntry; class DWARFDebugLine; -class DWARFDebugRanges; -class DWARFDebugRngLists; +class DWARFDebugRangesBase; class DWARFDeclContext; class DWARFDIECollection; class DWARFFormValue; @@ -266,9 +265,9 @@ const DWARFDebugInfo *DebugInfo() const; - DWARFDebugRanges *DebugRanges(); + DWARFDebugRangesBase *DebugRanges(); - const DWARFDebugRanges *DebugRanges() const; + const DWARFDebugRangesBase *DebugRanges() const; const lldb_private::DWARFDataExtractor &DebugLocData(); @@ -512,7 +511,7 @@ typedef std::shared_ptr> DIERefSetSP; typedef std::unordered_map NameToOffsetMap; NameToOffsetMap m_function_scope_qualified_name_map; - std::unique_ptr m_ranges; + std::unique_ptr m_ranges; UniqueDWARFASTTypeMap m_unique_ast_type_map; DIEToTypePtr m_die_to_type; DIEToVariableSP m_die_to_variable_sp; Ind
[Lldb-commits] [PATCH] D53361: [API] Extend the `SBThreadPlan` interface
aleksandr.urakov updated this revision to Diff 170852. aleksandr.urakov added a comment. I've added the test. Can you see, please, is it ok? https://reviews.llvm.org/D53361 Files: include/lldb/API/SBThreadPlan.h packages/Python/lldbsuite/test/functionalities/step_scripted/Makefile packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py packages/Python/lldbsuite/test/functionalities/step_scripted/main.c scripts/interface/SBThreadPlan.i source/API/SBThreadPlan.cpp Index: source/API/SBThreadPlan.cpp === --- source/API/SBThreadPlan.cpp +++ source/API/SBThreadPlan.cpp @@ -207,3 +207,13 @@ return SBThreadPlan(); } } + +SBThreadPlan +SBThreadPlan::QueueThreadPlanForStepScripted(const char *script_class_name) { + if (m_opaque_sp) { +return SBThreadPlan(m_opaque_sp->GetThread().QueueThreadPlanForStepScripted( +false, script_class_name, false)); + } else { +return SBThreadPlan(); + } +} Index: scripts/interface/SBThreadPlan.i === --- scripts/interface/SBThreadPlan.i +++ scripts/interface/SBThreadPlan.i @@ -106,6 +106,9 @@ SBThreadPlan QueueThreadPlanForRunToAddress (SBAddress address); +SBThreadPlan +QueueThreadPlanForStepScripted(const char *script_class_name); + protected: friend class SBBreakpoint; Index: packages/Python/lldbsuite/test/functionalities/step_scripted/main.c === --- /dev/null +++ packages/Python/lldbsuite/test/functionalities/step_scripted/main.c @@ -0,0 +1,10 @@ +#include + +void foo() { + printf("Set a breakpoint here.\n"); +} + +int main() { + foo(); + return 0; +} Index: packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py === --- /dev/null +++ packages/Python/lldbsuite/test/functionalities/step_scripted/TestStepScripted.py @@ -0,0 +1,41 @@ +""" +Tests stepping with scripted thread plans. +""" + +import lldb +import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.lldbtest import * + +class StepScriptedTestCase(TestBase): + +mydir = TestBase.compute_mydir(__file__) + +NO_DEBUG_INFO_TESTCASE = True + +def test_standard_step_out(self): +"""Tests stepping with the scripted thread plan laying over a standard thread plan for stepping out.""" +self.build() +self.main_source_file = lldb.SBFileSpec("main.c") +self.step_out_with_scripted_plan("Steps.StepOut") + +def test_scripted_step_out(self): +"""Tests stepping with the scripted thread plan laying over an another scripted thread plan for stepping out.""" +self.build() +self.main_source_file = lldb.SBFileSpec("main.c") +self.step_out_with_scripted_plan("Steps.StepScripted") + +def setUp(self): +TestBase.setUp(self) +self.runCmd("command script import Steps.py") + +def step_out_with_scripted_plan(self, name): +(target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, "Set a breakpoint here", self.main_source_file) + +frame = thread.GetFrameAtIndex(0) +self.assertEqual("foo", frame.GetFunctionName()) + +err = thread.StepUsingScriptedThreadPlan(name) +self.assertTrue(err.Success(), "Failed to step out") + +frame = thread.GetFrameAtIndex(0) +self.assertEqual("main", frame.GetFunctionName()) Index: packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py === --- /dev/null +++ packages/Python/lldbsuite/test/functionalities/step_scripted/Steps.py @@ -0,0 +1,37 @@ +import lldb + +class StepWithChild: +def __init__(self, thread_plan): +self.thread_plan = thread_plan +self.child_thread_plan = self.queue_child_thread_plan() + +def explains_stop(self, event): +return False + +def should_stop(self, event): +if not self.child_thread_plan.IsPlanComplete(): +return False + +self.thread_plan.SetPlanComplete(True) + +return True + +def should_step(self): +return False + +def queue_child_thread_plan(self): +return None + +class StepOut(StepWithChild): +def __init__(self, thread_plan, dict): +StepWithChild.__init__(self, thread_plan) + +def queue_child_thread_plan(self): +return self.thread_plan.QueueThreadPlanForStepOut(0) + +class StepScripted(StepWithChild): +def __init__(self, thread_plan, dict): +StepWithChild.__init__(self, thread_plan) + +def queue_child_thread_plan(self): +return self.thread_plan.QueueThreadPlanForStepScripted("Steps.StepOut") Index: packages/Python/lldbsuite/test/functionalities/step_scripted/Makefi
[Lldb-commits] [PATCH] D53646: [LLDB] - Parse the DW_LLE_startx_length correctly for DWARF v5 case.
grimar created this revision. grimar added reviewers: LLDB, clayborg. Herald added subscribers: JDevlieghere, aprantl. Currently, we always parse the `length` field of DW_LLE_startx_length entry as U32. That is correct for pre-standard definition: https://gcc.gnu.org/wiki/DebugFission - "A start/length entry contains one unsigned LEB128 number and a 4-byte unsigned value (as would be represented by the form code DW_FORM_const4u). The first number is an index into the .debug_addr section that selects the beginning offset, and the second number is the length of the range. ") But DWARF v5 says: "This is a form of bounded location description that has two unsigned ULEB operands. The first value is an address index (into the .debug_addr section) that indicates the beginning of the address range over which the location is valid. The second value is the length of the range." Fortunately, we can easily handle the difference. No test case because it seems impossible to test until we will be ready to use DWARF v5 in tests that need to run the executables. https://reviews.llvm.org/D53646 Files: include/lldb/Expression/DWARFExpression.h source/Expression/DWARFExpression.cpp Index: source/Expression/DWARFExpression.cpp === --- source/Expression/DWARFExpression.cpp +++ source/Expression/DWARFExpression.cpp @@ -3029,7 +3029,9 @@ if (!debug_loc_data.ValidOffset(*offset_ptr)) return false; - switch (dwarf_cu->GetSymbolFileDWARF()->GetLocationListFormat()) { + DWARFExpression::LocationListFormat format = + dwarf_cu->GetSymbolFileDWARF()->GetLocationListFormat(); + switch (format) { case NonLocationList: return false; case RegularLocationList: @@ -3051,7 +3053,9 @@ case DW_LLE_startx_length: { uint64_t index = debug_loc_data.GetULEB128(offset_ptr); low_pc = ReadAddressFromDebugAddrSection(dwarf_cu, index); - uint32_t length = debug_loc_data.GetU32(offset_ptr); + uint64_t length = (format == LocLists) +? debug_loc_data.GetULEB128(offset_ptr) +: debug_loc_data.GetU32(offset_ptr); high_pc = low_pc + length; return true; } Index: include/lldb/Expression/DWARFExpression.h === --- include/lldb/Expression/DWARFExpression.h +++ include/lldb/Expression/DWARFExpression.h @@ -40,8 +40,10 @@ enum LocationListFormat : uint8_t { NonLocationList, // Not a location list RegularLocationList, // Location list format used in non-split dwarf files -SplitDwarfLocationList, // Location list format used in split dwarf files -LocLists, // Location list format used in DWARF v5 (.debug_loclists). +SplitDwarfLocationList, // Location list format used in pre-DWARF v5 split +// dwarf files (.debug_loc.dwo) +LocLists, // Location list format used in DWARF v5 +// (.debug_loclists/.debug_loclists.dwo). }; //-- @@ -153,7 +155,7 @@ lldb::addr_t GetLocation_DW_OP_addr(uint32_t op_addr_idx, bool &error) const; bool Update_DW_OP_addr(lldb::addr_t file_addr); - + void SetModule(const lldb::ModuleSP &module) { m_module_wp = module; } bool ContainsThreadLocalStorage() const; Index: source/Expression/DWARFExpression.cpp === --- source/Expression/DWARFExpression.cpp +++ source/Expression/DWARFExpression.cpp @@ -3029,7 +3029,9 @@ if (!debug_loc_data.ValidOffset(*offset_ptr)) return false; - switch (dwarf_cu->GetSymbolFileDWARF()->GetLocationListFormat()) { + DWARFExpression::LocationListFormat format = + dwarf_cu->GetSymbolFileDWARF()->GetLocationListFormat(); + switch (format) { case NonLocationList: return false; case RegularLocationList: @@ -3051,7 +3053,9 @@ case DW_LLE_startx_length: { uint64_t index = debug_loc_data.GetULEB128(offset_ptr); low_pc = ReadAddressFromDebugAddrSection(dwarf_cu, index); - uint32_t length = debug_loc_data.GetU32(offset_ptr); + uint64_t length = (format == LocLists) +? debug_loc_data.GetULEB128(offset_ptr) +: debug_loc_data.GetU32(offset_ptr); high_pc = low_pc + length; return true; } Index: include/lldb/Expression/DWARFExpression.h === --- include/lldb/Expression/DWARFExpression.h +++ include/lldb/Expression/DWARFExpression.h @@ -40,8 +40,10 @@ enum LocationListFormat : uint8_t { NonLocationList, // Not a location list RegularLocationList, // Location list format used in non-split dwarf files -SplitDwarfLocationList, // Location list format used in split dwarf files -Loc
[Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file
aleksandr.urakov updated this revision to Diff 170889. aleksandr.urakov added a comment. I've implemented things as Greg have said, except for a few moments: - haven't added `AddSymbols` to `SymbolVendor` because it is not used anywhere, and we can just use `SymbolFile::AddSymbols` directly inside of `SymbolVendor`; - have made `AddSymbols`' parameter as a reference instead of a pointer, because there is no reason to send null to this function; - have cached symtab in `SymbolVendor` to not pass it every time through the `SymbolFile::AddSymbols`. But if you don't agree with some of these I'm ready to discuss. https://reviews.llvm.org/D53368 Files: include/lldb/Symbol/SymbolFile.h include/lldb/Symbol/SymbolVendor.h include/lldb/lldb-forward.h source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp source/Plugins/SymbolFile/PDB/SymbolFilePDB.h source/Symbol/SymbolVendor.cpp unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp Index: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp === --- unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp +++ unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp @@ -616,3 +616,20 @@ EXPECT_EQ(0u, num_results); EXPECT_EQ(0u, results.GetSize()); } + +TEST_F(SymbolFilePDBTests, TestFindSymbolsWithNameAndType) { + FileSpec fspec(m_pdb_test_exe.c_str(), false); + ArchSpec aspec("i686-pc-windows"); + lldb::ModuleSP module = std::make_shared(fspec, aspec); + + SymbolContextList sc_list; + EXPECT_EQ(1u, +module->FindSymbolsWithNameAndType(ConstString("?foo@@YAHH@Z"), + lldb::eSymbolTypeAny, sc_list)); + EXPECT_EQ(1u, sc_list.GetSize()); + + SymbolContext sc; + EXPECT_TRUE(sc_list.GetContextAtIndex(0, sc)); + EXPECT_STREQ("int foo(int)", + sc.GetFunctionName(Mangled::ePreferDemangled).AsCString()); +} Index: source/Symbol/SymbolVendor.cpp === --- source/Symbol/SymbolVendor.cpp +++ source/Symbol/SymbolVendor.cpp @@ -61,7 +61,7 @@ //-- SymbolVendor::SymbolVendor(const lldb::ModuleSP &module_sp) : ModuleChild(module_sp), m_type_list(), m_compile_units(), - m_sym_file_ap() {} + m_sym_file_ap(), m_symtab() {} //-- // Destructor @@ -438,14 +438,23 @@ Symtab *SymbolVendor::GetSymtab() { ModuleSP module_sp(GetModule()); - if (module_sp) { -ObjectFile *objfile = module_sp->GetObjectFile(); -if (objfile) { - // Get symbol table from unified section list. - return objfile->GetSymtab(); -} - } - return nullptr; + if (!module_sp) +return nullptr; + + std::lock_guard guard(module_sp->GetMutex()); + + if (m_symtab) +return m_symtab; + + ObjectFile *objfile = module_sp->GetObjectFile(); + if (!objfile) +return nullptr; + + m_symtab = objfile->GetSymtab(); + if (m_symtab && m_sym_file_ap) +m_sym_file_ap->AddSymbols(*m_symtab); + + return m_symtab; } void SymbolVendor::ClearSymtab() { Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h === --- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h +++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h @@ -132,6 +132,8 @@ const std::string &scope_qualified_name, std::vector &mangled_names) override; + void AddSymbols(lldb_private::Symtab &symtab) override; + uint32_t FindTypes(const lldb_private::SymbolContext &sc, const lldb_private::ConstString &name, @@ -236,9 +238,13 @@ uint32_t GetCompilandId(const llvm::pdb::PDBSymbolData &data); + lldb_private::Symbol * + GetPublicSymbol(const llvm::pdb::PDBSymbolPublicSymbol &pub_symbol); + llvm::DenseMap m_comp_units; llvm::DenseMap m_types; llvm::DenseMap m_variables; + llvm::DenseMap m_public_symbols; llvm::DenseMap m_public_names; SecContribsMap m_sec_contribs; Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp === --- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -1331,6 +1331,31 @@ const std::string &scope_qualified_name, std::vector &mangled_names) {} +void SymbolFilePDB::AddSymbols(lldb_private::Symtab &symtab) { + std::set sym_addresses; + for (size_t i = 0; i < symtab.GetNumSymbols(); i++) +sym_addresses.insert(symtab.SymbolAtIndex(i)->GetFileAddress()); + + auto results = m_global_scope_up->findAllChildren(); + if (!results) +return; + + while (auto pub_symbol = results->getNext()) { +auto symbol_ptr = GetPublicSymbol(*pub_symbol); +if (!symbol_ptr) + continue; + +auto file_addr = symbol_ptr->GetFileAddress(); +if (sym_addresses.find(file_addr) != sym_addresses.end()) + continue; +sym_add
[Lldb-commits] [PATCH] D53140: [LLDB] - Add support for DW_RLE_base_address and DW_RLE_offset_pair entries (.debug_rnglists)
aprantl added a comment. It looks like this might have broken the bots, could you please take a look? http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/11671/consoleFull#434797663d489585b-5106-414a-ac11-3ff90657619c TEST 'lldb :: Breakpoint/debug_rnglist_rlestartend.test' FAILED Script: -- : 'RUN: at line 1'; /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/yaml2obj /Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/lit/Breakpoint/Inputs/debug_rnglist_rlestartend.yaml > /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/tools/lldb/lit/Breakpoint/Output/debug_rnglist_rlestartend.test.tmptest : 'RUN: at line 2'; /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/lldb-test breakpoints /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/tools/lldb/lit/Breakpoint/Output/debug_rnglist_rlestartend.test.tmptest /Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/lit/Breakpoint/debug_rnglist_rlestartend.test | /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/FileCheck /Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/lit/Breakpoint/debug_rnglist_rlestartend.test -- Exit Code: 2 Command Output (stderr): -- unexpected encoding UNREACHABLE executed at /Users/buildslave/jenkins/workspace/lldb-cmake/llvm/tools/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugRanges.cpp:202! 0 lldb-test0x000105a27d65 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 37 1 lldb-test0x000105a26c96 llvm::sys::RunSignalHandlers() + 198 2 lldb-test0x000105a283c8 SignalHandler(int) + 264 3 libsystem_platform.dylib 0x7fff78d08f5a _sigtramp + 26 4 libsystem_platform.dylib 0x7ffeea2c9da8 _sigtramp + 1901858408 5 libsystem_c.dylib0x7fff78aa61ae abort + 127 6 lldb-test0x00010599797c llvm::llvm_unreachable_internal(char const*, char const*, unsigned int) + 540 7 lldb-test0x000105eaeba9 DWARFDebugRngLists::FindRanges(DWARFUnit const*, unsigned int, lldb_private::RangeArray&) const + 457 8 lldb-test0x000105ea9dcc DWARFDebugInfoEntry::GetAttributeAddressRanges(SymbolFileDWARF*, DWARFUnit const*, lldb_private::RangeArray&, bool, bool) const + 140 9 lldb-test0x000105eb4114 DWARFUnit::BuildAddressRangeTable(SymbolFileDWARF*, DWARFDebugAranges*) + 116 10 lldb-test0x000105ea6bd1 DWARFDebugInfo::GetCompileUnitAranges() + 1025 11 lldb-test0x000105ec4c0f SymbolFileDWARF::ResolveSymbolContext(lldb_private::Address const&, unsigned int, lldb_private::SymbolContext&) + 255 12 lldb-test0x000105ba76bf lldb_private::SymbolVendor::ResolveSymbolContext(lldb_private::Address const&, unsigned int, lldb_private::SymbolContext&) + 95 13 lldb-test0x000105a8305b lldb_private::Module::ResolveSymbolContextForAddress(lldb_private::Address const&, unsigned int, lldb_private::SymbolContext&, bool) + 459 14 lldb-test0x000105a49b3d lldb_private::Address::CalculateSymbolContext(lldb_private::SymbolContext*, unsigned int) const + 205 15 lldb-test0x000105a33e67 lldb_private::BreakpointLocation::GetDescription(lldb_private::Stream*, lldb::DescriptionLevel) + 279 16 lldb-test0x000105a2e591 lldb_private::Breakpoint::GetDescription(lldb_private::Stream*, lldb::DescriptionLevel, bool) + 849 17 lldb-test0x00010713ba1a CommandObjectBreakpointSet::DoExecute(lldb_private::Args&, lldb_private::CommandReturnObject&) + 3082 18 lldb-test0x000105b2daa2 lldb_private::CommandObjectParsed::Execute(char const*, lldb_private::CommandReturnObject&) + 418 19 lldb-test0x000105b26485 lldb_private::CommandInterpreter::HandleCommand(char const*, lldb_private::LazyBool, lldb_private::CommandReturnObject&, lldb_private::ExecutionContext*, bool, bool) + 2869 20 lldb-test0x000105b2eb31 lldb_private::CommandObjectRegexCommand::DoExecute(llvm::StringRef, lldb_private::CommandReturnObject&) + 849 21 lldb-test0x000105b2dcd6 lldb_private::CommandObjectRaw::Execute(char const*, lldb_private::CommandReturnObject&) + 470 22 lldb-test0x000105b26485 lldb_private::CommandInterpreter::HandleCommand(char const*, lldb_private::LazyBool, lldb_private::CommandReturnObject&, lldb_private::ExecutionContext*, bool, bool) + 2869 23 lldb-test0x00010593c474 main + 1716 24 libdyld.dylib0x7fff789fa015 start + 1 25 libdyld.dylib0x0004 start + 2271240176 Stack dump: 0.Program arguments: /Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/bin/lldb-test breakpoints /Users/buildslave
[Lldb-commits] [PATCH] D48226: [lldb] Remove enableThreadSanitizer from shared Xcode schemes
aprantl accepted this revision. aprantl added a comment. This revision is now accepted and ready to land. Seems reasonable. https://reviews.llvm.org/D48226 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r345155 - [lldb] Remove enableThreadSanitizer from shared Xcode schemes
Author: kuba.brecka Date: Wed Oct 24 08:59:31 2018 New Revision: 345155 URL: http://llvm.org/viewvc/llvm-project?rev=345155&view=rev Log: [lldb] Remove enableThreadSanitizer from shared Xcode schemes This was probably committed accidentally and default Xcode builds of LLDB now have TSan on. Let's turn it off. Modified: lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/LLDB.xcscheme lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/darwin-debug.xcscheme Modified: lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/LLDB.xcscheme URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/LLDB.xcscheme?rev=345155&r1=345154&r2=345155&view=diff == --- lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/LLDB.xcscheme (original) +++ lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/LLDB.xcscheme Wed Oct 24 08:59:31 2018 @@ -54,7 +54,6 @@ selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" displayScaleIsEnabled = "NO" displayScale = "1.00" - enableThreadSanitizer = "YES" launchStyle = "0" useCustomWorkingDirectory = "NO" ignoresPersistentStateOnLaunch = "NO" Modified: lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/darwin-debug.xcscheme URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/darwin-debug.xcscheme?rev=345155&r1=345154&r2=345155&view=diff == --- lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/darwin-debug.xcscheme (original) +++ lldb/trunk/lldb.xcodeproj/xcshareddata/xcschemes/darwin-debug.xcscheme Wed Oct 24 08:59:31 2018 @@ -45,7 +45,6 @@ selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" displayScaleIsEnabled = "NO" displayScale = "1.00" - enableThreadSanitizer = "YES" launchStyle = "0" useCustomWorkingDirectory = "NO" ignoresPersistentStateOnLaunch = "NO" ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D48226: [lldb] Remove enableThreadSanitizer from shared Xcode schemes
kubamracek closed this revision. kubamracek added a comment. r345155. https://reviews.llvm.org/D48226 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53656: Adding formatters for libc++ std::u16string and std::u32string
shafik created this revision. shafik added reviewers: jingham, davide. Herald added a reviewer: EricWF. Herald added a subscriber: christof. We currently support formatting for std::string and std::wstring but not std::u16string and std::u32string which was added with C++11. This adds support for these two types via refactoring for the existing std::string formatter. https://reviews.llvm.org/D53656 Files: packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/Makefile packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/TestDataFormatterLibcxxString.py packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/string/main.cpp source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp source/Plugins/Language/CPlusPlus/LibCxx.cpp source/Plugins/Language/CPlusPlus/LibCxx.h Index: source/Plugins/Language/CPlusPlus/LibCxx.h === --- source/Plugins/Language/CPlusPlus/LibCxx.h +++ source/Plugins/Language/CPlusPlus/LibCxx.h @@ -19,9 +19,17 @@ namespace lldb_private { namespace formatters { -bool LibcxxStringSummaryProvider( +bool LibcxxStringSummaryProviderASCII( ValueObject &valobj, Stream &stream, -const TypeSummaryOptions &options); // libc++ std::string +const TypeSummaryOptions &summary_options); // libc++ std::string + +bool LibcxxStringSummaryProviderUTF16( +ValueObject &valobj, Stream &stream, +const TypeSummaryOptions &summary_options); // libc++ std::u16string + +bool LibcxxStringSummaryProviderUTF32( +ValueObject &valobj, Stream &stream, +const TypeSummaryOptions &summary_options); // libc++ std::u32string bool LibcxxWStringSummaryProvider( ValueObject &valobj, Stream &stream, Index: source/Plugins/Language/CPlusPlus/LibCxx.cpp === --- source/Plugins/Language/CPlusPlus/LibCxx.cpp +++ source/Plugins/Language/CPlusPlus/LibCxx.cpp @@ -599,9 +599,10 @@ return true; } -bool lldb_private::formatters::LibcxxStringSummaryProvider( -ValueObject &valobj, Stream &stream, -const TypeSummaryOptions &summary_options) { +template +bool LibcxxStringSummaryProvider(ValueObject &valobj, Stream &stream, + const TypeSummaryOptions &summary_options, + std::string prefix_token = "") { uint64_t size = 0; ValueObjectSP location_sp; @@ -630,12 +631,37 @@ options.SetData(extractor); options.SetStream(&stream); - options.SetPrefixToken(nullptr); + + if (prefix_token.empty()) +options.SetPrefixToken(nullptr); + else +options.SetPrefixToken(prefix_token); + options.SetQuote('"'); options.SetSourceSize(size); options.SetBinaryZeroIsTerminator(false); - StringPrinter::ReadBufferAndDumpToStream< - StringPrinter::StringElementType::ASCII>(options); + StringPrinter::ReadBufferAndDumpToStream(options); return true; } + +bool lldb_private::formatters::LibcxxStringSummaryProviderASCII( +ValueObject &valobj, Stream &stream, +const TypeSummaryOptions &summary_options) { + return LibcxxStringSummaryProvider( + valobj, stream, summary_options); +} + +bool lldb_private::formatters::LibcxxStringSummaryProviderUTF16( +ValueObject &valobj, Stream &stream, +const TypeSummaryOptions &summary_options) { + return LibcxxStringSummaryProvider( + valobj, stream, summary_options, "u"); +} + +bool lldb_private::formatters::LibcxxStringSummaryProviderUTF32( +ValueObject &valobj, Stream &stream, +const TypeSummaryOptions &summary_options) { + return LibcxxStringSummaryProvider( + valobj, stream, summary_options, "U"); +} Index: source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp === --- source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp +++ source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp @@ -405,8 +405,17 @@ #ifndef LLDB_DISABLE_PYTHON lldb::TypeSummaryImplSP std_string_summary_sp(new CXXFunctionSummaryFormat( - stl_summary_flags, lldb_private::formatters::LibcxxStringSummaryProvider, + stl_summary_flags, + lldb_private::formatters::LibcxxStringSummaryProviderASCII, "std::string summary provider")); + lldb::TypeSummaryImplSP std_stringu16_summary_sp(new CXXFunctionSummaryFormat( + stl_summary_flags, + lldb_private::formatters::LibcxxStringSummaryProviderUTF16, + "std::u16string summary provider")); + lldb::TypeSummaryImplSP std_stringu32_summary_sp(new CXXFunctionSummaryFormat( + stl_summary_flags, + lldb_private::formatters::LibcxxStringSummaryProviderUTF32, + "std::u32string summary provider")); lldb::TypeSummaryImplSP std_wstring_summary_sp(new CXXFunctionSummaryFormat( stl_summary_flags, lldb_private::formatters::LibcxxWStringSummaryProvider,
[Lldb-commits] [PATCH] D53368: [Symbol] Search symbols with name and type in a symbol file
clayborg requested changes to this revision. clayborg added a comment. This revision now requires changes to proceed. Very close. Just a bit of cleanup now that we changed SymbolFilePDB where we don't seem to need m_public_symbols anymore. See inlined comments and let me know what you think Comment at: include/lldb/lldb-forward.h:444 StructuredDataPluginWP; +typedef std::shared_ptr SymbolSP; typedef std::shared_ptr SymbolFileSP; Do we need this anymore? See inlined comments below. Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:1344-1346 +auto symbol_ptr = GetPublicSymbol(*pub_symbol); +if (!symbol_ptr) + continue; Maybe get the file address from "pub_symbol" and avoid converting to a SymbolSP that we will never use? See more comments below. Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:1353 + +symtab.AddSymbol(*symbol_ptr); + } Just make a local symbol and convert it from PDB to lldb_private::Symbol here? Something like: ``` symtab.AddSymbol(ConvertPDBSymbol(pdb_symbol)); ``` So it seems we shouldn't need the m_public_symbols storage in this class anymore since "symtab" will now own the symbol we just created. Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:1968-1969 + +lldb_private::Symbol *SymbolFilePDB::GetPublicSymbol( +const llvm::pdb::PDBSymbolPublicSymbol &pub_symbol) { + auto it = m_public_symbols.find(pub_symbol.getSymIndexId()); Maybe convert this to: ``` lldb_private::Symbol ConvertPDBSymbol(const llvm::pdb::PDBSymbolPublicSymbol &pub_symbol) ``` And we only call this if we need to create a symbol we will add to the "symtab" in SymbolFilePDB::AddSymbols(...) Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h:247 llvm::DenseMap m_variables; + llvm::DenseMap m_public_symbols; llvm::DenseMap m_public_names; Do we need this mapping anymore? We should just add the symbols to the symbol table during SymbolFilePDB::AddSymbols(...). https://reviews.llvm.org/D53368 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53361: [API] Extend the `SBThreadPlan` interface
jingham accepted this revision. jingham added a comment. That looks right. Thanks for adding the test! https://reviews.llvm.org/D53361 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53094: [pecoff] Implement ObjectFilePECOFF::GetDependedModules()
clayborg added a comment. Zach: yes the module mutex needs to be recursive. Plenty of places where the symbol file stuff can call other symbol file APIs. To avoid A/B locking issues, the lldb_private::Module, lldb_private::ObjectFile and lldb_private::SymbolVendor and lldb_private::SymbolFile all use the module mutex. https://reviews.llvm.org/D53094 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53094: [pecoff] Implement ObjectFilePECOFF::GetDependedModules()
lemo added a comment. drive by CR notes: 1. does this handle forwarding imports? (it doesn't seem to from a quick glance at the code) 2. can you please add, or extend the existing test to cover the transitive case? A simple dag would suffice (ex. make both dllA and dllB implicitly import dllC) Comment at: source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.h:219 coff_data_dir_import_table = 1, +coff_data_dir_resource_table, +coff_data_dir_exception_table, nit: either explicitly assign values to all or none https://reviews.llvm.org/D53094 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner created this revision. zturner added reviewers: clayborg, labath, jingham. Herald added subscribers: JDevlieghere, aprantl. Previously the Module would parse a type name into scope and base name during a lookup and only give the SymbolFile plugin the base name, then it would filter the results down into the match set based on whether it was supposed to be an exact match. However, if this is supposed to be an exact match, a SymbolFile can do this lookup in O(1) time by using its internal accelerator tables, but it can't do this unless it actually receives the full name as well as the information that it is supposed to be an exact match. Rather than being too smart, we should just let the symbol file plugin be the arbiter to decide how it wants to do lookups, so we should pass it the full name. I'm trying to keep this as NFC for the DWARF plugin, so I took the code that was in Module.cpp and tried to rewrite it to be equivalent at the top of `SymbolFileDWARF::FindTypes` https://reviews.llvm.org/D53662 Files: lldb/include/lldb/Core/Module.h lldb/include/lldb/Symbol/SymbolFile.h lldb/include/lldb/Symbol/SymbolVendor.h lldb/lit/SymbolFile/NativePDB/Inputs/tag-types.lldbinit lldb/lit/SymbolFile/NativePDB/tag-types.cpp lldb/source/Core/Module.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h lldb/source/Symbol/SymbolFile.cpp lldb/source/Symbol/SymbolVendor.cpp lldb/tools/lldb-test/lldb-test.cpp Index: lldb/tools/lldb-test/lldb-test.cpp === --- lldb/tools/lldb-test/lldb-test.cpp +++ lldb/tools/lldb-test/lldb-test.cpp @@ -456,8 +456,8 @@ SymbolContext SC; DenseSet SearchedFiles; TypeMap Map; - Vendor.FindTypes(SC, ConstString(Name), ContextPtr, true, UINT32_MAX, - SearchedFiles, Map); + Vendor.FindTypes(SC, Name, ContextPtr, false, true, UINT32_MAX, SearchedFiles, + Map); outs() << formatv("Found {0} types:\n", Map.GetSize()); StreamString Stream; Index: lldb/source/Symbol/SymbolVendor.cpp === --- lldb/source/Symbol/SymbolVendor.cpp +++ lldb/source/Symbol/SymbolVendor.cpp @@ -315,17 +315,18 @@ } size_t SymbolVendor::FindTypes( -const SymbolContext &sc, const ConstString &name, -const CompilerDeclContext *parent_decl_ctx, bool append, size_t max_matches, +const SymbolContext &sc, llvm::StringRef name, +const CompilerDeclContext *parent_decl_ctx, bool exact_match, bool append, +size_t max_matches, llvm::DenseSet &searched_symbol_files, TypeMap &types) { ModuleSP module_sp(GetModule()); if (module_sp) { std::lock_guard guard(module_sp->GetMutex()); if (m_sym_file_ap.get()) - return m_sym_file_ap->FindTypes(sc, name, parent_decl_ctx, append, - max_matches, searched_symbol_files, - types); + return m_sym_file_ap->FindTypes(sc, name, parent_decl_ctx, exact_match, + append, max_matches, + searched_symbol_files, types); } if (!append) types.Clear(); Index: lldb/source/Symbol/SymbolFile.cpp === --- lldb/source/Symbol/SymbolFile.cpp +++ lldb/source/Symbol/SymbolFile.cpp @@ -140,8 +140,8 @@ } uint32_t SymbolFile::FindTypes( -const SymbolContext &sc, const ConstString &name, -const CompilerDeclContext *parent_decl_ctx, bool append, +const SymbolContext &sc, llvm::StringRef name, +const CompilerDeclContext *parent_decl_ctx, bool exact_match, bool append, uint32_t max_matches, llvm::DenseSet &searched_symbol_files, TypeMap &types) { Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h === --- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h +++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h @@ -134,10 +134,9 @@ std::vector &mangled_names) override; uint32_t - FindTypes(const lldb_private::SymbolContext &sc, -const lldb_private::ConstString &name, + FindTypes(const lldb_private::SymbolContext &sc, llvm::StringRef name, const lldb_private::CompilerDeclContext *parent_decl_ctx, -bool append, uint32_t max_matches, +bool exact_match, bool append, uint32_t max_matches, llvm::DenseSet &searched_symbol_files,
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner added a comment. Note that AFAICT, native PDB plugin is now the only plugin where you can do `type lookup -- NS::Struct::Local` and have it return instantly. On the regular PDB plugin it doesn't work at all (returns no results). On the DWARF plugin, I haven't tested, but it will either not work at all, or take a potentially long time if you have a lot of debug info). https://reviews.llvm.org/D53662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52772: [Settings] Make "settings set" without a value equivalent to "settings clear"
JDevlieghere added a comment. ping https://reviews.llvm.org/D52772 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D47889: Use llvm::VersionTuple instead of manual version marshalling
aprantl added a comment. Just FYI, I just came across this patch while debugging strangeness in PlatformDarwin.cpp and unfortunately this patch isn't NFC. `VersionTuple::tryParse()` can deal with an optional third (micro) component, but the parse will fail when there are extra characters after the version number (in my case trying to parse the substring "12.0.sdk" out of "iPhoneSimulator12.0.sdk" now fails after this patch). I'll fix it and try to add a unit test ... Repository: rL LLVM https://reviews.llvm.org/D47889 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52772: [Settings] Make "settings set" without a value equivalent to "settings clear"
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. That looks fine. https://reviews.llvm.org/D52772 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r345207 - [Settings] Add -force flag to "settings set"
Author: jdevlieghere Date: Wed Oct 24 15:04:20 2018 New Revision: 345207 URL: http://llvm.org/viewvc/llvm-project?rev=345207&view=rev Log: [Settings] Add -force flag to "settings set" The -force option allows you to pass an empty value to settings set to reset the value to its default. This means that the following operations are equivalent: settings set -f settings clear The motivation for this change is the ability to export and import settings from LLDB. Because of the way the dumpers work, we don't know whether a value is going to be the default or not. Hence we cannot use settings clear and use settings set -f, potentially providing an empty value. Differential revision: https://reviews.llvm.org/D52772 Added: lldb/trunk/lit/Settings/TestSettingsSet.test Modified: lldb/trunk/source/Commands/CommandObjectSettings.cpp Added: lldb/trunk/lit/Settings/TestSettingsSet.test URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/Settings/TestSettingsSet.test?rev=345207&view=auto == --- lldb/trunk/lit/Settings/TestSettingsSet.test (added) +++ lldb/trunk/lit/Settings/TestSettingsSet.test Wed Oct 24 15:04:20 2018 @@ -0,0 +1,15 @@ +# This tests setting setting values. + +# Check that setting an empty value with -f(orce) clears the value. +# RUN: %lldb -b -s %s 2>&1 | FileCheck %s + +settings set tab-size 16 +settings show tab-size +# CHECK: tab-size (unsigned) = 16 + +settings set -f tab-size +settings show tab-size +# CHECK: tab-size (unsigned) = 4 + +settings set tab-size +# CHECK: error: 'settings set' takes more arguments Modified: lldb/trunk/source/Commands/CommandObjectSettings.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectSettings.cpp?rev=345207&r1=345206&r2=345207&view=diff == --- lldb/trunk/source/Commands/CommandObjectSettings.cpp (original) +++ lldb/trunk/source/Commands/CommandObjectSettings.cpp Wed Oct 24 15:04:20 2018 @@ -30,7 +30,8 @@ using namespace lldb_private; static constexpr OptionDefinition g_settings_set_options[] = { // clang-format off - { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Apply the new value to the global default value." } + { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Apply the new value to the global default value." }, + { LLDB_OPT_SET_2, false, "force", 'f', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Force an empty value to be accepted as the default." } // clang-format on }; @@ -108,6 +109,9 @@ insert-before or insert-after."); const int short_option = m_getopt_table[option_idx].val; switch (short_option) { + case 'f': +m_force = true; +break; case 'g': m_global = true; break; @@ -122,6 +126,7 @@ insert-before or insert-after."); void OptionParsingStarting(ExecutionContext *execution_context) override { m_global = false; + m_force = false; } llvm::ArrayRef GetDefinitions() override { @@ -129,8 +134,8 @@ insert-before or insert-after."); } // Instance variables to hold the values for command options. - bool m_global; +bool m_force; }; int HandleArgumentCompletion( @@ -184,8 +189,10 @@ protected: if (!ParseOptions(cmd_args, result)) return false; +const size_t min_argc = m_options.m_force ? 1 : 2; const size_t argc = cmd_args.GetArgumentCount(); -if ((argc < 2) && (!m_options.m_global)) { + +if ((argc < min_argc) && (!m_options.m_global)) { result.AppendError("'settings set' takes more arguments"); result.SetStatus(eReturnStatusFailed); return false; @@ -199,6 +206,19 @@ protected: return false; } +// A missing value corresponds to clearing the setting when "force" is +// specified. +if (argc == 1 && m_options.m_force) { + Status error(m_interpreter.GetDebugger().SetPropertyValue( + &m_exe_ctx, eVarSetOperationClear, var_name, llvm::StringRef())); + if (error.Fail()) { +result.AppendError(error.AsCString()); +result.SetStatus(eReturnStatusFailed); +return false; + } + return result.Succeeded(); +} + // Split the raw command into var_name and value pair. llvm::StringRef raw_str(command); std::string var_value_string = raw_str.split(var_name).second.str(); ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52772: [Settings] Make "settings set" without a value equivalent to "settings clear"
This revision was automatically updated to reflect the committed changes. Closed by commit rL345207: [Settings] Add -force flag to "settings set" (authored by JDevlieghere, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit: https://reviews.llvm.org/D52772?vs=170257&id=170995#toc Repository: rL LLVM https://reviews.llvm.org/D52772 Files: lldb/trunk/lit/Settings/TestSettingsSet.test lldb/trunk/source/Commands/CommandObjectSettings.cpp Index: lldb/trunk/source/Commands/CommandObjectSettings.cpp === --- lldb/trunk/source/Commands/CommandObjectSettings.cpp +++ lldb/trunk/source/Commands/CommandObjectSettings.cpp @@ -30,7 +30,8 @@ static constexpr OptionDefinition g_settings_set_options[] = { // clang-format off - { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Apply the new value to the global default value." } + { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Apply the new value to the global default value." }, + { LLDB_OPT_SET_2, false, "force", 'f', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Force an empty value to be accepted as the default." } // clang-format on }; @@ -108,6 +109,9 @@ const int short_option = m_getopt_table[option_idx].val; switch (short_option) { + case 'f': +m_force = true; +break; case 'g': m_global = true; break; @@ -122,15 +126,16 @@ void OptionParsingStarting(ExecutionContext *execution_context) override { m_global = false; + m_force = false; } llvm::ArrayRef GetDefinitions() override { return llvm::makeArrayRef(g_settings_set_options); } // Instance variables to hold the values for command options. - bool m_global; +bool m_force; }; int HandleArgumentCompletion( @@ -184,8 +189,10 @@ if (!ParseOptions(cmd_args, result)) return false; +const size_t min_argc = m_options.m_force ? 1 : 2; const size_t argc = cmd_args.GetArgumentCount(); -if ((argc < 2) && (!m_options.m_global)) { + +if ((argc < min_argc) && (!m_options.m_global)) { result.AppendError("'settings set' takes more arguments"); result.SetStatus(eReturnStatusFailed); return false; @@ -199,6 +206,19 @@ return false; } +// A missing value corresponds to clearing the setting when "force" is +// specified. +if (argc == 1 && m_options.m_force) { + Status error(m_interpreter.GetDebugger().SetPropertyValue( + &m_exe_ctx, eVarSetOperationClear, var_name, llvm::StringRef())); + if (error.Fail()) { +result.AppendError(error.AsCString()); +result.SetStatus(eReturnStatusFailed); +return false; + } + return result.Succeeded(); +} + // Split the raw command into var_name and value pair. llvm::StringRef raw_str(command); std::string var_value_string = raw_str.split(var_name).second.str(); Index: lldb/trunk/lit/Settings/TestSettingsSet.test === --- lldb/trunk/lit/Settings/TestSettingsSet.test +++ lldb/trunk/lit/Settings/TestSettingsSet.test @@ -0,0 +1,15 @@ +# This tests setting setting values. + +# Check that setting an empty value with -f(orce) clears the value. +# RUN: %lldb -b -s %s 2>&1 | FileCheck %s + +settings set tab-size 16 +settings show tab-size +# CHECK: tab-size (unsigned) = 16 + +settings set -f tab-size +settings show tab-size +# CHECK: tab-size (unsigned) = 4 + +settings set tab-size +# CHECK: error: 'settings set' takes more arguments Index: lldb/trunk/source/Commands/CommandObjectSettings.cpp === --- lldb/trunk/source/Commands/CommandObjectSettings.cpp +++ lldb/trunk/source/Commands/CommandObjectSettings.cpp @@ -30,7 +30,8 @@ static constexpr OptionDefinition g_settings_set_options[] = { // clang-format off - { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Apply the new value to the global default value." } + { LLDB_OPT_SET_2, false, "global", 'g', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Apply the new value to the global default value." }, + { LLDB_OPT_SET_2, false, "force", 'f', OptionParser::eNoArgument, nullptr, {}, 0, eArgTypeNone, "Force an empty value to be accepted as the default." } // clang-format on }; @@ -108,6 +109,9 @@ const int short_option = m_getopt_table[option_idx].val; switch (short_option) { + case 'f': +m_force = true; +break; case 'g': m_global = true; break; @@ -122,15 +126,16 @@ void OptionParsingStarting(ExecutionContext *execution_context) override { m_global = false; + m_force = false;
[Lldb-commits] [PATCH] D52651: Add functionality to export settings
JDevlieghere added a comment. @teemperor Can you have another look? https://reviews.llvm.org/D52651 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53677: Fix a bug PlatformDarwin::SDKSupportsModule
aprantl created this revision. aprantl added reviewers: jingham, clayborg, labath. This fixes a bug PlatformDarwin::SDKSupportsModule introduced by https://reviews.llvm.org/D47889. VersionTuple::tryParse() can deal with an optional third (micro) component, but the parse will fail when there are extra characters after the version number (e.g.: trying to parse the substring "12.0.sdk" out of "iPhoneSimulator12.0.sdk" fails after that patch). Fixed here by stripping the ".sdk" suffix first. rdar://problem/45041492 https://reviews.llvm.org/D53677 Files: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp source/Plugins/Platform/MacOSX/PlatformDarwin.h unittests/Platform/PlatformDarwinTest.cpp Index: unittests/Platform/PlatformDarwinTest.cpp === --- unittests/Platform/PlatformDarwinTest.cpp +++ unittests/Platform/PlatformDarwinTest.cpp @@ -18,6 +18,13 @@ using namespace lldb; using namespace lldb_private; +struct PlatformDarwinTester : public PlatformDarwin { + static bool SDKSupportsModules(SDKType desired_type, + const lldb_private::FileSpec &sdk_path) { +return PlatformDarwin::SDKSupportsModules(desired_type, sdk_path); + } +}; + TEST(PlatformDarwinTest, TestParseVersionBuildDir) { llvm::VersionTuple V; llvm::StringRef D; @@ -44,4 +51,23 @@ std::tie(V, D) = PlatformDarwin::ParseVersionBuildDir("3.4.5"); EXPECT_EQ(llvm::VersionTuple(3, 4, 5), V); + + std::string base = "/Applications/Xcode.app/Contents/Developer/Platforms/"; + EXPECT_TRUE(PlatformDarwinTester::SDKSupportsModules( + PlatformDarwin::SDKType::iPhoneSimulator, + FileSpec(base + + "iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator12.0.sdk", + false))); + EXPECT_FALSE(PlatformDarwinTester::SDKSupportsModules( + PlatformDarwin::SDKType::iPhoneSimulator, + FileSpec(base + + "iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator7.2.sdk", + false))); + EXPECT_TRUE(PlatformDarwinTester::SDKSupportsModules( + PlatformDarwin::SDKType::MacOSX, + FileSpec(base + "MacOSX.platform/Developer/SDKs/MacOSX10.10.sdk", + false))); + EXPECT_FALSE(PlatformDarwinTester::SDKSupportsModules( + PlatformDarwin::SDKType::MacOSX, + FileSpec(base + "MacOSX.platform/Developer/SDKs/MacOSX10.9.sdk", false))); } Index: source/Plugins/Platform/MacOSX/PlatformDarwin.h === --- source/Plugins/Platform/MacOSX/PlatformDarwin.h +++ source/Plugins/Platform/MacOSX/PlatformDarwin.h @@ -85,6 +85,12 @@ static std::tuple ParseVersionBuildDir(llvm::StringRef str); + enum class SDKType { +MacOSX = 0, +iPhoneSimulator, +iPhoneOS, + }; + protected: void ReadLibdispatchOffsetsAddress(lldb_private::Process *process); @@ -95,12 +101,6 @@ const lldb_private::FileSpecList *module_search_paths_ptr, lldb::ModuleSP *old_module_sp_ptr, bool *did_create_ptr); - enum class SDKType { -MacOSX = 0, -iPhoneSimulator, -iPhoneOS, - }; - static bool SDKSupportsModules(SDKType sdk_type, llvm::VersionTuple version); static bool SDKSupportsModules(SDKType desired_type, Index: source/Plugins/Platform/MacOSX/PlatformDarwin.cpp === --- source/Plugins/Platform/MacOSX/PlatformDarwin.cpp +++ source/Plugins/Platform/MacOSX/PlatformDarwin.cpp @@ -1402,14 +1402,11 @@ if (last_path_component) { const llvm::StringRef sdk_name = last_path_component.GetStringRef(); -llvm::StringRef version_part; - -if (sdk_name.startswith(sdk_strings[(int)desired_type])) { - version_part = - sdk_name.drop_front(strlen(sdk_strings[(int)desired_type])); -} else { +if (!sdk_name.startswith(sdk_strings[(int)desired_type])) return false; -} +auto version_part = +sdk_name.drop_front(strlen(sdk_strings[(int)desired_type])); +version_part.consume_back(".sdk"); llvm::VersionTuple version; if (version.tryParse(version_part)) Index: unittests/Platform/PlatformDarwinTest.cpp === --- unittests/Platform/PlatformDarwinTest.cpp +++ unittests/Platform/PlatformDarwinTest.cpp @@ -18,6 +18,13 @@ using namespace lldb; using namespace lldb_private; +struct PlatformDarwinTester : public PlatformDarwin { + static bool SDKSupportsModules(SDKType desired_type, + const lldb_private::FileSpec &sdk_path) { +return PlatformDarwin::SDKSupportsModules(desired_type, sdk_path); + } +}; + TEST(PlatformDarwinTest, TestParseVersionBuildDir) { llvm::VersionTuple V; llvm::StringRef D; @@ -44,4 +51,23 @@ std::tie(V, D) = PlatformDarwin::ParseVersionBuildDir("3.4.5"); EXPECT_EQ(llvm::VersionTuple(3, 4, 5), V); + + std::string base = "/Applications/Xcode.app/Co
[Lldb-commits] [PATCH] D53597: Don't type-erase the SymbolContextItem enum
zturner added a comment. Hi Greg, any thoughts on this? https://reviews.llvm.org/D53597 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53656: Adding formatters for libc++ std::u16string and std::u32string
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. This looks reasonable to me. https://reviews.llvm.org/D53656 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52651: Add functionality to export settings
teemperor requested changes to this revision. teemperor added a comment. This revision now requires changes to proceed. Sorry, this somehow didn't show up in my review queue. I think this can land after two minor things are fixed: - I think unknown arguments to write/read shouldn't be silently ignored. - And I think we should report an error if writing to the output file fails. To give a practical example: This command below fails to write to the output file but also doesn't produce any error message. `settings write -f ~/ foo` Note that there is a space behind the /, so we write to a directory (which fails) and we silently ignore the second unknown arg. https://reviews.llvm.org/D52651 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52651: Add functionality to export settings
teemperor accepted this revision. teemperor added a comment. This revision is now accepted and ready to land. Woops, I wanted to accept in my previous comment. https://reviews.llvm.org/D52651 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D52651: Add functionality to export settings
JDevlieghere added a comment. Thanks Raphael! With r345207 the empty behavior only works if you explicitly specify `-force`. I'll address the second issue before landing this. https://reviews.llvm.org/D52651 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
jingham added a comment. I worry that your patch changes the behavior when you add the type_class explicitly in the lookup (i.e. look up "struct Struct" not "Struct". That should work... Note, this doesn't currently work in type lookup: (lldb) type lookup "struct Foo" no type was found matching '"struct Foo"' which I'll have to fix (grr...), but does work correctly in the SB API's: (lldb) script Python Interactive Interpreter. To exit, type 'quit()', 'exit()' or Ctrl-D. >>> module = lldb.target.modules[0] >>> print module (x86_64) /tmp/structs >>> result_list = module.FindTypes("struct Foo") >>> print result_list.GetSize() 1 >>> print result_list.GetTypeAtIndex(0) struct Foo { int First; } Other than that this looks correct to me. Comment at: lldb/source/Core/Module.cpp:1012 +TypeClass type_class = eTypeClassAny; +Type::GetTypeScopeAndBasename(type_name, type_scope, type_basename, + type_class); GetTypeScopeAndBasename's behavior is not well documented. It has a bool return which should mean some kind of failure. The code you are replacing checks for type_basename.empty() and the type_class not being set, and falls back on the input name if it is. You unconditionally use type_basename. The old code's test doesn't actually accord with the current behavior of GetTypeScopeAndBasename, which will only leave basename empty if the input name was empty, so far as I can see. So I don't think your version is wrong, but it is right only by accident. If we are going to rely on this behavior then it's probably a good idea to document it in the definition of GetTypeScopeAndBasename. Or go back to checking whether type_basename is empty... Also, by not calling GetTypeScopeAndBasename before you call FindTypes_Impl, your version of the code would pass it "struct Foo" if that's what the user typed, whereas the old code would pass "Foo" (the struct would get stripped by GetTypeScopeAndBasename). I'm not sure whether that matters, did you try 'type lookup "struct Struct"' or something like that? It doesn't look like you do that in your test cases. Also your "exact" would call "struct ::Foo" not exact, whereas the old code would call that exact. https://reviews.llvm.org/D53662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53662: Give the SymbolFile plugin enough information to efficiently do exact match lookups
zturner added a comment. In https://reviews.llvm.org/D53662#1275238, @jingham wrote: > I worry that your patch changes the behavior when you add the type_class > explicitly in the lookup (i.e. look up "struct Struct" not "Struct". That > should work... > > Note, this doesn't currently work in type lookup: > > (lldb) type lookup "struct Foo" > no type was found matching '"struct Foo"' > Ahh, nice catch. Luckily it should be easy to add a couple of tests for those alongside the other ones I added, so I'll try that and upload a new version. https://reviews.llvm.org/D53662 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53677: Fix a bug PlatformDarwin::SDKSupportsModule
jingham accepted this revision. jingham added a comment. This revision is now accepted and ready to land. Looks good to me. https://reviews.llvm.org/D53677 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53532: [FileSpec] Add VFS support to FileSpec convenience methods.
JDevlieghere added a comment. Note that there's also a File class which we'll need to patch up separately together with all other uses that actually open or read a file. For this class we can do the same as we did for FileSpec: deferring relevant operations to the FileSystem class. https://reviews.llvm.org/D53532 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D53532: [FileSpec] Add VFS support to FileSpec convenience methods.
It seems like FileSpec was moved out of Utility and into Host. I’m not a fan of this change, because it took a lot of effort to get it into Utility, which helped break a lot of bad dependencies. Can we invert this dependency and move FileSpec back into Utility? On Wed, Oct 24, 2018 at 5:53 PM Jonas Devlieghere via Phabricator < revi...@reviews.llvm.org> wrote: > JDevlieghere added a comment. > > Note that there's also a File class which we'll need to patch up > separately together with all other uses that actually open or read a file. > For this class we can do the same as we did for FileSpec: deferring > relevant operations to the FileSystem class. > > > https://reviews.llvm.org/D53532 > > > > ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D53532: [FileSpec] Add VFS support to FileSpec convenience methods.
zturner added a subscriber: JDevlieghere. zturner added a comment. It seems like FileSpec was moved out of Utility and into Host. I’m not a fan of this change, because it took a lot of effort to get it into Utility, which helped break a lot of bad dependencies. Can we invert this dependency and move FileSpec back into Utility? https://reviews.llvm.org/D53532 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D53532: [FileSpec] Add VFS support to FileSpec convenience methods.
Sent from my iPhone > On Oct 24, 2018, at 7:05 PM, Zachary Turner wrote: > > It seems like FileSpec was moved out of Utility and into Host. I’m not a fan > of this change, because it took a lot of effort to get it into Utility, which > helped break a lot of bad dependencies. Can we invert this dependency and > move FileSpec back into Utility? >> On Wed, Oct 24, 2018 at 5:53 PM Jonas Devlieghere via Phabricator >> wrote: >> JDevlieghere added a comment. >> >> Note that there's also a File class which we'll need to patch up separately >> together with all other uses that actually open or read a file. For this >> class we can do the same as we did for FileSpec: deferring relevant >> operations to the FileSystem class. >> >> >> https://reviews.llvm.org/D53532 >> >> >> ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits