[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

2019-11-29 Thread Martin Storsjö via Phabricator via lldb-commits
mstorsjo created this revision.
mstorsjo added reviewers: labath, amccarth.
Herald added subscribers: kristof.beyls, aprantl.
Herald added a reviewer: jdoerfert.
Herald added a project: LLDB.

Windows on ARM is always thumb, and contrary to ELF, there's no per-symbol flag 
to indicate arm vs thumb mode. Therefore, the linkers both Microsoft's link.exe 
and lld) sets the thumb bit in all relocations pointing to code sections.

In ELF, the thumb bit is only set for symbols marked as %function, that is, it 
isn't set in local labels used when constructing the dwarf debug info and line 
tables.

As the windows linkers sets the thumb bit in all code addresses, we'd have to 
cope with this and strip out the lowest bit from all code addresses if the 
architecture is ARM/Thumb.

The testcase uses a prelinked .yaml source instead of an assembly source file, 
to avoid needing to run a linker as part of the test.

There's already predecent for this in DWARFCallFrameInfo::GetFDEIndex, this 
extends the same concept to DWARF debug info and line tables. I'm not sure if I 
managed to get all spots covered here, but I tried to look for all cases of 
DW_AT_low_pc/DW_AT_high_pc and find the best place to handle it, where it would 
have to be handled in as few places as possible.

This is admittedly not very pretty, improvement suggestions are welcome.

Finally, this makes sure to propagate the correct address byte size to 
DataExtractors that are taken from e.g. ObjectFile::ReadSectionData. 
Previously, the DataExtractor got the host address size set, which later 
tripped up DWARFDebugLine::LineTable::parse as the address size of the line 
table (4) mismatched the one set in the DWARFDataExtractor (which was 
propagated from ObjectFile's internal m_data, which never got anything else set 
than the default from the host's sizeof(void*)).

This caused line tables to end up missing if inspecting a foreign object file 
(like in the testcase).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70840

Files:
  lldb/include/lldb/Symbol/LineTable.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Symbol/LineTable.cpp
  lldb/source/Symbol/ObjectFile.cpp
  lldb/test/Shell/SymbolFile/DWARF/lit.local.cfg
  lldb/test/Shell/SymbolFile/DWARF/thumb-windows.yaml

Index: lldb/test/Shell/SymbolFile/DWARF/thumb-windows.yaml
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/thumb-windows.yaml
@@ -0,0 +1,125 @@
+# Test that a linked windows executable, with a thumb bit in many address
+# fields, gets the thumb bit stripped out from addresses.
+
+# If the thumb bit isn't stripped out from subprogram ranges, 0x401006 is
+# associated with the function "entry", while it actually is the start of
+# the function "other".
+# If the thumb bit isn't stripped out from line tables, the LineEntry
+# points to the wrong line.
+
+# RUN: yaml2obj %s > %t.exe
+# RUN: %lldb %t.exe -o "image lookup -v -a 0x401006" -b | FileCheck %s
+
+# CHECK-LABEL: image lookup -v -a 0x401006
+# CHECK: Function: {{.*}}, name = "other", range = [0x00401006-0x0040100c)
+# CHECK: LineEntry: [0x00401006-0x0040100a): /path/to/src/dwarf-thumb.c:7:12
+
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4097
+  ImageBase:   4194304
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+  ExportTable:
+RelativeVirtualAddress: 0
+Size:0
+  ImportTable:
+RelativeVirtualAddress: 0
+Size:0
+  ResourceTable:
+RelativeVirtualAddress: 0
+Size:0
+  ExceptionTable:
+RelativeVirtualAddress: 0
+Size:0
+  CertificateTable:
+RelativeVirtualAddress: 0
+Size:0
+  BaseRelocationTable:
+RelativeVirtualAddress: 0
+Size:0
+  Debug:
+RelativeVirtualAddress: 8192
+Size:28
+  Architecture:
+RelativeVirtualAddress: 0
+Size:0
+  GlobalPtr:
+RelativeVirtualAddress: 0
+Size:0
+  TlsTable:
+RelativeVirtualAddress: 0
+Size:0
+  L

[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

2019-11-29 Thread Pavel Labath via Phabricator via lldb-commits
labath added reviewers: clayborg, jingham.
labath added a comment.

Yeah, this is going to be tricky... I don't really know what's the right way to 
do this, but here are the thoughts I have on this so far:

- The new DWARFDebugInfoEntry member is going to substantially increase our 
memory footprint -- that's a non-starter
- I don't like the inconsistency where all addresses are demangled in the DWARF 
code, except the line tables, which are in the "generic" code
- I think we ought to have some kind of a utility function to hold the logic 
for the `&~1` stuff. there is something like that in 
Architecture::GetOpcodeLoadAddress. The Architecture class is mostly 
independent of a target, so we may be able create one instance for each module 
or symbol file, but that feels quite heavy. I might even go for putting 
something like that into the ArchSpec class. The raison d'être of the 
Architecture class was to evict some heavy code out of ArchSpec -- this was 
ArchitectureArm::OverrideStopInfo. That one is definitely too heavy, so still 
don't thing it belongs in ArchSpec, but the `&~1` thingy seems like it could 
find a place there.)
- is this behavior reproducible with lld ? If so, I think it'd make the test 
more readable to run llvm-mc+lld as a part of the test
- the address byte size would best be handled separately


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70840/new/

https://reviews.llvm.org/D70840



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] a48b5e2 - [lldb][NFC] Fix header guard comment in ThreadSafeDenseMap.h

2019-11-29 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2019-11-29T11:34:18+01:00
New Revision: a48b5e24747ca83f9f18dff62a4bacb2f7dfd773

URL: 
https://github.com/llvm/llvm-project/commit/a48b5e24747ca83f9f18dff62a4bacb2f7dfd773
DIFF: 
https://github.com/llvm/llvm-project/commit/a48b5e24747ca83f9f18dff62a4bacb2f7dfd773.diff

LOG: [lldb][NFC] Fix header guard comment in ThreadSafeDenseMap.h

Added: 


Modified: 
lldb/include/lldb/Core/ThreadSafeDenseMap.h

Removed: 




diff  --git a/lldb/include/lldb/Core/ThreadSafeDenseMap.h 
b/lldb/include/lldb/Core/ThreadSafeDenseMap.h
index c485b91acb47..420cb5763586 100644
--- a/lldb/include/lldb/Core/ThreadSafeDenseMap.h
+++ b/lldb/include/lldb/Core/ThreadSafeDenseMap.h
@@ -62,4 +62,4 @@ class ThreadSafeDenseMap {
 
 } // namespace lldb_private
 
-#endif // liblldb_ThreadSafeSTLMap_h_
+#endif // liblldb_ThreadSafeDenseMap_h_



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 38870af - [lldb] Remove FileSpec->CompileUnit inheritance

2019-11-29 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2019-11-29T11:44:45+01:00
New Revision: 38870af8594726edf32aa0fd8fd9e8916df333af

URL: 
https://github.com/llvm/llvm-project/commit/38870af8594726edf32aa0fd8fd9e8916df333af
DIFF: 
https://github.com/llvm/llvm-project/commit/38870af8594726edf32aa0fd8fd9e8916df333af.diff

LOG: [lldb] Remove FileSpec->CompileUnit inheritance

Summary:
CompileUnit is a complicated class. Having it be implicitly convertible
to a FileSpec makes reasoning about it even harder.

This patch replaces the inheritance by a simple member and an accessor
function. This avoid the need for casting in places where one needed to
force a CompileUnit to be treated as a FileSpec, and does not add much
verbosity elsewhere.

It also fixes a bug where we were wrongly comparing CompileUnit& and a
CompileUnit*, which compiled due to a combination of this inheritance
and the FileSpec*->FileSpec implicit constructor.

Reviewers: teemperor, JDevlieghere, jdoerfert

Subscribers: lldb-commits

Tags: #lldb

Differential Revision: https://reviews.llvm.org/D70827

Added: 


Modified: 
lldb/include/lldb/Symbol/CompileUnit.h
lldb/source/API/SBCompileUnit.cpp
lldb/source/Breakpoint/Breakpoint.cpp
lldb/source/Breakpoint/BreakpointLocation.cpp
lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
lldb/source/Commands/CommandCompletions.cpp
lldb/source/Commands/CommandObjectSource.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Commands/CommandObjectThread.cpp
lldb/source/Core/FileLineResolver.cpp
lldb/source/Core/FormatEntity.cpp
lldb/source/Core/Module.cpp
lldb/source/Core/SearchFilter.cpp
lldb/source/Core/SourceManager.cpp
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
lldb/source/Symbol/CompileUnit.cpp
lldb/source/Symbol/Function.cpp
lldb/source/Symbol/SymbolContext.cpp
lldb/tools/lldb-test/lldb-test.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/CompileUnit.h 
b/lldb/include/lldb/Symbol/CompileUnit.h
index b5f37f678900..aec5cc7c8743 100644
--- a/lldb/include/lldb/Symbol/CompileUnit.h
+++ b/lldb/include/lldb/Symbol/CompileUnit.h
@@ -13,6 +13,7 @@
 #include "lldb/Core/ModuleChild.h"
 #include "lldb/Symbol/DebugMacros.h"
 #include "lldb/Symbol/Function.h"
+#include "lldb/Symbol/LineTable.h"
 #include "lldb/Symbol/SourceModule.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/UserID.h"
@@ -35,7 +36,6 @@ namespace lldb_private {
 /// table.
 class CompileUnit : public std::enable_shared_from_this,
 public ModuleChild,
-public FileSpec,
 public UserID,
 public SymbolContextScope {
 public:
@@ -116,9 +116,6 @@ class CompileUnit : public 
std::enable_shared_from_this,
   const FileSpec &file_spec, lldb::user_id_t uid,
   lldb::LanguageType language, lldb_private::LazyBool 
is_optimized);
 
-  /// Destructor
-  ~CompileUnit() override;
-
   /// Add a function to this compile unit.
   ///
   /// Typically called by the SymbolFile plug-ins as they partially parse the
@@ -225,6 +222,9 @@ class CompileUnit : public 
std::enable_shared_from_this,
  const FileSpec *file_spec_ptr, bool exact,
  LineEntry *line_entry);
 
+  /// Return the primary source file associated with this compile unit.
+  const FileSpec &GetPrimaryFile() const { return m_file_spec; }
+
   /// Get the line table for the compile unit.
   ///
   /// Called by clients and the SymbolFile plug-in. The SymbolFile plug-ins
@@ -415,6 +415,8 @@ class CompileUnit : public 
std::enable_shared_from_this,
   /// All modules, including the current module, imported by this
   /// compile unit.
   std::vector m_imported_modules;
+  /// The primary file associated with this compile unit.
+  FileSpec m_file_spec;
   /// Files associated with this compile unit's line table and
   /// declarations.
   FileSpecList m_support_files;

diff  --git a/lldb/source/API/SBCompileUnit.cpp 
b/lldb/source/API/SBCompileUnit.cpp
index 581bda363507..d52040d850a9 100644
--- a/lldb/source/API/SBCompileUnit.cpp
+++ b/lldb/source/API/SBCompileUnit.cpp
@@ -50,7 +50,7 @@ SBFileSpec SBCompileUnit::GetFileSpec() const {
 
   SBFileSpec file_spec;
   if (m_opaque_ptr)
-file_spec.SetFileSpec(*m_opaque_ptr);
+file_spec.SetFileSpec(m_opaque_ptr->GetPrimaryFile());
   return LLDB_RECORD_RESULT(file_spec);
 }
 
@@ -106,7 +106,7 @@ uint32_t SBCompileUnit::FindLineEntryIndex(uint32_t 
start_idx, uint32_t line,
 if (inline_file_spec && inline_file_spec->IsValid())
   file_spec = inline_file_spec->ref();
 else
-  file_spec = *m_opaque_ptr;
+  file_spec =

[Lldb-commits] [PATCH] D70827: [lldb] Remove FileSpec->CompileUnit inheritance

2019-11-29 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG38870af85947: [lldb] Remove FileSpec->CompileUnit 
inheritance (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D70827?vs=231438&id=231509#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70827/new/

https://reviews.llvm.org/D70827

Files:
  lldb/include/lldb/Symbol/CompileUnit.h
  lldb/source/API/SBCompileUnit.cpp
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Breakpoint/BreakpointLocation.cpp
  lldb/source/Breakpoint/BreakpointResolverFileRegex.cpp
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Core/FileLineResolver.cpp
  lldb/source/Core/FormatEntity.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/SearchFilter.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Symbol/Function.cpp
  lldb/source/Symbol/SymbolContext.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
@@ -549,7 +549,8 @@
 CompUnitSP CU;
 for (size_t Ind = 0; !CU && Ind < Module.GetNumCompileUnits(); ++Ind) {
   CompUnitSP Candidate = Module.GetCompileUnitAtIndex(Ind);
-  if (!Candidate || Candidate->GetFilename().GetStringRef() != File)
+  if (!Candidate ||
+  Candidate->GetPrimaryFile().GetFilename().GetStringRef() != File)
 continue;
   if (CU)
 return make_string_error("Multiple compile units for file `{0}` found.",
@@ -653,7 +654,8 @@
 if (!comp_unit)
   return make_string_error("Connot parse compile unit {0}.", i);
 
-outs() << "Processing '" << comp_unit->GetFilename().AsCString()
+outs() << "Processing '"
+   << comp_unit->GetPrimaryFile().GetFilename().AsCString()
<< "' compile unit.\n";
 
 LineTable *lt = comp_unit->GetLineTable();
Index: lldb/source/Symbol/SymbolContext.cpp
===
--- lldb/source/Symbol/SymbolContext.cpp
+++ lldb/source/Symbol/SymbolContext.cpp
@@ -316,7 +316,7 @@
   *s << "CompileUnit  = " << comp_unit;
   if (comp_unit != nullptr)
 s->Format(" {{{0:x-16}} {1}", comp_unit->GetID(),
-  *static_cast(comp_unit));
+  comp_unit->GetPrimaryFile());
   s->EOL();
   s->Indent();
   *s << "Function = " << function;
@@ -1055,7 +1055,8 @@
   // Next check the comp unit, but only if the SymbolContext was not
   // inlined.
   if (!was_inlined && sc.comp_unit != nullptr) {
-if (!FileSpec::Equal(*(sc.comp_unit), *(m_file_spec_up.get()), false))
+if (!FileSpec::Equal(sc.comp_unit->GetPrimaryFile(), *m_file_spec_up,
+ false))
   return false;
   }
 }
Index: lldb/source/Symbol/Function.cpp
===
--- lldb/source/Symbol/Function.cpp
+++ lldb/source/Symbol/Function.cpp
@@ -340,7 +340,8 @@
   "error: unable to find module "
   "shared pointer for function '%s' "
   "in %s\n",
-  GetName().GetCString(), m_comp_unit->GetPath().c_str());
+  GetName().GetCString(),
+  m_comp_unit->GetPrimaryFile().GetPath().c_str());
 }
 m_block.SetBlockInfoHasBeenParsed(true, true);
   }
Index: lldb/source/Symbol/CompileUnit.cpp
===
--- lldb/source/Symbol/CompileUnit.cpp
+++ lldb/source/Symbol/CompileUnit.cpp
@@ -21,30 +21,21 @@
  const char *pathname, const lldb::user_id_t cu_sym_id,
  lldb::LanguageType language,
  lldb_private::LazyBool is_optimized)
-: ModuleChild(module_sp), FileSpec(pathname), UserID(cu_sym_id),
-  m_user_data(user_data), m_language(language), m_flags(0),
-  m_support_files(), m_line_table_up(), m_variables(),
-  m_is_optimized(is_optimized) {
-  if (language != eLanguageTypeUnknown)
-m_flags.Set(flagsParsedLanguage);
-  assert(module_sp);
-}
+: CompileUnit(module_sp, user_data, FileSpec(pathname), cu_sym_id, language,
+  is_optimized) {}
 
 CompileUnit::CompileUnit(const lldb::ModuleSP &module_sp, void *user_data,
  const FileSpec &fspec, const lldb::user_id_t cu_sym_id,
  lldb::LanguageType

[Lldb-commits] [PATCH] D70845: [lldb][NFC] Remove ThreadSafeSTLVector and ThreadSafeSTLMap and their use in ValueObjectSynthetic

2019-11-29 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor created this revision.
teemperor added reviewers: labath, JDevlieghere, jingham.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

ThreadSafeSTLVector and ThreadSafeSTLMap are not useful for achieving any 
degree of thread safety in LLDB
and should be removed before they are used in more places. They are only used 
(unsurprisingly incorrectly) in
`ValueObjectSynthetic::GetChildAtIndex`, so this patch replaces their use there 
with a simple mutex with which
we guard the related data structures. This doesn't make 
ValueObjectSynthetic::GetChildAtIndex
any more thread-safe, but on the other hand it at least allows us to get rid of 
the ThreadSafeSTL* data structures
without changing the observable behaviour of ValueObjectSynthetic (beside that 
it is now a few bytes smaller).


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D70845

Files:
  lldb/include/lldb/Core/ThreadSafeSTLMap.h
  lldb/include/lldb/Core/ThreadSafeSTLVector.h
  lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
  lldb/source/Core/ValueObjectSyntheticFilter.cpp

Index: lldb/source/Core/ValueObjectSyntheticFilter.cpp
===
--- lldb/source/Core/ValueObjectSyntheticFilter.cpp
+++ lldb/source/Core/ValueObjectSyntheticFilter.cpp
@@ -48,8 +48,9 @@
 ValueObjectSynthetic::ValueObjectSynthetic(ValueObject &parent,
lldb::SyntheticChildrenSP filter)
 : ValueObject(parent), m_synth_sp(filter), m_children_byindex(),
-  m_name_toindex(), m_synthetic_children_count(UINT32_MAX),
-  m_synthetic_children_cache(), m_parent_type_name(parent.GetTypeName()),
+  m_name_toindex(), m_synthetic_children_cache(),
+  m_synthetic_children_count(UINT32_MAX),
+  m_parent_type_name(parent.GetTypeName()),
   m_might_have_children(eLazyBoolCalculate),
   m_provides_value(eLazyBoolCalculate) {
   SetName(parent.GetName());
@@ -177,14 +178,20 @@
   "filter said caches are stale - clearing",
   GetName().AsCString());
 // filter said that cached values are stale
-m_children_byindex.Clear();
-m_name_toindex.Clear();
+{
+  std::lock_guard guard(m_child_mutex);
+  m_children_byindex.clear();
+  m_name_toindex.clear();
+}
 // usually, an object's value can change but this does not alter its
 // children count for a synthetic VO that might indeed happen, so we need
 // to tell the upper echelons that they need to come back to us asking for
 // children
 m_children_count_valid = false;
-m_synthetic_children_cache.Clear();
+{
+  std::lock_guard guard(m_child_mutex);
+  m_synthetic_children_cache.clear();
+}
 m_synthetic_children_count = UINT32_MAX;
 m_might_have_children = eLazyBoolCalculate;
   } else {
@@ -232,7 +239,16 @@
   UpdateValueIfNeeded();
 
   ValueObject *valobj;
-  if (!m_children_byindex.GetValueForKey(idx, valobj)) {
+  bool child_is_cached;
+  {
+std::lock_guard guard(m_child_mutex);
+auto cached_child_it = m_children_byindex.find(idx);
+child_is_cached = cached_child_it != m_children_byindex.end();
+if (child_is_cached)
+  valobj = cached_child_it->second;
+  }
+
+  if (!child_is_cached) {
 if (can_create && m_synth_filter_up != nullptr) {
   LLDB_LOGF(log,
 "[ValueObjectSynthetic::GetChildAtIndex] name=%s, child at "
@@ -254,9 +270,12 @@
   if (!synth_guy)
 return synth_guy;
 
-  if (synth_guy->IsSyntheticChildrenGenerated())
-m_synthetic_children_cache.AppendObject(synth_guy);
-  m_children_byindex.SetValueForKey(idx, synth_guy.get());
+  {
+std::lock_guard guard(m_child_mutex);
+if (synth_guy->IsSyntheticChildrenGenerated())
+  m_synthetic_children_cache.push_back(synth_guy);
+m_children_byindex[idx] = synth_guy.get();
+  }
   synth_guy->SetPreferredDisplayLanguageIfNeeded(
   GetPreferredDisplayLanguage());
   return synth_guy;
@@ -297,13 +316,21 @@
   UpdateValueIfNeeded();
 
   uint32_t found_index = UINT32_MAX;
-  bool did_find = m_name_toindex.GetValueForKey(name.GetCString(), found_index);
+  bool did_find;
+  {
+std::lock_guard guard(m_child_mutex);
+auto name_to_index = m_name_toindex.find(name.GetCString());
+did_find = name_to_index != m_name_toindex.end();
+if (did_find)
+  found_index = name_to_index->second;
+  }
 
   if (!did_find && m_synth_filter_up != nullptr) {
 uint32_t index = m_synth_filter_up->GetIndexOfChildWithName(name);
 if (index == UINT32_MAX)
   return index;
-m_name_toindex.SetValueForKey(name.GetCString(), index);
+std::lock_guard guard(m_child_mutex);
+m_name_toindex[name.GetCString()] = index;
 return index;
   } else if (!did_find && m_synth_filter_up == nullptr)
 return UINT32_MAX;
Index: lldb/include/lldb/Core/ValueObjectSyntheticFilter.h

[Lldb-commits] [lldb] d1d6049 - [lldb][NFC] Remove dead logging code from DWARFASTParserClang::CompleteRecordType

2019-11-29 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2019-11-29T12:13:34+01:00
New Revision: d1d6049e9d6600f28746379290705b02ffb52d4b

URL: 
https://github.com/llvm/llvm-project/commit/d1d6049e9d6600f28746379290705b02ffb52d4b
DIFF: 
https://github.com/llvm/llvm-project/commit/d1d6049e9d6600f28746379290705b02ffb52d4b.diff

LOG: [lldb][NFC] Remove dead logging code from 
DWARFASTParserClang::CompleteRecordType

This code is behind a `if (log)` that is always a nullptr as the initializer
was commented out. One could uncomment the initializer code, but then this 
logging
code just leads to a deadlock as it tries to aquire the module lock.
This removes the logging code until I get this working again.

Added: 


Modified: 
lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

Removed: 




diff  --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
index ba17469ea998..43030c62cb40 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
@@ -1974,8 +1974,6 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE &die,
  CompilerType &clang_type) {
   const dw_tag_t tag = die.Tag();
   SymbolFileDWARF *dwarf = die.GetDWARF();
-  Log *log =
-  nullptr; // 
(LogChannelDWARF::GetLogIfAny(DWARF_LOG_DEBUG_INFO|DWARF_LOG_TYPE_COMPLETION));
 
   ClangASTImporter::LayoutInfo layout_info;
 
@@ -2125,75 +2123,8 @@ bool DWARFASTParserClang::CompleteRecordType(const 
DWARFDIE &die,
 
 clang::CXXRecordDecl *record_decl =
 m_ast.GetAsCXXRecordDecl(clang_type.GetOpaqueQualType());
-if (record_decl) {
-  if (log) {
-ModuleSP module_sp = dwarf->GetObjectFile()->GetModule();
-
-if (module_sp) {
-  module_sp->LogMessage(
-  log,
-  "ClangASTContext::CompleteTypeFromDWARF (clang_type = %p) "
-  "caching layout info for record_decl = %p, bit_size = %" PRIu64
-  ", alignment = %" PRIu64
-  ", field_offsets[%u], base_offsets[%u], vbase_offsets[%u])",
-  static_cast(clang_type.GetOpaqueQualType()),
-  static_cast(record_decl), layout_info.bit_size,
-  layout_info.alignment,
-  static_cast(layout_info.field_offsets.size()),
-  static_cast(layout_info.base_offsets.size()),
-  static_cast(layout_info.vbase_offsets.size()));
-
-  uint32_t idx;
-  {
-llvm::DenseMap::const_iterator
-pos,
-end = layout_info.field_offsets.end();
-for (idx = 0, pos = layout_info.field_offsets.begin(); pos != end;
- ++pos, ++idx) {
-  module_sp->LogMessage(
-  log,
-  "ClangASTContext::CompleteTypeFromDWARF (clang_type = "
-  "%p) field[%u] = { bit_offset=%u, name='%s' }",
-  static_cast(clang_type.GetOpaqueQualType()), idx,
-  static_cast(pos->second),
-  pos->first->getNameAsString().c_str());
-}
-  }
-
-  {
-llvm::DenseMap::const_iterator base_pos,
-base_end = layout_info.base_offsets.end();
-for (idx = 0, base_pos = layout_info.base_offsets.begin();
- base_pos != base_end; ++base_pos, ++idx) {
-  module_sp->LogMessage(
-  log,
-  "ClangASTContext::CompleteTypeFromDWARF (clang_type = "
-  "%p) base[%u] = { byte_offset=%u, name='%s' }",
-  clang_type.GetOpaqueQualType(), idx,
-  (uint32_t)base_pos->second.getQuantity(),
-  base_pos->first->getNameAsString().c_str());
-}
-  }
-  {
-llvm::DenseMap::const_iterator vbase_pos,
-vbase_end = layout_info.vbase_offsets.end();
-for (idx = 0, vbase_pos = layout_info.vbase_offsets.begin();
- vbase_pos != vbase_end; ++vbase_pos, ++idx) {
-  module_sp->LogMessage(
-  log,
-  "ClangASTContext::CompleteTypeFromDWARF (clang_type = "
-  "%p) vbase[%u] = { byte_offset=%u, name='%s' }",
-  static_cast(clang_type.GetOpaqueQualType()), idx,
-  static_cast(vbase_pos->second.getQuantity()),
-  vbase_pos->first->getNameAsString().c_str());
-}
-  }
-}
-  }
+if (record_decl)
   GetClangASTImporter().InsertRecordDecl(record_decl, layout_info);
-}
   }
 
   return (bool)clang_type;



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

2019-11-29 Thread Levon Ter-Grigoryan via Phabricator via lldb-commits
PatriosTheGreat created this revision.
PatriosTheGreat added reviewers: labath, jarin, aprantl.
PatriosTheGreat added a project: LLDB.
Herald added a subscriber: arphaman.

This change is connected with
https://reviews.llvm.org/D69843

In large codebases, we sometimes see Module::FindFunctions (when called from
ClangExpressionDeclMap::FindExternalVisibleDecls) returning huge amounts of
functions.

In current fix I trying to return only function_fullnames from 
ManualDWARFIndex::GetFunctions when eFunctionNameTypeFull is passed as argument.

I run check-lldb on this changes on linux machine and it seems like there are 
no additional failing tests compared with master. It seems a bit weird that 
there are no other places in code were it is expected that 
eFunctionNameTypeFull on GetFunctions will return fullnames + basenames + 
function_methods.
Is there any additional tests which I can run to be sure that I didn't break 
anything?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70846

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp


Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -401,8 +401,6 @@
 
   if (name_type_mask & eFunctionNameTypeFull) {
 DIEArray offsets;
-m_set.function_basenames.Find(name, offsets);
-m_set.function_methods.Find(name, offsets);
 m_set.function_fullnames.Find(name, offsets);
 for (const DIERef &die_ref: offsets) {
   DWARFDIE die = dwarf.GetDIE(die_ref);
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1210,7 +1210,7 @@
 // TODO Fix FindFunctions so that it doesn't return
 //   instance methods for eFunctionNameTypeBase.
 
-target->GetImages().FindFunctions(name, eFunctionNameTypeFull,
+target->GetImages().FindFunctions(name, eFunctionNameTypeFull | 
eFunctionNameTypeBase,
   include_symbols, include_inlines,
   sc_list);
   }


Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -401,8 +401,6 @@
 
   if (name_type_mask & eFunctionNameTypeFull) {
 DIEArray offsets;
-m_set.function_basenames.Find(name, offsets);
-m_set.function_methods.Find(name, offsets);
 m_set.function_fullnames.Find(name, offsets);
 for (const DIERef &die_ref: offsets) {
   DWARFDIE die = dwarf.GetDIE(die_ref);
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1210,7 +1210,7 @@
 // TODO Fix FindFunctions so that it doesn't return
 //   instance methods for eFunctionNameTypeBase.
 
-target->GetImages().FindFunctions(name, eFunctionNameTypeFull,
+target->GetImages().FindFunctions(name, eFunctionNameTypeFull | eFunctionNameTypeBase,
   include_symbols, include_inlines,
   sc_list);
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

2019-11-29 Thread Martin Storsjö via Phabricator via lldb-commits
mstorsjo added a comment.

In D70840#1763639 , @labath wrote:

> Yeah, this is going to be tricky... I don't really know what's the right way 
> to do this, but here are the thoughts I have on this so far:
>
> - The new DWARFDebugInfoEntry member is going to substantially increase our 
> memory footprint -- that's a non-starter


Ok, I feared that class was one where the size is critical yeah...

> - I don't like the inconsistency where all addresses are demangled in the 
> DWARF code, except the line tables, which are in the "generic" code

Yup. The reason for doing it like this was as I tried to find the most narrow 
interface where I could catch all of them.

> - I think we ought to have some kind of a utility function to hold the logic 
> for the `&~1` stuff. there is something like that in 
> Architecture::GetOpcodeLoadAddress. The Architecture class is mostly 
> independent of a target, so we may be able create one instance for each 
> module or symbol file, but that feels quite heavy. I might even go for 
> putting something like that into the ArchSpec class. The raison d'être of the 
> Architecture class was to evict some heavy code out of ArchSpec -- this was 
> ArchitectureArm::OverrideStopInfo. That one is definitely too heavy, so still 
> don't thing it belongs in ArchSpec, but the `&~1` thingy seems like it could 
> find a place there.)
> - is this behavior reproducible with lld ? If so, I think it'd make the test 
> more readable to run llvm-mc+lld as a part of the test

Yes it is. Ok, that's fine with me.

> - the address byte size would best be handled separately

On second thought, yes, this should be trivial to split out and make a similar 
test with inspecting line tables of an i386 binary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70840/new/

https://reviews.llvm.org/D70840



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D70847: [lldb-vscode] Ensure that target matches the executable file

2019-11-29 Thread Anton Kolesov via Phabricator via lldb-commits
anton.kolesov created this revision.
anton.kolesov added a project: LLDB.
Herald added a subscriber: lldb-commits.

Different architectures may use different target processes, so default
"gdb-remote" target process created by lldb-vscode may not work for all
targets. This commit changes behaviour of the request_launch function -
first it assigns an executable file to the debugger, which may create the
new SBTarget instance with a different target process implementation. In
contrast, if executable file is specified inside of the SBLaunchInfo
structure, then this structure is passed to an already existing target and
thus can't force recreation of the target to match a desired process
implementation.

New code has a behavior similar to LLDB MI [1], where typically IDE would
specify target file with -file-exec-and-symbols, and then only do -exec-run
command that would launch the process. In lldb-vscode those two actions are
merged into one request_launch function. Similarly in the interpreter
session, used would first do "file" command, then "process launch"


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70847

Files:
  lldb/tools/lldb-vscode/lldb-vscode.cpp


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1346,28 +1346,19 @@
   // Run any initialize LLDB commands the user specified in the launch.json
   g_vsc.RunInitCommands();
 
-  // Grab the current working directory if there is one and set it in the
-  // launch info.
-  const auto cwd = GetString(arguments, "cwd");
-  if (!cwd.empty())
-g_vsc.launch_info.SetWorkingDirectory(cwd.data());
-
-  // Grab the name of the program we need to debug and set it as the first
-  // argument that will be passed to the program we will debug.
+  // Grab the name of the program we need to debug and create a new target 
using
+  // the given program as an argument and replace previous dummy target with a
+  // new one. New target might have a different type then the previous one,
+  // based on the ABI or architecture of the file. Settings program path in
+  // LaunchInfo is useless, because launch info will recycle existing target,
+  // which might be incorrect for the selected file.
   llvm::StringRef program = GetString(arguments, "program");
   if (!program.empty()) {
-lldb::SBFileSpec program_fspec(program.data(), true /*resolve_path*/);
-g_vsc.launch_info.SetExecutableFile(program_fspec,
-true /*add_as_first_arg*/);
-const char *target_triple = nullptr;
-const char *uuid_cstr = nullptr;
-// Stand alone debug info file if different from executable
-const char *symfile = nullptr;
-lldb::SBModule module = g_vsc.target.AddModule(
-program.data(), target_triple, uuid_cstr, symfile);
-if (!module.IsValid()) {
-  response["success"] = llvm::json::Value(false);
+// Create a target that matches the file.
+g_vsc.target = g_vsc.debugger.CreateTarget(program.data());
 
+if (!g_vsc.target.IsValid()) {
+  response["success"] = llvm::json::Value(false);
   EmplaceSafeString(
   response, "message",
   llvm::formatv("Could not load program '{0}'.", program).str());
@@ -1376,6 +1367,15 @@
 }
   }
 
+  // Instantiate a launch info instance for the target.
+  g_vsc.launch_info = g_vsc.target.GetLaunchInfo();
+
+  // Grab the current working directory if there is one and set it in the
+  // launch info.
+  const auto cwd = GetString(arguments, "cwd");
+  if (!cwd.empty())
+g_vsc.launch_info.SetWorkingDirectory(cwd.data());
+
   // Extract any extra arguments and append them to our program arguments for
   // when we launch
   auto args = GetStrings(arguments, "args");


Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -1346,28 +1346,19 @@
   // Run any initialize LLDB commands the user specified in the launch.json
   g_vsc.RunInitCommands();
 
-  // Grab the current working directory if there is one and set it in the
-  // launch info.
-  const auto cwd = GetString(arguments, "cwd");
-  if (!cwd.empty())
-g_vsc.launch_info.SetWorkingDirectory(cwd.data());
-
-  // Grab the name of the program we need to debug and set it as the first
-  // argument that will be passed to the program we will debug.
+  // Grab the name of the program we need to debug and create a new target using
+  // the given program as an argument and replace previous dummy target with a
+  // new one. New target might have a different type then the previous one,
+  // based on the ABI or architecture of the file. Settings program path in
+  // LaunchInfo is useless, because launch info will recycle existing target,
+  // which might be incorrect for the selec

[Lldb-commits] [lldb] d752b75 - [lldb][NFC] Simplify regex_chars in CommandCompletions

2019-11-29 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2019-11-29T12:34:23+01:00
New Revision: d752b75d7fce2a77bb7656d33d2aa062372dc014

URL: 
https://github.com/llvm/llvm-project/commit/d752b75d7fce2a77bb7656d33d2aa062372dc014
DIFF: 
https://github.com/llvm/llvm-project/commit/d752b75d7fce2a77bb7656d33d2aa062372dc014.diff

LOG: [lldb][NFC] Simplify regex_chars in CommandCompletions

Added: 


Modified: 
lldb/source/Commands/CommandCompletions.cpp

Removed: 




diff  --git a/lldb/source/Commands/CommandCompletions.cpp 
b/lldb/source/Commands/CommandCompletions.cpp
index d325b724a38f..b382e26e2b70 100644
--- a/lldb/source/Commands/CommandCompletions.cpp
+++ b/lldb/source/Commands/CommandCompletions.cpp
@@ -413,10 +413,7 @@ void CommandCompletions::SourceFileCompleter::DoCompletion(
 // SymbolCompleter
 
 static bool regex_chars(const char comp) {
-  return (comp == '[' || comp == ']' || comp == '(' || comp == ')' ||
-  comp == '{' || comp == '}' || comp == '+' || comp == '.' ||
-  comp == '*' || comp == '|' || comp == '^' || comp == '$' ||
-  comp == '\\' || comp == '?');
+  return llvm::StringRef("[](){}+.*|^$\\?").contains(comp);
 }
 
 CommandCompletions::SymbolCompleter::SymbolCompleter(



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 656a812 - [lldb] Fix windows build for 38870af

2019-11-29 Thread Pavel Labath via lldb-commits

Author: Pavel Labath
Date: 2019-11-29T12:48:25+01:00
New Revision: 656a8123deed31d2d7aee313e87911dc153fa6d3

URL: 
https://github.com/llvm/llvm-project/commit/656a8123deed31d2d7aee313e87911dc153fa6d3
DIFF: 
https://github.com/llvm/llvm-project/commit/656a8123deed31d2d7aee313e87911dc153fa6d3.diff

LOG: [lldb] Fix windows build for 38870af

Added: 


Modified: 
lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Removed: 




diff  --git a/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp 
b/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
index 0470394d4255..e8a8690c1ff1 100644
--- a/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ b/lldb/unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -109,7 +109,7 @@ class SymbolFilePDBTests : public testing::Test {
const FileSpec &spec) const {
 for (size_t i = 0; i < sc_list.GetSize(); ++i) {
   const SymbolContext &sc = sc_list[i];
-  if (FileSpecMatchesAsBaseOrFull(*sc.comp_unit, spec))
+  if (FileSpecMatchesAsBaseOrFull(sc.comp_unit->GetPrimaryFile(), spec))
 return true;
 }
 return false;



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 76016f9 - [lldb][NFC] Early exit in ClangASTContext::CreateInstance

2019-11-29 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2019-11-29T12:49:33+01:00
New Revision: 76016f9b3a9acdba7728561a7ddfb48b1245dfa7

URL: 
https://github.com/llvm/llvm-project/commit/76016f9b3a9acdba7728561a7ddfb48b1245dfa7
DIFF: 
https://github.com/llvm/llvm-project/commit/76016f9b3a9acdba7728561a7ddfb48b1245dfa7.diff

LOG: [lldb][NFC] Early exit in ClangASTContext::CreateInstance

Added: 


Modified: 
lldb/source/Symbol/ClangASTContext.cpp

Removed: 




diff  --git a/lldb/source/Symbol/ClangASTContext.cpp 
b/lldb/source/Symbol/ClangASTContext.cpp
index e70b005550d1..adb8d57a74f6 100644
--- a/lldb/source/Symbol/ClangASTContext.cpp
+++ b/lldb/source/Symbol/ClangASTContext.cpp
@@ -562,47 +562,47 @@ uint32_t ClangASTContext::GetPluginVersion() { return 1; }
 lldb::TypeSystemSP ClangASTContext::CreateInstance(lldb::LanguageType language,
lldb_private::Module 
*module,
Target *target) {
-  if (ClangASTContextSupportsLanguage(language)) {
-ArchSpec arch;
-if (module)
-  arch = module->GetArchitecture();
-else if (target)
-  arch = target->GetArchitecture();
-
-if (arch.IsValid()) {
-  ArchSpec fixed_arch = arch;
-  // LLVM wants this to be set to iOS or MacOSX; if we're working on
-  // a bare-boards type image, change the triple for llvm's benefit.
-  if (fixed_arch.GetTriple().getVendor() == llvm::Triple::Apple &&
-  fixed_arch.GetTriple().getOS() == llvm::Triple::UnknownOS) {
-if (fixed_arch.GetTriple().getArch() == llvm::Triple::arm ||
-fixed_arch.GetTriple().getArch() == llvm::Triple::aarch64 ||
-fixed_arch.GetTriple().getArch() == llvm::Triple::aarch64_32 ||
-fixed_arch.GetTriple().getArch() == llvm::Triple::thumb) {
-  fixed_arch.GetTriple().setOS(llvm::Triple::IOS);
-} else {
-  fixed_arch.GetTriple().setOS(llvm::Triple::MacOSX);
-}
-  }
-
-  if (module) {
-std::shared_ptr ast_sp(
-new ClangASTContext(fixed_arch));
-return ast_sp;
-  } else if (target && target->IsValid()) {
-std::shared_ptr ast_sp(
-new ClangASTContextForExpressions(*target, fixed_arch));
-ast_sp->m_scratch_ast_source_up.reset(
-new ClangASTSource(target->shared_from_this()));
-lldbassert(ast_sp->getFileManager());
-ast_sp->m_scratch_ast_source_up->InstallASTContext(
-*ast_sp->getASTContext(), *ast_sp->getFileManager(), true);
-llvm::IntrusiveRefCntPtr proxy_ast_source(
-ast_sp->m_scratch_ast_source_up->CreateProxy());
-ast_sp->SetExternalSource(proxy_ast_source);
-return ast_sp;
-  }
-}
+  if (!ClangASTContextSupportsLanguage(language))
+return lldb::TypeSystemSP();
+  ArchSpec arch;
+  if (module)
+arch = module->GetArchitecture();
+  else if (target)
+arch = target->GetArchitecture();
+
+  if (!arch.IsValid())
+return lldb::TypeSystemSP();
+
+  ArchSpec fixed_arch = arch;
+  // LLVM wants this to be set to iOS or MacOSX; if we're working on
+  // a bare-boards type image, change the triple for llvm's benefit.
+  if (fixed_arch.GetTriple().getVendor() == llvm::Triple::Apple &&
+  fixed_arch.GetTriple().getOS() == llvm::Triple::UnknownOS) {
+if (fixed_arch.GetTriple().getArch() == llvm::Triple::arm ||
+fixed_arch.GetTriple().getArch() == llvm::Triple::aarch64 ||
+fixed_arch.GetTriple().getArch() == llvm::Triple::aarch64_32 ||
+fixed_arch.GetTriple().getArch() == llvm::Triple::thumb) {
+  fixed_arch.GetTriple().setOS(llvm::Triple::IOS);
+} else {
+  fixed_arch.GetTriple().setOS(llvm::Triple::MacOSX);
+}
+  }
+
+  if (module) {
+std::shared_ptr ast_sp(new ClangASTContext(fixed_arch));
+return ast_sp;
+  } else if (target && target->IsValid()) {
+std::shared_ptr ast_sp(
+new ClangASTContextForExpressions(*target, fixed_arch));
+ast_sp->m_scratch_ast_source_up.reset(
+new ClangASTSource(target->shared_from_this()));
+lldbassert(ast_sp->getFileManager());
+ast_sp->m_scratch_ast_source_up->InstallASTContext(
+*ast_sp->getASTContext(), *ast_sp->getFileManager(), true);
+llvm::IntrusiveRefCntPtr proxy_ast_source(
+ast_sp->m_scratch_ast_source_up->CreateProxy());
+ast_sp->SetExternalSource(proxy_ast_source);
+return ast_sp;
   }
   return lldb::TypeSystemSP();
 }



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D70848: [LLDB] Set the right address size on output DataExtractors from ObjectFile

2019-11-29 Thread Martin Storsjö via Phabricator via lldb-commits
mstorsjo created this revision.
mstorsjo added reviewers: labath, amccarth.
Herald added a subscriber: aprantl.
Herald added a project: LLDB.

If filling in a DataExtractor from an ObjectFile, e.g. via the ReadSectionData 
method, the output DataExtractor gets the address size from the m_data member.

ObjectFile's m_data member is initialized without knowledge about the address 
size (so the address size is set based on the host's sizeof(void*), and at that 
point within ObjectFile's constructor, virtual methods implemented in 
subclasses (like GetAddressByteSize()) can't be called, therefore fix it up 
when filling in external DataExtractors.

This makes sure that line tables from executables with a different address size 
are parsed properly; previously this tripped up 
DWARFDebugLine::LineTable::parse for 32 bit executables on a 64 bit host, as 
the address size in the line table (4) didn't match the one set in the 
DWARFDataExtractor.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70848

Files:
  lldb/source/Symbol/ObjectFile.cpp
  lldb/test/Shell/SymbolFile/DWARF/win-i386-line-table.s


Index: lldb/test/Shell/SymbolFile/DWARF/win-i386-line-table.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/win-i386-line-table.s
@@ -0,0 +1,55 @@
+# Test that lldb can read a line table for an architecture with a different
+# address size than the one that of the host.
+
+# REQUIRES: lld, x86
+
+# RUN: llvm-mc -triple i686-windows-gnu %s -filetype=obj > %t.o
+# RUN: lld-link %t.o -out:%t.exe -debug:dwarf -entry:entry -subsystem:console 
-lldmingw
+# RUN: %lldb %t.exe -o "image dump line-table -v win-i386-line-table.c" -b | 
FileCheck %s
+
+# CHECK: Line table for win-i386-line-table.c in `win-i386-line-table.s.tmp.exe
+# CHECK: 0x00401000: /path/to/src/win-i386-line-table.c:2:1
+# CHECK: 0x00401001: /path/to/src/win-i386-line-table.c:2:1
+
+.text
+.file   "win-i386-line-table.c"
+.globl  _entry  # -- Begin function entry
+_entry: # @entry
+.file   1 "/path/to/src" "win-i386-line-table.c"
+.loc1 1 0   # win-i386-line-table.c:1:0
+.cfi_sections .debug_frame
+.cfi_startproc
+.loc1 2 1 prologue_end  # win-i386-line-table.c:2:1
+retl
+.cfi_endproc
+# -- End function
+.section.debug_str,"dr"
+Linfo_string1:
+.asciz  "win-i386-line-table.c"
+.section.debug_abbrev,"dr"
+Lsection_abbrev:
+.byte   1   # Abbreviation Code
+.byte   17  # DW_TAG_compile_unit
+.byte   1   # DW_CHILDREN_yes
+.byte   3   # DW_AT_name
+.byte   14  # DW_FORM_strp
+.byte   16  # DW_AT_stmt_list
+.byte   23  # DW_FORM_sec_offset
+.byte   0   # EOM(1)
+.byte   0   # EOM(2)
+.byte   0   # EOM(3)
+.section.debug_info,"dr"
+Lsection_info:
+Lcu_begin0:
+.long   Ldebug_info_end0-Ldebug_info_start0 # Length of Unit
+Ldebug_info_start0:
+.short  4   # DWARF version number
+.secrel32   Lsection_abbrev # Offset Into Abbrev. Section
+.byte   4   # Address Size (in bytes)
+.byte   1   # Abbrev [1] 0xb:0x2d 
DW_TAG_compile_unit
+.secrel32   Linfo_string1   # DW_AT_name
+.secrel32   Lline_table_start0 # DW_AT_stmt_list
+.byte   0   # End Of Children Mark
+Ldebug_info_end0:
+.section.debug_line,"dr"
+Lline_table_start0:
Index: lldb/source/Symbol/ObjectFile.cpp
===
--- lldb/source/Symbol/ObjectFile.cpp
+++ lldb/source/Symbol/ObjectFile.cpp
@@ -477,7 +477,13 @@
DataExtractor &data) const {
   // The entire file has already been mmap'ed into m_data, so just copy from
   // there as the back mmap buffer will be shared with shared pointers.
-  return data.SetData(m_data, offset, length);
+  size_t ret = data.SetData(m_data, offset, length);
+  // DataExtractor::SetData copies the address byte size from m_data, but
+  // m_data's address byte size is only set from sizeof(void*), and we can't
+  // access subclasses GetAddressByteSize() when setting up m_data in the
+  // constructor.
+  data.SetAddressByteSize(GetAddressByteSize());
+  return ret;
 }
 
 size_t ObjectFile::CopyData(lldb::offset_t offset, size_t length,


Index: lldb/test/Shell/SymbolFile/DWARF/win-i386-line-table.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/win-i

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

2019-11-29 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

There's `lldb-shell :: SymbolFile/DWARF/find-basic-function.cpp`, which 
probably didn't get run for you as you didn't have lld enabled (cmake 
-DLLVM_ENABLE_PROJECTS=...;lld). You'll need to update that test to match the 
new behavior, but other than that, I think this is a good change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70846/new/

https://reviews.llvm.org/D70846



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

2019-11-29 Thread Martin Storsjö via Phabricator via lldb-commits
mstorsjo updated this revision to Diff 231517.
mstorsjo edited the summary of this revision.
mstorsjo added a comment.

Converted the test to .s form, using lld, split out the ObjectFile changes to 
D70848  (which this change now depends on for 
the tests).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70840/new/

https://reviews.llvm.org/D70840

Files:
  lldb/include/lldb/Symbol/LineTable.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Symbol/LineTable.cpp
  lldb/test/Shell/SymbolFile/DWARF/thumb-windows.s

Index: lldb/test/Shell/SymbolFile/DWARF/thumb-windows.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/thumb-windows.s
@@ -0,0 +1,183 @@
+# Test that a linked windows executable, with a thumb bit in many address
+# fields, gets the thumb bit stripped out from addresses.
+
+# If the thumb bit isn't stripped out from subprogram ranges, 0x401006 is
+# associated with the function "entry", while it actually is the start of
+# the function "other".
+# If the thumb bit isn't stripped out from line tables, the LineEntry
+# points to the wrong line.
+
+# REQUIRES: lld, arm
+
+# RUN: llvm-mc -triple armv7-windows-gnu %s -filetype=obj > %t.o
+# RUN: lld-link %t.o -out:%t.exe -debug:dwarf -entry:entry -subsystem:console -lldmingw
+# RUN: %lldb %t.exe -o "image lookup -v -a 0x401006" -b | FileCheck %s
+
+# CHECK-LABEL: image lookup -v -a 0x401006
+# CHECK: Function: {{.*}}, name = "other", range = [0x00401006-0x0040100c)
+# CHECK: LineEntry: [0x00401006-0x0040100a): /path/to/src/dwarf-thumb.c:7:12
+
+.text
+.syntax unified
+.file   "dwarf-thumb.c"
+.file   1 "" "dwarf-thumb.c"
+.def entry;
+.scl2;
+.type   32;
+.endef
+.globl  entry   @ -- Begin function entry
+.p2align1
+.code   16  @ @entry
+.thumb_func
+entry:
+.Lfunc_begin0:
+.loc1 2 0   @ dwarf-thumb.c:2:0
+.cfi_sections .debug_frame
+.cfi_startproc
+.loc1 4 9 prologue_end  @ dwarf-thumb.c:4:9
+mov r1, r0
+.Ltmp0:
+b   other
+.Ltmp1:
+.Lfunc_end0:
+.cfi_endproc
+@ -- End function
+.def other;
+.scl2;
+.type   32;
+.endef
+.globl  other   @ -- Begin function other
+.p2align1
+.code   16  @ @other
+.thumb_func
+other:
+.Lfunc_begin1:
+.loc1 6 0   @ dwarf-thumb.c:6:0
+.cfi_startproc
+.loc1 7 12 prologue_end @ dwarf-thumb.c:7:12
+add.w   r0, r1, r1, lsl #1
+.loc1 7 2 is_stmt 0 @ dwarf-thumb.c:7:2
+bx  lr
+.Ltmp2:
+.Lfunc_end1:
+.cfi_endproc
+@ -- End function
+.section.debug_str,"dr"
+.Linfo_string:
+.Linfo_string1:
+.asciz  "dwarf-thumb.c"
+.Linfo_string2:
+.asciz  "/path/to/src"
+.Linfo_string3:
+.asciz  "other"
+.Linfo_string6:
+.asciz  "entry"
+.section.debug_loc,"dr"
+.Lsection_debug_loc:
+.Ldebug_loc0:
+.long   .Lfunc_begin0-.Lfunc_begin0
+.long   .Ltmp0-.Lfunc_begin0
+.short  1   @ Loc expr size
+.byte   80  @ DW_OP_reg0
+.long   .Ltmp0-.Lfunc_begin0
+.long   .Lfunc_end0-.Lfunc_begin0
+.short  1   @ Loc expr size
+.byte   81  @ DW_OP_reg1
+.long   0
+.long   0
+.section.debug_abbrev,"dr"
+.Lsection_abbrev:
+.byte   1   @ Abbreviation Code
+.byte   17  @ DW_TAG_compile_unit
+.byte   1   @ DW_CHILDREN_yes
+.byte   37  @ DW_AT_producer
+.byte   37  @ DW_FORM_strx1
+.byte   19  @ DW_AT_language
+.byte   5   @ DW_FORM_data2
+.byte   3   @ DW_AT_name
+.byte   14  @ DW_FORM_strp
+.byte   16  @ DW_AT_stmt_list
+.byte   23  @ DW_FORM_sec_offset
+.byte   27  @ DW_AT_comp_dir
+.byte   14  @ DW_FORM_strp
+ 

[Lldb-commits] [lldb] bc7f1df - [lldb][NFC] Explicitly ask for a ClangASTContext in ClangASTSource

2019-11-29 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2019-11-29T13:28:55+01:00
New Revision: bc7f1df6b61a3c8f88f2541ef9ba73f4ee0ee4fe

URL: 
https://github.com/llvm/llvm-project/commit/bc7f1df6b61a3c8f88f2541ef9ba73f4ee0ee4fe
DIFF: 
https://github.com/llvm/llvm-project/commit/bc7f1df6b61a3c8f88f2541ef9ba73f4ee0ee4fe.diff

LOG: [lldb][NFC] Explicitly ask for a ClangASTContext in ClangASTSource

ClangASTSource currently takes a clang::ASTContext and keeps that
around, but a lot of LLDB's functionality for doing operations
on a clang::ASTContext is in its ClangASTContext twin class. We
currently constantly recompute the respective ClangASTContext
from the clang::ASTContext while we instead could just pass and
store a ClangASTContext in the ClangASTSource. This also allows
us to get rid of a bunch of unreachable error checking for cases
where recomputation fails for some reason.

Added: 


Modified: 
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
lldb/source/Symbol/ClangASTContext.cpp

Removed: 




diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
index 2b484db3a188..51540902e2dc 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -57,10 +57,11 @@ ClangASTSource::ClangASTSource(const lldb::TargetSP &target)
   }
 }
 
-void ClangASTSource::InstallASTContext(clang::ASTContext &ast_context,
+void ClangASTSource::InstallASTContext(ClangASTContext &clang_ast_context,
clang::FileManager &file_manager,
bool is_shared_context) {
-  m_ast_context = &ast_context;
+  m_ast_context = clang_ast_context.getASTContext();
+  m_clang_ast_context = &clang_ast_context;
   m_file_manager = &file_manager;
   if (m_target->GetUseModernTypeLookup()) {
 // Configure the ExternalASTMerger.  The merger needs to be able to import
@@ -69,7 +70,7 @@ void ClangASTSource::InstallASTContext(clang::ASTContext 
&ast_context,
 // AST contexts.
 
 lldbassert(!m_merger_up);
-clang::ExternalASTMerger::ImporterTarget target = {ast_context,
+clang::ExternalASTMerger::ImporterTarget target = {*m_ast_context,
file_manager};
 std::vector sources;
 for (lldb::ModuleSP module_sp : m_target->GetImages().Modules()) {
@@ -132,7 +133,7 @@ void ClangASTSource::InstallASTContext(clang::ASTContext 
&ast_context,
 m_merger_up =
 std::make_unique(target, sources);
   } else {
-m_ast_importer_sp->InstallMapCompleter(&ast_context, *this);
+m_ast_importer_sp->InstallMapCompleter(m_ast_context, *this);
   }
 }
 
@@ -775,7 +776,7 @@ void 
ClangASTSource::FindExternalVisibleDecls(NameSearchContext &context) {
 }
 
 clang::Sema *ClangASTSource::getSema() {
-  return ClangASTContext::GetASTContext(m_ast_context)->getSema();
+  return m_clang_ast_context->getSema();
 }
 
 bool ClangASTSource::IgnoreName(const ConstString name,
@@ -2058,8 +2059,7 @@ CompilerType ClangASTSource::GuardedCopyType(const 
CompilerType &src_type) {
 // seems to be generating bad types on occasion.
 return CompilerType();
 
-  return CompilerType(ClangASTContext::GetASTContext(m_ast_context),
-  copied_qual_type.getAsOpaquePtr());
+  return CompilerType(m_clang_ast_context, copied_qual_type.getAsOpaquePtr());
 }
 
 clang::NamedDecl *NameSearchContext::AddVarDecl(const CompilerType &type) {
@@ -2186,10 +2186,9 @@ clang::NamedDecl *NameSearchContext::AddGenericFunDecl() 
{
   ArrayRef(), // argument types
   proto_info));
 
-  return AddFunDecl(
-  CompilerType(ClangASTContext::GetASTContext(m_ast_source.m_ast_context),
-   generic_function_type.getAsOpaquePtr()),
-  true);
+  return AddFunDecl(CompilerType(m_ast_source.m_clang_ast_context,
+ generic_function_type.getAsOpaquePtr()),
+true);
 }
 
 clang::NamedDecl *

diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
index d8e784f49b10..194233e4a028 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h
@@ -57,7 +57,7 @@ class ClangASTSource : public ClangExternalASTSourceCommon,
   }
   void MaterializeVisibleDecls(const clang::DeclContext *DC) { return; }
 
-  void InstallASTContext(clang::ASTContext &ast_context,
+  void InstallASTContext(ClangASTContext &ast_context,
 

[Lldb-commits] [PATCH] D70840: [LLDB] [DWARF] Strip out the thumb bit from addresses on ARM

2019-11-29 Thread Martin Storsjö via Phabricator via lldb-commits
mstorsjo added a comment.

In D70840#1763705 , @mstorsjo wrote:

> In D70840#1763639 , @labath wrote:
>
> > Yeah, this is going to be tricky... I don't really know what's the right 
> > way to do this, but here are the thoughts I have on this so far:
> >
> > - The new DWARFDebugInfoEntry member is going to substantially increase our 
> > memory footprint -- that's a non-starter
>
>
> Ok, I feared that class was one where the size is critical yeah...
>
> > - I don't like the inconsistency where all addresses are demangled in the 
> > DWARF code, except the line tables, which are in the "generic" code
>
> Yup. The reason for doing it like this was as I tried to find the most narrow 
> interface where I could catch all of them.


Is there any corresponding place in generic code where one could do the same 
operation on the addresses that comes from pc_lo/pc_hi ranges from .debug_info 
and .debug_aranges (not familiar with this section and when it's generated 
etc), if that would end up touching fewer places? The existing predecent in 
DWARFCallFrameInfo::GetFDEIndex is dwarf specific, but in generic code outside 
of the dwarf symbolfile plugin.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70840/new/

https://reviews.llvm.org/D70840



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D69371: [ARM64] Cleanup and speedup NativeRegisterContextLinux_arm64

2019-11-29 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

Thanks. I now agree with this change in principle, but I don't think its ready 
for an LGTM yet.

The main thing I'd like to discuss is this `InvalidateAllRegisters` function. I 
think we should come up with a story of when is this function expected to be 
called, and document it somewhere. Right now, the placement of call sites seems 
pretty random. For instance, it's not clear to me why one would need to call it 
in `NativeProcessLinux::SetupSoftwareSingleStepping`.

The reason we want to call this function is because the values that the kernel 
holds for this thread have (potentially) changed, and so we want to ensure we 
don't hold stale values. The way I see it, the values can change when:
a) we do a "register write" operation. It looks like you have this part 
covered, and the invalidation happens pretty close to the actual "write" syscall
b) when the thread gets a chance to run. I think this what the other 
`InvalidateAllRegisters` are trying to cover, but it's hard to tell because 
they are very far from the place where the actual thread gets resumed. In 
particular, the call in `NativeProcessLinux::Resume` seems troublesome, as it 
will call `NativeThreadLinux::Resume` *after* it "invalidates" the caches. 
However, `NativeProcessLinux::Resume` is responsible for setting the 
watchpoints, and this involves register read/writes, which may end up 
re-populating some caches. I guess that doesn't matter on arm because of how 
watchpoints are set, but I think it would make it easier to reason about these 
things if the invalidation happened right before we resume the thread, or 
immediately after it stops.




Comment at: lldb/include/lldb/Host/common/NativeRegisterContext.h:112-113
 
+  virtual void InvalidateAllRegisters(){};
+
   // Subclasses should not override these

Maybe this could be a method on `NativeRegisterContextLinux`. We can always 
later lift it to the main class if we find a reason to do that...



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:255-258
+error = WriteGPR();
+if (error.Fail())
+  return error;
+  } else if (IsFPR(reg)) {

There's nothing happening after this if block, so you could write this so that 
it always returns (`return WriteGPR()`), and then drop the "else" from the next 
condition.



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:305-306
 
+  InvalidateAllRegisters();
+
   if (!data_sp) {

I guess this isn't needed, since Write[FG]PR already handle invalidation on 
their own?



Comment at: 
lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp:874
+
+  if (!m_fpu_is_valid) {
+struct iovec ioVec;

Please change this to an early return (as well as the ReadGPR above).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69371/new/

https://reviews.llvm.org/D69371



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] c214c92 - [lldb][NFC] Remove ClangASTContext::GetBuiltinTypeForEncodingAndBitSize overload

2019-11-29 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2019-11-29T13:57:02+01:00
New Revision: c214c92f3be7c15abc458f23c7be05a5790e6aed

URL: 
https://github.com/llvm/llvm-project/commit/c214c92f3be7c15abc458f23c7be05a5790e6aed
DIFF: 
https://github.com/llvm/llvm-project/commit/c214c92f3be7c15abc458f23c7be05a5790e6aed.diff

LOG: [lldb][NFC] Remove ClangASTContext::GetBuiltinTypeForEncodingAndBitSize 
overload

Added: 


Modified: 
lldb/include/lldb/Symbol/ClangASTContext.h
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
lldb/source/Symbol/ClangASTContext.cpp
lldb/unittests/Symbol/TestClangASTContext.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ClangASTContext.h 
b/lldb/include/lldb/Symbol/ClangASTContext.h
index 7018f3b71b4f..a55307ef632d 100644
--- a/lldb/include/lldb/Symbol/ClangASTContext.h
+++ b/lldb/include/lldb/Symbol/ClangASTContext.h
@@ -150,9 +150,6 @@ class ClangASTContext : public TypeSystem {
   CompilerType GetBuiltinTypeForEncodingAndBitSize(lldb::Encoding encoding,
size_t bit_size) override;
 
-  static CompilerType GetBuiltinTypeForEncodingAndBitSize(
-  clang::ASTContext *ast, lldb::Encoding encoding, uint32_t bit_size);
-
   CompilerType GetBasicType(lldb::BasicType type);
 
   CompilerType GetBasicType(ConstString name);

diff  --git 
a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
index b33547529deb..22966e8023d6 100644
--- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -1762,8 +1762,8 @@ void 
ClangExpressionDeclMap::AddOneRegister(NameSearchContext &context,
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
 
   CompilerType clang_type =
-  ClangASTContext::GetBuiltinTypeForEncodingAndBitSize(
-  m_ast_context, reg_info->encoding, reg_info->byte_size * 8);
+  m_clang_ast_context->GetBuiltinTypeForEncodingAndBitSize(
+  reg_info->encoding, reg_info->byte_size * 8);
 
   if (!clang_type) {
 LLDB_LOGF(log, "  Tried to add a type for %s, but couldn't get one",

diff  --git a/lldb/source/Symbol/ClangASTContext.cpp 
b/lldb/source/Symbol/ClangASTContext.cpp
index 9988f0615651..8428dfe8c4fa 100644
--- a/lldb/source/Symbol/ClangASTContext.cpp
+++ b/lldb/source/Symbol/ClangASTContext.cpp
@@ -843,77 +843,62 @@ static inline bool QualTypeMatchesBitSize(const uint64_t 
bit_size,
 CompilerType
 ClangASTContext::GetBuiltinTypeForEncodingAndBitSize(Encoding encoding,
  size_t bit_size) {
-  return ClangASTContext::GetBuiltinTypeForEncodingAndBitSize(
-  getASTContext(), encoding, bit_size);
-}
-
-CompilerType ClangASTContext::GetBuiltinTypeForEncodingAndBitSize(
-ASTContext *ast, Encoding encoding, uint32_t bit_size) {
-  auto *clang_ast_context = ClangASTContext::GetASTContext(ast);
+  ASTContext *ast = this->getASTContext();
   if (!ast)
 return CompilerType();
   switch (encoding) {
   case eEncodingInvalid:
 if (QualTypeMatchesBitSize(bit_size, ast, ast->VoidPtrTy))
-  return CompilerType(clang_ast_context, ast->VoidPtrTy.getAsOpaquePtr());
+  return CompilerType(this, ast->VoidPtrTy.getAsOpaquePtr());
 break;
 
   case eEncodingUint:
 if (QualTypeMatchesBitSize(bit_size, ast, ast->UnsignedCharTy))
-  return CompilerType(clang_ast_context,
-  ast->UnsignedCharTy.getAsOpaquePtr());
+  return CompilerType(this, ast->UnsignedCharTy.getAsOpaquePtr());
 if (QualTypeMatchesBitSize(bit_size, ast, ast->UnsignedShortTy))
-  return CompilerType(clang_ast_context,
-  ast->UnsignedShortTy.getAsOpaquePtr());
+  return CompilerType(this, ast->UnsignedShortTy.getAsOpaquePtr());
 if (QualTypeMatchesBitSize(bit_size, ast, ast->UnsignedIntTy))
-  return CompilerType(clang_ast_context,
-  ast->UnsignedIntTy.getAsOpaquePtr());
+  return CompilerType(this, ast->UnsignedIntTy.getAsOpaquePtr());
 if (QualTypeMatchesBitSize(bit_size, ast, ast->UnsignedLongTy))
-  return CompilerType(clang_ast_context,
-  ast->UnsignedLongTy.getAsOpaquePtr());
+  return CompilerType(this, ast->UnsignedLongTy.getAsOpaquePtr());
 if (QualTypeMatchesBitSize(bit_size, ast, ast->UnsignedLongLongTy))
-  return CompilerType(clang_ast_context,
-  ast->UnsignedLongLongTy.getAsOpaquePtr());
+  return CompilerType(this, ast->UnsignedLongLongTy.getAsOpaquePtr());
 if (QualTypeMatchesBitSize(bit_size, ast, ast->UnsignedInt128Ty))
-  return CompilerType(clang_ast_context,
-  ast->UnsignedInt128Ty.getAsOpaquePtr());
+  return CompilerType(thi

[Lldb-commits] [PATCH] D70845: [lldb][NFC] Remove ThreadSafeSTLVector and ThreadSafeSTLMap and their use in ValueObjectSynthetic

2019-11-29 Thread Pavel Labath via Phabricator via lldb-commits
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

I'm yet to see a non-trivial algorithm, which can be made thread-safe by 
employing "thread safe" containers. +1 for deleting these and sorting the 
ValueObjectSynthetic issues later.


Repository:
  rLLDB LLDB

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70845/new/

https://reviews.llvm.org/D70845



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] 8059188 - [lldb][NFC] Remove unused ClangASTContext::GetBasicType(ConstString)

2019-11-29 Thread Raphael Isemann via lldb-commits

Author: Raphael Isemann
Date: 2019-11-29T14:11:25+01:00
New Revision: 8059188c45f049b52b779d6684ea78b6ef8b168c

URL: 
https://github.com/llvm/llvm-project/commit/8059188c45f049b52b779d6684ea78b6ef8b168c
DIFF: 
https://github.com/llvm/llvm-project/commit/8059188c45f049b52b779d6684ea78b6ef8b168c.diff

LOG: [lldb][NFC] Remove unused ClangASTContext::GetBasicType(ConstString)

Added: 


Modified: 
lldb/include/lldb/Symbol/ClangASTContext.h
lldb/source/Symbol/ClangASTContext.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/ClangASTContext.h 
b/lldb/include/lldb/Symbol/ClangASTContext.h
index a55307ef632d..b2c284282f11 100644
--- a/lldb/include/lldb/Symbol/ClangASTContext.h
+++ b/lldb/include/lldb/Symbol/ClangASTContext.h
@@ -152,8 +152,6 @@ class ClangASTContext : public TypeSystem {
 
   CompilerType GetBasicType(lldb::BasicType type);
 
-  CompilerType GetBasicType(ConstString name);
-
   static lldb::BasicType GetBasicTypeEnumeration(ConstString name);
 
   CompilerType GetBuiltinTypeForDWARFEncodingAndBitSize(const char *type_name,

diff  --git a/lldb/source/Symbol/ClangASTContext.cpp 
b/lldb/source/Symbol/ClangASTContext.cpp
index 8428dfe8c4fa..e683a0a9f4be 100644
--- a/lldb/source/Symbol/ClangASTContext.cpp
+++ b/lldb/source/Symbol/ClangASTContext.cpp
@@ -971,11 +971,6 @@ ClangASTContext::GetBasicTypeEnumeration(ConstString name) 
{
   return eBasicTypeInvalid;
 }
 
-CompilerType ClangASTContext::GetBasicType(ConstString name) {
-  lldb::BasicType basic_type = ClangASTContext::GetBasicTypeEnumeration(name);
-  return GetBasicType(basic_type);
-}
-
 uint32_t ClangASTContext::GetPointerByteSize() {
   if (m_pointer_byte_size == 0)
 if (auto size = GetBasicType(lldb::eBasicTypeVoid)



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D70847: [lldb-vscode] Ensure that target matches the executable file

2019-11-29 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

This needs a test case, though judging by your description it looks like pretty 
much every test case on affected platforms (I guess, windows) should hit this. 
Can you check if any of the existing vscode tests which are marked 
@skipIfWindows start passing now?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70847/new/

https://reviews.llvm.org/D70847



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D70851: [lldb] s/FileSpec::Equal/FileSpec::Match

2019-11-29 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.
labath added reviewers: teemperor, JDevlieghere.
Herald added a subscriber: emaste.
Herald added a reviewer: jdoerfert.
Herald added a project: LLDB.

The FileSpec class is often used as a sort of a pattern -- one specifies
a bare file name to search, and we check if in matches the full file
name of an existing module (for example).

These comparisons used FileSpec::Equal, which had some support for it
(via the full=false argument), but it was not a good fit for this job.

For one, it did a symmetric comparison, which makes sense for a function
called "equal", but not for typical searches (when searching for
"/foo/bar.so", we don't want to find a module whose name is just
"bar.so"). This resulted in patterns like:

  if (FileSpec::Equal(pattern, file, pattern.GetDirectory()))

which would request a "full" match only if the pattern really contained
a directory. This worked, but the intended behavior was very unobvious.

On top of that, a lot of the code wanted to handle the case of an
"empty" pattern, and treat it as matching everything. This resulted in
conditions like:

  if (pattern && !FileSpec::Equal(pattern, file, pattern.GetDirectory())

which are nearly impossible to decipher.

This patch introduces a FileSpec::Match function, which does exactly
what most of FileSpec::Equal callers want, an asymmetric match between a
"pattern" FileSpec and a an actual FileSpec. Empty paterns match
everything, filename-only patterns match only the filename component.

I've tried to update all callers of FileSpec::Equal to use a simpler
interface. Those that hardcoded full=true have been changed to use
operator==. Those passing full=pattern.GetDirectory() have been changed
to use FileSpec::Match.

There was also a handful of places which hardcoded full=false. I've
changed these to use FileSpec::Match too. This is a slight change in
semantics, but it does not look like that was ever intended, and it was
more likely a result of a misunderstanding of the "proper" way to use
FileSpec::Equal.

[In an ideal world a "FileSpec" and a "FileSpec pattern" would be two
different types, but given how widespread FileSpec is, it is unlikely
we'll get there in one go. This at least provides a good starting point
by centralizing all matching behavior.]


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70851

Files:
  lldb/include/lldb/Core/ModuleSpec.h
  lldb/include/lldb/Core/SourceManager.h
  lldb/include/lldb/Utility/FileSpec.h
  lldb/source/Breakpoint/Breakpoint.cpp
  lldb/source/Commands/CommandObjectSource.cpp
  lldb/source/Core/IOHandler.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/SearchFilter.cpp
  lldb/source/Core/SourceManager.cpp
  lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Symbol/Declaration.cpp
  lldb/source/Symbol/SymbolContext.cpp
  lldb/source/Target/TargetList.cpp
  lldb/source/Target/ThreadPlanStepInRange.cpp
  lldb/source/Utility/FileSpec.cpp
  lldb/unittests/Utility/FileSpecTest.cpp

Index: lldb/unittests/Utility/FileSpecTest.cpp
===
--- lldb/unittests/Utility/FileSpecTest.cpp
+++ lldb/unittests/Utility/FileSpecTest.cpp
@@ -399,3 +399,24 @@
   EXPECT_FALSE(Eq("foo", "/bar/foo", true));
   EXPECT_TRUE(Eq("foo", "/bar/foo", false));
 }
+
+TEST(FileSpecTest, Match) {
+  auto Match = [](const char *pattern, const char *file) {
+return FileSpec::Match(PosixSpec(pattern), PosixSpec(file));
+  };
+  EXPECT_TRUE(Match("/foo/bar", "/foo/bar"));
+  EXPECT_FALSE(Match("/foo/bar", "/oof/bar"));
+  EXPECT_FALSE(Match("/foo/bar", "/foo/baz"));
+  EXPECT_FALSE(Match("/foo/bar", "bar"));
+  EXPECT_FALSE(Match("/foo/bar", ""));
+
+  EXPECT_TRUE(Match("bar", "/foo/bar"));
+  EXPECT_FALSE(Match("bar", "/foo/baz"));
+  EXPECT_TRUE(Match("bar", "bar"));
+  EXPECT_FALSE(Match("bar", "baz"));
+  EXPECT_FALSE(Match("bar", ""));
+
+  EXPECT_TRUE(Match("", "/foo/bar"));
+  EXPECT_TRUE(Match("", ""));
+
+}
Index: lldb/source/Utility/FileSpec.cpp
===
--- lldb/source/Utility/FileSpec.cpp
+++ lldb/source/Utility/FileSpec.cpp
@@ -308,6 +308,14 @@
   return a.FileEquals(b);
 }
 
+bool FileSpec::Match(const FileSpec &pattern, const FileSpec &file) {
+  if (pattern.GetDirectory())
+return pattern == file;
+  if (pattern.GetFilename())
+return pattern.FileEquals(file);
+  return true;
+}
+
 llvm::Optional FileSpec::GuessPathStyle(llvm::StringRef absolute_path) {
   if (absolute_path.startswith("/"))
 return Style::posix;
Index: lldb/source/Target/ThreadPlanStepInRange.cpp
===
--- lldb/source/Target/ThreadPlanStepInRange.cpp
+++ lldb/source/Target/ThreadPlanStepInRange.cpp
@@ -339,7 +339,7 @@
 if (frame_

[Lldb-commits] [PATCH] D70846: Expression eval lookup speedup by not returning methods in ManualDWARFIndex::GetFunctions

2019-11-29 Thread Raphael Isemann via Phabricator via lldb-commits
teemperor added a comment.

This LGTM, but the TODO directly above the change in ClangExpressionDeclMap.cpp 
worries me a bit. I am not sure how good our test coverage is for calling 
functions when there are instance methods in scope (e.g., we are in instance 
method and try to call another instance method).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70846/new/

https://reviews.llvm.org/D70846



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D70797: [LLDB] Use r11 as frame pointer on Windows on ARM

2019-11-29 Thread Martin Storsjö via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG45c843de4eb8: [LLDB] [ARM] Use r11 as frame pointer on 
Windows on ARM (authored by mstorsjo).

Changed prior to commit:
  https://reviews.llvm.org/D70797?vs=231416&id=231527#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70797/new/

https://reviews.llvm.org/D70797

Files:
  lldb/source/Plugins/Instruction/ARM/EmulateInstructionARM.cpp
  lldb/test/Shell/Minidump/Windows/Inputs/arm-fp-unwind.dmp.yaml
  lldb/test/Shell/Minidump/Windows/Inputs/arm-fp-unwind.exe.yaml
  lldb/test/Shell/Minidump/Windows/arm-fp-unwind.test

Index: lldb/test/Shell/Minidump/Windows/arm-fp-unwind.test
===
--- /dev/null
+++ lldb/test/Shell/Minidump/Windows/arm-fp-unwind.test
@@ -0,0 +1,17 @@
+Test that unwind plans use the frame pointer register correctly.
+
+REQUIRES: arm
+
+RUN: yaml2obj %S/Inputs/arm-fp-unwind.exe.yaml > %T/arm-fp-unwind.exe
+RUN: yaml2obj %S/Inputs/arm-fp-unwind.dmp.yaml > %T/arm-fp-unwind.dmp
+RUN: %lldb -O "settings set target.exec-search-paths %T" \
+RUN:   -c %T/arm-fp-unwind.dmp -o "image show-unwind -a 0x00c71010" -b \
+RUN:   | FileCheck %s
+
+CHECK: Assembly language inspection UnwindPlan:
+CHECK-NEXT: This UnwindPlan originally sourced from EmulateInstructionARM
+CHECK-NEXT: This UnwindPlan is sourced from the compiler: no.
+CHECK-NEXT: This UnwindPlan is valid at all instruction locations: yes.
+CHECK-NEXT: row[0]:0: CFA=sp +0 =>
+CHECK-NEXT: row[1]:4: CFA=sp +8 => fp=[CFA-8] lr=[CFA-4]
+CHECK-NEXT: row[2]:6: CFA=fp +8 => fp=[CFA-8] lr=[CFA-4]
Index: lldb/test/Shell/Minidump/Windows/Inputs/arm-fp-unwind.exe.yaml
===
--- /dev/null
+++ lldb/test/Shell/Minidump/Windows/Inputs/arm-fp-unwind.exe.yaml
@@ -0,0 +1,92 @@
+--- !COFF
+OptionalHeader:
+  AddressOfEntryPoint: 4097
+  ImageBase:   4194304
+  SectionAlignment: 4096
+  FileAlignment:   512
+  MajorOperatingSystemVersion: 6
+  MinorOperatingSystemVersion: 0
+  MajorImageVersion: 0
+  MinorImageVersion: 0
+  MajorSubsystemVersion: 6
+  MinorSubsystemVersion: 0
+  Subsystem:   IMAGE_SUBSYSTEM_WINDOWS_CUI
+  DLLCharacteristics: [ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE, IMAGE_DLL_CHARACTERISTICS_NX_COMPAT, IMAGE_DLL_CHARACTERISTICS_TERMINAL_SERVER_AWARE ]
+  SizeOfStackReserve: 1048576
+  SizeOfStackCommit: 4096
+  SizeOfHeapReserve: 1048576
+  SizeOfHeapCommit: 4096
+  ExportTable:
+RelativeVirtualAddress: 0
+Size:0
+  ImportTable:
+RelativeVirtualAddress: 0
+Size:0
+  ResourceTable:
+RelativeVirtualAddress: 0
+Size:0
+  ExceptionTable:
+RelativeVirtualAddress: 0
+Size:0
+  CertificateTable:
+RelativeVirtualAddress: 0
+Size:0
+  BaseRelocationTable:
+RelativeVirtualAddress: 0
+Size:0
+  Debug:
+RelativeVirtualAddress: 0
+Size:0
+  Architecture:
+RelativeVirtualAddress: 0
+Size:0
+  GlobalPtr:
+RelativeVirtualAddress: 0
+Size:0
+  TlsTable:
+RelativeVirtualAddress: 0
+Size:0
+  LoadConfigTable:
+RelativeVirtualAddress: 0
+Size:0
+  BoundImport:
+RelativeVirtualAddress: 0
+Size:0
+  IAT:
+RelativeVirtualAddress: 0
+Size:0
+  DelayImportDescriptor:
+RelativeVirtualAddress: 0
+Size:0
+  ClrRuntimeHeader:
+RelativeVirtualAddress: 0
+Size:0
+header:
+  Machine: IMAGE_FILE_MACHINE_ARMNT
+  Characteristics: [ IMAGE_FILE_EXECUTABLE_IMAGE, IMAGE_FILE_32BIT_MACHINE ]
+sections:
+  - Name:.text
+Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+VirtualAddress:  4096
+VirtualSize: 38
+SectionData: 2DE90048EB46ADF5007D684600F004F80DF5007DBDE8008800BE01784278415C805C08447047
+symbols:
+  - Name:.text
+Value:   0
+SectionNumber:   1
+SimpleType:  IMAGE_SYM_TYPE_NULL
+ComplexType: IMAGE_SYM_DTYPE_NULL
+StorageClass:IMAGE_SYM_CLASS_STATIC
+  - Name:entry
+Value:   0
+SectionNumber:   1
+SimpleType:  IMAGE_SYM_TYPE_NULL
+ComplexType: IMAGE_SYM_DTYPE_FUNCTION
+StorageClass:IMAGE_SYM_CLASS_EXTERNAL
+  - Name:other
+Value:   24
+SectionNumber:   1
+SimpleType:  IMAGE_SYM_TYPE_NULL
+ComplexType: IMAGE_SYM_DTYPE_FUNCTION
+StorageClass:IMAGE_SYM_CLASS_EXTERNAL
+...
Index: lldb/test/Shell/Minidump/Windows/Inputs/arm-fp-unwind.dmp.yaml
===
--- /dev/null
+++ lldb/test/Shell/Minidump/Windows/Inputs/arm-fp-unwind.dmp.yaml
@@ -0,0 +1,37 @@
+--- !minidump
+Version: 0xA0BAA793
+Flags:   0x000