[Lldb-commits] [PATCH] D53140: [LLDB] - Add support for DW_RLE_base_address and DW_RLE_offset_pair entries (.debug_rnglists)

2018-10-24 Thread George Rimar via Phabricator via lldb-commits
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

2018-10-24 Thread Aleksandr Urakov via Phabricator via lldb-commits
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.

2018-10-24 Thread George Rimar via Phabricator via lldb-commits
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

2018-10-24 Thread Aleksandr Urakov via Phabricator via lldb-commits
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)

2018-10-24 Thread Adrian Prantl via Phabricator via lldb-commits
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

2018-10-24 Thread Adrian Prantl via Phabricator via lldb-commits
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

2018-10-24 Thread Kuba Mracek via lldb-commits
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

2018-10-24 Thread Kuba (Brecka) Mracek via Phabricator via lldb-commits
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

2018-10-24 Thread Shafik Yaghmour via Phabricator via lldb-commits
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

2018-10-24 Thread Greg Clayton via Phabricator via lldb-commits
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

2018-10-24 Thread Jim Ingham via Phabricator via lldb-commits
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()

2018-10-24 Thread Greg Clayton via Phabricator via lldb-commits
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()

2018-10-24 Thread Leonard Mosescu via Phabricator via lldb-commits
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

2018-10-24 Thread Zachary Turner via Phabricator via lldb-commits
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

2018-10-24 Thread Zachary Turner via Phabricator via lldb-commits
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"

2018-10-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
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

2018-10-24 Thread Adrian Prantl via Phabricator via lldb-commits
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"

2018-10-24 Thread Jim Ingham via Phabricator via lldb-commits
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"

2018-10-24 Thread Jonas Devlieghere via lldb-commits
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"

2018-10-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
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

2018-10-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
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

2018-10-24 Thread Adrian Prantl via Phabricator via lldb-commits
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

2018-10-24 Thread Zachary Turner via Phabricator via lldb-commits
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

2018-10-24 Thread Jim Ingham via Phabricator via lldb-commits
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

2018-10-24 Thread Raphael Isemann via Phabricator via lldb-commits
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

2018-10-24 Thread Raphael Isemann via Phabricator via lldb-commits
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

2018-10-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
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

2018-10-24 Thread Jim Ingham via Phabricator via lldb-commits
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

2018-10-24 Thread Zachary Turner via Phabricator via lldb-commits
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

2018-10-24 Thread Jim Ingham via Phabricator via lldb-commits
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.

2018-10-24 Thread Jonas Devlieghere via Phabricator via lldb-commits
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.

2018-10-24 Thread Zachary Turner via lldb-commits
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.

2018-10-24 Thread Zachary Turner via Phabricator via lldb-commits
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.

2018-10-24 Thread Jonas Devlieghere via lldb-commits


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