[Lldb-commits] [PATCH] D47492: DWARFUnit::m_die_array swap()->shrink_to_fit()

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In https://reviews.llvm.org/D47492#1121543, @dblaikie wrote:

> Happy to help explain it - which part(s) are you having a bit of trouble with?


What's wrong on this implementation 
?

> It seems like the main one is that the implementation can't be sure that 
> malloc was used to allocate the memory - since the global allocation function 
> can be replaced & there's no convenient way to detect that.

The example above does verify whether the vector uses default libstdc++ 
std::allocator which uses libstdc++ ::operator new which uses malloc().


Repository:
  rL LLVM

https://reviews.llvm.org/D47492



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

[Lldb-commits] [lldb] r333987 - Protect DWARFCompileUnit::m_die_array by new mutexes

[Jan Kratochvil via lldb-commits]

Author: jankratochvil
Date: Tue Jun  5 01:52:18 2018
New Revision: 333987

URL: http://llvm.org/viewvc/llvm-project?rev=333987&view=rev
Log:
Protect DWARFCompileUnit::m_die_array by new mutexes

If BuildAddressRangeTable called ExtractDIEsIfNeeded(false), then another
thread started processing data from m_die_array and then the first thread
called final ClearDIEs() the second thread would crash.

It is also required without multithreaded debugger using DW_TAG_partial_unit
for DWZ.

Differential revision: https://reviews.llvm.org/D40470

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp?rev=333987&r1=333986&r2=333987&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp Tue Jun  5 
01:52:18 2018
@@ -31,7 +31,8 @@ using namespace std;
 
 extern int g_verbose;
 
-DWARFUnit::DWARFUnit(SymbolFileDWARF *dwarf) : m_dwarf(dwarf) {}
+DWARFUnit::DWARFUnit(SymbolFileDWARF *dwarf)
+: m_dwarf(dwarf), m_cancel_scopes(false) {}
 
 DWARFUnit::~DWARFUnit() {}
 
@@ -39,6 +40,12 @@ DWARFUnit::~DWARFUnit() {}
 // Parses first DIE of a compile unit.
 //--
 void DWARFUnit::ExtractUnitDIEIfNeeded() {
+  {
+llvm::sys::ScopedReader lock(m_first_die_mutex);
+if (m_first_die)
+  return; // Already parsed
+  }
+  llvm::sys::ScopedWriter lock(m_first_die_mutex);
   if (m_first_die)
 return; // Already parsed
 
@@ -67,10 +74,88 @@ void DWARFUnit::ExtractUnitDIEIfNeeded()
 
 //--
 // Parses a compile unit and indexes its DIEs if it hasn't already been done.
+// It will leave this compile unit extracted forever.
 //--
-bool DWARFUnit::ExtractDIEsIfNeeded() {
+void DWARFUnit::ExtractDIEsIfNeeded() {
+  m_cancel_scopes = true;
+
+  {
+llvm::sys::ScopedReader lock(m_die_array_mutex);
+if (!m_die_array.empty())
+  return; // Already parsed
+  }
+  llvm::sys::ScopedWriter lock(m_die_array_mutex);
   if (!m_die_array.empty())
-return 0; // Already parsed
+return; // Already parsed
+
+  ExtractDIEsRWLocked();
+}
+
+//--
+// Parses a compile unit and indexes its DIEs if it hasn't already been done.
+// It will clear this compile unit after returned instance gets out of scope,
+// no other ScopedExtractDIEs instance is running for this compile unit
+// and no ExtractDIEsIfNeeded() has been executed during this ScopedExtractDIEs
+// lifetime.
+//--
+DWARFUnit::ScopedExtractDIEs DWARFUnit::ExtractDIEsScoped() {
+  ScopedExtractDIEs scoped(this);
+
+  {
+llvm::sys::ScopedReader lock(m_die_array_mutex);
+if (!m_die_array.empty())
+  return std::move(scoped); // Already parsed
+  }
+  llvm::sys::ScopedWriter lock(m_die_array_mutex);
+  if (!m_die_array.empty())
+return std::move(scoped); // Already parsed
+
+  // Otherwise m_die_array would be already populated.
+  lldbassert(!m_cancel_scopes);
+
+  ExtractDIEsRWLocked();
+  scoped.m_clear_dies = true;
+  return scoped;
+}
+
+DWARFUnit::ScopedExtractDIEs::ScopedExtractDIEs(DWARFUnit *cu) : m_cu(cu) {
+  lldbassert(m_cu);
+  m_cu->m_die_array_scoped_mutex.lock_shared();
+}
+
+DWARFUnit::ScopedExtractDIEs::~ScopedExtractDIEs() {
+  if (!m_cu)
+return;
+  m_cu->m_die_array_scoped_mutex.unlock_shared();
+  if (!m_clear_dies || m_cu->m_cancel_scopes)
+return;
+  // Be sure no other ScopedExtractDIEs is running anymore.
+  llvm::sys::ScopedWriter lock_scoped(m_cu->m_die_array_scoped_mutex);
+  llvm::sys::ScopedWriter lock(m_cu->m_die_array_mutex);
+  if (m_cu->m_cancel_scopes)
+return;
+  m_cu->ClearDIEsRWLocked();
+}
+
+DWARFUnit::ScopedExtractDIEs::ScopedExtractDIEs(ScopedExtractDIEs &&rhs)
+: m_cu(rhs.m_cu), m_clear_dies(rhs.m_clear_dies) {
+  rhs.m_cu = nullptr;
+}
+
+DWARFUnit::ScopedExtractDIEs &DWARFUnit::ScopedExtractDIEs::operator=(
+DWARFUnit::ScopedExtractDIEs &&rhs) {
+  m_cu = rhs.m_cu;
+  rhs.m_cu = nullptr;
+  m_clear_dies = rhs.m_clear_dies;
+  return *this;
+}
+
+//--
+// Parses a compile unit and indexes its DIEs, m_die_array_mutex must be
+// held R/W and m_die_array must be empty.
+//--
+void DWARFUnit::ExtractDIEsRWLocked() {
+  llvm::sys::ScopedWriter first_die_lock(m_fi

[Lldb-commits] [PATCH] D40470: Protect DWARFCompileUnit::m_die_array by a new mutex

[Jan Kratochvil via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL333987: Protect DWARFCompileUnit::m_die_array by new mutexes 
(authored by jankratochvil, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D40470?vs=149609&id=149917#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D40470

Files:
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -13,6 +13,7 @@
 #include "DWARFDIE.h"
 #include "DWARFDebugInfoEntry.h"
 #include "lldb/lldb-enumerations.h"
+#include "llvm/Support/RWMutex.h"
 
 class DWARFUnit;
 class DWARFCompileUnit;
@@ -40,7 +41,20 @@
   virtual ~DWARFUnit();
 
   void ExtractUnitDIEIfNeeded();
-  bool ExtractDIEsIfNeeded();
+  void ExtractDIEsIfNeeded();
+
+  class ScopedExtractDIEs {
+DWARFUnit *m_cu;
+  public:
+bool m_clear_dies = false;
+ScopedExtractDIEs(DWARFUnit *cu);
+~ScopedExtractDIEs();
+DISALLOW_COPY_AND_ASSIGN(ScopedExtractDIEs);
+ScopedExtractDIEs(ScopedExtractDIEs &&rhs);
+ScopedExtractDIEs &operator=(ScopedExtractDIEs &&rhs);
+  };
+  ScopedExtractDIEs ExtractDIEsScoped();
+
   DWARFDIE LookupAddress(const dw_addr_t address);
   size_t AppendDIEsWithTag(const dw_tag_t tag,
DWARFDIECollection &matching_dies,
@@ -100,7 +114,6 @@
   dw_addr_t GetRangesBase() const { return m_ranges_base; }
   void SetAddrBase(dw_addr_t addr_base, dw_addr_t ranges_base,
dw_offset_t base_obj_offset);
-  void ClearDIEs();
   void BuildAddressRangeTable(SymbolFileDWARF *dwarf,
   DWARFDebugAranges *debug_aranges);
 
@@ -172,10 +185,17 @@
   void *m_user_data = nullptr;
   // The compile unit debug information entry item
   DWARFDebugInfoEntry::collection m_die_array;
+  mutable llvm::sys::RWMutex m_die_array_mutex;
+  // It is used for tracking of ScopedExtractDIEs instances.
+  mutable llvm::sys::RWMutex m_die_array_scoped_mutex;
+  // ScopedExtractDIEs instances should not call ClearDIEsRWLocked()
+  // as someone called ExtractDIEsIfNeeded().
+  std::atomic m_cancel_scopes;
   // GetUnitDIEPtrOnly() needs to return pointer to the first DIE.
   // But the first element of m_die_array after ExtractUnitDIEIfNeeded()
   // would possibly move in memory after later ExtractDIEsIfNeeded().
   DWARFDebugInfoEntry m_first_die;
+  llvm::sys::RWMutex m_first_die_mutex;
   // A table similar to the .debug_aranges table, but this one points to the
   // exact DW_TAG_subprogram DIEs
   std::unique_ptr m_func_aranges_ap;
@@ -201,11 +221,14 @@
 
 private:
   void ParseProducerInfo();
+  void ExtractDIEsRWLocked();
+  void ClearDIEsRWLocked();
 
   // Get the DWARF unit DWARF debug informration entry. Parse the single DIE
   // if needed.
   const DWARFDebugInfoEntry *GetUnitDIEPtrOnly() {
 ExtractUnitDIEIfNeeded();
+// m_first_die_mutex is not required as m_first_die is never cleared.
 if (!m_first_die)
   return NULL;
 return &m_first_die;
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -31,14 +31,21 @@
 
 extern int g_verbose;
 
-DWARFUnit::DWARFUnit(SymbolFileDWARF *dwarf) : m_dwarf(dwarf) {}
+DWARFUnit::DWARFUnit(SymbolFileDWARF *dwarf)
+: m_dwarf(dwarf), m_cancel_scopes(false) {}
 
 DWARFUnit::~DWARFUnit() {}
 
 //--
 // Parses first DIE of a compile unit.
 //--
 void DWARFUnit::ExtractUnitDIEIfNeeded() {
+  {
+llvm::sys::ScopedReader lock(m_first_die_mutex);
+if (m_first_die)
+  return; // Already parsed
+  }
+  llvm::sys::ScopedWriter lock(m_first_die_mutex);
   if (m_first_die)
 return; // Already parsed
 
@@ -67,10 +74,88 @@
 
 //--
 // Parses a compile unit and indexes its DIEs if it hasn't already been done.
+// It will leave this compile unit extracted forever.
 //--
-bool DWARFUnit::ExtractDIEsIfNeeded() {
+void DWARFUnit::ExtractDIEsIfNeeded() {
+  m_cancel_scopes = true;
+
+  {
+llvm::sys::ScopedReader lock(m_die_array_mutex);
+if (!m_die_array.empty())
+  return; // Already parsed
+  }
+  llvm::sys::ScopedWriter lock(m_die_array_mutex);
   if (!m_die_array.empty())
-return 0; // Already parsed
+return; // Already parsed
+
+  ExtractDIEsRW

[Lldb-commits] [PATCH] D40470: Protect DWARFCompileUnit::m_die_array by a new mutex

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

FYI I have reordered `m_cu->m_die_array_scoped_mutex.unlock_shared();` in 
`DWARFUnit::ScopedExtractDIEs::~ScopedExtractDIEs()` during commit (it was 
written this new way in some other of the 4 offered variants).


Repository:
  rL LLVM

https://reviews.llvm.org/D40470



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

[Lldb-commits] [PATCH] D47492: DWARFUnit::m_die_array swap()->shrink_to_fit()

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D47492#1122039, @jankratochvil wrote:

> In https://reviews.llvm.org/D47492#1121543, @dblaikie wrote:
>
> > Happy to help explain it - which part(s) are you having a bit of trouble 
> > with?
>
>
> What's wrong on this implementation 
> ?
>
> > It seems like the main one is that the implementation can't be sure that 
> > malloc was used to allocate the memory - since the global allocation 
> > function can be replaced & there's no convenient way to detect that.
>
> The example above does verify whether the vector uses default libstdc++ 
> std::allocator which uses libstdc++ ::operator new which uses malloc().


It doesn't look like your code verifies that the user hasn't replaced the 
global `operator new` by defining a custom version. 
http://en.cppreference.com/w/cpp/memory/new/operator_new:

> The versions (1-4) are implicitly declared in each translation unit even if 
> the  header is not included. Versions (1-8) are replaceable: a 
> user-provided non-member function with the same signature defined anywhere in 
> the program, in any source file, replaces the default version. Its 
> declaration does not need to be visible.

The requirement that the declaration need not even be visible makes me believe 
this is not possible to detect at compile time. This kind of overriding is 
usually done via weak symbols, and the only way of detecting that I can think 
of is if libstdc++ exposed a non-weak alias to its `operator new` and then 
`shrink_to_fit` did a runtime check like `if(&__standard_operator_new == 
&::operator new)`. That seems like a huge hack.


Repository:
  rL LLVM

https://reviews.llvm.org/D47492



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

[Lldb-commits] [PATCH] D47629: [DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:122
+PropertyDefinition g_experimental_properties[] = {
+{"use-debug-names", OptionValue::eTypeBoolean, true, 0, nullptr, nullptr,
+ "Use .debug_names index section."},

clayborg wrote:
> Would it be better to use an inverse setting like "always-manually-index"? 
> That way if there is something wrong with any of the indexes that are 
> generated, then we can fall back to manually indexing? Otherwise we will need 
> to remove this later.
In the interests of non-proliferation of settings I have deliberately chosen a 
path that does not make the setting permanent. However, this does sound like a 
thing that could conceivably be useful as a setting in general, so I can do 
that too if you think it's better.

Would you then also make the setting non-experimental?


https://reviews.llvm.org/D47629



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

[Lldb-commits] [lldb] r333992 - Fix ClangParserTest.cpp

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jun  5 02:33:26 2018
New Revision: 333992

URL: http://llvm.org/viewvc/llvm-project?rev=333992&view=rev
Log:
Fix ClangParserTest.cpp

The test does not use a test fixture, so it needs to be declared with
the TEST macro.

Modified:
lldb/trunk/unittests/Expression/ClangParserTest.cpp

Modified: lldb/trunk/unittests/Expression/ClangParserTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Expression/ClangParserTest.cpp?rev=333992&r1=333991&r2=333992&view=diff
==
--- lldb/trunk/unittests/Expression/ClangParserTest.cpp (original)
+++ lldb/trunk/unittests/Expression/ClangParserTest.cpp Tue Jun  5 02:33:26 2018
@@ -24,7 +24,7 @@ static std::string ComputeClangDir(std::
   return clang_dir.GetPath();
 }
 
-TEST_F(HostInfoTest, MacOSX) {
+TEST(ClangHostTest, MacOSX) {
   // This returns whatever the POSIX fallback returns.
   std::string posix = "/usr/lib/liblldb.dylib";
   EXPECT_FALSE(ComputeClangDir(posix).empty());


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

[Lldb-commits] [PATCH] D47492: DWARFUnit::m_die_array swap()->shrink_to_fit()

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In https://reviews.llvm.org/D47492#1122080, @labath wrote:

> It doesn't look like your code verifies that the user hasn't replaced the 
> global `operator new` by defining a custom version. 
> http://en.cppreference.com/w/cpp/memory/new/operator_new:


I asked about it in the code, I see I could find it is permitted:

  // Is permitted a symbol interposition of ::operator new to verify even that?



> if libstdc++ exposed a non-weak alias to its `operator new` and then 
> `shrink_to_fit` did a runtime check like `if(&__standard_operator_new == 
> &::operator new)`.

Yes, that is what I planned to write if the symbol interpoisiton is really 
permitted.

> That seems like a huge hack.

Yes but possible.


Repository:
  rL LLVM

https://reviews.llvm.org/D47492



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

[Lldb-commits] [lldb] r333996 - Fixup r333987

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jun  5 02:56:14 2018
New Revision: 333996

URL: http://llvm.org/viewvc/llvm-project?rev=333996&view=rev
Log:
Fixup r333987

- add #include  (fixes windows build)
- remove std::move (fixes "using move prevents copy ellision" warnings)

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp?rev=333996&r1=333995&r2=333996&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp Tue Jun  5 
02:56:14 2018
@@ -104,11 +104,11 @@ DWARFUnit::ScopedExtractDIEs DWARFUnit::
   {
 llvm::sys::ScopedReader lock(m_die_array_mutex);
 if (!m_die_array.empty())
-  return std::move(scoped); // Already parsed
+  return scoped; // Already parsed
   }
   llvm::sys::ScopedWriter lock(m_die_array_mutex);
   if (!m_die_array.empty())
-return std::move(scoped); // Already parsed
+return scoped; // Already parsed
 
   // Otherwise m_die_array would be already populated.
   lldbassert(!m_cancel_scopes);

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h?rev=333996&r1=333995&r2=333996&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFUnit.h Tue Jun  5 02:56:14 
2018
@@ -14,6 +14,7 @@
 #include "DWARFDebugInfoEntry.h"
 #include "lldb/lldb-enumerations.h"
 #include "llvm/Support/RWMutex.h"
+#include 
 
 class DWARFUnit;
 class DWARFCompileUnit;


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

[Lldb-commits] [PATCH] D47492: DWARFUnit::m_die_array swap()->shrink_to_fit()

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Well, with these kinds of safeguards **I think** this could work.
If I was the maintainer of std::vector in libstdc++ I am not sure if I would 
accept such a patch, but I am not, so feel free to try. :)


Repository:
  rL LLVM

https://reviews.llvm.org/D47492



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

[Lldb-commits] [lldb] r334003 - Really fix ClangParserTest

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jun  5 03:29:48 2018
New Revision: 334003

URL: http://llvm.org/viewvc/llvm-project?rev=334003&view=rev
Log:
Really fix ClangParserTest

It turns out the test needs a fixture after all (to initialize HostInfo), so
provide one that does that.

Modified:
lldb/trunk/unittests/Expression/ClangParserTest.cpp

Modified: lldb/trunk/unittests/Expression/ClangParserTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Expression/ClangParserTest.cpp?rev=334003&r1=334002&r2=334003&view=diff
==
--- lldb/trunk/unittests/Expression/ClangParserTest.cpp (original)
+++ lldb/trunk/unittests/Expression/ClangParserTest.cpp Tue Jun  5 03:29:48 2018
@@ -9,12 +9,20 @@
 
 #include "Plugins/ExpressionParser/Clang/ClangHost.h"
 #include "TestingSupport/TestUtilities.h"
+#include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/FileSpec.h"
 #include "lldb/lldb-defines.h"
 #include "gtest/gtest.h"
 
 using namespace lldb_private;
 
+namespace {
+struct ClangHostTest : public testing::Test {
+  static void SetUpTestCase() { HostInfo::Initialize(); }
+  static void TearDownTestCase() { HostInfo::Terminate(); }
+};
+} // namespace
+
 #ifdef __APPLE__
 static std::string ComputeClangDir(std::string lldb_shlib_path,
bool verify = false) {
@@ -24,7 +32,7 @@ static std::string ComputeClangDir(std::
   return clang_dir.GetPath();
 }
 
-TEST(ClangHostTest, MacOSX) {
+TEST_F(ClangHostTest, MacOSX) {
   // This returns whatever the POSIX fallback returns.
   std::string posix = "/usr/lib/liblldb.dylib";
   EXPECT_FALSE(ComputeClangDir(posix).empty());


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

[Lldb-commits] [lldb] r334004 - DWARFIndex: simplify GetFunctions methods

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jun  5 03:33:56 2018
New Revision: 334004

URL: http://llvm.org/viewvc/llvm-project?rev=334004&view=rev
Log:
DWARFIndex: simplify GetFunctions methods

Now that Apple index determines method-ness straight from the debug
info, we don't need to resolve the functions into SymbolContexts inside
the Index classes. This removes the need for callback arguments and
allows us to pull the common parts out of the two implementations of
these functions back into the SymbolFileDWARF class.

Reviewers: JDevlieghere, clayborg

Subscribers: aprantl, lldb-commits

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

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp?rev=334004&r1=334003&r2=334004&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp Tue Jun  5 
03:33:56 2018
@@ -154,101 +154,66 @@ static bool KeepFunctionDIE(DWARFDIE die
   return looking_for_methods == die.IsMethod();
 }
 
-void AppleDWARFIndex::GetFunctions(
-ConstString name, DWARFDebugInfo &info,
-llvm::function_ref
-resolve_function,
-llvm::function_ref
-get_decl_context_containing_uid,
-const CompilerDeclContext *parent_decl_ctx, uint32_t name_type_mask,
-bool include_inlines, SymbolContextList &sc_list) {
-  if (!m_apple_names_up)
-return;
-
-  std::set resolved_dies;
-  DIEArray offsets;
-
-  uint32_t num_matches = 0;
-
+void AppleDWARFIndex::GetFunctions(ConstString name, DWARFDebugInfo &info,
+   const CompilerDeclContext &parent_decl_ctx,
+   uint32_t name_type_mask,
+   std::vector &dies) {
   if (name_type_mask & eFunctionNameTypeFull) {
 // If they asked for the full name, match what they typed.  At some
 // point we may want to canonicalize this (strip double spaces, etc.
 // For now, we just add all the dies that we find by exact match.
-num_matches = m_apple_names_up->FindByName(name.GetStringRef(), offsets);
-for (uint32_t i = 0; i < num_matches; i++) {
-  const DIERef &die_ref = offsets[i];
+DIEArray offsets;
+m_apple_names_up->FindByName(name.GetStringRef(), offsets);
+for (const DIERef &die_ref: offsets) {
   DWARFDIE die = info.GetDIE(die_ref);
-  if (die) {
-if (!SymbolFileDWARF::DIEInDeclContext(parent_decl_ctx, die))
-  continue; // The containing decl contexts don't match
-
-if (resolved_dies.find(die.GetDIE()) == resolved_dies.end()) {
-  if (resolve_function(die, include_inlines, sc_list))
-resolved_dies.insert(die.GetDIE());
-}
-  } else
+  if (!die) {
 ReportInvalidDIEOffset(die_ref.die_offset, name.GetStringRef());
+continue;
+  }
+  if (SymbolFileDWARF::DIEInDeclContext(&parent_decl_ctx, die))
+dies.push_back(die);
 }
   }
+  if (name_type_mask & eFunctionNameTypeSelector &&
+  !parent_decl_ctx.IsValid()) {
+DIEArray offsets;
+m_apple_names_up->FindByName(name.GetStringRef(), offsets);
 
-  if (name_type_mask & eFunctionNameTypeSelector) {
-if (parent_decl_ctx && parent_decl_ctx->IsValid())
-  return; // no selectors in namespaces
-
-num_matches = m_apple_names_up->FindByName(name.GetStringRef(), offsets);
 // Now make sure these are actually ObjC methods.  In this case we can
 // simply look up the name, and if it is an ObjC method name, we're
 // good.
-
-for (uint32_t i = 0; i < num_matches; i++) {
-  const DIERef &die_ref = offsets[i];
+for (const DIERef &die_ref: offsets) {
   DWARFDIE die = info.GetDIE(die_ref);
-  if (die) {
-const char *die_name = die.GetName();
-if (ObjCLanguage::IsPossibleObjCMethodName(die_name)) {
-  if (resolved_dies.find(die.GetDIE()) == resolved_dies.end()) {
-if (resolve_function(die, include_inlines, sc_list))
-  resolved_dies.insert(die.GetDIE());
-  }
-}
-  } else
+  if (!die) {
 ReportInvalidDIEOffset(die_ref.die_offset, name.GetStringRef());
+continue;
+  }
+  const char *die_name = die.GetName();
+  if (ObjCLanguage::IsPossibleObjCMethodName(die_name))
+dies.push_back(die);
 }
-offsets.clear();
   }
-
-  if (((name_type_ma

[Lldb-commits] [PATCH] D47147: DWARFIndex: Reduce duplication in the GetFunctions methods

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL334004: DWARFIndex: simplify GetFunctions methods (authored 
by labath, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47147?vs=149702&id=149928#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47147

Files:
  lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -36,15 +36,10 @@
   virtual void GetTypes(ConstString name, DIEArray &offsets) = 0;
   virtual void GetTypes(const DWARFDeclContext &context, DIEArray &offsets) = 0;
   virtual void GetNamespaces(ConstString name, DIEArray &offsets) = 0;
-  virtual void GetFunctions(
-  ConstString name, DWARFDebugInfo &info,
-  llvm::function_ref
-  resolve_function,
-  llvm::function_ref
-  get_decl_context_containing_uid,
-  const CompilerDeclContext *parent_decl_ctx, uint32_t name_type_mask,
-  bool include_inlines, SymbolContextList &sc_list) = 0;
+  virtual void GetFunctions(ConstString name, DWARFDebugInfo &info,
+const CompilerDeclContext &parent_decl_ctx,
+uint32_t name_type_mask,
+std::vector &dies);
   virtual void GetFunctions(
   const RegularExpression ®ex, DWARFDebugInfo &info,
   llvm::function_refGetFunctions(name, *info,
-[this](const DWARFDIE &die, bool include_inlines,
-   lldb_private::SymbolContextList &sc_list) {
-  return ResolveFunction(die, include_inlines, sc_list);
-},
-[this](lldb::user_id_t type_uid) {
-  return GetDeclContextContainingUID(type_uid);
-},
-parent_decl_ctx, name_type_mask, include_inlines,
-sc_list);
+  llvm::DenseSet resolved_dies;
+  DIEArray offsets;
+  CompilerDeclContext empty_decl_ctx;
+  if (!parent_decl_ctx)
+parent_decl_ctx = &empty_decl_ctx;
+
+  std::vector dies;
+  m_index->GetFunctions(name, *info, *parent_decl_ctx, name_type_mask, dies);
+  for (const DWARFDIE &die: dies) {
+if (resolved_dies.insert(die.GetDIE()).second)
+  ResolveFunction(die, include_inlines, sc_list);
+  }
 
   // Return the number of variable that were appended to the list
   const uint32_t num_matches = sc_list.GetSize() - original_size;
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
@@ -154,101 +154,66 @@
   return looking_for_methods == die.IsMethod();
 }
 
-void AppleDWARFIndex::GetFunctions(
-ConstString name, DWARFDebugInfo &info,
-llvm::function_ref
-resolve_function,
-llvm::function_ref
-get_decl_context_containing_uid,
-const CompilerDeclContext *parent_decl_ctx, uint32_t name_type_mask,
-bool include_inlines, SymbolContextList &sc_list) {
-  if (!m_apple_names_up)
-return;
-
-  std::set resolved_dies;
-  DIEArray offsets;
-
-  uint32_t num_matches = 0;
-
+void AppleDWARFIndex::GetFunctions(ConstString name, DWARFDebugInfo &info,
+   const CompilerDeclContext &parent_decl_ctx,
+   uint32_t name_type_mask,
+   std::vector &dies) {
   if (name_type_mask & eFunctionNameTypeFull) {
 // If they asked for the full name, match what they typed.  At some
 // point we may want to canonicalize this (strip double spaces, etc.
 // For now, we just add all the dies that we find by exact match.
-num_matches = m_apple_names_up->FindByName(name.GetStringRef(), offsets);
-for (uint32_t i = 0; i < num_matches; i++) {
-  const DIERef &die_ref = offsets[i];
+DIEArray offsets;
+m_apple_names_up->FindByName(name.GetStringRef(), offsets);
+for (const DIERef &die_ref: offsets) {
   DWARFDIE die = info.GetDIE(die_ref);
-  if (die) {
-if (!SymbolFileDWARF::DIEInDeclContext(parent_decl_ctx, die))
-  continue; // The containing decl contexts don't match
-
-if (resolved_dies.find(die.GetDIE()) == resolved_dies.end()) {
-  if (resolve_function(die,

[Lldb-commits] [lldb] r334006 - Fix windows build broken by r334004

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jun  5 03:49:56 2018
New Revision: 334006

URL: http://llvm.org/viewvc/llvm-project?rev=334006&view=rev
Log:
Fix windows build broken by r334004

The problem was a link error due to a missing =0 on an abstract method.
Interestingly, this was not a problem for clang/linux.

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h?rev=334006&r1=334005&r2=334006&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h Tue Jun  5 03:49:56 
2018
@@ -39,7 +39,7 @@ public:
   virtual void GetFunctions(ConstString name, DWARFDebugInfo &info,
 const CompilerDeclContext &parent_decl_ctx,
 uint32_t name_type_mask,
-std::vector &dies);
+std::vector &dies) = 0;
   virtual void GetFunctions(
   const RegularExpression ®ex, DWARFDebugInfo &info,
   llvm::function_refhttp://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r334009 - dotest: make inline tests compatible with -f

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jun  5 03:58:44 2018
New Revision: 334009

URL: http://llvm.org/viewvc/llvm-project?rev=334009&view=rev
Log:
dotest: make inline tests compatible with -f

Summary:
This is split off from D47265 where I needed to be able to invoke every test
with -f. That patch is kinda dead now, but this part seems like a good
cleanup anyway.

The problem with inline tests was in the way we were adding methods to
the class, which left them with an incorrect __name__ property. This
prevented dotest from finding them with -f.

I fix this with (what I think is) the correct way of dynamically
creating classes -- passing the list of methods during type construction
instead of fixing up the class afterwards. Among other things this has
the advantage of not needing to do anything special for debug info
variants. As our test method will be visible to the metaclass, it will
automagically do the multiplication for us.

Reviewers: JDevlieghere, aprantl, tberghammer

Subscribers: eraman, lldb-commits

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

Modified:
lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py?rev=334009&r1=334008&r2=334009&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py Tue Jun  5 03:58:44 
2018
@@ -84,18 +84,6 @@ class CommandParser:
 class InlineTest(TestBase):
 # Internal implementation
 
-def getRerunArgs(self):
-# The -N option says to NOT run a if it matches the option argument, so
-# if we are using dSYM we say to NOT run dwarf (-N dwarf) and vice
-# versa.
-if self.using_dsym is None:
-# The test was skipped altogether.
-return ""
-elif self.using_dsym:
-return "-N dwarf " + self.mydir
-else:
-return "-N dsym " + self.mydir
-
 def BuildMakefile(self):
 makefilePath = self.getBuildArtifact("Makefile")
 if os.path.exists(makefilePath):
@@ -135,37 +123,10 @@ class InlineTest(TestBase):
 makefile.flush()
 makefile.close()
 
-@add_test_categories(["dsym"])
-def __test_with_dsym(self):
-self.using_dsym = True
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dsym.debug_info = "dsym"
-
-@add_test_categories(["dwarf"])
-def __test_with_dwarf(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dwarf.debug_info = "dwarf"
-
-@add_test_categories(["dwo"])
-def __test_with_dwo(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dwo.debug_info = "dwo"
-
-@add_test_categories(["gmodules"])
-def __test_with_gmodules(self):
-self.using_dsym = False
+def _test(self):
 self.BuildMakefile()
 self.build()
 self.do_test()
-__test_with_gmodules.debug_info = "gmodules"
 
 def execute_user_command(self, __command):
 exec(__command, globals(), locals())
@@ -237,23 +198,15 @@ def MakeInlineTest(__file, __globals, de
 InlineTest.mydir = TestBase.compute_mydir(__file)
 
 test_name, _ = os.path.splitext(file_basename)
-# Build the test case
-test = type(test_name, (InlineTest,), {'using_dsym': None})
-test.name = test_name
 
-test.test_with_dsym = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dsym, decorators)
-test.test_with_dwarf = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dwarf, decorators)
-test.test_with_dwo = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dwo, decorators)
-test.test_with_gmodules = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_gmodules, decorators)
+test_func = ApplyDecoratorsToFunction(InlineTest._test, decorators)
+# Build the test case
+test_class = type(test_name, (InlineTest,), dict(test=test_func, 
name=test_name))
 
 # Add the test case to the globals, and hide InlineTest
-__globals.update({test_name: test})
+__globals.update({test_name: test_class})
 
 # Keep track of the original test filename so we report it
 # correctly in test results.
-test.test_filename = __file
-return test
+test_class.test_filename = __file
+return test_class


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

[Lldb-commits] [PATCH] D47579: dotest: make inline tests compatible with -f

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL334009: dotest: make inline tests compatible with -f 
(authored by labath, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D47579

Files:
  lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py


Index: lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
@@ -84,18 +84,6 @@
 class InlineTest(TestBase):
 # Internal implementation
 
-def getRerunArgs(self):
-# The -N option says to NOT run a if it matches the option argument, so
-# if we are using dSYM we say to NOT run dwarf (-N dwarf) and vice
-# versa.
-if self.using_dsym is None:
-# The test was skipped altogether.
-return ""
-elif self.using_dsym:
-return "-N dwarf " + self.mydir
-else:
-return "-N dsym " + self.mydir
-
 def BuildMakefile(self):
 makefilePath = self.getBuildArtifact("Makefile")
 if os.path.exists(makefilePath):
@@ -135,37 +123,10 @@
 makefile.flush()
 makefile.close()
 
-@add_test_categories(["dsym"])
-def __test_with_dsym(self):
-self.using_dsym = True
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dsym.debug_info = "dsym"
-
-@add_test_categories(["dwarf"])
-def __test_with_dwarf(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dwarf.debug_info = "dwarf"
-
-@add_test_categories(["dwo"])
-def __test_with_dwo(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dwo.debug_info = "dwo"
-
-@add_test_categories(["gmodules"])
-def __test_with_gmodules(self):
-self.using_dsym = False
+def _test(self):
 self.BuildMakefile()
 self.build()
 self.do_test()
-__test_with_gmodules.debug_info = "gmodules"
 
 def execute_user_command(self, __command):
 exec(__command, globals(), locals())
@@ -237,23 +198,15 @@
 InlineTest.mydir = TestBase.compute_mydir(__file)
 
 test_name, _ = os.path.splitext(file_basename)
-# Build the test case
-test = type(test_name, (InlineTest,), {'using_dsym': None})
-test.name = test_name
 
-test.test_with_dsym = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dsym, decorators)
-test.test_with_dwarf = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dwarf, decorators)
-test.test_with_dwo = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_dwo, decorators)
-test.test_with_gmodules = ApplyDecoratorsToFunction(
-test._InlineTest__test_with_gmodules, decorators)
+test_func = ApplyDecoratorsToFunction(InlineTest._test, decorators)
+# Build the test case
+test_class = type(test_name, (InlineTest,), dict(test=test_func, 
name=test_name))
 
 # Add the test case to the globals, and hide InlineTest
-__globals.update({test_name: test})
+__globals.update({test_name: test_class})
 
 # Keep track of the original test filename so we report it
 # correctly in test results.
-test.test_filename = __file
-return test
+test_class.test_filename = __file
+return test_class


Index: lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
@@ -84,18 +84,6 @@
 class InlineTest(TestBase):
 # Internal implementation
 
-def getRerunArgs(self):
-# The -N option says to NOT run a if it matches the option argument, so
-# if we are using dSYM we say to NOT run dwarf (-N dwarf) and vice
-# versa.
-if self.using_dsym is None:
-# The test was skipped altogether.
-return ""
-elif self.using_dsym:
-return "-N dwarf " + self.mydir
-else:
-return "-N dsym " + self.mydir
-
 def BuildMakefile(self):
 makefilePath = self.getBuildArtifact("Makefile")
 if os.path.exists(makefilePath):
@@ -135,37 +123,10 @@
 makefile.flush()
 makefile.close()
 
-@add_test_categories(["dsym"])
-def __test_with_dsym(self):
-self.using_dsym = True
-self.BuildMakefile()
-self.build()
-self.do_test()
-__test_with_dsym.debug_info = "dsym"
-
-@add_test_categories(["dwarf"])
-def __test_with_dwarf(self):
-self.using_dsym = False
-self.BuildMakefile()
-self.build()
-self.do_test()
-__

[Lldb-commits] [PATCH] D47265: WIP: lit: Run each test separately

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 149936.
labath added a comment.

No real change in functionality, just updating the diff to remove the unrelated
changes.


https://reviews.llvm.org/D47265

Files:
  lit/Suite/lldbtest.py


Index: lit/Suite/lldbtest.py
===
--- lit/Suite/lldbtest.py
+++ lit/Suite/lldbtest.py
@@ -13,38 +13,41 @@
 class LLDBTest(TestFormat):
 def __init__(self, dotest_cmd):
 self.dotest_cmd = dotest_cmd
+self.tests = None
+
+def getTests(self, bin_dir):
+if self.tests:
+return self.tests
+dumper = os.path.join(os.path.dirname(__file__), "lldbtestdumper.py")
+lldb_path = os.path.join(bin_dir, "lldb")
+output = subprocess.check_output([sys.executable, dumper, lldb_path])
+self.tests = [line.split() for line in output.splitlines()]
+return self.tests
+
 
 def getTestsInDirectory(self, testSuite, path_in_suite, litConfig,
 localConfig):
-source_path = testSuite.getSourcePath(path_in_suite)
-for filename in os.listdir(source_path):
-# Ignore dot files and excluded tests.
-if (filename.startswith('.') or filename in localConfig.excludes):
-continue
-
-# Ignore files that don't start with 'Test'.
-if not filename.startswith('Test'):
+for test in self.getTests(testSuite.config.llvm_tools_dir):
+if test[0:-3] != list(path_in_suite):
 continue
-
-filepath = os.path.join(source_path, filename)
-if not os.path.isdir(filepath):
-base, ext = os.path.splitext(filename)
-if ext in localConfig.suffixes:
-yield lit.Test.Test(testSuite, path_in_suite +
-(filename, ), localConfig)
+yield lit.Test.Test(testSuite, test, localConfig)
 
 def execute(self, test, litConfig):
 if litConfig.noExecute:
 return lit.Test.PASS, ''
 
 if test.config.unsupported:
 return (lit.Test.UNSUPPORTED, 'Test is unsupported')
 
-testPath, testFile = os.path.split(test.getSourcePath())
+testDir = test.getSourcePath()
+testDir, testMethod = os.path.split(testDir)
+testDir, testClass = os.path.split(testDir)
+testDir, testFile = os.path.split(testDir)
+
 # On Windows, the system does not always correctly interpret shebang 
lines.
 # To make sure we can execute the tests, add python exe as the first 
parameter
 # of the command.
-cmd = [sys.executable] + self.dotest_cmd + [testPath, '-p', testFile]
+cmd = [sys.executable] + self.dotest_cmd + [testDir, '-p', testFile, 
'-f', testClass+"."+testMethod]
 
 try:
 out, err, exitCode = lit.util.executeCommand(


Index: lit/Suite/lldbtest.py
===
--- lit/Suite/lldbtest.py
+++ lit/Suite/lldbtest.py
@@ -13,38 +13,41 @@
 class LLDBTest(TestFormat):
 def __init__(self, dotest_cmd):
 self.dotest_cmd = dotest_cmd
+self.tests = None
+
+def getTests(self, bin_dir):
+if self.tests:
+return self.tests
+dumper = os.path.join(os.path.dirname(__file__), "lldbtestdumper.py")
+lldb_path = os.path.join(bin_dir, "lldb")
+output = subprocess.check_output([sys.executable, dumper, lldb_path])
+self.tests = [line.split() for line in output.splitlines()]
+return self.tests
+
 
 def getTestsInDirectory(self, testSuite, path_in_suite, litConfig,
 localConfig):
-source_path = testSuite.getSourcePath(path_in_suite)
-for filename in os.listdir(source_path):
-# Ignore dot files and excluded tests.
-if (filename.startswith('.') or filename in localConfig.excludes):
-continue
-
-# Ignore files that don't start with 'Test'.
-if not filename.startswith('Test'):
+for test in self.getTests(testSuite.config.llvm_tools_dir):
+if test[0:-3] != list(path_in_suite):
 continue
-
-filepath = os.path.join(source_path, filename)
-if not os.path.isdir(filepath):
-base, ext = os.path.splitext(filename)
-if ext in localConfig.suffixes:
-yield lit.Test.Test(testSuite, path_in_suite +
-(filename, ), localConfig)
+yield lit.Test.Test(testSuite, test, localConfig)
 
 def execute(self, test, litConfig):
 if litConfig.noExecute:
 return lit.Test.PASS, ''
 
 if test.config.unsupported:
 return (lit.Test.UNSUPPORTED, 'Test is unsupported')
 
-testPath, testFile = os.path.split(test.getSourcePath())
+testDir = test.ge

[Lldb-commits] [PATCH] D47062: Suggest lldb-dotest to reproduce a failure.

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D47062#1109399, @JDevlieghere wrote:

> I'm going to hold off on this until we have decided what to do for 
> https://reviews.llvm.org/D46005. If we run the test cases as separate lit 
> invocations, then the test format is in a better position to report this 
> information.


It looks like that patch has gotten a bit stuck, and I'm not sure when/if I'll 
have the time to unstuck it. So, if you still want to go ahead with this 
approach, then feel free to commit it.


https://reviews.llvm.org/D47062



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

[Lldb-commits] [lldb] r334012 - DWARFIndex: more GetFunctions cleanup

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jun  5 05:13:22 2018
New Revision: 334012

URL: http://llvm.org/viewvc/llvm-project?rev=334012&view=rev
Log:
DWARFIndex: more GetFunctions cleanup

This applies similar simplification as r334004, only it touches the
regex version of the method.

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp?rev=334012&r1=334011&r2=334012&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp Tue Jun  5 
05:13:22 2018
@@ -217,21 +217,14 @@ void AppleDWARFIndex::GetFunctions(Const
   }
 }
 
-void AppleDWARFIndex::GetFunctions(
-const RegularExpression ®ex, DWARFDebugInfo &info,
-llvm::function_ref
-resolve_function,
-bool include_inlines, SymbolContextList &sc_list) {
+void AppleDWARFIndex::GetFunctions(const RegularExpression ®ex,
+   DIEArray &offsets) {
   if (!m_apple_names_up)
 return;
 
-  DIEArray offsets;
   DWARFMappedHash::DIEInfoArray hash_data;
-  if (m_apple_names_up->AppendAllDIEsThatMatchingRegex(regex, hash_data)) {
+  if (m_apple_names_up->AppendAllDIEsThatMatchingRegex(regex, hash_data))
 DWARFMappedHash::ExtractDIEArray(hash_data, offsets);
-ParseFunctions(offsets, info, resolve_function, include_inlines, sc_list);
-  }
 }
 
 void AppleDWARFIndex::ReportInvalidDIEOffset(dw_offset_t offset,

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h?rev=334012&r1=334011&r2=334012&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h Tue Jun  5 
05:13:22 2018
@@ -47,12 +47,7 @@ public:
 const CompilerDeclContext &parent_decl_ctx,
 uint32_t name_type_mask,
 std::vector &dies) override;
-  void GetFunctions(
-  const RegularExpression ®ex, DWARFDebugInfo &info,
-  llvm::function_ref
-  resolve_function,
-  bool include_inlines, SymbolContextList &sc_list) override;
+  void GetFunctions(const RegularExpression ®ex, DIEArray &offsets) 
override;
 
   void ReportInvalidDIEOffset(dw_offset_t offset,
   llvm::StringRef name) override;

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp?rev=334012&r1=334011&r2=334012&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp Tue Jun  5 
05:13:22 2018
@@ -15,17 +15,3 @@ using namespace lldb_private;
 using namespace lldb;
 
 DWARFIndex::~DWARFIndex() = default;
-
-void DWARFIndex::ParseFunctions(
-const DIEArray &offsets, DWARFDebugInfo &info,
-llvm::function_ref
-resolve_function,
-bool include_inlines, SymbolContextList &sc_list) {
-  const size_t num_matches = offsets.size();
-  for (size_t i = 0; i < num_matches; ++i) {
-DWARFDIE die = info.GetDIE(offsets[i]);
-if (die)
-  resolve_function(die, include_inlines, sc_list);
-  }
-}

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h?rev=334012&r1=334011&r2=334012&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFIndex.h Tue Jun  5 05:13:22 
2018
@@ -40,12 +40,8 @@ public:
 const CompilerDeclContext &parent_decl_ctx,
 uint32_t name_type_mask,
 std::vector &dies) = 0;
-  virtual void GetFunctions(
-  const RegularExpression ®ex, DWARFDebugInfo &info,
-  llvm::function_ref
-  resolve_function,
-  bool include_inlines, SymbolContextList &sc_list) = 0;
+  virtual void GetFunctions(const RegularExpress

[Lldb-commits] [PATCH] D40470: Protect DWARFCompileUnit::m_die_array by a new mutex

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

Thanks for https://reviews.llvm.org/rL333996 fixups.


Repository:
  rL LLVM

https://reviews.llvm.org/D40470



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

[Lldb-commits] [PATCH] D47629: [DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 149964.
labath added a comment.

This makes the setting non-experimental and renames it to "ignore-file-indexes".
I've also wired up the index dump methods to Module::Dump so that I could write
a test that apple indexes are indeed being used after this.


https://reviews.llvm.org/D47629

Files:
  include/lldb/Symbol/SymbolFile.h
  lit/SymbolFile/DWARF/apple-index-is-used.cpp
  source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/CMakeLists.txt
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Symbol/SymbolVendor.cpp

Index: source/Symbol/SymbolVendor.cpp
===
--- source/Symbol/SymbolVendor.cpp
+++ source/Symbol/SymbolVendor.cpp
@@ -392,6 +392,8 @@
   }
 }
 s->EOL();
+if (m_sym_file_ap)
+  m_sym_file_ap->Dump(*s);
 s->IndentMore();
 m_type_list.Dump(s, show_context);
 
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -316,6 +316,8 @@
   DIEInDeclContext(const lldb_private::CompilerDeclContext *parent_decl_ctx,
const DWARFDIE &die);
 
+  void Dump(lldb_private::Stream &s) override;
+
 protected:
   typedef llvm::DenseMap
   DIEToTypePtr;
@@ -402,8 +404,6 @@
   lldb::TypeSP GetTypeForDIE(const DWARFDIE &die,
  bool resolve_function_context = false);
 
-  void DumpIndexes();
-
   void SetDebugMapModule(const lldb::ModuleSP &module_sp) {
 m_debug_map_module_wp = module_sp;
   }
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -64,6 +64,7 @@
 #include "DWARFDeclContext.h"
 #include "DWARFFormValue.h"
 #include "DWARFUnit.h"
+#include "DebugNamesDWARFIndex.h"
 #include "LogChannelDWARF.h"
 #include "ManualDWARFIndex.h"
 #include "SymbolFileDWARFDebugMap.h"
@@ -111,11 +112,20 @@
 
 PropertyDefinition g_properties[] = {
 {"comp-dir-symlink-paths", OptionValue::eTypeFileSpecList, true, 0, nullptr,
- nullptr, "If the DW_AT_comp_dir matches any of these paths the symbolic "
-  "links will be resolved at DWARF parse time."},
-{nullptr, OptionValue::eTypeInvalid, false, 0, nullptr, nullptr, nullptr}};
+ nullptr,
+ "If the DW_AT_comp_dir matches any of these paths the symbolic "
+ "links will be resolved at DWARF parse time."},
+{"ignore-file-indexes", OptionValue::eTypeBoolean, true, 0, nullptr,
+ nullptr,
+ "Ignore indexes present in the object files and always index DWARF "
+ "manually."},
+{nullptr, OptionValue::eTypeInvalid, false, 0, nullptr, nullptr, nullptr},
+};
 
-enum { ePropertySymLinkPaths };
+enum {
+  ePropertySymLinkPaths,
+  ePropertyIgnoreIndexes,
+};
 
 class PluginProperties : public Properties {
 public:
@@ -135,6 +145,11 @@
 assert(option_value);
 return option_value->GetCurrentValue();
   }
+
+  bool IgnoreFileIndexes() const {
+return m_collection_sp->GetPropertyAtIndexAsBoolean(
+nullptr, ePropertyIgnoreIndexes, false);
+  }
 };
 
 typedef std::shared_ptr SymbolFileDWARFPropertiesSP;
@@ -432,6 +447,7 @@
 }
 
 void SymbolFileDWARF::InitializeObject() {
+  Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
   ModuleSP module_sp(m_obj_file->GetModule());
   if (module_sp) {
 const SectionList *section_list = module_sp->GetSectionList();
@@ -442,19 +458,38 @@
   m_obj_file->ReadSectionData(section, m_dwarf_data);
   }
 
-  DWARFDataExtractor apple_names, apple_namespaces, apple_types, apple_objc;
-  LoadSectionData(eSectionTypeDWARFAppleNames, apple_names);
-  LoadSectionData(eSectionTypeDWARFAppleNamespaces, apple_namespaces);
-  LoadSectionData(eSectionTypeDWARFAppleTypes, apple_types);
-  LoadSectionData(eSectionTypeDWARFAppleObjC, apple_objc);
+  if (!GetGlobalPluginProperties()->IgnoreFileIndexes()) {
+DWARFDataExtractor apple_names, apple_namespaces, apple_types, apple_objc;
+LoadSectionData(eSectionTypeDWARFAppleNames, apple_names);
+LoadSectionData(eSectionTypeDWARFAppleNamespaces, apple_namespaces);
+LoadSectionData(eSectionTypeDWARFAppleTypes, apple_types);
+LoadSectionData(eSectionTypeDWARFAppleObjC, apple_objc);
+
+m_index = AppleDWARFIndex::Create(
+*GetObjectFile()->GetModule(), apple_names, apple_namespaces,
+apple_types, apple_objc, get_debug_str_data());
 
-  m_index = AppleDWARFIndex::Create(*GetObjectFile()->GetModule(), apple_name

[Lldb-commits] [PATCH] D47781: DebugNamesDWARFIndex: Add ability to lookup variables

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, JDevlieghere.
Herald added a subscriber: aprantl.

This adds the ability to lookup variables to the DWARF v5 index class.
find-basic-variable.cpp test is extended to cover this scenario as well,
and I have added a new test which verifies that the dwarf v5 index class
is indeed used.


https://reviews.llvm.org/D47781

Files:
  lit/SymbolFile/DWARF/dwarf5-index-is-used.cpp
  lit/SymbolFile/DWARF/find-basic-variable.cpp
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h

Index: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
===
--- source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
@@ -11,6 +11,7 @@
 #define LLDB_DEBUGNAMESDWARFINDEX_H
 
 #include "Plugins/SymbolFile/DWARF/DWARFIndex.h"
+#include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
 #include "lldb/Utility/ConstString.h"
 #include "llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h"
 
@@ -23,9 +24,9 @@
 
   void Preload() override {}
 
-  void GetGlobalVariables(ConstString name, DIEArray &offsets) override {}
+  void GetGlobalVariables(ConstString name, DIEArray &offsets) override;
   void GetGlobalVariables(const RegularExpression ®ex,
-  DIEArray &offsets) override {}
+  DIEArray &offsets) override;
   void GetGlobalVariables(const DWARFUnit &cu, DIEArray &offsets) override {}
   void GetObjCMethods(ConstString class_name, DIEArray &offsets) override {}
   void GetCompleteObjCClass(ConstString class_name, bool must_be_implementation,
@@ -42,7 +43,7 @@
 
   void ReportInvalidDIEOffset(dw_offset_t offset,
   llvm::StringRef name) override {}
-  void Dump(Stream &s) override {}
+  void Dump(Stream &s) override;
 
 private:
   DebugNamesDWARFIndex(Module &module,
@@ -57,7 +58,12 @@
   DWARFDataExtractor m_debug_names_data;
   DWARFDataExtractor m_debug_str_data;
 
-  std::unique_ptr m_debug_names_up;
+  using DebugNames = llvm::DWARFDebugNames;
+  std::unique_ptr m_debug_names_up;
+
+  void Append(const DebugNames::Entry &entry, DIEArray &offsets);
+  void MaybeLogLookupError(llvm::Error error, const DebugNames::NameIndex &ni,
+   llvm::StringRef name);
 };
 
 } // namespace lldb_private
Index: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -9,6 +9,8 @@
 
 #include "Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h"
 
+#include "Plugins/Language/CPlusPlus/CPlusPlusLanguage.h"
+
 using namespace lldb_private;
 using namespace lldb;
 
@@ -23,11 +25,76 @@
 DebugNamesDWARFIndex::Create(Module &module, DWARFDataExtractor debug_names,
  DWARFDataExtractor debug_str,
  DWARFDebugInfo *debug_info) {
-  auto index_up = llvm::make_unique(ToLLVM(debug_names),
-   ToLLVM(debug_str));
+  auto index_up =
+  llvm::make_unique(ToLLVM(debug_names), ToLLVM(debug_str));
   if (llvm::Error E = index_up->extract())
 return std::move(E);
 
   return std::unique_ptr(new DebugNamesDWARFIndex(
   module, std::move(index_up), debug_names, debug_str, debug_info));
 }
+
+void DebugNamesDWARFIndex::Append(const DebugNames::Entry &entry,
+  DIEArray &offsets) {
+  llvm::Optional cu_offset = entry.getCUOffset();
+  llvm::Optional die_offset = entry.getDIESectionOffset();
+  if (cu_offset && die_offset)
+offsets.emplace_back(*cu_offset, *die_offset);
+}
+
+void DebugNamesDWARFIndex::MaybeLogLookupError(llvm::Error error,
+   const DebugNames::NameIndex &ni,
+   llvm::StringRef name) {
+  // Ignore SentinelErrors, log everything else.
+  LLDB_LOG_ERROR(
+  LogChannelDWARF::GetLogIfAll(DWARF_LOG_LOOKUPS),
+  handleErrors(std::move(error), [](const DebugNames::SentinelError &) {}),
+  "Failed to parse index entries for index at {1:x}, name {2}: {0}",
+  ni.getUnitOffset(), name);
+}
+
+void DebugNamesDWARFIndex::GetGlobalVariables(ConstString name,
+  DIEArray &offsets) {
+  const char *name_cstr = name.GetCString();
+  llvm::StringRef basename;
+  llvm::StringRef context;
+
+  if (!CPlusPlusLanguage::ExtractContextAndIdentifier(name_cstr, context,
+  basename))
+basename = name_cstr;
+
+  for (const DebugNames::Entry &entry :
+   m_debug_names_up->equal_range(basename)) {
+if (entry.tag() != DW_TAG_variable)
+  continue;
+
+Append(entry, offsets);
+  }
+}
+
+void D

[Lldb-commits] [lldb] r334025 - [lit, pdb] Fix func-symbols.test (on Windows)

[Stella Stamenova via lldb-commits]

Author: stella.stamenova
Date: Tue Jun  5 09:20:36 2018
New Revision: 334025

URL: http://llvm.org/viewvc/llvm-project?rev=334025&view=rev
Log:
[lit, pdb] Fix func-symbols.test (on Windows)

Summary: This test was failing sporadically on windows because the order in 
which the symbols are generated was different between builds. To fix the test, 
we need to run FileCheck twice - once for each set of symbols we want to 
verify. The test only runs on Windows.

Reviewers: asmith, zturner, labath

Subscribers: stella.stamenova, llvm-commits

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

Modified:
lldb/trunk/lit/SymbolFile/PDB/func-symbols.test

Modified: lldb/trunk/lit/SymbolFile/PDB/func-symbols.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/SymbolFile/PDB/func-symbols.test?rev=334025&r1=334024&r2=334025&view=diff
==
--- lldb/trunk/lit/SymbolFile/PDB/func-symbols.test (original)
+++ lldb/trunk/lit/SymbolFile/PDB/func-symbols.test Tue Jun  5 09:20:36 2018
@@ -2,46 +2,47 @@ REQUIRES: windows
 RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/FuncSymbolsTestMain.cpp /o 
%T/FuncSymbolsTestMain.cpp.obj
 RUN: clang-cl -m32 /Z7 /c /GS- %S/Inputs/FuncSymbols.cpp /o 
%T/FuncSymbols.cpp.obj
 RUN: link %T/FuncSymbolsTestMain.cpp.obj %T/FuncSymbols.cpp.obj /DEBUG 
/nodefaultlib /Entry:main /OUT:%T/FuncSymbolsTest.exe
-RUN: lldb-test symbols %T/FuncSymbolsTest.exe | FileCheck %s
+RUN: lldb-test symbols %T/FuncSymbolsTest.exe | FileCheck 
--check-prefix=CHECK-ONE %s
+RUN: lldb-test symbols %T/FuncSymbolsTest.exe | FileCheck 
--check-prefix=CHECK-TWO %s
 
 ; Link multiple objects
 ; In this test, We don't check demangled name of a mangled function.
 
-CHECK: Module [[MD:.*]]
-CHECK-DAG: {{.*}}: SymbolVendor ([[MD]])
-CHECK-DAG: [[TY0:.*]]:   Type{[[UID0:.*]]} , name = "Func_arg_array", decl = 
FuncSymbolsTestMain.cpp:3, compiler_type = {{.*}} int (int *)
-CHECK-DAG: [[TY1:.*]]:   Type{[[UID1:.*]]} , name = "Func_arg_void", decl = 
FuncSymbolsTestMain.cpp:4, compiler_type = {{.*}} void (void)
-CHECK-DAG: [[TY2:.*]]:   Type{[[UID2:.*]]} , name = "Func_arg_none", decl = 
FuncSymbolsTestMain.cpp:5, compiler_type = {{.*}} void (void)
-CHECK-DAG: [[TY3:.*]]:   Type{[[UID3:.*]]} , name = "Func_varargs", decl = 
FuncSymbolsTestMain.cpp:6, compiler_type = {{.*}} void (...)
-CHECK-DAG: [[TY4:.*]]:   Type{[[UID4:.*]]} , name = "NS::Func", decl = 
FuncSymbolsTestMain.cpp:28, compiler_type = {{.*}} void (signed char, int)
-CHECK-DAG: [[TY5:.*]]:   Type{[[UID5:.*]]} , name = "main", decl = 
FuncSymbolsTestMain.cpp:44, compiler_type = {{.*}} int (void)
-CHECK-DAG: [[TY6:.*]]:   Type{[[UID6:.*]]} , name = "`anonymous 
namespace'::Func", decl = FuncSymbolsTestMain.cpp:24, compiler_type = {{.*}} 
void (int, const long, volatile _Bool, ...)
-CHECK-DAG: [[TY7:.*]]:   Type{[[UID7:.*]]} , name = "StaticFunction", decl = 
FuncSymbolsTestMain.cpp:35, compiler_type = {{.*}} long (int)
-CHECK-DAG: [[TY8:.*]]:   Type{[[UID8:.*]]} , name = "MemberTest::A::Func", 
decl = FuncSymbolsTestMain.cpp:12, compiler_type = {{.*}} int (int, ...)
-CHECK-DAG: [[TY9:.*]]:   Type{[[UID9:.*]]} , name = "TemplateFunc<1,int>", 
decl = FuncSymbolsTestMain.cpp:18, compiler_type = {{.*}} void (int)
-CHECK-DAG: [[TY10:.*]]:   Type{[[UID10:.*]]} , name = 
"TemplateFunc<1,int,int,int>", decl = FuncSymbolsTestMain.cpp:18, compiler_type 
= {{.*}} void (int, int, int)
-CHECK-DAG: [[TY11:.*]]:   Type{[[UID11:.*]]} , name = "InlinedFunction", decl 
= FuncSymbolsTestMain.cpp:40, compiler_type = {{.*}} void (long)
+CHECK-ONE: Module [[MD:.*]]
+CHECK-ONE-DAG: {{.*}}: SymbolVendor ([[MD]])
+CHECK-ONE-DAG: [[TY0:.*]]:   Type{[[UID0:.*]]} , name = "Func_arg_array", decl 
= FuncSymbolsTestMain.cpp:3, compiler_type = {{.*}} int (int *)
+CHECK-ONE-DAG: [[TY1:.*]]:   Type{[[UID1:.*]]} , name = "Func_arg_void", decl 
= FuncSymbolsTestMain.cpp:4, compiler_type = {{.*}} void (void)
+CHECK-ONE-DAG: [[TY2:.*]]:   Type{[[UID2:.*]]} , name = "Func_arg_none", decl 
= FuncSymbolsTestMain.cpp:5, compiler_type = {{.*}} void (void)
+CHECK-ONE-DAG: [[TY3:.*]]:   Type{[[UID3:.*]]} , name = "Func_varargs", decl = 
FuncSymbolsTestMain.cpp:6, compiler_type = {{.*}} void (...)
+CHECK-ONE-DAG: [[TY4:.*]]:   Type{[[UID4:.*]]} , name = "NS::Func", decl = 
FuncSymbolsTestMain.cpp:28, compiler_type = {{.*}} void (signed char, int)
+CHECK-ONE-DAG: [[TY5:.*]]:   Type{[[UID5:.*]]} , name = "main", decl = 
FuncSymbolsTestMain.cpp:44, compiler_type = {{.*}} int (void)
+CHECK-ONE-DAG: [[TY6:.*]]:   Type{[[UID6:.*]]} , name = "`anonymous 
namespace'::Func", decl = FuncSymbolsTestMain.cpp:24, compiler_type = {{.*}} 
void (int, const long, volatile _Bool, ...)
+CHECK-ONE-DAG: [[TY7:.*]]:   Type{[[UID7:.*]]} , name = "StaticFunction", decl 
= FuncSymbolsTestMain.cpp:35, compiler_type = {{.*}} long (int)
+CHECK-ONE-DAG: [[TY8:.*]]:   Type{[[UID8:.*]]} , name = "MemberTest::A::Func", 
decl = FuncSymbolsTestMain.cpp:12, compiler_ty

[Lldb-commits] [PATCH] D47629: [DWARF] Add (empty) DebugNamesDWARFIndex class and a setting to control its use

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.

I like this new setting much better. Looks good.


https://reviews.llvm.org/D47629



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

[Lldb-commits] [PATCH] D47781: DebugNamesDWARFIndex: Add ability to lookup variables

[Greg Clayton via Phabricator via lldb-commits]

clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.

Check the context as noted in inline comments.




Comment at: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp:70
+  continue;
+
+Append(entry, offsets);

Need to check for the context matches here if context is not empty.



Comment at: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h:14
 #include "Plugins/SymbolFile/DWARF/DWARFIndex.h"
+#include "Plugins/SymbolFile/DWARF/LogChannelDWARF.h"
 #include "lldb/Utility/ConstString.h"

move to .cpp file?


https://reviews.llvm.org/D47781



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

[Lldb-commits] [PATCH] D47708: PDB support of function-level linking and splitted functions

[Aaron Smith via Phabricator via lldb-commits]

asmith accepted this revision.
asmith added a comment.
This revision is now accepted and ready to land.

LGTM - will commit for you


https://reviews.llvm.org/D47708



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

[Lldb-commits] [PATCH] D47791: Initial support for Hexagon target.

[Sid Manning via Phabricator via lldb-commits]

sidneym created this revision.
sidneym added reviewers: ruiu, shankar.easwaran, kparzysz, bcahoon.
sidneym added a project: lld.
Herald added subscribers: MaskRay, arichardson, mgorny, emaste.
Herald added a reviewer: espindola.

Support B22_PCREL relocation along with a simple test.

I'm also working on a llvmSupport patch that will detail the instruction set 
encoding.  Many of Hexagon's instructions have immediates that are peppered 
throughout the word makeing application of fixups tedious since the same 
relocation fixup may need to be placed in different bits depending on the 
instruction.  That change will be made in llvmSupport so that both lld and 
llvm-rtdyld can share encoding information.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D47791

Files:
  ELF/Arch/Hexagon.cpp
  ELF/CMakeLists.txt
  ELF/Target.cpp
  ELF/Target.h
  test/ELF/Inputs/hexagon.s
  test/ELF/hexagon.s
  test/lit.cfg.py

Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -65,6 +65,7 @@
   'AMDGPU': 'amdgpu',
   'ARM': 'arm',
   'AVR': 'avr',
+  'Hexagon': 'hexagon',
   'Mips': 'mips',
   'PowerPC': 'ppc',
   'Sparc': 'sparc',
Index: test/ELF/hexagon.s
===
--- /dev/null
+++ test/ELF/hexagon.s
@@ -0,0 +1,9 @@
+# REQUIRES: hexagon
+# RUN: llvm-mc  -filetype=obj -triple=hexagon-unknown-elf %s -o %t
+# RUN: llvm-mc  -filetype=obj -triple=hexagon-unknown-elf %S/Inputs/hexagon.s -o %t2
+# RUN: ld.lld %t2 %t  -o %t3
+# RUN: llvm-objdump -d  %t3 | FileCheck %s
+
+# R_HEX_B22_PCREL
+call #_start
+# CHECK:call 0x11000
Index: test/ELF/Inputs/hexagon.s
===
--- /dev/null
+++ test/ELF/Inputs/hexagon.s
@@ -0,0 +1,7 @@
+
+.global _start
+_start:
+  nop
+.global foo
+foo:
+  jumpr lr
Index: ELF/Target.h
===
--- ELF/Target.h
+++ ELF/Target.h
@@ -134,6 +134,7 @@
 TargetInfo *getAMDGPUTargetInfo();
 TargetInfo *getARMTargetInfo();
 TargetInfo *getAVRTargetInfo();
+TargetInfo *getHexagonTargetInfo();
 TargetInfo *getPPC64TargetInfo();
 TargetInfo *getPPCTargetInfo();
 TargetInfo *getSPARCV9TargetInfo();
Index: ELF/Target.cpp
===
--- ELF/Target.cpp
+++ ELF/Target.cpp
@@ -60,6 +60,8 @@
 return getARMTargetInfo();
   case EM_AVR:
 return getAVRTargetInfo();
+  case EM_HEXAGON:
+return getHexagonTargetInfo();
   case EM_MIPS:
 switch (Config->EKind) {
 case ELF32LEKind:
Index: ELF/CMakeLists.txt
===
--- ELF/CMakeLists.txt
+++ ELF/CMakeLists.txt
@@ -12,6 +12,7 @@
   Arch/AMDGPU.cpp
   Arch/ARM.cpp
   Arch/AVR.cpp
+  Arch/Hexagon.cpp
   Arch/Mips.cpp
   Arch/MipsArchTree.cpp
   Arch/PPC.cpp
Index: ELF/Arch/Hexagon.cpp
===
--- /dev/null
+++ ELF/Arch/Hexagon.cpp
@@ -0,0 +1,87 @@
+//===-- Hexagon.cpp ---===//
+//
+// The LLVM Linker
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "InputFiles.h"
+#include "Symbols.h"
+#include "Target.h"
+#include "lld/Common/ErrorHandler.h"
+#include "llvm/BinaryFormat/ELF.h"
+#include "llvm/Object/ELF.h"
+#include "llvm/Support/Endian.h"
+
+using namespace llvm;
+using namespace llvm::object;
+using namespace llvm::support::endian;
+using namespace llvm::ELF;
+using namespace lld;
+using namespace lld::elf;
+
+namespace {
+class HexagonReloc final : public TargetInfo {
+public:
+  uint32_t calcEFlags() const override;
+  uint32_t applyMask(uint32_t Mask, uint32_t Data) const;
+  RelExpr getRelExpr(RelType Type, const Symbol &S,
+ const uint8_t *Loc) const override;
+  void relocateOne(uint8_t *Loc, RelType Type, uint64_t Val) const override;
+};
+} // namespace
+
+// Support V60 only at the moment.
+uint32_t HexagonReloc::calcEFlags() const {
+  assert(!ObjectFiles.empty());
+  return 0x60;
+}
+
+uint32_t HexagonReloc::applyMask(uint32_t Mask, uint32_t Data) const {
+  uint32_t Result = 0;
+  size_t Off = 0;
+
+  for (size_t Bit = 0; Bit != sizeof(uint32_t) * 8; ++Bit) {
+const bool ValBit = (Data >> Off) & 1;
+const bool MaskBit = (Mask >> Bit) & 1;
+if (MaskBit) {
+  Result |= (ValBit << Bit);
+  ++Off;
+}
+  }
+  return Result;
+}
+
+RelExpr HexagonReloc::getRelExpr(RelType Type, const Symbol &S,
+ const uint8_t *Loc) const {
+  switch (Typ

[Lldb-commits] [PATCH] D47792: Fix up Info.plist when building LLDB.framework with CMake

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added reviewers: sas, clayborg.
Herald added a subscriber: mgorny.

We weren't using the Info.plist template in resources previously.
When using that template, some of the key's values weren't being populated
because some variables were not being defined. In one case, CMake didn't
like the substring expansion syntax of CFBundleIdentifier so I got rid of that.


https://reviews.llvm.org/D47792

Files:
  resources/LLDB-Info.plist
  source/API/CMakeLists.txt


Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -179,10 +179,14 @@
 COMMAND ${LLDB_SOURCE_DIR}/scripts/framework-header-fix.sh 
${CMAKE_CURRENT_BINARY_DIR}/FrameworkHeaders ${LLDB_VERSION})
   add_dependencies(liblldb lldb-framework-headers)
 
+  set(PRODUCT_NAME "LLDB")
+  set(EXECUTABLE_NAME "LLDB")
+  set(CURRENT_PROJECT_VERSION "360.99.0")
   set_target_properties(liblldb PROPERTIES
 OUTPUT_NAME LLDB
 FRAMEWORK On
 FRAMEWORK_VERSION ${LLDB_FRAMEWORK_VERSION}
+MACOSX_FRAMEWORK_INFO_PLIST ${LLDB_SOURCE_DIR}/resources/LLDB-Info.plist
 LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${LLDB_FRAMEWORK_INSTALL_DIR}
 PUBLIC_HEADER "${framework_headers}")
 
Index: resources/LLDB-Info.plist
===
--- resources/LLDB-Info.plist
+++ resources/LLDB-Info.plist
@@ -7,7 +7,7 @@
CFBundleExecutable
${EXECUTABLE_NAME}
CFBundleIdentifier
-   com.apple.${PRODUCT_NAME:rfc1034identifier}.framework
+   com.apple.${PRODUCT_NAME}.framework
CFBundleInfoDictionaryVersion
6.0
CFBundlePackageType


Index: source/API/CMakeLists.txt
===
--- source/API/CMakeLists.txt
+++ source/API/CMakeLists.txt
@@ -179,10 +179,14 @@
 COMMAND ${LLDB_SOURCE_DIR}/scripts/framework-header-fix.sh ${CMAKE_CURRENT_BINARY_DIR}/FrameworkHeaders ${LLDB_VERSION})
   add_dependencies(liblldb lldb-framework-headers)
 
+  set(PRODUCT_NAME "LLDB")
+  set(EXECUTABLE_NAME "LLDB")
+  set(CURRENT_PROJECT_VERSION "360.99.0")
   set_target_properties(liblldb PROPERTIES
 OUTPUT_NAME LLDB
 FRAMEWORK On
 FRAMEWORK_VERSION ${LLDB_FRAMEWORK_VERSION}
+MACOSX_FRAMEWORK_INFO_PLIST ${LLDB_SOURCE_DIR}/resources/LLDB-Info.plist
 LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/${LLDB_FRAMEWORK_INSTALL_DIR}
 PUBLIC_HEADER "${framework_headers}")
 
Index: resources/LLDB-Info.plist
===
--- resources/LLDB-Info.plist
+++ resources/LLDB-Info.plist
@@ -7,7 +7,7 @@
 	CFBundleExecutable
 	${EXECUTABLE_NAME}
 	CFBundleIdentifier
-	com.apple.${PRODUCT_NAME:rfc1034identifier}.framework
+	com.apple.${PRODUCT_NAME}.framework
 	CFBundleInfoDictionaryVersion
 	6.0
 	CFBundlePackageType
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47791: Initial support for Hexagon target.

[Rui Ueyama via Phabricator via lldb-commits]

ruiu added inline comments.



Comment at: ELF/Arch/Hexagon.cpp:46
+
+  for (size_t Bit = 0; Bit != sizeof(uint32_t) * 8; ++Bit) {
+const bool ValBit = (Data >> Off) & 1;

sizeof(uint32_t) * 8 is always 32.



Comment at: ELF/Arch/Hexagon.cpp:77
+  }
+
+  default:

Remove extraneous blank line. If you take a look at other files in lld, you'd 
notice that we normally don't add a blank line before "default:".



Comment at: test/ELF/Inputs/hexagon.s:1
+
+.global _start

Remove a leading empty line.



Comment at: test/ELF/hexagon.s:2
+# REQUIRES: hexagon
+# RUN: llvm-mc  -filetype=obj -triple=hexagon-unknown-elf %s -o %t
+# RUN: llvm-mc  -filetype=obj -triple=hexagon-unknown-elf %S/Inputs/hexagon.s 
-o %t2

Remove extranesous space between "mc" and "-filetype"



Comment at: test/ELF/hexagon.s:9
+call #_start
+# CHECK:call 0x11000

Remove extraneous space characters.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D47791



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

[Lldb-commits] [PATCH] D47791: Initial support for Hexagon target.

[Sid Manning via Phabricator via lldb-commits]

sidneym updated this revision to Diff 150013.
sidneym added a comment.

Made the changes Rui suggested.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D47791

Files:
  ELF/Arch/Hexagon.cpp
  ELF/CMakeLists.txt
  ELF/Target.cpp
  ELF/Target.h
  test/ELF/Inputs/hexagon.s
  test/ELF/hexagon.s
  test/lit.cfg.py

Index: test/lit.cfg.py
===
--- test/lit.cfg.py
+++ test/lit.cfg.py
@@ -65,6 +65,7 @@
   'AMDGPU': 'amdgpu',
   'ARM': 'arm',
   'AVR': 'avr',
+  'Hexagon': 'hexagon',
   'Mips': 'mips',
   'PowerPC': 'ppc',
   'Sparc': 'sparc',
Index: test/ELF/hexagon.s
===
--- /dev/null
+++ test/ELF/hexagon.s
@@ -0,0 +1,9 @@
+# REQUIRES: hexagon
+# RUN: llvm-mc -filetype=obj -triple=hexagon-unknown-elf %s -o %t
+# RUN: llvm-mc -filetype=obj -triple=hexagon-unknown-elf %S/Inputs/hexagon.s -o %t2
+# RUN: ld.lld %t2 %t  -o %t3
+# RUN: llvm-objdump -d  %t3 | FileCheck %s
+
+# R_HEX_B22_PCREL
+call #_start
+# CHECK: call 0x11000
Index: test/ELF/Inputs/hexagon.s
===
--- /dev/null
+++ test/ELF/Inputs/hexagon.s
@@ -0,0 +1,6 @@
+.global _start
+_start:
+  nop
+.global foo
+foo:
+  jumpr lr
Index: ELF/Target.h
===
--- ELF/Target.h
+++ ELF/Target.h
@@ -134,6 +134,7 @@
 TargetInfo *getAMDGPUTargetInfo();
 TargetInfo *getARMTargetInfo();
 TargetInfo *getAVRTargetInfo();
+TargetInfo *getHexagonTargetInfo();
 TargetInfo *getPPC64TargetInfo();
 TargetInfo *getPPCTargetInfo();
 TargetInfo *getSPARCV9TargetInfo();
Index: ELF/Target.cpp
===
--- ELF/Target.cpp
+++ ELF/Target.cpp
@@ -60,6 +60,8 @@
 return getARMTargetInfo();
   case EM_AVR:
 return getAVRTargetInfo();
+  case EM_HEXAGON:
+return getHexagonTargetInfo();
   case EM_MIPS:
 switch (Config->EKind) {
 case ELF32LEKind:
Index: ELF/CMakeLists.txt
===
--- ELF/CMakeLists.txt
+++ ELF/CMakeLists.txt
@@ -12,6 +12,7 @@
   Arch/AMDGPU.cpp
   Arch/ARM.cpp
   Arch/AVR.cpp
+  Arch/Hexagon.cpp
   Arch/Mips.cpp
   Arch/MipsArchTree.cpp
   Arch/PPC.cpp
Index: ELF/Arch/Hexagon.cpp
===
--- /dev/null
+++ ELF/Arch/Hexagon.cpp
@@ -0,0 +1,86 @@
+//===-- Hexagon.cpp ---===//
+//
+// The LLVM Linker
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "InputFiles.h"
+#include "Symbols.h"
+#include "Target.h"
+#include "lld/Common/ErrorHandler.h"
+#include "llvm/BinaryFormat/ELF.h"
+#include "llvm/Object/ELF.h"
+#include "llvm/Support/Endian.h"
+
+using namespace llvm;
+using namespace llvm::object;
+using namespace llvm::support::endian;
+using namespace llvm::ELF;
+using namespace lld;
+using namespace lld::elf;
+
+namespace {
+class HexagonReloc final : public TargetInfo {
+public:
+  uint32_t calcEFlags() const override;
+  uint32_t applyMask(uint32_t Mask, uint32_t Data) const;
+  RelExpr getRelExpr(RelType Type, const Symbol &S,
+ const uint8_t *Loc) const override;
+  void relocateOne(uint8_t *Loc, RelType Type, uint64_t Val) const override;
+};
+} // namespace
+
+// Support V60 only at the moment.
+uint32_t HexagonReloc::calcEFlags() const {
+  assert(!ObjectFiles.empty());
+  return 0x60;
+}
+
+uint32_t HexagonReloc::applyMask(uint32_t Mask, uint32_t Data) const {
+  uint32_t Result = 0;
+  size_t Off = 0;
+
+  for (size_t Bit = 0; Bit != 32; ++Bit) {
+const bool ValBit = (Data >> Off) & 1;
+const bool MaskBit = (Mask >> Bit) & 1;
+if (MaskBit) {
+  Result |= (ValBit << Bit);
+  ++Off;
+}
+  }
+  return Result;
+}
+
+RelExpr HexagonReloc::getRelExpr(RelType Type, const Symbol &S,
+ const uint8_t *Loc) const {
+  switch (Type) {
+  case R_HEX_B22_PCREL:
+return R_PC;
+  default:
+return R_ABS;
+  }
+}
+
+void HexagonReloc::relocateOne(uint8_t *Loc, RelType Type, uint64_t Val) const {
+  switch (Type) {
+  case R_HEX_NONE:
+break;
+  case R_HEX_B22_PCREL: {
+uint32_t EffectiveValue = (Val >> 2) & 0x3f;
+EffectiveValue = applyMask(0x01ff3ffe, EffectiveValue);
+write32le(Loc, (read32le(Loc) | EffectiveValue));
+break;
+  }
+  default:
+error(getErrorLocation(Loc) + "unrecognized reloc " + toString(Type));
+break;
+  }
+}
+
+TargetInfo *elf::getHexagonTargetInfo() {
+  static HexagonReloc Target;
+  return &

[Lldb-commits] [PATCH] D47791: Initial support for Hexagon target.

[Rui Ueyama via Phabricator via lldb-commits]

ruiu added inline comments.



Comment at: ELF/Arch/Hexagon.cpp:47
+  for (size_t Bit = 0; Bit != 32; ++Bit) {
+const bool ValBit = (Data >> Off) & 1;
+const bool MaskBit = (Mask >> Bit) & 1;

We normally don't add `const` if the variable's scope is narrow.



Comment at: ELF/Arch/Hexagon.cpp:72
+  case R_HEX_B22_PCREL: {
+uint32_t EffectiveValue = (Val >> 2) & 0x3f;
+EffectiveValue = applyMask(0x01ff3ffe, EffectiveValue);

We usually use much shorter variable name for a variable whose scope is very 
narrow. Just `V` would be enough for this.




Comment at: ELF/Arch/Hexagon.cpp:74
+EffectiveValue = applyMask(0x01ff3ffe, EffectiveValue);
+write32le(Loc, (read32le(Loc) | EffectiveValue));
+break;

I believe we have `or32le`.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D47791



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

[Lldb-commits] [PATCH] D47781: DebugNamesDWARFIndex: Add ability to lookup variables

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp:70
+  continue;
+
+Append(entry, offsets);

clayborg wrote:
> Need to check for the context matches here if context is not empty.
Hmm... thanks for pointing this out. There is some index-independent filtering 
based on CompilerDeclContexts happening in SymbolFileDWARF (which is why my 
context test actually passes here).

I was basing this off of how apple and manual indexes work, and neither of them 
check the context. In fact, the manual index does not even extract the 
identifier portion of the name (it just looks up the whole string), so maybe 
nobody passes qualified names names into this function anyway? I am starting to 
think I should just remove the `ExtractContextAndIdentifier` call. If we see 
failures down the line, at least we will learn why was this call there in the 
first place.


https://reviews.llvm.org/D47781



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

[Lldb-commits] [PATCH] D47781: DebugNamesDWARFIndex: Add ability to lookup variables

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Feel free to check into it and modify the contract for GetGlobalVariables and 
possibly rename "name" to "basename" to be clear what the argument is. Then 
modify the headerdoc to clearly state what we are looking for if the filtering 
is going on higher up


https://reviews.llvm.org/D47781



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

[Lldb-commits] [PATCH] D47797: [lldb-mi] Re-implement MI -exec-next command.

[Alexander Polyakov via Phabricator via lldb-commits]

polyakov.alex created this revision.
polyakov.alex added reviewers: aprantl, clayborg.
Herald added a subscriber: ki.stfu.

Now -exec-next command uses SB API for stepping over.


https://reviews.llvm.org/D47797

Files:
  lit/tools/lldb-mi/exec/exec-next.test
  lit/tools/lldb-mi/exec/inputs/main.c
  tools/lldb-mi/MICmdCmdExec.cpp


Index: tools/lldb-mi/MICmdCmdExec.cpp
===
--- tools/lldb-mi/MICmdCmdExec.cpp
+++ tools/lldb-mi/MICmdCmdExec.cpp
@@ -22,6 +22,7 @@
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/API/SBThread.h"
 #include "lldb/lldb-enumerations.h"
 
 // In-house headers:
@@ -378,12 +379,11 @@
 
   CMICmnLLDBDebugSessionInfo &rSessionInfo(
   CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBDebugger &rDebugger = rSessionInfo.GetDebugger();
-  CMIUtilString strCmd("thread step-over");
+
   if (nThreadId != UINT64_MAX)
-strCmd += CMIUtilString::Format(" %llu", nThreadId);
-  rDebugger.GetCommandInterpreter().HandleCommand(strCmd.c_str(), m_lldbResult,
-  false);
+rSessionInfo.GetProcess().GetThreadByID(nThreadId).StepOver();
+  else
+rSessionInfo.GetProcess().GetSelectedThread().StepOver();
 
   return MIstatus::success;
 }
Index: lit/tools/lldb-mi/exec/inputs/main.c
===
--- /dev/null
+++ lit/tools/lldb-mi/exec/inputs/main.c
@@ -0,0 +1,4 @@
+int main(void) {
+  int x = 0;
+  return x;
+}
Index: lit/tools/lldb-mi/exec/exec-next.test
===
--- /dev/null
+++ lit/tools/lldb-mi/exec/exec-next.test
@@ -0,0 +1,21 @@
+# XFAIL: windows
+# -> llvm.org/pr24452
+#
+# RUN: %cc -o %t %p/inputs/main.c -g
+# RUN: %lldbmi %t < %s | FileCheck %s
+
+# Test lldb-mi -exec-next command.
+
+# Check that we have a valid target created via '%lldbmi %t'.
+# CHECK: ^done
+
+-break-insert main
+# CHECK: ^done,bkpt={number="1"
+
+-exec-run
+# CHECK: ^running
+# CHECK: *stopped,reason="breakpoint-hit"
+
+-exec-next
+# CHECK: ^running
+# CHECK: *stopped,reason="end-stepping-range"


Index: tools/lldb-mi/MICmdCmdExec.cpp
===
--- tools/lldb-mi/MICmdCmdExec.cpp
+++ tools/lldb-mi/MICmdCmdExec.cpp
@@ -22,6 +22,7 @@
 #include "lldb/API/SBCommandInterpreter.h"
 #include "lldb/API/SBProcess.h"
 #include "lldb/API/SBStream.h"
+#include "lldb/API/SBThread.h"
 #include "lldb/lldb-enumerations.h"
 
 // In-house headers:
@@ -378,12 +379,11 @@
 
   CMICmnLLDBDebugSessionInfo &rSessionInfo(
   CMICmnLLDBDebugSessionInfo::Instance());
-  lldb::SBDebugger &rDebugger = rSessionInfo.GetDebugger();
-  CMIUtilString strCmd("thread step-over");
+
   if (nThreadId != UINT64_MAX)
-strCmd += CMIUtilString::Format(" %llu", nThreadId);
-  rDebugger.GetCommandInterpreter().HandleCommand(strCmd.c_str(), m_lldbResult,
-  false);
+rSessionInfo.GetProcess().GetThreadByID(nThreadId).StepOver();
+  else
+rSessionInfo.GetProcess().GetSelectedThread().StepOver();
 
   return MIstatus::success;
 }
Index: lit/tools/lldb-mi/exec/inputs/main.c
===
--- /dev/null
+++ lit/tools/lldb-mi/exec/inputs/main.c
@@ -0,0 +1,4 @@
+int main(void) {
+  int x = 0;
+  return x;
+}
Index: lit/tools/lldb-mi/exec/exec-next.test
===
--- /dev/null
+++ lit/tools/lldb-mi/exec/exec-next.test
@@ -0,0 +1,21 @@
+# XFAIL: windows
+# -> llvm.org/pr24452
+#
+# RUN: %cc -o %t %p/inputs/main.c -g
+# RUN: %lldbmi %t < %s | FileCheck %s
+
+# Test lldb-mi -exec-next command.
+
+# Check that we have a valid target created via '%lldbmi %t'.
+# CHECK: ^done
+
+-break-insert main
+# CHECK: ^done,bkpt={number="1"
+
+-exec-run
+# CHECK: ^running
+# CHECK: *stopped,reason="breakpoint-hit"
+
+-exec-next
+# CHECK: ^running
+# CHECK: *stopped,reason="end-stepping-range"
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47801: Make lldb tools dependent on liblldb target when building LLDB.framework with CMake

[Alex Langford via Phabricator via lldb-commits]

xiaobai created this revision.
xiaobai added reviewers: sas, clayborg, labath.
Herald added a subscriber: mgorny.

When you build LLDB.framework with cmake, the liblldb target is built
as a framework. When lldb tools are built with INCLUDE_IN_FRAMEWORK, then
LLDB.framework should depend on those tools. This means making liblldb depend on
those tools.


https://reviews.llvm.org/D47801

Files:
  cmake/modules/AddLLDB.cmake


Index: cmake/modules/AddLLDB.cmake
===
--- cmake/modules/AddLLDB.cmake
+++ cmake/modules/AddLLDB.cmake
@@ -105,6 +105,7 @@
 set(resource_dir "/Resources")
 set(resource_dots "../")
   endif()
+  add_dependencies(liblldb ${name})
   string(REGEX REPLACE "[^/]+" ".." _dots ${LLDB_FRAMEWORK_INSTALL_DIR})
   set_target_properties(${name} PROPERTIES
 RUNTIME_OUTPUT_DIRECTORY $${resource_dir}


Index: cmake/modules/AddLLDB.cmake
===
--- cmake/modules/AddLLDB.cmake
+++ cmake/modules/AddLLDB.cmake
@@ -105,6 +105,7 @@
 set(resource_dir "/Resources")
 set(resource_dots "../")
   endif()
+  add_dependencies(liblldb ${name})
   string(REGEX REPLACE "[^/]+" ".." _dots ${LLDB_FRAMEWORK_INSTALL_DIR})
   set_target_properties(${name} PROPERTIES
 RUNTIME_OUTPUT_DIRECTORY $${resource_dir}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D47797: [lldb-mi] Re-implement MI -exec-next command.

[Adrian Prantl via Phabricator via lldb-commits]

aprantl accepted this revision.
aprantl added inline comments.
This revision is now accepted and ready to land.



Comment at: lit/tools/lldb-mi/exec/exec-next.test:2
+# XFAIL: windows
+# -> llvm.org/pr24452
+#

@stella.stemanova: Would be interesting to understand why (/if ?) this doesn't 
work on windows


https://reviews.llvm.org/D47797



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

[Lldb-commits] [PATCH] D47801: Make lldb tools dependent on liblldb target when building LLDB.framework with CMake

[Stephane Sezer via Phabricator via lldb-commits]

sas added a comment.

Don't we risk creating circular dependencies with this? What happens when a 
tool depends on liblldb? you'll have theTool depend on liblldb and liblldb 
depend on theTool.


https://reviews.llvm.org/D47801



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

[Lldb-commits] [PATCH] D47792: Fix up Info.plist when building LLDB.framework with CMake

[Stephane Sezer via Phabricator via lldb-commits]

sas added inline comments.



Comment at: source/API/CMakeLists.txt:184
+  set(EXECUTABLE_NAME "LLDB")
+  set(CURRENT_PROJECT_VERSION "360.99.0")
   set_target_properties(liblldb PROPERTIES

Hardcoding this here isn't ideal. Where did you get this number from? Is there 
a way to fetch it from a single location?


https://reviews.llvm.org/D47792



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

[Lldb-commits] [PATCH] D47792: Fix up Info.plist when building LLDB.framework with CMake

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/API/CMakeLists.txt:184
+  set(EXECUTABLE_NAME "LLDB")
+  set(CURRENT_PROJECT_VERSION "360.99.0")
   set_target_properties(liblldb PROPERTIES

Currently the apple generic versioning is used and the value for this is 
grabbed from CURRENT_PROJECT_VERSION in the Xcode project in 
lldb.xcodeproj/project.pbxproj. We will need to let people know that they will 
need to update this variable from now on.



Comment at: source/API/CMakeLists.txt:184
+  set(EXECUTABLE_NAME "LLDB")
+  set(CURRENT_PROJECT_VERSION "360.99.0")
   set_target_properties(liblldb PROPERTIES

sas wrote:
> clayborg wrote:
> > Currently the apple generic versioning is used and the value for this is 
> > grabbed from CURRENT_PROJECT_VERSION in the Xcode project in 
> > lldb.xcodeproj/project.pbxproj. We will need to let people know that they 
> > will need to update this variable from now on.
> Hardcoding this here isn't ideal. Where did you get this number from? Is 
> there a way to fetch it from a single location?
It was hardcoded into the Xcode project before. The numbers would be manually 
bumped by using "agvtool bump -all". That would update the Xcode project and a 
few plist files. Feel free to try running that from the root LLDB directory to 
see all files that get updated, then revert once you see what changed.


https://reviews.llvm.org/D47792



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

[Lldb-commits] [PATCH] D47797: [lldb-mi] Re-implement MI -exec-next command.

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added inline comments.



Comment at: lit/tools/lldb-mi/exec/exec-next.test:2
+# XFAIL: windows
+# -> llvm.org/pr24452
+#

aprantl wrote:
> @stella.stemanova: Would be interesting to understand why (/if ?) this 
> doesn't work on windows
I can test it out after it's committed.


https://reviews.llvm.org/D47797



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