[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[Pavel Labath via Phabricator via lldb-commits]

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

LGTM, modulo the inline comments.




Comment at: lldb/source/Host/common/NativeProcessProtocol.cpp:25-26
 // NativeProcessProtocol Members
+const size_t NativeProcessProtocol::g_cache_line_size =
+llvm::sys::Process::getPageSizeEstimate();
 

Make this a local static in `ReadCStringFromMemory`. There's no need to execute 
this immediately on program startup..



Comment at: lldb/unittests/Host/NativeProcessProtocolTest.cpp:100
+
+TEST(NativeProcessProtocolTest, ReadCStringFromMemory) {
+  NiceMock DummyDelegate;

labath wrote:
> It would be nice to have one more test where reads cross the page boundary, 
> to make sure chunks are concatenated properly.
How about this? I still think it would be useful to have it, as the cross-page 
handling is the main source of complexity in this function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62503



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

[Lldb-commits] [PATCH] D63380: [lldb] [test] Skip watchpoint tests on NetBSD if userdbregs is disabled

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D63380#1545428 , @labath wrote:

> This definitely aren't all of our watchpoint tests. What's the reason for 
> picking this particular bunch? Are they the only ones enabled on NetBSD right 
> now?


Buildbot was down at the moment, so I went only for those that I was sure 
started failing once I fixed dbreg error reporting.

> Anyway, given that all of the watchpoint tests are already annotated with the 
> "watchpoint" category, I think it would be better to handle this similar to 
> how we handle other category-based skips (debug info flavours, c++ library 
> types, etc.). This would also enable us to get rid of the 
> "expectedFailure(windows)" decorator on all of these tests by implementing 
> the skip centrally (though you don't have to do that right now). The 
> category-based skipping happens around here: 
> https://github.com/llvm-mirror/lldb/blob/master/packages/Python/lldbsuite/test/dotest.py#L1143

Will do that.


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

https://reviews.llvm.org/D63380



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

[Lldb-commits] [PATCH] D63322: DWARF: Avoid storing DIERefs in long-lived containers

[Pavel Labath via Phabricator via lldb-commits]

labath marked 2 inline comments as done.
labath added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:2970
 
   if (die) {
 TypeSystem *type_system =

aprantl wrote:
> If you find the time:
> 
> ```
> if (!die)
>   return {};
> auto *type_system = GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
> if (!type_system)
>   return {};
>  DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();
> if (!dwarf_ast)
>   return {};
> ```
Sure.


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

https://reviews.llvm.org/D63322



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

[Lldb-commits] [lldb] r363528 - DWARF: Avoid storing DIERefs in long-lived containers

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Jun 17 00:32:56 2019
New Revision: 363528

URL: http://llvm.org/viewvc/llvm-project?rev=363528&view=rev
Log:
DWARF: Avoid storing DIERefs in long-lived containers

Summary:
A user_id_t carries the same information as a DIERef, but it takes up
less space.

Furthermore, DIERef::operator<'s implementation is very
questionable, as it does not take the cu_offset and section fields into
account. Using just the die offset was correct in the days when all
debug info lived in a single section, but since we started supporting
DWO debug info, this was no longer true. The comparison operator could
be fixed, but it seems like using the user_id_t for these purposes is a
better idea overall.

I think this did not cause any bugs, because the only place the
comparison operator was used is in m_function_scope_qualified_name_map,
and this one is local to a dwo file, but I am not 100% sure of that.

Reviewers: clayborg, JDevlieghere

Subscribers: aprantl, lldb-commits

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

Modified:
lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.h
lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.h?rev=363528&r1=363527&r2=363528&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.h (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.h Mon Jun 17 00:32:56 2019
@@ -25,10 +25,6 @@ struct DIERef {
 
   explicit DIERef(const DWARFFormValue &form_value);
 
-  bool operator<(const DIERef &ref) const {
-return die_offset < ref.die_offset;
-  }
-
   explicit operator bool() const {
 return cu_offset != DW_INVALID_OFFSET || die_offset != DW_INVALID_OFFSET;
   }

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp?rev=363528&r1=363527&r2=363528&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp 
(original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp Mon Jun 
17 00:32:56 2019
@@ -998,7 +998,7 @@ TypeSP DWARFASTParserClang::ParseTypeFro
 clang_type.GetOpaqueQualType();
 dwarf->GetForwardDeclClangTypeToDie()
 [ClangUtil::RemoveFastQualifiers(clang_type).GetOpaqueQualType()] =
-die.GetDIERef();
+die.GetID();
 m_ast.SetHasExternalStorage(clang_type.GetOpaqueQualType(), true);
   }
 }

Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=363528&r1=363527&r2=363528&view=diff
==
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Mon Jun 17 
00:32:56 2019
@@ -2327,15 +2327,9 @@ void SymbolFileDWARF::GetMangledNamesFor
   dwo->GetMangledNamesForFunction(scope_qualified_name, mangled_names);
   }
 
-  NameToOffsetMap::iterator iter =
-  m_function_scope_qualified_name_map.find(scope_qualified_name);
-  if (iter == m_function_scope_qualified_name_map.end())
-return;
-
-  DIERefSetSP set_sp = (*iter).second;
-  std::set::iterator set_iter;
-  for (set_iter = set_sp->begin(); set_iter != set_sp->end(); set_iter++) {
-DWARFDIE die = DebugInfo()->GetDIE(*set_iter);
+  for (lldb::user_id_t uid :
+   m_function_scope_qualified_name_map.lookup(scope_qualified_name)) {
+DWARFDIE die = GetDIE(uid);
 mangled_names.push_back(ConstString(die.GetMangledName()));
   }
 }
@@ -2952,42 +2946,32 @@ TypeSP SymbolFileDWARF::FindDefinitionTy
 
 TypeSP SymbolFileDWARF::ParseType(const SymbolContext &sc, const DWARFDIE &die,
   bool *type_is_new_ptr) {
-  TypeSP type_sp;
-
-  if (die) {
-TypeSystem *type_system =
-GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
+  if (!die)
+return {};
 
-if (type_system) {
-  DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();
-  if (dwarf_ast) {
-Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
-type_sp = dwarf_ast->ParseTypeFromDWARF(sc, die, log, type_is_new_ptr);
-if (type_sp) {
-  TypeList *type_list = GetTypeList();
-  if (type_list)
-type_list->Insert(type_sp);
-
-  if (die.Tag() == DW_TAG_subprogram) {
-DIERef die_ref = die.GetDIERef()

[Lldb-commits] [PATCH] D63322: DWARF: Avoid storing DIERefs in long-lived containers

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
labath marked an inline comment as done.
Closed by commit rL363528: DWARF: Avoid storing DIERefs in long-lived 
containers (authored by labath, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63322?vs=204717&id=204993#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63322

Files:
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.h
  lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -2327,15 +2327,9 @@
   dwo->GetMangledNamesForFunction(scope_qualified_name, mangled_names);
   }
 
-  NameToOffsetMap::iterator iter =
-  m_function_scope_qualified_name_map.find(scope_qualified_name);
-  if (iter == m_function_scope_qualified_name_map.end())
-return;
-
-  DIERefSetSP set_sp = (*iter).second;
-  std::set::iterator set_iter;
-  for (set_iter = set_sp->begin(); set_iter != set_sp->end(); set_iter++) {
-DWARFDIE die = DebugInfo()->GetDIE(*set_iter);
+  for (lldb::user_id_t uid :
+   m_function_scope_qualified_name_map.lookup(scope_qualified_name)) {
+DWARFDIE die = GetDIE(uid);
 mangled_names.push_back(ConstString(die.GetMangledName()));
   }
 }
@@ -2952,42 +2946,32 @@
 
 TypeSP SymbolFileDWARF::ParseType(const SymbolContext &sc, const DWARFDIE &die,
   bool *type_is_new_ptr) {
-  TypeSP type_sp;
+  if (!die)
+return {};
 
-  if (die) {
-TypeSystem *type_system =
-GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
+  TypeSystem *type_system =
+  GetTypeSystemForLanguage(die.GetCU()->GetLanguageType());
+  if (!type_system)
+return {};
 
-if (type_system) {
-  DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();
-  if (dwarf_ast) {
-Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
-type_sp = dwarf_ast->ParseTypeFromDWARF(sc, die, log, type_is_new_ptr);
-if (type_sp) {
-  TypeList *type_list = GetTypeList();
-  if (type_list)
-type_list->Insert(type_sp);
-
-  if (die.Tag() == DW_TAG_subprogram) {
-DIERef die_ref = die.GetDIERef();
-std::string scope_qualified_name(GetDeclContextForUID(die.GetID())
- .GetScopeQualifiedName()
- .AsCString(""));
-if (scope_qualified_name.size()) {
-  NameToOffsetMap::iterator iter =
-  m_function_scope_qualified_name_map.find(
-  scope_qualified_name);
-  if (iter != m_function_scope_qualified_name_map.end())
-(*iter).second->insert(die_ref);
-  else {
-DIERefSetSP new_set(new std::set);
-new_set->insert(die_ref);
-m_function_scope_qualified_name_map.emplace(
-std::make_pair(scope_qualified_name, new_set));
-  }
-}
-  }
-}
+  DWARFASTParser *dwarf_ast = type_system->GetDWARFParser();
+  if (!dwarf_ast)
+return {};
+
+  Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO);
+  TypeSP type_sp = dwarf_ast->ParseTypeFromDWARF(sc, die, log, type_is_new_ptr);
+  if (type_sp) {
+TypeList *type_list = GetTypeList();
+if (type_list)
+  type_list->Insert(type_sp);
+
+if (die.Tag() == DW_TAG_subprogram) {
+  std::string scope_qualified_name(GetDeclContextForUID(die.GetID())
+   .GetScopeQualifiedName()
+   .AsCString(""));
+  if (scope_qualified_name.size()) {
+m_function_scope_qualified_name_map[scope_qualified_name].insert(
+die.GetID());
   }
 }
   }
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.h
===
--- lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.h
+++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DIERef.h
@@ -25,10 +25,6 @@
 
   explicit DIERef(const DWARFFormValue &form_value);
 
-  bool operator<(const DIERef &ref) const {
-return die_offset < ref.die_offset;
-  }
-
   explicit operator bool() const {
 return cu_offset != DW_INVALID_OFFSET || die_offset != DW_INVALID_OFFSET;
   }
Index: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
==

[Lldb-commits] [PATCH] D63380: [lldb] [test] Skip watchpoint tests on NetBSD if userdbregs is disabled

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 204995.
mgorny added a comment.

Updated as suggested above.


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

https://reviews.llvm.org/D63380

Files:
  lldb/packages/Python/lldbsuite/test/decorators.py
  lldb/packages/Python/lldbsuite/test/dotest.py


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -1181,6 +1181,30 @@
 print("libstdcxx tests will not be run because: " + reason)
 configuration.skipCategories.append("libstdcxx")
 
+def canRunWatchpointTests():
+from lldbsuite.test import lldbplatformutil
+
+platform = lldbplatformutil.getPlatform()
+if platform == "netbsd":
+  try:
+output = subprocess.check_output(["/sbin/sysctl", "-n",
+  "security.models.extensions.user_set_dbregs"]).decode().strip()
+if output == "1":
+  return True, "security.models.extensions.user_set_dbregs enabled"
+  except subprocess.CalledProcessError:
+pass
+  return False, "security.models.extensions.user_set_dbregs disabled"
+return True, "dbregs always writable"
+
+def checkWatchpointSupport():
+result, reason = canRunWatchpointTests()
+if result:
+return # watchpoints supported
+if "watchpoint" in configuration.categoriesList:
+return # watchpoint category explicitly requested, let it run.
+print("watchpoint tests will not be run because: " + reason)
+configuration.skipCategories.append("watchpoint")
+
 def checkDebugInfoSupport():
 import lldb
 
@@ -1305,6 +1329,7 @@
 
 checkLibcxxSupport()
 checkLibstdcxxSupport()
+checkWatchpointSupport()
 checkDebugInfoSupport()
 
 # Don't do debugserver tests on anything except OS X.
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -826,3 +826,18 @@
 return (('DYLD_INSERT_LIBRARIES' in os.environ) and
 'libclang_rt.asan' in os.environ['DYLD_INSERT_LIBRARIES'])
 return skipTestIfFn(is_sanitized)(func)
+
+def skipIfDbRegsReadOnly(func):
+"""Skip this test if platform does not permit writing to debug 
registers."""
+def is_dbregs_rdonly():
+if platform.system() != 'NetBSD':
+return None
+try:
+output = subprocess.check_output(["/sbin/sysctl", "-n",
+"security.models.extensions.user_set_dbregs"]).decode().strip()
+if output == "1":
+return None
+except subprocess.CalledProcessError:
+pass
+return "security.models.extensions.user_set_dbregs is disabled"
+return skipTestIfFn(is_dbregs_rdonly)(func)


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -1181,6 +1181,30 @@
 print("libstdcxx tests will not be run because: " + reason)
 configuration.skipCategories.append("libstdcxx")
 
+def canRunWatchpointTests():
+from lldbsuite.test import lldbplatformutil
+
+platform = lldbplatformutil.getPlatform()
+if platform == "netbsd":
+  try:
+output = subprocess.check_output(["/sbin/sysctl", "-n",
+  "security.models.extensions.user_set_dbregs"]).decode().strip()
+if output == "1":
+  return True, "security.models.extensions.user_set_dbregs enabled"
+  except subprocess.CalledProcessError:
+pass
+  return False, "security.models.extensions.user_set_dbregs disabled"
+return True, "dbregs always writable"
+
+def checkWatchpointSupport():
+result, reason = canRunWatchpointTests()
+if result:
+return # watchpoints supported
+if "watchpoint" in configuration.categoriesList:
+return # watchpoint category explicitly requested, let it run.
+print("watchpoint tests will not be run because: " + reason)
+configuration.skipCategories.append("watchpoint")
+
 def checkDebugInfoSupport():
 import lldb
 
@@ -1305,6 +1329,7 @@
 
 checkLibcxxSupport()
 checkLibstdcxxSupport()
+checkWatchpointSupport()
 checkDebugInfoSupport()
 
 # Don't do debugserver tests on anything except OS X.
Index: lldb/packages/Python/lldbsuite/test/decorators.py
===
--- lldb/packages/Python/lldbsuite/test/decorators.py
+++ lldb/packages/Python/lldbsuite/test/decorators.py
@@ -826,3 +826,18 @@
 return (('DYLD_INSERT_LIBRARIES' in os.environ) and
 'libclang_rt.asan' in os.environ['DYLD_INSERT_LIBRARIES'])
 return skipTestIfFn(is_sanitized)(func)
+
+def skipIfDb

[Lldb-commits] [PATCH] D63380: [lldb] [test] Skip watchpoint tests on NetBSD if userdbregs is disabled

[Michał Górny via Phabricator via lldb-commits]

mgorny updated this revision to Diff 204996.
mgorny added a comment.

Removed stale decorator.


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

https://reviews.llvm.org/D63380

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -1181,6 +1181,30 @@
 print("libstdcxx tests will not be run because: " + reason)
 configuration.skipCategories.append("libstdcxx")
 
+def canRunWatchpointTests():
+from lldbsuite.test import lldbplatformutil
+
+platform = lldbplatformutil.getPlatform()
+if platform == "netbsd":
+  try:
+output = subprocess.check_output(["/sbin/sysctl", "-n",
+  "security.models.extensions.user_set_dbregs"]).decode().strip()
+if output == "1":
+  return True, "security.models.extensions.user_set_dbregs enabled"
+  except subprocess.CalledProcessError:
+pass
+  return False, "security.models.extensions.user_set_dbregs disabled"
+return True, "dbregs always writable"
+
+def checkWatchpointSupport():
+result, reason = canRunWatchpointTests()
+if result:
+return # watchpoints supported
+if "watchpoint" in configuration.categoriesList:
+return # watchpoint category explicitly requested, let it run.
+print("watchpoint tests will not be run because: " + reason)
+configuration.skipCategories.append("watchpoint")
+
 def checkDebugInfoSupport():
 import lldb
 
@@ -1305,6 +1329,7 @@
 
 checkLibcxxSupport()
 checkLibstdcxxSupport()
+checkWatchpointSupport()
 checkDebugInfoSupport()
 
 # Don't do debugserver tests on anything except OS X.


Index: lldb/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/packages/Python/lldbsuite/test/dotest.py
+++ lldb/packages/Python/lldbsuite/test/dotest.py
@@ -1181,6 +1181,30 @@
 print("libstdcxx tests will not be run because: " + reason)
 configuration.skipCategories.append("libstdcxx")
 
+def canRunWatchpointTests():
+from lldbsuite.test import lldbplatformutil
+
+platform = lldbplatformutil.getPlatform()
+if platform == "netbsd":
+  try:
+output = subprocess.check_output(["/sbin/sysctl", "-n",
+  "security.models.extensions.user_set_dbregs"]).decode().strip()
+if output == "1":
+  return True, "security.models.extensions.user_set_dbregs enabled"
+  except subprocess.CalledProcessError:
+pass
+  return False, "security.models.extensions.user_set_dbregs disabled"
+return True, "dbregs always writable"
+
+def checkWatchpointSupport():
+result, reason = canRunWatchpointTests()
+if result:
+return # watchpoints supported
+if "watchpoint" in configuration.categoriesList:
+return # watchpoint category explicitly requested, let it run.
+print("watchpoint tests will not be run because: " + reason)
+configuration.skipCategories.append("watchpoint")
+
 def checkDebugInfoSupport():
 import lldb
 
@@ -1305,6 +1329,7 @@
 
 checkLibcxxSupport()
 checkLibstdcxxSupport()
+checkWatchpointSupport()
 checkDebugInfoSupport()
 
 # Don't do debugserver tests on anything except OS X.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63380: [lldb] [test] Skip watchpoint tests on NetBSD if userdbregs is disabled

[Pavel Labath via Phabricator via lldb-commits]

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



Comment at: lldb/packages/Python/lldbsuite/test/dotest.py:1197
+  return False, "security.models.extensions.user_set_dbregs disabled"
+return True, "dbregs always writable"
+

The string isn't used anywhere, but I'd use a slightly more generic value here, 
because there are a number of reasons watchpoints can/cannot work on a given 
platform...


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

https://reviews.llvm.org/D63380



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

[Lldb-commits] [PATCH] D63399: DWARF: Make DIERefs always valid

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: JDevlieghere, clayborg, aprantl.
Herald added a subscriber: arphaman.
labath marked an inline comment as done.
labath added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DIERef.cpp:1
 //===-- DIERef.cpp --*- C++ 
-*-===//
 //

I'm not deleting this file, because I'm going to add some stuff to it in the 
next patch in this series.


This patch makes the DIERef class always valid by default constructor
and operator bool. This allows one to express the validity of a DIERef
in the type system. Places which are working with potentially-invalid
DIERefs have been updated to use Optional instead.

The constructor taking a DWARFFormValue was not needed, as all places
which were constructing a DIERef this way were immediately converting it
into a DWARFDIE or a user_id. This can be done without constructing an
intermediate DIERef.


https://reviews.llvm.org/D63399

Files:
  source/Plugins/SymbolFile/DWARF/DIERef.cpp
  source/Plugins/SymbolFile/DWARF/DIERef.h
  source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.h
  source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDIE.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  source/Plugins/SymbolFile/DWARF/HashedNameToDIE.cpp
  source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -269,7 +269,13 @@
   DWARFDIE GetDIE(lldb::user_id_t uid);
 
   lldb::user_id_t GetUID(const DWARFBaseDIE &die) {
-return GetUID(die.GetDIERef());
+if (die)
+  return GetUID(*die.GetDIERef());
+return LLDB_INVALID_UID;
+  }
+
+  lldb::user_id_t GetUID(const llvm::Optional &ref) {
+return ref ? GetUID(*ref) : LLDB_INVALID_UID;
   }
 
   lldb::user_id_t GetUID(const DIERef &ref) {
@@ -437,10 +443,10 @@
   llvm::Optional GetDWARFUnitIndex(uint32_t cu_idx);
 
   struct DecodedUID {
-SymbolFileDWARF *dwarf;
+SymbolFileDWARF &dwarf;
 DIERef ref;
   };
-  DecodedUID DecodeUID(lldb::user_id_t uid);
+  llvm::Optional DecodeUID(lldb::user_id_t uid);
 
   SymbolFileDWARFDwp *GetDwpSymbolFile();
 
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1199,7 +1199,8 @@
   ast_parser->GetDeclForUIDFromDWARF(decl);
 }
 
-SymbolFileDWARF::DecodedUID SymbolFileDWARF::DecodeUID(lldb::user_id_t uid) {
+llvm::Optional
+SymbolFileDWARF::DecodeUID(lldb::user_id_t uid) {
   // This method can be called without going through the symbol vendor so we
   // need to lock the module.
   std::lock_guard guard(GetModuleMutex());
@@ -1213,7 +1214,9 @@
   if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile()) {
 SymbolFileDWARF *dwarf = debug_map->GetSymbolFileByOSOIndex(
 debug_map->GetOSOIndexFromUserID(uid));
-return {dwarf, {DIERef::Section::DebugInfo, DW_INVALID_OFFSET, dw_offset_t(uid)}};
+return DecodedUID{
+*dwarf,
+{DIERef::Section::DebugInfo, DW_INVALID_OFFSET, dw_offset_t(uid)}};
   }
   DIERef::Section section =
   uid >> 63 ? DIERef::Section::DebugTypes : DIERef::Section::DebugInfo;
@@ -1221,7 +1224,7 @@
   dw_offset_t die_offset = uid;
 
   if (die_offset == DW_INVALID_OFFSET)
-return {nullptr, DIERef()};
+return llvm::None;
 
   SymbolFileDWARF *dwarf = this;
   if (DebugInfo()) {
@@ -1230,7 +1233,7 @@
 dwarf = unit->GetDwoSymbolFile();
 }
   }
-  return {dwarf, {section, DW_INVALID_OFFSET, die_offset}};
+  return DecodedUID{*dwarf, {section, DW_INVALID_OFFSET, die_offset}};
 }
 
 DWARFDIE
@@ -1239,10 +1242,10 @@
   // need to lock the module.
   std::lock_guard guard(GetModuleMutex());
 
-  DecodedUID decoded = DecodeUID(uid);
+  llvm::Optional decoded = DecodeUID(uid);
 
-  if (decoded.dwarf)
-return decoded.dwarf->GetDIE(decoded.ref);
+  if (decoded)
+return decoded->dwarf.GetDIE(decoded->ref);
 
   return DWARFDIE();
 }
@@ -3453,7 +3456,7 @@
 
   if (symbol_context_scope) {
 SymbolFileTypeSP type_sp(
-new SymbolFileType(*this, GetUID(DIERef(type_die_form;
+new SymbolFileType(*this, GetUID(type_die_form.Reference(;
 
 if (const_value.Form() && type_sp && type_sp->GetType())
   location.UpdateValue(const_value.Unsigned(),
I

[Lldb-commits] [PATCH] D63399: DWARF: Make DIERefs always valid

[Pavel Labath via Phabricator via lldb-commits]

labath marked an inline comment as done.
labath added inline comments.



Comment at: source/Plugins/SymbolFile/DWARF/DIERef.cpp:1
 //===-- DIERef.cpp --*- C++ 
-*-===//
 //

I'm not deleting this file, because I'm going to add some stuff to it in the 
next patch in this series.


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

https://reviews.llvm.org/D63399



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

[Lldb-commits] [PATCH] D63400: DWARF: Provide accessors to DIERef fields

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: JDevlieghere, aprantl, clayborg.
Herald added a subscriber: arphaman.
labath added a parent revision: D63399: DWARF: Make DIERefs always valid.
labath added a comment.

(also, cu_offset is renamed to unit_offset, as we now support type units too)


Instead of accessing the fields directly, use accessor functions to
provide access to the DIERef components. This allows us to decouple the
external interface, from the internal representation. The external
interface can use llvm::Optional and similar goodies, while the data can
still be stored internally in a more compact representation.

I also document the purpose of the existing DIERef fields.

The main motivation for this change is a need to introduce an additional
field to the DIERef class, but I believe the change has its own merit.


https://reviews.llvm.org/D63400

Files:
  source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/DIERef.cpp
  source/Plugins/SymbolFile/DWARF/DIERef.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -124,7 +124,8 @@
 
 DWARFDIE
 SymbolFileDWARFDwo::GetDIE(const DIERef &die_ref) {
-  lldbassert(die_ref.cu_offset == m_base_dwarf_cu.GetOffset() ||
- die_ref.cu_offset == DW_INVALID_OFFSET);
-  return DebugInfo()->GetDIEForDIEOffset(die_ref.section, die_ref.die_offset);
+  lldbassert(!die_ref.unit_offset() ||
+ *die_ref.unit_offset() == m_base_dwarf_cu.GetOffset());
+  return DebugInfo()->GetDIEForDIEOffset(die_ref.section(),
+ die_ref.die_offset());
 }
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -279,8 +279,9 @@
   }
 
   lldb::user_id_t GetUID(const DIERef &ref) {
-return GetID() | ref.die_offset |
-   (lldb::user_id_t(ref.section == DIERef::Section::DebugTypes) << 63);
+return GetID() | ref.die_offset() |
+   (lldb::user_id_t(ref.section() == DIERef::Section::DebugTypes)
+<< 63);
   }
 
   virtual std::unique_ptr
Index: source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
===
--- source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
+++ source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
@@ -26,6 +26,7 @@
 }
 
 void NameToDIE::Insert(ConstString name, const DIERef &die_ref) {
+  assert(die_ref.unit_offset().hasValue());
   m_map.Append(name, die_ref);
 }
 
@@ -44,7 +45,7 @@
   const uint32_t size = m_map.GetSize();
   for (uint32_t i = 0; i < size; ++i) {
 const DIERef &die_ref = m_map.GetValueAtIndexUnchecked(i);
-if (cu_offset == die_ref.cu_offset)
+if (cu_offset == *die_ref.unit_offset())
   info_array.push_back(die_ref);
   }
   return info_array.size() - initial_size;
@@ -53,10 +54,8 @@
 void NameToDIE::Dump(Stream *s) {
   const uint32_t size = m_map.GetSize();
   for (uint32_t i = 0; i < size; ++i) {
-ConstString cstr = m_map.GetCStringAtIndex(i);
-const DIERef &die_ref = m_map.GetValueAtIndexUnchecked(i);
-s->Printf("%p: {0x%8.8x/0x%8.8x} \"%s\"\n", (const void *)cstr.GetCString(),
-  die_ref.cu_offset, die_ref.die_offset, cstr.GetCString());
+s->Format("{0} \"{1}\"\n", m_map.GetValueAtIndexUnchecked(i),
+  m_map.GetCStringAtIndexUnchecked(i));
   }
 }
 
Index: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -166,7 +166,7 @@
   continue;
 
 DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo,
- ref->cu_offset);
+ *ref->unit_offset());
 if (!cu || !cu->Supports_DW_AT_APPLE_objc_complete_type()) {
   incomplete_types.push_back(*ref);
   continue;
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -149,10 +149,9 @@
 }
 
 DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef &die_ref) {
-  if (die_ref.cu_offset == DW_INVALID_OFFSET)
-return GetUnitContainingDIEOffset(die_ref.sec

[Lldb-commits] [PATCH] D63400: DWARF: Provide accessors to DIERef fields

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

(also, cu_offset is renamed to unit_offset, as we now support type units too)


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

https://reviews.llvm.org/D63400



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

[Lldb-commits] [PATCH] D63400: DWARF: Provide accessors to DIERef fields

[Pavel Labath via Phabricator via lldb-commits]

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

- s/DW_INVALID_OFFSET/llvm::None/ in HashedNameToDIE.h


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

https://reviews.llvm.org/D63400

Files:
  source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/DIERef.cpp
  source/Plugins/SymbolFile/DWARF/DIERef.h
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
  source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -124,7 +124,8 @@
 
 DWARFDIE
 SymbolFileDWARFDwo::GetDIE(const DIERef &die_ref) {
-  lldbassert(die_ref.cu_offset == m_base_dwarf_cu.GetOffset() ||
- die_ref.cu_offset == DW_INVALID_OFFSET);
-  return DebugInfo()->GetDIEForDIEOffset(die_ref.section, die_ref.die_offset);
+  lldbassert(!die_ref.unit_offset() ||
+ *die_ref.unit_offset() == m_base_dwarf_cu.GetOffset());
+  return DebugInfo()->GetDIEForDIEOffset(die_ref.section(),
+ die_ref.die_offset());
 }
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -279,8 +279,9 @@
   }
 
   lldb::user_id_t GetUID(const DIERef &ref) {
-return GetID() | ref.die_offset |
-   (lldb::user_id_t(ref.section == DIERef::Section::DebugTypes) << 63);
+return GetID() | ref.die_offset() |
+   (lldb::user_id_t(ref.section() == DIERef::Section::DebugTypes)
+<< 63);
   }
 
   virtual std::unique_ptr
Index: source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
===
--- source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
+++ source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
@@ -26,6 +26,7 @@
 }
 
 void NameToDIE::Insert(ConstString name, const DIERef &die_ref) {
+  assert(die_ref.unit_offset().hasValue());
   m_map.Append(name, die_ref);
 }
 
@@ -44,7 +45,7 @@
   const uint32_t size = m_map.GetSize();
   for (uint32_t i = 0; i < size; ++i) {
 const DIERef &die_ref = m_map.GetValueAtIndexUnchecked(i);
-if (cu_offset == die_ref.cu_offset)
+if (cu_offset == *die_ref.unit_offset())
   info_array.push_back(die_ref);
   }
   return info_array.size() - initial_size;
@@ -53,10 +54,8 @@
 void NameToDIE::Dump(Stream *s) {
   const uint32_t size = m_map.GetSize();
   for (uint32_t i = 0; i < size; ++i) {
-ConstString cstr = m_map.GetCStringAtIndex(i);
-const DIERef &die_ref = m_map.GetValueAtIndexUnchecked(i);
-s->Printf("%p: {0x%8.8x/0x%8.8x} \"%s\"\n", (const void *)cstr.GetCString(),
-  die_ref.cu_offset, die_ref.die_offset, cstr.GetCString());
+s->Format("{0} \"{1}\"\n", m_map.GetValueAtIndexUnchecked(i),
+  m_map.GetCStringAtIndexUnchecked(i));
   }
 }
 
Index: source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
===
--- source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
+++ source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
@@ -61,7 +61,7 @@
 DIEInfo(dw_offset_t o, dw_tag_t t, uint32_t f, uint32_t h);
 
 explicit operator DIERef() const {
-  return DIERef(DIERef::Section::DebugInfo, DW_INVALID_OFFSET, die_offset);
+  return DIERef(DIERef::Section::DebugInfo, llvm::None, die_offset);
 }
   };
 
Index: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
===
--- source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -166,7 +166,7 @@
   continue;
 
 DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo,
- ref->cu_offset);
+ *ref->unit_offset());
 if (!cu || !cu->Supports_DW_AT_APPLE_objc_complete_type()) {
   incomplete_types.push_back(*ref);
   continue;
Index: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
===
--- source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -149,10 +149,9 @@
 }
 
 DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef &die_ref) {
-  if (die_ref.cu_offset == DW_INVALID_OFFSET)
-return GetUnitContainingDIEOffset(die_ref.section, die_ref.die_offset);
-  else
-return GetUnitAtOffset(die_ref.section

[Lldb-commits] [lldb] r363536 - [lldb] [test] Skip watchpoint tests on NetBSD if userdbregs is disabled

[Michal Gorny via lldb-commits]

Author: mgorny
Date: Mon Jun 17 02:49:05 2019
New Revision: 363536

URL: http://llvm.org/viewvc/llvm-project?rev=363536&view=rev
Log:
[lldb] [test] Skip watchpoint tests on NetBSD if userdbregs is disabled

Skip watchpoint tests if security.models.extensions.user_set_dbregs
is disabled.  This indicates that unprivileged processes are not allowed
to write to debug registers which is a prerequisite for using hardware
watchpoints.

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

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

Modified: lldb/trunk/packages/Python/lldbsuite/test/dotest.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dotest.py?rev=363536&r1=363535&r2=363536&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/dotest.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py Mon Jun 17 02:49:05 2019
@@ -1181,6 +1181,30 @@ def checkLibstdcxxSupport():
 print("libstdcxx tests will not be run because: " + reason)
 configuration.skipCategories.append("libstdcxx")
 
+def canRunWatchpointTests():
+from lldbsuite.test import lldbplatformutil
+
+platform = lldbplatformutil.getPlatform()
+if platform == "netbsd":
+  try:
+output = subprocess.check_output(["/sbin/sysctl", "-n",
+  "security.models.extensions.user_set_dbregs"]).decode().strip()
+if output == "1":
+  return True, "security.models.extensions.user_set_dbregs enabled"
+  except subprocess.CalledProcessError:
+pass
+  return False, "security.models.extensions.user_set_dbregs disabled"
+return True, "watchpoint support available"
+
+def checkWatchpointSupport():
+result, reason = canRunWatchpointTests()
+if result:
+return # watchpoints supported
+if "watchpoint" in configuration.categoriesList:
+return # watchpoint category explicitly requested, let it run.
+print("watchpoint tests will not be run because: " + reason)
+configuration.skipCategories.append("watchpoint")
+
 def checkDebugInfoSupport():
 import lldb
 
@@ -1305,6 +1329,7 @@ def run_suite():
 
 checkLibcxxSupport()
 checkLibstdcxxSupport()
+checkWatchpointSupport()
 checkDebugInfoSupport()
 
 # Don't do debugserver tests on anything except OS X.


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

[Lldb-commits] [PATCH] D63380: [lldb] [test] Skip watchpoint tests on NetBSD if userdbregs is disabled

[Michał Górny via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363536: [lldb] [test] Skip watchpoint tests on NetBSD if 
userdbregs is disabled (authored by mgorny, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63380?vs=204996&id=205007#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63380

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


Index: lldb/trunk/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/dotest.py
+++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py
@@ -1181,6 +1181,30 @@
 print("libstdcxx tests will not be run because: " + reason)
 configuration.skipCategories.append("libstdcxx")
 
+def canRunWatchpointTests():
+from lldbsuite.test import lldbplatformutil
+
+platform = lldbplatformutil.getPlatform()
+if platform == "netbsd":
+  try:
+output = subprocess.check_output(["/sbin/sysctl", "-n",
+  "security.models.extensions.user_set_dbregs"]).decode().strip()
+if output == "1":
+  return True, "security.models.extensions.user_set_dbregs enabled"
+  except subprocess.CalledProcessError:
+pass
+  return False, "security.models.extensions.user_set_dbregs disabled"
+return True, "watchpoint support available"
+
+def checkWatchpointSupport():
+result, reason = canRunWatchpointTests()
+if result:
+return # watchpoints supported
+if "watchpoint" in configuration.categoriesList:
+return # watchpoint category explicitly requested, let it run.
+print("watchpoint tests will not be run because: " + reason)
+configuration.skipCategories.append("watchpoint")
+
 def checkDebugInfoSupport():
 import lldb
 
@@ -1305,6 +1329,7 @@
 
 checkLibcxxSupport()
 checkLibstdcxxSupport()
+checkWatchpointSupport()
 checkDebugInfoSupport()
 
 # Don't do debugserver tests on anything except OS X.


Index: lldb/trunk/packages/Python/lldbsuite/test/dotest.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/dotest.py
+++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py
@@ -1181,6 +1181,30 @@
 print("libstdcxx tests will not be run because: " + reason)
 configuration.skipCategories.append("libstdcxx")
 
+def canRunWatchpointTests():
+from lldbsuite.test import lldbplatformutil
+
+platform = lldbplatformutil.getPlatform()
+if platform == "netbsd":
+  try:
+output = subprocess.check_output(["/sbin/sysctl", "-n",
+  "security.models.extensions.user_set_dbregs"]).decode().strip()
+if output == "1":
+  return True, "security.models.extensions.user_set_dbregs enabled"
+  except subprocess.CalledProcessError:
+pass
+  return False, "security.models.extensions.user_set_dbregs disabled"
+return True, "watchpoint support available"
+
+def checkWatchpointSupport():
+result, reason = canRunWatchpointTests()
+if result:
+return # watchpoints supported
+if "watchpoint" in configuration.categoriesList:
+return # watchpoint category explicitly requested, let it run.
+print("watchpoint tests will not be run because: " + reason)
+configuration.skipCategories.append("watchpoint")
+
 def checkDebugInfoSupport():
 import lldb
 
@@ -1305,6 +1329,7 @@
 
 checkLibcxxSupport()
 checkLibstdcxxSupport()
+checkWatchpointSupport()
 checkDebugInfoSupport()
 
 # Don't do debugserver tests on anything except OS X.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D63370: Specify log level for CMake messages (less stderr)

[Stefan Gränitz via Phabricator via lldb-commits]

sgraenitz added a comment.

Thanks for the initiative! Three inline comments for cosmetics.




Comment at: lldb/cmake/modules/LLDBConfig.cmake:143
  PYTHONLIBS_VERSION_STRING "${python_version_str}")
-message("-- Found Python version ${PYTHONLIBS_VERSION_STRING}")
+message(STATUS "-- Found Python version ${PYTHONLIBS_VERSION_STRING}")
 string(REGEX REPLACE "([0-9]+)[.]([0-9]+)[.][0-9]+" "python\\1\\2" 
PYTHONLIBS_BASE_NAME "${PYTHONLIBS_VERSION_STRING}")

`STATUS` messages are automatically prefixed with `-- ` so it can be removed 
from the text here.



Comment at: lldb/cmake/modules/LLDBConfig.cmake:187
+  message(STATUS "-- LLDB Found PythonDLL: ${PYTHON_DLL}")
+  message(STATUS "-- LLDB Found PythonIncludeDirs: ${PYTHON_INCLUDE_DIR}")
 endfunction(find_python_libs_windows)

Same as above. Please remove `-- `.



Comment at: lldb/cmake/modules/LLDBStandalone.cmake:98
   else()
-message("-- Found PythonInterp: ${PYTHON_EXECUTABLE}")
+message(STATUS "-- Found PythonInterp: ${PYTHON_EXECUTABLE}")
   endif()

Same as above. Please remove `-- `.


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

https://reviews.llvm.org/D63370



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

[Lldb-commits] [PATCH] D63380: [lldb] [test] Skip watchpoint tests on NetBSD if userdbregs is disabled

[Kamil Rytarowski via Phabricator via lldb-commits]

krytarowski added a comment.
Herald added a subscriber: ormris.

This check should contain additional check for uid==root. If we are root we can 
read and write to DB registers.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63380



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

[Lldb-commits] [PATCH] D63380: [lldb] [test] Skip watchpoint tests on NetBSD if userdbregs is disabled

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

In D63380#1545856 , @krytarowski wrote:

> This check should contain additional check for uid==root. If we are root we 
> can read and write to DB registers.


Will do.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63380



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

[Lldb-commits] [lldb] r363551 - [lldb] [test] Watchpoint tests can be always run as root on NetBSD

[Michal Gorny via lldb-commits]

Author: mgorny
Date: Mon Jun 17 05:32:09 2019
New Revision: 363551

URL: http://llvm.org/viewvc/llvm-project?rev=363551&view=rev
Log:
[lldb] [test] Watchpoint tests can be always run as root on NetBSD

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

Modified: lldb/trunk/packages/Python/lldbsuite/test/dotest.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/dotest.py?rev=363551&r1=363550&r2=363551&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/dotest.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/dotest.py Mon Jun 17 05:32:09 2019
@@ -1186,6 +1186,8 @@ def canRunWatchpointTests():
 
 platform = lldbplatformutil.getPlatform()
 if platform == "netbsd":
+  if os.geteuid() == 0:
+return True, "root can always write dbregs"
   try:
 output = subprocess.check_output(["/sbin/sysctl", "-n",
   "security.models.extensions.user_set_dbregs"]).decode().strip()


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

[Lldb-commits] [PATCH] D63428: DWARF: Add "dwo_num" field to the DIERef class

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: JDevlieghere, clayborg, aprantl.
Herald added subscribers: arphaman, dexonsmith, mehdi_amini.

When dwo support was introduced, it used a trick where debug info
entries were referenced by the offset of the compile unit in the main
file, but the die offset was relative to the dwo file. Although there
was some elegance to it, this representation was starting to reach its
breaking point:

- the fact that the skeleton compile unit owned the DWO file meant that it was 
impossible (or at least hard and unintuitive) to support DWO files containing 
more than one compile unit. These kinds of files are produced by LTO for 
example.
- it made it impossible to reference any DIEs in the skeleton compile unit 
(although the skeleton units are generally empty, clang still puts some info 
into them with -fsplit-dwarf-inlining).
- (current motivation) it made it very hard to support type units placed in DWO 
files, as type units don't have any skeleton units which could be referenced in 
the main file

This patch addresses this problem by introducing an additional
"dwo_num" field to the DIERef class, whose purpose is to identify the
dwo file. It's kind of similar to the dwo_id field in DWARF5 unit
headers, but while this is a 64bit hash whose main purpose is to catch
file mismatches, this is just a smaller integer used to indentify a
loaded dwo file. Currently, this is based on the index of the skeleton
compile unit which owns the dwo file, but it is intended to be
eventually independent of that (to support the LTO use case).

Simultaneously the unit_offset field is changed to reference the compile
unit containing the referenced die instead of the main unit. This means
we can remove the "BaseObjectOffset" field from the DWARFUnit class. It
also means we can remove some of the workarounds put in place to support
the skeleton-unit+dwo-die combo. More work is needed to remove all of
them, which is out of scope of this patch.


https://reviews.llvm.org/D63428

Files:
  source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
  source/Plugins/SymbolFile/DWARF/DIERef.cpp
  source/Plugins/SymbolFile/DWARF/DIERef.h
  source/Plugins/SymbolFile/DWARF/DWARFBaseDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
  source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/DWARFIndex.h
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h

Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.h
@@ -44,6 +44,8 @@
 
   DWARFCompileUnit *GetBaseCompileUnit() override { return &m_base_dwarf_cu; }
 
+  llvm::Optional GetDwoNum() override { return GetID() >> 32; }
+
 protected:
   void LoadSectionData(lldb::SectionType sect_type,
lldb_private::DWARFDataExtractor &data) override;
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -124,8 +124,7 @@
 
 DWARFDIE
 SymbolFileDWARFDwo::GetDIE(const DIERef &die_ref) {
-  lldbassert(!die_ref.unit_offset() ||
- *die_ref.unit_offset() == m_base_dwarf_cu.GetOffset());
-  return DebugInfo()->GetDIEForDIEOffset(die_ref.section(),
- die_ref.die_offset());
+  if (*die_ref.dwo_num() == GetDwoNum())
+return DebugInfo()->GetDIE(die_ref);
+  return GetBaseSymbolFile().GetDIE(die_ref);
 }
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
===
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
@@ -293,6 +293,8 @@
   // the method returns a pointer to the base compile unit.
   virtual DWARFCompileUnit *GetBaseCompileUnit() { return nullptr; }
 
+  virtual llvm::Optional GetDwoNum() { return llvm::None; }
+
   static bool
   DIEInDeclContext(const lldb_private::CompilerDeclContext *parent_decl_ctx,
const DWARFDIE &die);
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

[Lldb-commits] [PATCH] D63165: Initial support for native debugging of x86/x64 Windows processes

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp:315
+ lldb::addr_t &load_addr) {
+  return GetProcessBaseAddress(m_pid, load_addr);
+}

This doesn't look right. You're completely ignoring the `file_name` argument. 
The idea of this function is to return the load (base) address of the module 
specified by that argument. Here I guess you're always returning the load 
address of the main module.



Comment at: 
source/Plugins/Process/Windows/Common/NativeProcessWindows.cpp:529-530
+  ProcessAttachInfo attach_info;
+  attach_info.SetProcessID(pid);
+  attach_info.SetArchitecture(process_info.GetArchitecture());
+

Hui wrote:
> labath wrote:
> > Fetching the architecture this way looks like it could be racy if the pid 
> > is recycled before you get a chance to attach to the process (can that 
> > happen?). Is there a way to fetch the architecture *after* you perform the 
> > attach operation.
> This factory attach will return a constructed native process which needs 
> process architecture ahead to construct its native thread with a proper 
> register context. So I think can't do it after the attach operation. At least 
> need to do it before the factory routine returns. Do you mean to put these 
> codes before line 540, i.e. return std::move(process_up)?
I meant doing this *before* the factory returns, but *after* you perform the 
actual OS attach syscall is completed.

Combining this with the constructor suggestion below, I'd imagine doing 
something like
```
Error E = Error::success();
auto process_up = std::make_unique(pid, -1, 
native_delegate, E);
if (E)
  return E;
return std::move(process_up);
```

Then in the process constructor, you'd do:
```
ErrorAsOutParameter EOut(E);
auto delegate_sp = std::make_shared(*this);
ProcessAttachInfo attach_info;
attach_info.SetPID(pid); // TODO: This is dumb, we are already passing the pid 
separately
E = AttachProcess(pid, attach_info, delegate).ToError();
if (E)
  return;
SetID(GetDebuggedProcessId());
ProcessInstanceInfo info;
if (!Host::GetProcessInfo(pid, info)) {
  E = createStringError(inconvertibleErrorCode(), "Cannot get process 
information")
  return;
}
m_arch = info.GetArchitecture();
```

Among other things, I'm trying to cut down on the number of layers one has to 
go through in order to launch/attach. This file already has a number of 
functions called "attach", D63166 has a bunch more, and then there even more in 
DebuggerThread. By the time one gets to the bottom of the stack and figures out 
what's actually happening, he's forgotten what was he actually trying to 
achieve.



Comment at: source/Plugins/Process/Windows/Common/NativeProcessWindows.h:31
+class NativeProcessWindows : public NativeProcessProtocol,
+ public ProcessDebugger {
+

I'm not sure this multiple inheritance is really helping anything here. While 
looking through the code i kept wondering "why doesn't this function just do 
the job itself instead of delegating to somebody else" only to have to remind 
myself that the function it is delegating to is actually a different object, 
which does not know anything about the bigger picture.

If using composition here wouldn't introduce additional complexities/code 
(which it doesn't look like it would), I think it might be better to do that 
instead.



Comment at: source/Plugins/Process/Windows/Common/NativeProcessWindows.h:133
+public:
+  NativeDebugDelegate(NativeProcessWindows *process) : m_process(process) {}
+

It doesn't look like `process` should ever be null. So, I'd suggest replacing 
the pointer by a reference, and deleting all the null checks.



Comment at: 
source/Plugins/Process/Windows/Common/NativeRegisterContextWindows.h:31
+protected:
+  Status ReadAllRegisterValues(lldb::DataBufferSP &data_sp,
+   const size_t data_size);

Hui wrote:
> labath wrote:
> > Is this overriding something? Can you please use `override` to indicate 
> > that (throughout this patch)?
> No, it doesn't override anything. It has different signature from  the pure 
> virtual method with the same name.
> 
> 
> ```
> NativeRegisterContext::virtual Status 
> ReadAllRegisterValues(lldb::DataBufferSP &data_sp) = 0;
> ```
> 
> It would be better to change the name to be ReadAllRegisterValuesWithSize or 
> something else.
Hm.., interesting. I think I am starting to understand what's going on here. 
IIUC, the size of the register context is a constant for any given 
architecture, but it varies between architectures (i.e., 
NativeRegisterContextWindows_XXX instances). If that's the case, then it sounds 
like the data_size should be an argument to the NativeRegisterContextWindows 
constructor, instead it being passed down for every call. Would that w

[Lldb-commits] [PATCH] D63166: Move common functionality from processwindows into processdebugger

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: source/Plugins/Process/Windows/Common/ProcessDebugger.cpp:169-170
+
+Status ProcessDebugger::AttachProcess(lldb::pid_t pid,
+  const ProcessAttachInfo &attach_info,
+  DebugDelegateSP delegate) {

Hui wrote:
> Hui wrote:
> > labath wrote:
> > > BTW, looking at the other patch, I just realized that this api is 
> > > extremely redundant:
> > > a) `ProcessAttachInfo` already contains a `pid` field so passing it 
> > > separately makes no sense
> > > b) `DebuggerThread` does not seem to be doing anything with the 
> > > ProcessAttachInfo struct anyway.
> > I once checked the pid shipped in attach_info. It was 
> > LLDB_INVALID_PROCESS_ID.  Maybe it is an issue to fix.
> Just double checked. The pid shipped in attach_info is correct. So the pid 
> field can be removed. 
> However the Process class does have a virtual method DoAttachToProcessWithID 
> that requires pid as an input.
Hm.., I didn't realize this comes all the way down from the Process. Maybe 
let's keep this as is for now, since this patch is just about moving code.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63166



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

[Lldb-commits] [PATCH] D63339: Extend D55859 symbols.enable-external-lookup=false for more testcases

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

It looks like lldb-mi has the `--source` option. Could that be used to set this 
setting automatically via lit, as it is done with `%lldb` ?


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63339



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

[Lldb-commits] [PATCH] D63339: Extend D55859 symbols.enable-external-lookup=false for more testcases

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

In D63339#1546076 , @labath wrote:

> It looks like lldb-mi has the `--source` option. Could that be used to set 
> this setting automatically via lit, as it is done with `%lldb` ?


The problem is it generates additional `^done` line

  echo 'settings set symbols.enable-external-lookup false' >/tmp/1j;lldb-mi 
--source /tmp/1j
  (gdb)
  settings set symbols.enable-external-lookup false
  ^done
  (gdb)

which the scripts do not expect. They already expect such "unexpected" response 
from a commandline parameter the scripts pass themselves (`%t`):

  # RUN: %lldbmi %t < %s | FileCheck %s
  # Check that we have a valid target created via '%lldbmi %t'.
  # CHECK: ^done

This would mean adding additional one `# CHECK: ^done` for the `settings set 
symbols.enable-external-lookup false` command which is not present in the 
script which I hope you agree is worse than adding both that command+its 
response as I did.
Or did you mean it some different way? Thanks for the review.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63339



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

[Lldb-commits] [PATCH] D63339: Extend D55859 symbols.enable-external-lookup=false for more testcases

[Pavel Labath via Phabricator via lldb-commits]

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

In D63339#1546091 , @jankratochvil 
wrote:

> In D63339#1546076 , @labath wrote:
>
> > It looks like lldb-mi has the `--source` option. Could that be used to set 
> > this setting automatically via lit, as it is done with `%lldb` ?
>
>
> The problem is it generates additional `^done` line
>
>   echo 'settings set symbols.enable-external-lookup false' >/tmp/1j;lldb-mi 
> --source /tmp/1j
>   (gdb)
>   settings set symbols.enable-external-lookup false
>   ^done
>   (gdb)
>
>
>   which the scripts do not expect. They already expect such "unexpected" 
> response from a commandline parameter the scripts pass themselves (`%t`):
>
>   # RUN: %lldbmi %t < %s | FileCheck %s
>   # Check that we have a valid target created via '%lldbmi %t'.
>   # CHECK: ^done
>
>
> This would mean adding additional one `# CHECK: ^done` for the `settings set 
> symbols.enable-external-lookup false` command which is not present in the 
> script which I hope you agree is worse than adding both that command+its 
> response as I did.
>  Or did you mean it some different way? Thanks for the review.


Nope, that's exactly what I meant. I think the fact that you're not able to do 
this is a pretty big flaw in these lldb-mi tests, but I've already kind of 
given up on lldb-mi, so I don't care either way..

The rest of the changes seem fine to me.




Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:736
 # init file unless told otherwise.
-if "NO_LLDBINIT" in os.environ and "NO" == os.environ["NO_LLDBINIT"]:
-self.lldbOption = ""
-else:
-self.lldbOption = "--no-lldbinit"
+if "NO_LLDBINIT" not in os.environ or "NO" != 
os.environ["NO_LLDBINIT"]:
+self.lldbOption += " --no-lldbinit"

```
if os.environ.get("NO_LLDBINIT") != "NO":
```


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D63339



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

[Lldb-commits] [PATCH] D63400: DWARF: Provide accessors to DIERef fields

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Just questioning if we want to increase the size of the DIERef struct.




Comment at: source/Plugins/SymbolFile/DWARF/DIERef.h:47-48
+private:
+  unsigned m_section : 1;
+  dw_offset_t m_unit_offset;
+  dw_offset_t m_die_offset;

We we want this to be:
```
dw_offset_t m_section:1;
dw_offset_t m_unit_offset:31;
```

Otherwise we increase the size of the DIERef struct?


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

https://reviews.llvm.org/D63400



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

[Lldb-commits] [lldb] r363567 - [lldb] [test] Extend D55859 symbols.enable-external-lookup=false for more testcases

[Jan Kratochvil via lldb-commits]

Author: jankratochvil
Date: Mon Jun 17 07:46:17 2019
New Revision: 363567

URL: http://llvm.org/viewvc/llvm-project?rev=363567&view=rev
Log:
[lldb] [test] Extend D55859 symbols.enable-external-lookup=false for more 
testcases

D55859  has no effect for some of the
testcases so this patch extends it even for (all?) other testcases known to me.
LLDB was failing when LLDB prints errors reading system debug infos
(`*-debuginfo.rpm`, DWZ-optimized) which should never happen as LLDB testcases
should not be affected by system debug infos.

`lldb/packages/Python/lldbsuite/test/api/multithreaded/driver.cpp.template` is
using only SB API which does not expose `ModuleList` so I had to call
`HandleCommand()` there.

`lldb-test.cpp` could also use `HandleCommand` and then there would be no need
for `ModuleListProperties::SetEnableExternalLookup()` but I think it is cleaner
with API and not on based on text commands.

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

Modified:
lldb/trunk/include/lldb/Core/ModuleList.h
lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test
lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert.test
lldb/trunk/lit/tools/lldb-mi/data/data-info-line.test
lldb/trunk/lit/tools/lldb-mi/exec/exec-continue.test
lldb/trunk/lit/tools/lldb-mi/exec/exec-finish.test
lldb/trunk/lit/tools/lldb-mi/exec/exec-interrupt.test
lldb/trunk/lit/tools/lldb-mi/exec/exec-next-instruction.test
lldb/trunk/lit/tools/lldb-mi/exec/exec-next.test
lldb/trunk/lit/tools/lldb-mi/exec/exec-step-instruction.test
lldb/trunk/lit/tools/lldb-mi/exec/exec-step.test

lldb/trunk/packages/Python/lldbsuite/test/api/multithreaded/driver.cpp.template
lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
lldb/trunk/source/Core/ModuleList.cpp
lldb/trunk/tools/lldb-test/lldb-test.cpp

Modified: lldb/trunk/include/lldb/Core/ModuleList.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/ModuleList.h?rev=363567&r1=363566&r2=363567&view=diff
==
--- lldb/trunk/include/lldb/Core/ModuleList.h (original)
+++ lldb/trunk/include/lldb/Core/ModuleList.h Mon Jun 17 07:46:17 2019
@@ -52,6 +52,7 @@ public:
   FileSpec GetClangModulesCachePath() const;
   bool SetClangModulesCachePath(llvm::StringRef path);
   bool GetEnableExternalLookup() const;
+  bool SetEnableExternalLookup(bool new_value);
 }; 
 
 /// \class ModuleList ModuleList.h "lldb/Core/ModuleList.h"

Modified: 
lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test?rev=363567&r1=363566&r2=363567&view=diff
==
--- lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test 
(original)
+++ lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test 
Mon Jun 17 07:46:17 2019
@@ -7,6 +7,9 @@
 
 # Test for enabling pending breakpoints globally
 
+settings set symbols.enable-external-lookup false
+# CHECK: ^done
+
 -break-insert printf
 # CHECK: ^error,msg="Command 'break-insert'. Breakpoint location 'printf' not 
found
 

Modified: lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert.test?rev=363567&r1=363566&r2=363567&view=diff
==
--- lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert.test (original)
+++ lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert.test Mon Jun 17 
07:46:17 2019
@@ -6,6 +6,9 @@
 
 # Test that a breakpoint can be inserted before creating a target.
 
+settings set symbols.enable-external-lookup false
+# CHECK: ^done
+
 -break-insert breakpoint
 # CHECK: ^done,bkpt={number="1"
 

Modified: lldb/trunk/lit/tools/lldb-mi/data/data-info-line.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/tools/lldb-mi/data/data-info-line.test?rev=363567&r1=363566&r2=363567&view=diff
==
--- lldb/trunk/lit/tools/lldb-mi/data/data-info-line.test (original)
+++ lldb/trunk/lit/tools/lldb-mi/data/data-info-line.test Mon Jun 17 07:46:17 
2019
@@ -8,6 +8,9 @@
 # Check that we have a valid target created via '%lldbmi %t'.
 # CHECK: ^done
 
+settings set symbols.enable-external-lookup false
+# CHECK: ^done
+
 -break-insert main
 # CHECK: ^done,bkpt={number="1"
 

Modified: lldb/trunk/lit/tools/lldb-mi/exec/exec-continue.test
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/lit/tools/lldb-mi/exec/exec-continue.test?rev=363567&r1=363566&r2=363567&view=diff
==

[Lldb-commits] [PATCH] D63339: Extend D55859 symbols.enable-external-lookup=false for more testcases

[Jan Kratochvil via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363567: [lldb] [test] Extend D55859 
symbols.enable-external-lookup=false for more… (authored by jankratochvil, 
committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D63339?vs=204785&id=205070#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D63339

Files:
  lldb/trunk/include/lldb/Core/ModuleList.h
  lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert-enable-pending.test
  lldb/trunk/lit/tools/lldb-mi/breakpoint/break-insert.test
  lldb/trunk/lit/tools/lldb-mi/data/data-info-line.test
  lldb/trunk/lit/tools/lldb-mi/exec/exec-continue.test
  lldb/trunk/lit/tools/lldb-mi/exec/exec-finish.test
  lldb/trunk/lit/tools/lldb-mi/exec/exec-interrupt.test
  lldb/trunk/lit/tools/lldb-mi/exec/exec-next-instruction.test
  lldb/trunk/lit/tools/lldb-mi/exec/exec-next.test
  lldb/trunk/lit/tools/lldb-mi/exec/exec-step-instruction.test
  lldb/trunk/lit/tools/lldb-mi/exec/exec-step.test
  
lldb/trunk/packages/Python/lldbsuite/test/api/multithreaded/driver.cpp.template
  lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
  lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
  lldb/trunk/source/Core/ModuleList.cpp
  lldb/trunk/tools/lldb-test/lldb-test.cpp

Index: lldb/trunk/include/lldb/Core/ModuleList.h
===
--- lldb/trunk/include/lldb/Core/ModuleList.h
+++ lldb/trunk/include/lldb/Core/ModuleList.h
@@ -52,6 +52,7 @@
   FileSpec GetClangModulesCachePath() const;
   bool SetClangModulesCachePath(llvm::StringRef path);
   bool GetEnableExternalLookup() const;
+  bool SetEnableExternalLookup(bool new_value);
 }; 
 
 /// \class ModuleList ModuleList.h "lldb/Core/ModuleList.h"
Index: lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py
@@ -729,12 +729,12 @@
 else:
 self.lldbVSCodeExec = None
 
+self.lldbOption = "-o 'settings set symbols.enable-external-lookup false'"
+
 # If we spawn an lldb process for test (via pexpect), do not load the
 # init file unless told otherwise.
-if "NO_LLDBINIT" in os.environ and "NO" == os.environ["NO_LLDBINIT"]:
-self.lldbOption = ""
-else:
-self.lldbOption = "--no-lldbinit"
+if os.environ.get("NO_LLDBINIT") != "NO":
+self.lldbOption += " --no-lldbinit"
 
 # Assign the test method name to self.testMethodName.
 #
Index: lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
+++ lldb/trunk/packages/Python/lldbsuite/test/tools/lldb-vscode/vscode.py
@@ -447,8 +447,10 @@
 args_dict['waitFor'] = waitFor
 if trace:
 args_dict['trace'] = trace
+args_dict['initCommands'] = [
+'settings set symbols.enable-external-lookup false']
 if initCommands:
-args_dict['initCommands'] = initCommands
+args_dict['initCommands'].extend(initCommands)
 if preRunCommands:
 args_dict['preRunCommands'] = preRunCommands
 if stopCommands:
@@ -582,8 +584,10 @@
 args_dict['shellExpandArguments'] = shellExpandArguments
 if trace:
 args_dict['trace'] = trace
+args_dict['initCommands'] = [
+'settings set symbols.enable-external-lookup false']
 if initCommands:
-args_dict['initCommands'] = initCommands
+args_dict['initCommands'].extend(initCommands)
 if preRunCommands:
 args_dict['preRunCommands'] = preRunCommands
 if stopCommands:
Index: lldb/trunk/packages/Python/lldbsuite/test/api/multithreaded/driver.cpp.template
===
--- lldb/trunk/packages/Python/lldbsuite/test/api/multithreaded/driver.cpp.template
+++ lldb/trunk/packages/Python/lldbsuite/test/api/multithreaded/driver.cpp.template
@@ -31,6 +31,7 @@
 
   SBDebugger::Initialize();
   SBDebugger dbg = SBDebugger::Create();
+  dbg.HandleCommand("settings set symbols.enable-external-lookup false");
 
   try {
 if (!dbg.IsValid())
Index: lldb/trunk/source/Core/ModuleList.cpp
===
--- lldb/trunk/source/Core/ModuleList.cpp
+++ lldb/trunk/source/Core/ModuleList.cpp
@@ -102,6 +102,11 @@
   nullptr, idx, g_properties[idx].default_uint_value != 0);
 }
 
+bool ModuleListProperties::SetEnableExternalLookup(bool new_value) {
+  return m_collection_sp->SetPrope

[Lldb-commits] [PATCH] D63400: DWARF: Provide accessors to DIERef fields

[Greg Clayton via Phabricator via lldb-commits]

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



Comment at: source/Plugins/SymbolFile/DWARF/DIERef.h:48
+  unsigned m_section : 1;
+  dw_offset_t m_unit_offset;
+  dw_offset_t m_die_offset;

Never mind, saw your other DWO related patch. Makes sense now.


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

https://reviews.llvm.org/D63400



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

[Lldb-commits] [PATCH] D63428: DWARF: Add "dwo_num" field to the DIERef class

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I am concerned with increasing the size of DIERef objects. If we go to 12 bytes 
per DIERef, and we mostly store these in NameToDIE in 
"lldb_private::UniqueCStringMap m_map;" maps, which contain a 
std::vector of UniqueCStringMap::Entry objects which are:

  struct Entry {
ConstString cstring;
T value;
  };

Where T is DIERef. This will align to 24 bytes. Before we would be at 16 when 
DIERef was 8 bytes. I know we already exceeded the 8 byte size of DIERef with 
the added "section" field in DIERef with SVN revision 360872, so we already 
have 24 byte alignment, but the memory used by LLDB for large debug sessions 
will have increased significantly since a month ago.

If DIERef becomes the same size as DWARFDIE objects, we still need DIERef as 
when we index the DWARF we end up with DIERef objects and then we can unload 
the actual DWARF for the compile unit (so we can't switch over to using 
DWARFDIE since they have pointers to the loaded DWARF DIEs, but we will need to 
watch for code that might have been using a DIERef in lieu of DWARFDIE objects 
due to the DIERef size being smaller, but now that they are closer or will 
usually align to the same size, some clients might be able to switch over to 
use DWARFDIE objects now that there is really no size win.

So to address my concern with increase in DIERef size is wether we can use the 
"lldb::user_id_t" in the NameToDie map if we can always change a user_id_t into 
a DIERef and get back a size win in the process? Possibly look at anyone else 
storing DIERef objects and see if they can use lldb::user_id_t values too?




Comment at: source/Plugins/SymbolFile/DWARF/DIERef.h:59-62
+  unsigned m_dwo_num : 31;
   unsigned m_section : 1;
   dw_offset_t m_unit_offset;
   dw_offset_t m_die_offset;

I'm a bit concerned about increasing the size by 4 bytes here. All of our 
indexes are using maps of name to DIERef objects right? 



Comment at: source/Plugins/SymbolFile/DWARF/DIERef.h:64
 };
+static_assert(sizeof(DIERef) == 12, "");
 

Insert a message here? 



Comment at: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp:245-246
 
-DIERef ref(unit.GetDebugSection(), cu_offset, die.GetOffset());
+DIERef ref(unit.GetSymbolFileDWARF().GetDwoNum(), unit.GetDebugSection(),
+   unit.GetOffset(), die.GetOffset());
 switch (tag) {

Make a DWARFDIE accessor function?:
```
DIERef DWARFDIE::GetDIERef() const;
```



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1494
+return DebugInfo()
+->GetUnitAtIndex(*die_ref.dwo_num())
+->GetDwoSymbolFile()

Can a DWO file be missing and ever return NULL here?



Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1495
+->GetUnitAtIndex(*die_ref.dwo_num())
+->GetDwoSymbolFile()
+->GetDIE(die_ref);

NULL can never be returned here?


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

https://reviews.llvm.org/D63428



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

[Lldb-commits] [PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

[Gabor Marton via Phabricator via lldb-commits]

martong updated this revision to Diff 205093.
martong added a comment.

- Fix regression of TestFormatters.py


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61333

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  lldb/packages/Python/lldbsuite/test/lang/c/ast/Makefile
  lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py
  lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c
  lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

Index: lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
@@ -609,6 +609,8 @@
   if (!original_decl_context)
 return;
 
+  // Indicates whether we skipped any Decls of the original DeclContext.
+  bool SkippedDecls = false;
   for (TagDecl::decl_iterator iter = original_decl_context->decls_begin();
iter != original_decl_context->decls_end(); ++iter) {
 Decl *decl = *iter;
@@ -637,21 +639,22 @@
 
 m_ast_importer_sp->RequireCompleteType(copied_field_type);
   }
-
-  DeclContext *decl_context_non_const =
-  const_cast(decl_context);
-
-  if (copied_decl->getDeclContext() != decl_context) {
-if (copied_decl->getDeclContext()->containsDecl(copied_decl))
-  copied_decl->getDeclContext()->removeDecl(copied_decl);
-copied_decl->setDeclContext(decl_context_non_const);
-  }
-
-  if (!decl_context_non_const->containsDecl(copied_decl))
-decl_context_non_const->addDeclInternal(copied_decl);
+} else {
+  SkippedDecls = true;
 }
   }
 
+  // CopyDecl may build a lookup table which may set up ExternalLexicalStorage
+  // to false.  However, since we skipped some of the external Decls we must
+  // set it back!
+  if (SkippedDecls) {
+decl_context->setHasExternalLexicalStorage(true);
+// This sets HasLazyExternalLexicalLookups to true.  By setting this bit we
+// ensure that the lookup table is rebuilt, which means the external source
+// is consulted again when a clang::DeclContext::lookup is called.
+const_cast(decl_context)->setMustBuildLookupTable();
+  }
+
   return;
 }
 
Index: lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
===
--- lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
+++ lldb/packages/Python/lldbsuite/test/lang/c/modules/main.c
@@ -5,11 +5,11 @@
 typedef struct {
 int a;
 int b;
-} FILE;
+} MYFILE;
 
 int main()
 {
-FILE *myFile = malloc(sizeof(FILE));
+MYFILE *myFile = malloc(sizeof(MYFILE));
 
 myFile->a = 5;
 myFile->b = 9;
Index: lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/c/ast/main.c
@@ -0,0 +1,5 @@
+int main()
+{
+int a = 0; // Set breakpoint 0 here.
+return 0;
+}
Index: lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py
===
--- /dev/null
+++ lldb/packages/Python/lldbsuite/test/lang/c/ast/TestAST.py
@@ -0,0 +1,77 @@
+"""Test that importing modules in C works as expected."""
+
+from __future__ import print_function
+
+
+from distutils.version import StrictVersion
+import os
+import time
+import platform
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class CModulesTestCase(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+@skipIfFreeBSD
+@skipIfLinux
+@skipIfWindows
+@skipIfNetBSD
+@skipIf(macos_version=["<", "10.12"])
+def test_expr(self):
+self.build()
+exe = self.getBuildArtifact("a.out")
+self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+lldbutil.run_break_set_by_file_and_line(
+self, "main.c", self.line, num_expected_locations=1, loc_exact=True)
+
+self.runCmd("run", RUN_SUCCEEDED)
+
+# The stop reason of the thread should be breakpoint.
+self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stopped',
+ 'stop reason = breakpoint'])
+
+# The breakpoint should have a hit count of 1.
+self.expect("breakpoint list -f", BREAKPOINT_HIT_ONCE,
+substrs=[' resolved, hit count = 1'])
+
+# Enable logging of the imported AST.
+log_file = os.path.join(self.getBuildDir(), "lldb-ast-log.txt")
+self.runCmd("log enable lldb ast -f '%s'" % log_file)
+
+self.expect(
+"expr -l objc++ -- @import Darwin; 3",
+VARIABLE

[Lldb-commits] [PATCH] D61333: [ASTImporter] Fix LLDB lookup in transparent ctx and with ext src

[Gabor Marton via Phabricator via lldb-commits]

martong added a comment.

About the regression of TestFormatters.py: I realized that the problem is about 
the wrong implementation of the ExternalASTSource interface.
In the implementation of FindExternalLexicalDecls of this interface, we simply 
ignored those cases when the given predicate (passed as a param) is false. When 
that happens, that means we still have some more external decls which should be 
dug up by the upcoming calls of DeclContext::lookup. The fix is about to 
indicate this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61333



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

[Lldb-commits] [PATCH] D63399: DWARF: Make DIERefs always valid

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I am concerned that our mapping from DIERef to lldb::user_id_t won't work for 
all cases now that we are/have expanded the DIERef class (including as we add 
the DWO field). I voiced this concern in https://reviews.llvm.org/D63428. Let 
me know what you think.




Comment at: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp:59
 
   DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, 
*cu_offset);
   if (!cu)

Shouldn't we be using the section from the DIERef in "entry" instead of hard 
coding to "DIERef::Section::DebugInfo" here? If so we need a test to cover this 
case so we don't regress



Comment at: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp:70
   if (llvm::Optional die_offset = entry.getDIEUnitOffset())
 return DIERef(DIERef::Section::DebugInfo, *cu_offset, die_bias + 
*die_offset);
 

Shouldn't we be using the section from the DIERef in "entry" instead of hard 
coding to "DIERef::Section::DebugInfo" here?




Comment at: source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h:64
+explicit operator DIERef() const {
+  return DIERef(DIERef::Section::DebugInfo, DW_INVALID_OFFSET, die_offset);
+}

DIEInfo objects are only ever used in .debug_info?


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

https://reviews.llvm.org/D63399



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

[Lldb-commits] [PATCH] D63441: [zorg] Add lldb-arm-ubuntu builder

[Muhammad Omair Javaid via Phabricator via lldb-commits]

omjavaid created this revision.
omjavaid added reviewers: labath, jankratochvil, gkistanova.
Herald added subscribers: kristof.beyls, javed.absar.

This patch adds lldb aarch64 linux builder. It ll run on staging master until 
tests become stable.


https://reviews.llvm.org/D63441

Files:
  buildbot/osuosl/master/config/builders.py
  buildbot/osuosl/master/config/slaves.py
  buildbot/osuosl/master/config/status.py


Index: buildbot/osuosl/master/config/status.py
===
--- buildbot/osuosl/master/config/status.py
+++ buildbot/osuosl/master/config/status.py
@@ -279,7 +279,7 @@
 extraRecipients = ["omair.jav...@linaro.org"],
 subject="Build %(builder)s Failure",
 mode = "failing",
-builders = ["lldb-aarch64-ubuntu"],
+builders = ["lldb-arm-ubuntu","lldb-aarch64-ubuntu"],
 addLogs=False),
 InformativeMailNotifier(
 fromaddr = "llvm.buildmas...@lab.llvm.org",
Index: buildbot/osuosl/master/config/slaves.py
===
--- buildbot/osuosl/master/config/slaves.py
+++ buildbot/osuosl/master/config/slaves.py
@@ -43,6 +43,7 @@
 # sure we have plenty CPU cycle to satisfy timing assumptions.
 create_slave("linaro-armv8-01-arm-libcxx", properties={'jobs' : 1}, 
max_builds=1),
 create_slave("linaro-armv8-01-arm-libcxx-noeh", properties={'jobs' : 
1}, max_builds=1),
+create_slave("linaro-armv8-01-lldb-arm", properties={'jobs' : 64}, 
max_builds=1),
 # Packet.Net ThunderX1 for LLDB buildbot - Ubuntu Xenial 16.04 arm64 
container
 create_slave("linaro-thx1-lldb-aarch64", properties={'jobs': 16}, 
max_builds=1),
 
Index: buildbot/osuosl/master/config/builders.py
===
--- buildbot/osuosl/master/config/builders.py
+++ buildbot/osuosl/master/config/builders.py
@@ -856,6 +856,17 @@
   '-DLLVM_USE_LINKER=gold',
   '-DCMAKE_C_COMPILER=clang',
   '-DCMAKE_CXX_COMPILER=clang++'])},
+{'name': "lldb-arm-ubuntu",
+ 'slavenames': ["linaro-armv8-01-lldb-arm"],
+ 'builddir': "lldb-cmake-arm",
+ 'category' : 'lldb',
+ 'factory': LLDBBuilder.getLLDBCMakeBuildFactory(
+test=True,
+extra_cmake_args=['-DLLVM_ENABLE_ASSERTIONS=True',
+  '-DLLVM_LIT_ARGS="-sv --threads=8"',
+  '-DLLVM_USE_LINKER=gold',
+  '-DCMAKE_C_COMPILER=clang',
+  '-DCMAKE_CXX_COMPILER=clang++'])},
 {'name': "lldb-x64-windows-ninja",
  'slavenames': ["win-py3-buildbot"],
  'builddir': "lldb-x64-windows-ninja",


Index: buildbot/osuosl/master/config/status.py
===
--- buildbot/osuosl/master/config/status.py
+++ buildbot/osuosl/master/config/status.py
@@ -279,7 +279,7 @@
 extraRecipients = ["omair.jav...@linaro.org"],
 subject="Build %(builder)s Failure",
 mode = "failing",
-builders = ["lldb-aarch64-ubuntu"],
+builders = ["lldb-arm-ubuntu","lldb-aarch64-ubuntu"],
 addLogs=False),
 InformativeMailNotifier(
 fromaddr = "llvm.buildmas...@lab.llvm.org",
Index: buildbot/osuosl/master/config/slaves.py
===
--- buildbot/osuosl/master/config/slaves.py
+++ buildbot/osuosl/master/config/slaves.py
@@ -43,6 +43,7 @@
 # sure we have plenty CPU cycle to satisfy timing assumptions.
 create_slave("linaro-armv8-01-arm-libcxx", properties={'jobs' : 1}, max_builds=1),
 create_slave("linaro-armv8-01-arm-libcxx-noeh", properties={'jobs' : 1}, max_builds=1),
+create_slave("linaro-armv8-01-lldb-arm", properties={'jobs' : 64}, max_builds=1),
 # Packet.Net ThunderX1 for LLDB buildbot - Ubuntu Xenial 16.04 arm64 container
 create_slave("linaro-thx1-lldb-aarch64", properties={'jobs': 16}, max_builds=1),
 
Index: buildbot/osuosl/master/config/builders.py
===
--- buildbot/osuosl/master/config/builders.py
+++ buildbot/osuosl/master/config/builders.py
@@ -856,6 +856,17 @@
   '-DLLVM_USE_LINKER=gold',
   '-DCMAKE_C_COMPILER=clang',
   '-DCMAKE_CXX_COMPILER=clang++'])},
+{'name': "lldb-arm-ubuntu",
+ 'slavenames': ["linaro-armv8-01-lldb-arm"],
+ 'builddir': "lldb-cmake-arm",
+ 'category' : 'lldb',
+ 'factory': LLDBBuilder.getLLDBCMakeBuildFactory(
+test=Tr

[Lldb-commits] [PATCH] D63363: [Signals] Create a plugin directory just for signals

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D63363#1545427 , @labath wrote:

> Although this is technically correct and pretty consistent with our other 
> "plugins", I can't help but feel that it's incredibly wasteful. Each of the 
> XXXSignals.cpp files is less than a 100 lines long (with the licence header 
> and all bolierplate) and it's unlikely to ever grow beyond that. And 
> essentially, all these files do is define a single enum. The reason they are 
> this long is because the UnixSignals class is already over-engineered (e.g. I 
> don't see why LinuxSignals needs to be a separate class, or why it needs to 
> repeat the descriptions/default stop values for each of the signals). Making 
> this a plugin would just add another chunk of boilerplate on top of that.
>
> I don't know about others, but I'd rather us move in a direction which 
> reduces the amount of boilerplate instead of adding more of it. In my ideal 
> world, each of these signal definitions would just be a bunch of (number, 
> name) pairs. This doesn't have/need to be a class or a plugin; a single 
> constexpr variable would suffice for that. Then we'd just cross-reference 
> this mapping with another one which gives the default stop values and 
> descriptions for each of the signals, and that's it.
>
> I know I am repeating myself, but each time I say this, it's because I find 
> another reason for it: I think we should start a new library which I would 
> roughly define as "utilities for describing and manipulating various 
> low-level aspects of processes, but which is agnostic of any actual process 
> class". The idea would be that we can shove here all classes which are shared 
> between lldb-server liblldb. UnixSignals would be one candidate for it. 
> AuxVector, MemoryRegionInfo are others. `Plugins/Process/Utility` (where most 
> of the signal classes live) would be a pretty good place for it already, were 
> it not for the "Plugins" part (it would be weird for non-plugin code to 
> depend on something called a "plugin"). However, maybe we could just create a 
> new top-level library called "ProcessUtil" (or whatever name we come up with) 
> and move the relevant stuff there.
>
> Anyway, TL;DR: I think this should be handled differently. However, if others 
> are fine with this approach, then feel free to ignore me.


I really don't like my approach here either and agree that the Signals support 
is over-engineered as is. I also think it's quite unfortunate that UnixSignals 
inside of Target by default has the Darwin signals instead of them being broken 
out into a separate file like the rest of the platforms. I am okay with this 
approach in the short term, though.

I think that the "ProcessUtil" idea is a good one. I kind of feel like 
"Plugins/Process/Utility" is a dumping ground for loosely related classes for 
manipulating processes', but let's see what other people think about that.


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

https://reviews.llvm.org/D63363



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

[Lldb-commits] [PATCH] D63363: [Signals] Create a plugin directory just for signals

[JF Bastien via Phabricator via lldb-commits]

jfb added a comment.

Can you describe what the goal of your plugin architecture is? Maybe you need 
an RFC by email before moving stuff around.

I want to understand what you're going for because as they are today the 
signals mostly work, and aren't really tested (because injecting these 
conditions isn't trivial). Anything you change is likely to break some subtle 
invariant which will only repro when your change is deployed at scale.


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

https://reviews.llvm.org/D63363



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

[Lldb-commits] [PATCH] D63363: [Signals] Create a plugin directory just for signals

[Alex Langford via Phabricator via lldb-commits]

xiaobai added a comment.

In D63363#1546464 , @jfb wrote:

> Can you describe what the goal of your plugin architecture is? Maybe you need 
> an RFC by email before moving stuff around.
>
> I want to understand what you're going for because as they are today the 
> signals mostly work, and aren't really tested (because injecting these 
> conditions isn't trivial). Anything you change is likely to break some subtle 
> invariant which will only repro when your change is deployed at scale.


My goal is to remove non-plugin libraries dependencies on plugins. Today, 
Target depends on the ProcessUtility plugin because of UnixSignals. If Signals 
were their own plugin that could be accessed through the PluginManager 
interface, that dependency would go away. As Pavel said, this feels somewhat 
over engineered and contrived, but it is the simplest path forward. I am 
willing to drop this patch and go for a different solution if there is a nicer 
solution agreed upon by the community, so an RFC sounds like a nice idea. :)


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

https://reviews.llvm.org/D63363



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

[Lldb-commits] [PATCH] D63363: [Signals] Create a plugin directory just for signals

[JF Bastien via Phabricator via lldb-commits]

jfb added a comment.

In D63363#1546490 , @xiaobai wrote:

> In D63363#1546464 , @jfb wrote:
>
> > Can you describe what the goal of your plugin architecture is? Maybe you 
> > need an RFC by email before moving stuff around.
> >
> > I want to understand what you're going for because as they are today the 
> > signals mostly work, and aren't really tested (because injecting these 
> > conditions isn't trivial). Anything you change is likely to break some 
> > subtle invariant which will only repro when your change is deployed at 
> > scale.
>
>
> My goal is to remove non-plugin libraries dependencies on plugins. Today, 
> Target depends on the ProcessUtility plugin because of UnixSignals. If 
> Signals were their own plugin that could be accessed through the 
> PluginManager interface, that dependency would go away. As Pavel said, this 
> feels somewhat over engineered and contrived, but it is the simplest path 
> forward. I am willing to drop this patch and go for a different solution if 
> there is a nicer solution agreed upon by the community, so an RFC sounds like 
> a nice idea. :)


Removing the dependency seems fine, but we have to be careful with how signals 
work: they're inherently global state, and you want to register / de-register 
them exactly where they're registered / de-registered right now to keep them 
working exactly the same. If you change this, we need to really think through 
why that's a good idea (it might be!).


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

https://reviews.llvm.org/D63363



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

[Lldb-commits] [PATCH] D63441: [zorg] Add lldb-arm-ubuntu builder

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

> It ll run on staging master until tests become stable.

BTW the silent master is now dead since 2019-06-13.


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

https://reviews.llvm.org/D63441



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

[Lldb-commits] [PATCH] D63441: [zorg] Add lldb-arm-ubuntu builder

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

LGTM but I will leave the approval for @labath.


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

https://reviews.llvm.org/D63441



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

[Lldb-commits] [PATCH] D63399: DWARF: Make DIERefs always valid

[Pavel Labath via Phabricator via lldb-commits]

labath marked 2 inline comments as done.
labath added a comment.

In D63399#1546330 , @clayborg wrote:

> I am concerned that our mapping from DIERef to lldb::user_id_t won't work for 
> all cases now that we are/have expanded the DIERef class (including as we add 
> the DWO field). I voiced this concern in https://reviews.llvm.org/D63428. Let 
> me know what you think.


I'll reply to that review tomorrow, for now here's a quick reply to the 
comments in this review.




Comment at: source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp:59
 
   DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, 
*cu_offset);
   if (!cu)

clayborg wrote:
> Shouldn't we be using the section from the DIERef in "entry" instead of hard 
> coding to "DIERef::Section::DebugInfo" here? If so we need a test to cover 
> this case so we don't regress
debug_names is DWARF5 only, which has moved back type units into the debug_info 
section. So `entry` does not contain the section, nor does it need to because 
we can always assume all units will be in the debug_info section.



Comment at: source/Plugins/SymbolFile/DWARF/HashedNameToDIE.h:64
+explicit operator DIERef() const {
+  return DIERef(DIERef::Section::DebugInfo, DW_INVALID_OFFSET, die_offset);
+}

clayborg wrote:
> DIEInfo objects are only ever used in .debug_info?
Yes. They are only used with apple indexes, and those only work with debug_info 
sections. -fdebug-types-section is hard-disabled on apple targets nowadays (it 
used to crash the compiler before that).


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

https://reviews.llvm.org/D63399



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

[Lldb-commits] [PATCH] D63363: [Signals] Create a plugin directory just for signals

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D63363#1546504 , @jfb wrote:

> In D63363#1546490 , @xiaobai wrote:
>
> > In D63363#1546464 , @jfb wrote:
> >
> > > Can you describe what the goal of your plugin architecture is? Maybe you 
> > > need an RFC by email before moving stuff around.
> > >
> > > I want to understand what you're going for because as they are today the 
> > > signals mostly work, and aren't really tested (because injecting these 
> > > conditions isn't trivial). Anything you change is likely to break some 
> > > subtle invariant which will only repro when your change is deployed at 
> > > scale.
> >
> >
> > My goal is to remove non-plugin libraries dependencies on plugins. Today, 
> > Target depends on the ProcessUtility plugin because of UnixSignals. If 
> > Signals were their own plugin that could be accessed through the 
> > PluginManager interface, that dependency would go away. As Pavel said, this 
> > feels somewhat over engineered and contrived, but it is the simplest path 
> > forward. I am willing to drop this patch and go for a different solution if 
> > there is a nicer solution agreed upon by the community, so an RFC sounds 
> > like a nice idea. :)
>
>
> Removing the dependency seems fine, but we have to be careful with how 
> signals work: they're inherently global state, and you want to register / 
> de-register them exactly where they're registered / de-registered right now 
> to keep them working exactly the same. If you change this, we need to really 
> think through why that's a good idea (it might be!).


I think there's some misunderstading here. This code has nothing to do with 
signal handlers. All these classes do is describe the set of possible signals 
on a given target (e.g. if remote-debugging a linux mips machine from a darwin 
x86 host, we need to know what number does linux use to represent SIGSEGV on 
mips, etc.)...


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

https://reviews.llvm.org/D63363



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

[Lldb-commits] [PATCH] D63363: [Signals] Create a plugin directory just for signals

[JF Bastien via Phabricator via lldb-commits]

jfb added a comment.

In D63363#1546837 , @labath wrote:

> In D63363#1546504 , @jfb wrote:
>
> > In D63363#1546490 , @xiaobai wrote:
> >
> > > In D63363#1546464 , @jfb wrote:
> > >
> > > > Can you describe what the goal of your plugin architecture is? Maybe 
> > > > you need an RFC by email before moving stuff around.
> > > >
> > > > I want to understand what you're going for because as they are today 
> > > > the signals mostly work, and aren't really tested (because injecting 
> > > > these conditions isn't trivial). Anything you change is likely to break 
> > > > some subtle invariant which will only repro when your change is 
> > > > deployed at scale.
> > >
> > >
> > > My goal is to remove non-plugin libraries dependencies on plugins. Today, 
> > > Target depends on the ProcessUtility plugin because of UnixSignals. If 
> > > Signals were their own plugin that could be accessed through the 
> > > PluginManager interface, that dependency would go away. As Pavel said, 
> > > this feels somewhat over engineered and contrived, but it is the simplest 
> > > path forward. I am willing to drop this patch and go for a different 
> > > solution if there is a nicer solution agreed upon by the community, so an 
> > > RFC sounds like a nice idea. :)
> >
> >
> > Removing the dependency seems fine, but we have to be careful with how 
> > signals work: they're inherently global state, and you want to register / 
> > de-register them exactly where they're registered / de-registered right now 
> > to keep them working exactly the same. If you change this, we need to 
> > really think through why that's a good idea (it might be!).
>
>
> I think there's some misunderstading here. This code has nothing to do with 
> signal handlers. All these classes do is describe the set of possible signals 
> on a given target (e.g. if remote-debugging a linux mips machine from a 
> darwin x86 host, we need to know what number does linux use to represent 
> SIGSEGV on mips, etc.)...


OK, if that's the intended change, and scope doesn't widen further, it would be 
good to say so in the review. The description leads me to believe that all 
things signal-related are in scope, and this refactoring is step #1 in that 
goal.


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

https://reviews.llvm.org/D63363



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

[Lldb-commits] [PATCH] D63441: [zorg] Add lldb-arm-ubuntu builder

[Pavel Labath via Phabricator via lldb-commits]

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

lgtm


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

https://reviews.llvm.org/D63441



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

[Lldb-commits] [lldb] r363608 - Add color to the default thread and frame format.

[Jonas Devlieghere via lldb-commits]

Author: jdevlieghere
Date: Mon Jun 17 12:53:11 2019
New Revision: 363608

URL: http://llvm.org/viewvc/llvm-project?rev=363608&view=rev
Log:
Add color to the default thread and frame format.

Now that we correctly ignore ASCII escape sequences when colors are
disabled (r362240), I'd like to change the default frame and thread
format to include color in their output, in line with the syntax
highlighting that Raphael added a while ago.

This patch adds highlighting for the stop reason, the file, line and
column number. With colors disabled, this of course is a no-op.

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/macosx/nslog/TestDarwinNSLogOutput.py
lldb/trunk/source/Core/Debugger.cpp
lldb/trunk/source/Core/FormatEntity.cpp

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/macosx/nslog/TestDarwinNSLogOutput.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/nslog/TestDarwinNSLogOutput.py?rev=363608&r1=363607&r2=363608&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/macosx/nslog/TestDarwinNSLogOutput.py 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/macosx/nslog/TestDarwinNSLogOutput.py 
Mon Jun 17 12:53:11 2019
@@ -97,7 +97,7 @@ class DarwinNSLogOutputTestCase(TestBase
 
 # Ensure we stopped at a breakpoint.
 self.runCmd("thread list")
-self.expect(re.compile(r"stop reason = breakpoint"))
+self.expect(re.compile(r"stop reason = .*breakpoint"))
 
 def runCmd(self, cmd):
 if self.child:

Modified: lldb/trunk/source/Core/Debugger.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Debugger.cpp?rev=363608&r1=363607&r2=363608&view=diff
==
--- lldb/trunk/source/Core/Debugger.cpp (original)
+++ lldb/trunk/source/Core/Debugger.cpp Mon Jun 17 12:53:11 2019
@@ -121,7 +121,10 @@ static constexpr OptionEnumValueElement
   "{${frame.no-debug}${function.pc-offset"
 
 #define FILE_AND_LINE  
\
-  "{ at ${line.file.basename}:${line.number}{:${line.column}}}"
+  "{ at ${ansi.fg.cyan}${line.file.basename}${ansi.normal}"
\
+  ":${ansi.fg.yellow}${line.number}${ansi.normal}" 
\
+  "{:${ansi.fg.yellow}${line.column}${ansi.normal}}}"
+
 #define IS_OPTIMIZED "{${function.is-optimized} [opt]}"
 
 #define IS_ARTIFICIAL "{${frame.is-artificial} [artificial]}"
@@ -129,32 +132,36 @@ static constexpr OptionEnumValueElement
 #define DEFAULT_THREAD_FORMAT  
\
   "thread #${thread.index}: tid = ${thread.id%tid}"
\
   "{, ${frame.pc}}" MODULE_WITH_FUNC FILE_AND_LINE 
\
-  "{, name = '${thread.name}'}"
\
-  "{, queue = '${thread.queue}'}"  
\
-  "{, activity = '${thread.info.activity.name}'}"  
\
+  "{, name = ${ansi.fg.green}'${thread.name}'${ansi.normal}}"  
\
+  "{, queue = ${ansi.fg.green}'${thread.queue}'${ansi.normal}}"
\
+  "{, activity = " 
\
+  "${ansi.fg.green}'${thread.info.activity.name}'${ansi.normal}}"  
\
   "{, ${thread.info.trace_messages} messages}" 
\
-  "{, stop reason = ${thread.stop-reason}}"
\
+  "{, stop reason = ${ansi.fg.red}${thread.stop-reason}${ansi.normal}}"
\
   "{\\nReturn value: ${thread.return-value}}"  
\
   "{\\nCompleted expression: ${thread.completed-expression}}"  
\
   "\\n"
 
 #define DEFAULT_THREAD_STOP_FORMAT 
\
   "thread #${thread.index}{, name = '${thread.name}'}" 
\
-  "{, queue = '${thread.queue}'}"  
\
-  "{, activity = '${thread.info.activity.name}'}"  
\
+  "{, queue = ${ansi.fg.green}'${thread.queue}'${ansi.normal}}"
\
+  "{, activity = " 
\
+  "${ansi.fg.green}'${thread.info.activity.name}'${ansi.normal}}"  
\
   "{, ${thread.info.trace_messages} messages}" 
\
-  "{, stop reason = ${thread.stop-reason}}"
\
+  "{, stop reason = ${ansi.fg.red}${thread.stop-reason}${ansi.normal}}"
\
   "{\\nReturn value: ${thread.return-value}}"  
\
   "{\\nCompleted expression: ${thread.completed-expression}}"  
\
   "\\n"
 
 #define DEFAULT_FRAME_FORMAT

[Lldb-commits] [PATCH] D62743: Add color to the default thread and frame format.

[Jonas Devlieghere via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363608: Add color to the default thread and frame format. 
(authored by JDevlieghere, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62743?vs=204855&id=205155#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62743

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/macosx/nslog/TestDarwinNSLogOutput.py
  lldb/trunk/source/Core/Debugger.cpp
  lldb/trunk/source/Core/FormatEntity.cpp


Index: lldb/trunk/source/Core/FormatEntity.cpp
===
--- lldb/trunk/source/Core/FormatEntity.cpp
+++ lldb/trunk/source/Core/FormatEntity.cpp
@@ -1109,11 +1109,11 @@
 Debugger &debugger = target->GetDebugger();
 if (debugger.GetUseColor()) {
   s.PutCString(entry.string);
-  return true;
 }
   }
 }
-return false;
+// Always return true, so colors being disabled is transparent.
+return true;
 
   case Entry::Type::Root:
 for (const auto &child : entry.children) {
Index: lldb/trunk/source/Core/Debugger.cpp
===
--- lldb/trunk/source/Core/Debugger.cpp
+++ lldb/trunk/source/Core/Debugger.cpp
@@ -121,7 +121,10 @@
   "{${frame.no-debug}${function.pc-offset"
 
 #define FILE_AND_LINE  
\
-  "{ at ${line.file.basename}:${line.number}{:${line.column}}}"
+  "{ at ${ansi.fg.cyan}${line.file.basename}${ansi.normal}"
\
+  ":${ansi.fg.yellow}${line.number}${ansi.normal}" 
\
+  "{:${ansi.fg.yellow}${line.column}${ansi.normal}}}"
+
 #define IS_OPTIMIZED "{${function.is-optimized} [opt]}"
 
 #define IS_ARTIFICIAL "{${frame.is-artificial} [artificial]}"
@@ -129,32 +132,36 @@
 #define DEFAULT_THREAD_FORMAT  
\
   "thread #${thread.index}: tid = ${thread.id%tid}"
\
   "{, ${frame.pc}}" MODULE_WITH_FUNC FILE_AND_LINE 
\
-  "{, name = '${thread.name}'}"
\
-  "{, queue = '${thread.queue}'}"  
\
-  "{, activity = '${thread.info.activity.name}'}"  
\
+  "{, name = ${ansi.fg.green}'${thread.name}'${ansi.normal}}"  
\
+  "{, queue = ${ansi.fg.green}'${thread.queue}'${ansi.normal}}"
\
+  "{, activity = " 
\
+  "${ansi.fg.green}'${thread.info.activity.name}'${ansi.normal}}"  
\
   "{, ${thread.info.trace_messages} messages}" 
\
-  "{, stop reason = ${thread.stop-reason}}"
\
+  "{, stop reason = ${ansi.fg.red}${thread.stop-reason}${ansi.normal}}"
\
   "{\\nReturn value: ${thread.return-value}}"  
\
   "{\\nCompleted expression: ${thread.completed-expression}}"  
\
   "\\n"
 
 #define DEFAULT_THREAD_STOP_FORMAT 
\
   "thread #${thread.index}{, name = '${thread.name}'}" 
\
-  "{, queue = '${thread.queue}'}"  
\
-  "{, activity = '${thread.info.activity.name}'}"  
\
+  "{, queue = ${ansi.fg.green}'${thread.queue}'${ansi.normal}}"
\
+  "{, activity = " 
\
+  "${ansi.fg.green}'${thread.info.activity.name}'${ansi.normal}}"  
\
   "{, ${thread.info.trace_messages} messages}" 
\
-  "{, stop reason = ${thread.stop-reason}}"
\
+  "{, stop reason = ${ansi.fg.red}${thread.stop-reason}${ansi.normal}}"
\
   "{\\nReturn value: ${thread.return-value}}"  
\
   "{\\nCompleted expression: ${thread.completed-expression}}"  
\
   "\\n"
 
 #define DEFAULT_FRAME_FORMAT   
\
-  "frame #${frame.index}: ${frame.pc}" MODULE_WITH_FUNC FILE_AND_LINE  
\
+  "frame #${frame.index}: "
\
+  "${ansi.fg.yellow}${frame.pc}${ansi.normal}" MODULE_WITH_FUNC FILE_AND_LINE  
\
   IS_OPTIMIZED IS_ARTIFICIAL "\\n"
 
 #define DEFAULT_FRAME_FORMAT_NO_ARGS   
\
-  "frame #${frame.index}: ${frame.pc}" MODULE_WITH_FUNC_NO_ARGS FILE_AND_LINE  
\
-  IS_OPTIMIZED IS_ARTIFICIAL "\\n"
+  "frame #${frame.index}: "
\
+  "${ansi.fg.yellow}${frame.pc}${ansi.normal}" MODULE_WITH

[Lldb-commits] [PATCH] D63268: Make UniqueCStringMap work with non-default-constructible types and other improvements/cleanups

[Stella Stamenova via Phabricator via lldb-commits]

stella.stamenova added a comment.

It looks like this broke the Windows bot. I am no longer receiving emails when 
the bot fails (as bot owner I used to always get the failure emails), so I am 
assuming for some reason notifications aren't working correctly.

Can you have a look at the failure?

http://lab.llvm.org:8011/buildslaves/win-py3-buildbot


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63268



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

[Lldb-commits] [PATCH] D63166: Move common functionality from processwindows into processdebugger

[Aaron Smith via Phabricator via lldb-commits]

asmith updated this revision to Diff 205178.

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

https://reviews.llvm.org/D63166

Files:
  source/Plugins/Process/Windows/Common/CMakeLists.txt
  source/Plugins/Process/Windows/Common/ProcessDebugger.cpp
  source/Plugins/Process/Windows/Common/ProcessDebugger.h
  source/Plugins/Process/Windows/Common/ProcessWindows.cpp
  source/Plugins/Process/Windows/Common/ProcessWindows.h

Index: source/Plugins/Process/Windows/Common/ProcessWindows.h
===
--- source/Plugins/Process/Windows/Common/ProcessWindows.h
+++ source/Plugins/Process/Windows/Common/ProcessWindows.h
@@ -13,17 +13,14 @@
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-forward.h"
 
-#include "llvm/Support/Mutex.h"
-
-#include "IDebugDelegate.h"
 #include "Plugins/DynamicLoader/Windows-DYLD/DynamicLoaderWindowsDYLD.h"
+#include "ProcessDebugger.h"
 
 namespace lldb_private {
 
 class HostProcess;
-class ProcessWindowsData;
 
-class ProcessWindows : public Process, public IDebugDelegate {
+class ProcessWindows : public Process, public ProcessDebugger {
 public:
   // Static functions.
   static lldb::ProcessSP CreateInstance(lldb::TargetSP target_sp,
@@ -101,19 +98,7 @@
   void OnUnloadDll(lldb::addr_t module_addr) override;
   void OnDebugString(const std::string &string) override;
   void OnDebuggerError(const Status &error, uint32_t type) override;
-
-private:
-  Status WaitForDebuggerConnection(DebuggerThreadSP debugger,
-   HostProcess &process);
-
-  // These decode the page protection bits.
-  static bool IsPageReadable(uint32_t protect);
-  static bool IsPageWritable(uint32_t protect);
-  static bool IsPageExecutable(uint32_t protect);
-
-  llvm::sys::Mutex m_mutex;
-  std::unique_ptr m_session_data;
 };
-}
+} // namespace lldb_private
 
 #endif // liblldb_Plugins_Process_Windows_Common_ProcessWindows_H_
Index: source/Plugins/Process/Windows/Common/ProcessWindows.cpp
===
--- source/Plugins/Process/Windows/Common/ProcessWindows.cpp
+++ source/Plugins/Process/Windows/Common/ProcessWindows.cpp
@@ -70,46 +70,10 @@
   }
   return file_name;
 }
-
-DWORD ConvertLldbToWinApiProtect(uint32_t protect) {
-  // We also can process a read / write permissions here, but if the debugger
-  // will make later a write into the allocated memory, it will fail. To get
-  // around it is possible inside DoWriteMemory to remember memory permissions,
-  // allow write, write and restore permissions, but for now we process only
-  // the executable permission.
-  //
-  // TODO: Process permissions other than executable
-  if (protect & ePermissionsExecutable)
-return PAGE_EXECUTE_READWRITE;
-
-  return PAGE_READWRITE;
-}
-
 } // anonymous namespace
 
 namespace lldb_private {
 
-// We store a pointer to this class in the ProcessWindows, so that we don't
-// expose Windows-specific types and implementation details from a public
-// header file.
-class ProcessWindowsData {
-public:
-  ProcessWindowsData(bool stop_at_entry) : m_stop_at_entry(stop_at_entry) {
-m_initial_stop_event = ::CreateEvent(nullptr, TRUE, FALSE, nullptr);
-  }
-
-  ~ProcessWindowsData() { ::CloseHandle(m_initial_stop_event); }
-
-  Status m_launch_error;
-  DebuggerThreadSP m_debugger;
-  StopInfoSP m_pending_stop_info;
-  HANDLE m_initial_stop_event = nullptr;
-  bool m_initial_stop_received = false;
-  bool m_stop_at_entry;
-  std::map m_new_threads;
-  std::set m_exited_threads;
-};
-
 ProcessSP ProcessWindows::CreateInstance(lldb::TargetSP target_sp,
  lldb::ListenerSP listener_sp,
  const FileSpec *) {
@@ -192,159 +156,41 @@
 }
 
 Status ProcessWindows::DoDetach(bool keep_stopped) {
-  Log *log = ProcessWindowsLog::GetLogIfAny(WINDOWS_LOG_PROCESS);
-  DebuggerThreadSP debugger_thread;
-  StateType private_state;
-  {
-// Acquire the lock only long enough to get the DebuggerThread.
-// StopDebugging() will trigger a call back into ProcessWindows which will
-// also acquire the lock.  Thus we have to release the lock before calling
-// StopDebugging().
-llvm::sys::ScopedLock lock(m_mutex);
-
-private_state = GetPrivateState();
-
-if (!m_session_data) {
-  LLDB_LOG(log, "state = {0}, but there is no active session.",
-   private_state);
-  return Status();
-}
-
-debugger_thread = m_session_data->m_debugger;
-  }
-
   Status error;
+  Log *log = ProcessWindowsLog::GetLogIfAny(WINDOWS_LOG_PROCESS);
+  StateType private_state = GetPrivateState();
   if (private_state != eStateExited && private_state != eStateDetached) {
-LLDB_LOG(log, "detaching from process {0} while state = {1}.",
- debugger_thread->GetProcess().GetNativeProcess().GetSystemHandle(),
- private_state);
-error = debugger_thread->StopDebugg

[Lldb-commits] [PATCH] D62503: Add ReadCStringFromMemory for faster string reads

[António Afonso via Phabricator via lldb-commits]

aadsm updated this revision to Diff 205211.
aadsm added a comment.

- Add a test to cover cross page reads
- Make cache_line_size a static const variable inside the function.
- Fix how we're passing name_buffer to ReadCStringFromMemory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62503

Files:
  lldb/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/source/Host/common/NativeProcessProtocol.cpp
  lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
  lldb/unittests/Host/NativeProcessProtocolTest.cpp

Index: lldb/unittests/Host/NativeProcessProtocolTest.cpp
===
--- lldb/unittests/Host/NativeProcessProtocolTest.cpp
+++ lldb/unittests/Host/NativeProcessProtocolTest.cpp
@@ -9,6 +9,7 @@
 #include "TestingSupport/Host/NativeProcessTestUtils.h"
 
 #include "lldb/Host/common/NativeProcessProtocol.h"
+#include "llvm/Support/Process.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 
@@ -96,3 +97,53 @@
   EXPECT_THAT_EXPECTED(Process.ReadMemoryWithoutTrap(4, 2),
llvm::HasValue(std::vector{4, 5}));
 }
+
+TEST(NativeProcessProtocolTest, ReadCStringFromMemory) {
+  NiceMock DummyDelegate;
+  MockProcess Process(DummyDelegate,
+ ArchSpec("aarch64-pc-linux"));
+  FakeMemory M({'h', 'e', 'l', 'l', 'o', 0, 'w', 'o'});
+  EXPECT_CALL(Process, ReadMemory(_, _))
+  .WillRepeatedly(Invoke(&M, &FakeMemory::Read));
+
+  char string[1024];
+  size_t bytes_read;
+  EXPECT_THAT_EXPECTED(Process.ReadCStringFromMemory(
+   0x0, &string[0], sizeof(string), bytes_read),
+   llvm::HasValue(llvm::StringRef("hello")));
+  EXPECT_EQ(bytes_read, 6UL);
+}
+
+TEST(NativeProcessProtocolTest, ReadCStringFromMemory_MaxSize) {
+  NiceMock DummyDelegate;
+  MockProcess Process(DummyDelegate,
+ ArchSpec("aarch64-pc-linux"));
+  FakeMemory M({'h', 'e', 'l', 'l', 'o', 0, 'w', 'o'});
+  EXPECT_CALL(Process, ReadMemory(_, _))
+  .WillRepeatedly(Invoke(&M, &FakeMemory::Read));
+
+  char string[4];
+  size_t bytes_read;
+  EXPECT_THAT_EXPECTED(Process.ReadCStringFromMemory(
+   0x0, &string[0], sizeof(string), bytes_read),
+   llvm::HasValue(llvm::StringRef("hel")));
+  EXPECT_EQ(bytes_read, 3UL);
+}
+
+TEST(NativeProcessProtocolTest, ReadCStringFromMemory_CrossPageBoundary) {
+  NiceMock DummyDelegate;
+  MockProcess Process(DummyDelegate,
+ ArchSpec("aarch64-pc-linux"));
+  unsigned string_start = llvm::sys::Process::getPageSizeEstimate() - 3;
+  FakeMemory M({'h', 'e', 'l', 'l', 'o', 0, 'w', 'o'}, string_start);
+  EXPECT_CALL(Process, ReadMemory(_, _))
+  .WillRepeatedly(Invoke(&M, &FakeMemory::Read));
+
+  char string[1024];
+  size_t bytes_read;
+  EXPECT_THAT_EXPECTED(Process.ReadCStringFromMemory(string_start, &string[0],
+ sizeof(string),
+ bytes_read),
+   llvm::HasValue(llvm::StringRef("hello")));
+  EXPECT_EQ(bytes_read, 6UL);
+}
\ No newline at end of file
Index: lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
===
--- lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
+++ lldb/source/Plugins/Process/POSIX/NativeProcessELF.cpp
@@ -118,14 +118,13 @@
 return error.ToError();
 
   char name_buffer[PATH_MAX];
-  error = ReadMemory(link_map.l_name, &name_buffer, sizeof(name_buffer),
- bytes_read);
-  if (!error.Success())
-return error.ToError();
-  name_buffer[PATH_MAX - 1] = '\0';
+  llvm::Expected string_or_error = ReadCStringFromMemory(
+  link_map.l_name, &name_buffer[0], sizeof(name_buffer), bytes_read);
+  if (!string_or_error)
+return string_or_error.takeError();
 
   SVR4LibraryInfo info;
-  info.name = std::string(name_buffer);
+  info.name = string_or_error->str();
   info.link_map = link_map_addr;
   info.base_addr = link_map.l_addr;
   info.ld_addr = link_map.l_ld;
Index: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -2076,4 +2076,4 @@
   m_processor_trace_monitor.erase(iter);
 
   return error;
-}
\ No newline at end of file
+}
Index: lldb/source/Host/common/NativeProcessProtocol.cpp
===
--- lldb/source/Host/common/NativeProcessProtocol.cpp
+++ lldb/source/Host/common/NativeProcessProtocol.cpp
@@ -16,6 +16,8 @@
 #include "lldb/Utility/State.h"
 #include "

[Lldb-commits] [PATCH] D63467: [Reproducers] Make reproducer relocatable

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere created this revision.
JDevlieghere added reviewers: labath, aprantl, bruno.
Herald added a subscriber: dexonsmith.
Herald added a project: LLDB.

Before this patch, reproducers weren't relocatable. The reproducer contained 
hard coded paths in the VFS mapping, as well in the yaml file listing the 
different input files for the command interpreter. This patch changes that:

- Use relative paths for the `DataCollector`.
- Use an overlay prefix for the `FileCollector`.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D63467

Files:
  lldb/include/lldb/Utility/FileCollector.h
  lldb/include/lldb/Utility/Reproducer.h
  lldb/lit/Reproducer/TestReuseDirectory.test
  lldb/source/API/SBDebugger.cpp
  lldb/source/Host/common/FileSystem.cpp
  lldb/source/Utility/FileCollector.cpp
  lldb/unittests/Utility/FileCollectorTest.cpp

Index: lldb/unittests/Utility/FileCollectorTest.cpp
===
--- lldb/unittests/Utility/FileCollectorTest.cpp
+++ lldb/unittests/Utility/FileCollectorTest.cpp
@@ -105,7 +105,7 @@
 TEST(FileCollectorTest, AddFile) {
   ScopedDir root("add_file_root", true);
   FileSpec root_fs(root.Path);
-  TestingFileCollector file_collector(root_fs);
+  TestingFileCollector file_collector(root_fs, root_fs);
 
   file_collector.AddFile(FileSpec("/path/to/a"));
   file_collector.AddFile(FileSpec("/path/to/b"));
@@ -132,7 +132,7 @@
   // Create file collector and add files.
   ScopedDir root("copy_files_root", true);
   FileSpec root_fs(root.Path);
-  TestingFileCollector file_collector(root_fs);
+  TestingFileCollector file_collector(root_fs, root_fs);
   file_collector.AddFile(a.Path);
   file_collector.AddFile(b.Path);
   file_collector.AddFile(c.Path);
@@ -174,7 +174,7 @@
   // Root where files are copied to.
   ScopedDir reproducer_root("reproducer_root", true);
   FileSpec root_fs(reproducer_root.Path);
-  TestingFileCollector file_collector(root_fs);
+  TestingFileCollector file_collector(root_fs, root_fs);
 
   // Add all the files to the collector.
   file_collector.AddFile(a.Path);
Index: lldb/source/Utility/FileCollector.cpp
===
--- lldb/source/Utility/FileCollector.cpp
+++ lldb/source/Utility/FileCollector.cpp
@@ -33,7 +33,8 @@
   return true;
 }
 
-FileCollector::FileCollector(const FileSpec &root) : m_root(root) {
+FileCollector::FileCollector(const FileSpec &root, const FileSpec &overlay_root)
+: m_root(root), m_overlay_root(overlay_root) {
   sys::fs::create_directories(m_root.GetPath(), true);
 }
 
@@ -133,7 +134,9 @@
 std::error_code FileCollector::WriteMapping(const FileSpec &mapping_file) {
   std::lock_guard lock(m_mutex);
 
-  const std::string root = m_root.GetPath();
+  llvm::StringRef root = m_overlay_root.GetPath();
+
+  m_vfs_writer.setOverlayDir(root);
   m_vfs_writer.setCaseSensitivity(IsCaseSensitivePath(root));
   m_vfs_writer.setUseExternalNames(false);
 
Index: lldb/source/Host/common/FileSystem.cpp
===
--- lldb/source/Host/common/FileSystem.cpp
+++ lldb/source/Host/common/FileSystem.cpp
@@ -63,8 +63,9 @@
   if (!buffer)
 return llvm::errorCodeToError(buffer.getError());
 
-  InstanceImpl().emplace(
-  llvm::vfs::getVFSFromYAML(std::move(buffer.get()), nullptr, ""), true);
+  InstanceImpl().emplace(llvm::vfs::getVFSFromYAML(std::move(buffer.get()),
+   nullptr, mapping.GetPath()),
+ true);
 
   return llvm::Error::success();
 }
Index: lldb/source/API/SBDebugger.cpp
===
--- lldb/source/API/SBDebugger.cpp
+++ lldb/source/API/SBDebugger.cpp
@@ -82,6 +82,12 @@
 if (auto err = yin.error())
   return {};
 
+for (auto &file : files) {
+  FileSpec absolute_path =
+  loader->GetRoot().CopyByAppendingPathComponent(file);
+  file = absolute_path.GetPath();
+}
+
 return llvm::make_unique(std::move(files));
   }
 
Index: lldb/lit/Reproducer/TestReuseDirectory.test
===
--- lldb/lit/Reproducer/TestReuseDirectory.test
+++ lldb/lit/Reproducer/TestReuseDirectory.test
@@ -8,3 +8,10 @@
 # RUN: %lldb -x -b -s %S/Inputs/GDBRemoteCapture.in --capture --capture-path %t.repro %t.out | FileCheck %S/TestGDBRemoteRepro.test --check-prefix CHECK --check-prefix CAPTURE
 # RUN: %lldb -x -b -s %S/Inputs/GDBRemoteCapture.in --capture --capture-path %t.repro %t.out | FileCheck %S/TestGDBRemoteRepro.test --check-prefix CHECK --check-prefix CAPTURE
 # RUN: %lldb --replay %t.repro | FileCheck %S/TestGDBRemoteRepro.test --check-prefix CHECK --check-prefix REPLAY
+
+# Test that we can replay from a different location, i.e. that the reproducer
+# is relocatable.
+
+# RUN: rm -rf %t.repro_moved
+# RUN: mv %t.repro %t.repro_moved
+# RUN: %lldb --replay %t.repro_moved | FileChe

[Lldb-commits] [PATCH] D63370: Specify log level for CMake messages (less stderr)

[Christoph Siedentop via Phabricator via lldb-commits]

siedentop updated this revision to Diff 205242.
siedentop added a comment.

@sgraenitz , good find. I've removed the leading dashes.


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

https://reviews.llvm.org/D63370

Files:
  compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/cmake/modules/LLDBStandalone.cmake
  llvm/cmake/modules/LLVMInstallSymlink.cmake

Index: llvm/cmake/modules/LLVMInstallSymlink.cmake
===
--- llvm/cmake/modules/LLVMInstallSymlink.cmake
+++ llvm/cmake/modules/LLVMInstallSymlink.cmake
@@ -12,7 +12,7 @@
 
   set(bindir "${DESTDIR}${CMAKE_INSTALL_PREFIX}/${outdir}/")
 
-  message("Creating ${name}")
+  message(STATUS "Creating ${name}")
 
   execute_process(
 COMMAND "${CMAKE_COMMAND}" -E ${LINK_OR_COPY} "${target}" "${name}"
Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -95,7 +95,7 @@
Please install Python or specify the PYTHON_EXECUTABLE CMake variable.")
 endif()
   else()
-message("-- Found PythonInterp: ${PYTHON_EXECUTABLE}")
+message(STATUS "Found PythonInterp: ${PYTHON_EXECUTABLE}")
   endif()
 
   set(PACKAGE_VERSION "${LLVM_PACKAGE_VERSION}")
Index: lldb/cmake/modules/LLDBConfig.cmake
===
--- lldb/cmake/modules/LLDBConfig.cmake
+++ lldb/cmake/modules/LLDBConfig.cmake
@@ -128,7 +128,7 @@
 # addresses them, but it can be improved and extended on an as-needed basis.
 function(find_python_libs_windows)
   if ("${PYTHON_HOME}" STREQUAL "")
-message("LLDB embedded Python on Windows requires specifying a value for PYTHON_HOME.  Python support disabled.")
+message(WARNING "LLDB embedded Python on Windows requires specifying a value for PYTHON_HOME.  Python support disabled.")
 set(LLDB_DISABLE_PYTHON 1 PARENT_SCOPE)
 return()
   endif()
@@ -140,12 +140,12 @@
  REGEX "^#define[ \t]+PY_VERSION[ \t]+\"[^\"]+\"")
 string(REGEX REPLACE "^#define[ \t]+PY_VERSION[ \t]+\"([^\"+]+)[+]?\".*" "\\1"
  PYTHONLIBS_VERSION_STRING "${python_version_str}")
-message("-- Found Python version ${PYTHONLIBS_VERSION_STRING}")
+message(STATUS "Found Python version ${PYTHONLIBS_VERSION_STRING}")
 string(REGEX REPLACE "([0-9]+)[.]([0-9]+)[.][0-9]+" "python\\1\\2" PYTHONLIBS_BASE_NAME "${PYTHONLIBS_VERSION_STRING}")
 unset(python_version_str)
   else()
-message("Unable to find ${PYTHON_INCLUDE_DIR}/patchlevel.h, Python installation is corrupt.")
-message("Python support will be disabled for this build.")
+message(WARNING "Unable to find ${PYTHON_INCLUDE_DIR}/patchlevel.h, Python installation is corrupt.")
+message(WARNING "Python support will be disabled for this build.")
 set(LLDB_DISABLE_PYTHON 1 PARENT_SCOPE)
 return()
   endif()
@@ -165,13 +165,13 @@
 
   foreach(component PYTHON_EXE;PYTHON_LIB;PYTHON_DLL)
 if(NOT EXISTS ${${component}})
-  message("unable to find ${component}")
+  message(WARNING "unable to find ${component}")
   unset(${component})
 endif()
   endforeach()
 
   if (NOT PYTHON_EXE OR NOT PYTHON_LIB OR NOT PYTHON_DLL)
-message("Unable to find all Python components.  Python support will be disabled for this build.")
+message(WARNING "Unable to find all Python components.  Python support will be disabled for this build.")
 set(LLDB_DISABLE_PYTHON 1 PARENT_SCOPE)
 return()
   endif()
@@ -181,10 +181,10 @@
   set (PYTHON_DLL ${PYTHON_DLL} PARENT_SCOPE)
   set (PYTHON_INCLUDE_DIR ${PYTHON_INCLUDE_DIR} PARENT_SCOPE)
 
-  message("-- LLDB Found PythonExecutable: ${PYTHON_EXE}")
-  message("-- LLDB Found PythonLibs: ${PYTHON_LIB}")
-  message("-- LLDB Found PythonDLL: ${PYTHON_DLL}")
-  message("-- LLDB Found PythonIncludeDirs: ${PYTHON_INCLUDE_DIR}")
+  message(STATUS "LLDB Found PythonExecutable: ${PYTHON_EXE}")
+  message(STATUS "LLDB Found PythonLibs: ${PYTHON_LIB}")
+  message(STATUS "LLDB Found PythonDLL: ${PYTHON_DLL}")
+  message(STATUS "LLDB Found PythonIncludeDirs: ${PYTHON_INCLUDE_DIR}")
 endfunction(find_python_libs_windows)
 
 if (NOT LLDB_DISABLE_PYTHON)
Index: compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
===
--- compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
+++ compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
@@ -410,8 +410,8 @@
 INCLUDE ${arch}_FUNCTIONS
 ${${arch}_SOURCES})
   if(NOT ${arch}_filtered_sources)
-message("${arch}_SOURCES: ${${arch}_SOURCES}")
-message("${arch}_FUNCTIONS: ${${arch}_FUNCTIONS}")
+message(WARNING "${arch}_SOURCES: ${${arch}_SOURCES}")
+message(WARNING "${arch}_FUNCTIONS: ${${arch}_FUNCTIONS}")
 message(FATAL_ERROR "Empty filte

[Lldb-commits] [lldb] r363653 - Fix windows build for r363357

[Pavel Labath via lldb-commits]

Author: labath
Date: Tue Jun 18 00:02:53 2019
New Revision: 363653

URL: http://llvm.org/viewvc/llvm-project?rev=363653&view=rev
Log:
Fix windows build for r363357

MSVC has trouble referencing the protected Compare class from from the
friend Entry operator<. Just delete that operator, as it's used only
once.

Modified:
lldb/trunk/include/lldb/Core/UniqueCStringMap.h

Modified: lldb/trunk/include/lldb/Core/UniqueCStringMap.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/UniqueCStringMap.h?rev=363653&r1=363652&r2=363653&view=diff
==
--- lldb/trunk/include/lldb/Core/UniqueCStringMap.h (original)
+++ lldb/trunk/include/lldb/Core/UniqueCStringMap.h Tue Jun 18 00:02:53 2019
@@ -28,10 +28,6 @@ public:
   struct Entry {
 Entry(ConstString cstr, const T &v) : cstring(cstr), value(v) {}
 
-friend bool operator<(const Entry &lhs, const Entry &rhs) {
-  return Compare()(lhs, rhs);
-}
-
 ConstString cstring;
 T value;
   };
@@ -165,7 +161,7 @@ public:
   //  my_map.Append (UniqueCStringMap::Entry(GetName(...), GetValue(...)));
   // }
   // my_map.Sort();
-  void Sort() { llvm::sort(m_map.begin(), m_map.end()); }
+  void Sort() { llvm::sort(m_map.begin(), m_map.end(), Compare()); }
 
   // Since we are using a vector to contain our items it will always double its
   // memory consumption as things are added to the vector, so if you intend to


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