date:20191125

[Lldb-commits] [lldb] aa16bf1 - [lldb-vscode] Fix a race in test_extra_launch_commands

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


Author: Pavel Labath
Date: 2019-11-25T10:07:38+01:00
New Revision: aa16bf15fe38ae4d12d85e7610891dd48e6c0cda

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

LOG: [lldb-vscode] Fix a race in test_extra_launch_commands

Summary:
The test used a non-stopping "run" command to launch the process. This
is different from the regular launch with no extra launch commands,
which uses eLaunchFlagStopAtEntry to ensure that the process stops
straight away.

I'm not really sure what's supposed to happen in non-stop-at-entry mode,
or if that's even supported, but what ended up happening was the launch
packet got a reply while the process was running. Then the test case did
a continue_to_next_stop(), which queued a *second* resume request
(along with the internal "resumes" which were being issued as a part of
normal process startup). These two resumes ended up chasing each other's
tails inside lldb in a way which produced hilarious log traces.
Surprisingly, the test ended up passing most of the time, but it did
cause spurious failures when the test seemed to miss a breakpoint.

This changes the test to use stop-at-entry mode in the manual launch
sequence too, which seems to be enough to make the test pass reliably.

Reviewers: clayborg, kusmour, jankratochvil

Subscribers: lldb-commits

Tags: #lldb

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

Added: 


Modified: 

lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py

Removed: 




diff  --git 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
index 9c75b7e324af..414417f7d7c1 100644
--- 
a/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ 
b/lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -352,7 +352,7 @@ def test_extra_launch_commands(self):
 'target create "%s"' % (program),
 'br s -f main.c -l %d' % first_line,
 'br s -f main.c -l %d' % second_line,
-'run'
+'process launch --stop-at-entry'
 ]
 
 initCommands = ['target list', 'platform list']



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

[Lldb-commits] [PATCH] D70127: [lldb-vscode] Fix a race in test_extra_launch_commands

2019-11-25 Thread Pavel Labath via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGaa16bf15fe38: [lldb-vscode] Fix a race in 
test_extra_launch_commands (authored by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70127

Files:
  
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py


Index: 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ 
lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -352,7 +352,7 @@
 'target create "%s"' % (program),
 'br s -f main.c -l %d' % first_line,
 'br s -f main.c -l %d' % second_line,
-'run'
+'process launch --stop-at-entry'
 ]
 
 initCommands = ['target list', 'platform list']


Index: lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
===
--- lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
+++ lldb/packages/Python/lldbsuite/test/tools/lldb-vscode/launch/TestVSCode_launch.py
@@ -352,7 +352,7 @@
 'target create "%s"' % (program),
 'br s -f main.c -l %d' % first_line,
 'br s -f main.c -l %d' % second_line,
-'run'
+'process launch --stop-at-entry'
 ]
 
 initCommands = ['target list', 'platform list']
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D70023: [lldb] [Process/NetBSD] Copy watchpoints to newly-created threads

2019-11-25 Thread Pavel Labath via Phabricator via lldb-commits

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



Comment at: lldb/test/Shell/Watchpoint/netbsd-nouserdbregs.test:5
+# REQUIRES: native && system-netbsd && (target-x86 || target-x86_64) && 
!dbregs-set
+# RUN: %clang %p/Inputs/thread-dbreg.c -pthread -g -o %t.out
+# RUN: %lldb -b -o 'settings set interpreter.stop-command-source-on-error 
false' -s %s %t.out 2>&1 | FileCheck %s

mgorny wrote:
> labath wrote:
> > mgorny wrote:
> > > Note to self: try `%clang_host` as suggested in D70050.
> > So, did you try it?
> Sorry, I've forgotten about it. Done now. I presume I don't need `native` in 
> REQUIRES now.
> 
> Will address remaining comments soonish.
`system-netbsd` should be enough, but we never really bothered to add  "native" 
 even before we had %clang_host. I'm pretty sure a _lot_ of things would blow 
up if you configured your build so that the default clang target is not the 
host system.


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

https://reviews.llvm.org/D70023



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

[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

2019-11-25 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

Thanks for looking into this. This is something I've missed too, but never got 
around to checking it out.

Jonas, what do you think about this? Should we do this, or move the testcases 
themselves into the "test/API" folder?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70622



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

[Lldb-commits] [PATCH] D70646: RFC 2/3: Move non-DWARF code: `DWARFUnit` -> `SymbolFileDWARF`

2019-11-25 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

> @labath, maybe this is the only patch you did mean?

This is (more or less -- I can't say I had a really exact picture in my head) 
what I had in mind, when I said that moving the high-level code out of the 
DWARFUnit class would be useful for the llvm<->lldb dwarf parser convergence. I 
am aware that you need more for the DWZ stuff -- that is fine, and I like that 
you have split that into a separate patch.

Anyway, I think this is a good direction to move in. When the time comes, I'd 
like to get some of clang ast folks to give this a look too, but maybe that's 
not needed yet...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70646



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

[Lldb-commits] [PATCH] D70645: RFC 1/3: Unify src<->dst DWARFASTParser for CopyUniqueClassMethodTypes()

2019-11-25 Thread Pavel Labath via Phabricator via lldb-commits

labath added a reviewer: teemperor.
labath added a subscriber: teemperor.
labath added a comment.

@shafik or @teemperor will know more about this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70645



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

[Lldb-commits] [PATCH] D70647: RFC 3/3: Remove DWARFDIE dependency from functions moved by D70646

2019-11-25 Thread Pavel Labath via Phabricator via lldb-commits

labath added inline comments.



Comment at: lldb/include/lldb/Symbol/TypeSystem.h:108-110
+  virtual DWARFASTParser *GetDWARFParser(SymbolFileDWARF &dwarf) {
+return nullptr;
+  }

This part looks pretty dodgy. I'd like to avoid introducing plugin references 
in non-plugin code. It looks like this class isn't particularly clean already 
(DWARFDIE forward decl), but this seems to make the problem much worse.

Since this class already contains a `SymbolFile` pointer, maybe we could create 
some kind of a ast-parser constructing method on the SymbolFile class to avoid 
mentioning the SymbolFileDWARF directly.

If I was doing this, I'd probably try to take this one step further and merge 
the `GetDWARFParser`/`GetPDBParser` methods (whose only calls are in the 
relevant symbol file plugins) into a single `GetASTParser` method and do 
appropriate casts in the plugins themselves.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70647



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

[Lldb-commits] [PATCH] D70645: RFC 1/3: Unify src<->dst DWARFASTParser for CopyUniqueClassMethodTypes()

2019-11-25 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor added a comment.

I think it's possible that for example with -gmodules we have multiple 
ClangASTContext's (and therefore multiple DWARFASTParserClang), but the only 
place where we call this function seems to only concerned about a single 
SymbolFileDWARF so this seems safe.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70645



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

[Lldb-commits] [PATCH] D70663: [lldb] Remove lldb's own ASTDumper

2019-11-25 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor created this revision.
teemperor added a reviewer: labath.
Herald added subscribers: lldb-commits, JDevlieghere, mgorny.
Herald added a project: LLDB.

LLDB's ASTDumper is just a clone of Clang's ASTDumper but with some scary code 
and
some unrelated functionality (like dumping name/attributes of types). This 
removes LLDB's ASTDumper
and replaces its uses with the `ClangUtils::DumpDecl` method that just calls 
Clang's ASTDumper
and returns the result as a string.

The few uses where we just want a textual representation of a type (which will 
print their name/attributes but not
dump any AST) are now also in ClangUtil under a `ToString` name until we find a 
better home for them.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D70663

Files:
  lldb/include/lldb/Symbol/ClangUtil.h
  lldb/source/Plugins/ExpressionParser/Clang/ASTDumper.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ASTDumper.h
  lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
  lldb/source/Symbol/ClangUtil.cpp

Index: lldb/source/Symbol/ClangUtil.cpp
===
--- lldb/source/Symbol/ClangUtil.cpp
+++ lldb/source/Symbol/ClangUtil.cpp
@@ -59,3 +59,24 @@
 
   return qual_type->getAsTagDecl();
 }
+
+std::string ClangUtil::DumpDecl(const clang::Decl *d) {
+  if (!d)
+return "nullptr";
+
+  std::string result;
+  llvm::raw_string_ostream stream(result);
+  bool deserialize = false;
+  d->dump(stream, deserialize);
+
+  stream.flush();
+  return result;
+}
+
+std::string ClangUtil::ToString(const clang::Type *t) {
+  return clang::QualType(t, 0).getAsString();
+}
+
+std::string ClangUtil::ToString(const CompilerType &c) {
+  return ClangUtil::GetQualType(c).getAsString();
+}
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
@@ -8,7 +8,6 @@
 
 #include "AppleObjCDeclVendor.h"
 
-#include "Plugins/ExpressionParser/Clang/ASTDumper.h"
 #include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/ClangExternalASTSourceCommon.h"
@@ -77,24 +76,18 @@
 Log *log(GetLogIfAllCategoriesSet(
 LIBLLDB_LOG_EXPRESSIONS)); // FIXME - a more appropriate log channel?
 
-if (log) {
-  LLDB_LOGF(log,
-"AppleObjCExternalASTSource::CompleteType[%u] on "
-"(ASTContext*)%p Completing (TagDecl*)%p named %s",
-current_id, static_cast(&tag_decl->getASTContext()),
-static_cast(tag_decl),
-tag_decl->getName().str().c_str());
+LLDB_LOGF(log,
+  "AppleObjCExternalASTSource::CompleteType[%u] on "
+  "(ASTContext*)%p Completing (TagDecl*)%p named %s",
+  current_id, static_cast(&tag_decl->getASTContext()),
+  static_cast(tag_decl), tag_decl->getName().str().c_str());
 
-  LLDB_LOGF(log, "  AOEAS::CT[%u] Before:", current_id);
-  ASTDumper dumper((clang::Decl *)tag_decl);
-  dumper.ToLog(log, "[CT] ");
-}
+LLDB_LOG(log, "  AOEAS::CT[{0}] Before:\n{1}", current_id,
+ ClangUtil::DumpDecl(tag_decl));
+
+LLDB_LOG(log, "  AOEAS::CT[{1}] After:{1}", current_id,
+ ClangUtil::DumpDecl(tag_decl));
 
-if (log) {
-  LLDB_LOGF(log, "  AOEAS::CT[%u] After:", current_id);
-  ASTDumper dumper((clang::Decl *)tag_decl);
-  dumper.ToLog(log, "[CT] ");
-}
 return;
   }
 
@@ -115,16 +108,14 @@
 interface_decl->getName().str().c_str());
 
   LLDB_LOGF(log, "  AOEAS::CT[%u] Before:", current_id);
-  ASTDumper dumper((clang::Decl *)interface_decl);
-  dumper.ToLog(log, "[CT] ");
+  LLDB_LOG(log, "[CT] {0}", ClangUtil::DumpDecl(interface_decl));
 }
 
 m_decl_vendor.FinishDecl(interface_decl);
 
 if (log) {
   LLDB_LOGF(log, "  [CT] After:");
-  ASTDumper dumper((clang::Decl *)interface_decl);
-  dumper.ToLog(log, "[CT] ");
+  LLDB_LOG(log, "[CT] {0}", ClangUtil::DumpDecl(interface_decl));
 }
 return;
   }
@@ -526,27 +517,21 @@
 return false;
   };
 
-  if (log) {
-ASTDumper method_dumper((clang::Decl *)interface_decl);
-
-LLDB_LOGF(log,
-  "[AppleObjCDeclVendor::FinishDecl] Finishing Objective-C "
-  "interface for %s",
-  descriptor->GetClassName().AsCString());
-  }
+  LLDB_LOG(log,
+   "[AppleObjCDeclVendor::FinishDecl] Finishing Objective-C "
+   "interface for %s",
+   descriptor->GetClassN

[Lldb-commits] [PATCH] D70663: [lldb] Remove lldb's own ASTDumper

2019-11-25 Thread Pavel Labath via Phabricator via lldb-commits

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

Yay




Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1675
+   "  CEDM::FEVD[{0}] Found variable {1}, returned\n{2} (original 
{3})",
+   current_id, decl_name.c_str(), ClangUtil::DumpDecl(var_decl),
+   ClangUtil::ToString(ut));

drop .c_str()



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1709
+  LLDB_LOG(log, "  CEDM::FEVD[{0}] Added pvar {1}, returned\n{2}", current_id,
+   pvar_sp->GetName().GetCString(), ClangUtil::DumpDecl(var_decl));
 }

drop .GetCString()



Comment at: 
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp:1762
+  LLDB_LOG(log, "  CEDM::FEVD[{0}] Found variable {1}, returned\n{2}",
+   current_id, decl_name.c_str(), ClangUtil::DumpDecl(var_decl));
 }

drop .c_str()


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D70663



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

[Lldb-commits] [lldb] 7a6588a - [lldb] Remove lldb's own ASTDumper

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


Author: Raphael Isemann
Date: 2019-11-25T13:27:51+01:00
New Revision: 7a6588abf8bac99d716188608adfbfb4928714db

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

LOG: [lldb] Remove lldb's own ASTDumper

Summary:
LLDB's ASTDumper is just a clone of Clang's ASTDumper but with some scary code 
and
some unrelated functionality (like dumping name/attributes of types). This 
removes LLDB's ASTDumper
and replaces its uses with the `ClangUtils::DumpDecl` method that just calls 
Clang's ASTDumper
and returns the result as a string.

The few uses where we just want a textual representation of a type (which will 
print their name/attributes but not
dump any AST) are now also in ClangUtil under a `ToString` name until we find a 
better home for them.

Reviewers: labath

Reviewed By: labath

Subscribers: mgorny, JDevlieghere, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/Symbol/ClangUtil.h
lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
lldb/source/Symbol/ClangUtil.cpp

Removed: 
lldb/source/Plugins/ExpressionParser/Clang/ASTDumper.cpp
lldb/source/Plugins/ExpressionParser/Clang/ASTDumper.h



diff  --git a/lldb/include/lldb/Symbol/ClangUtil.h 
b/lldb/include/lldb/Symbol/ClangUtil.h
index d6106032190c..5ffbce340e59 100644
--- a/lldb/include/lldb/Symbol/ClangUtil.h
+++ b/lldb/include/lldb/Symbol/ClangUtil.h
@@ -11,6 +11,7 @@
 #ifndef LLDB_SYMBOL_CLANGUTIL_H
 #define LLDB_SYMBOL_CLANGUTIL_H
 
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/Type.h"
 
 #include "lldb/Symbol/CompilerType.h"
@@ -30,6 +31,15 @@ struct ClangUtil {
   static CompilerType RemoveFastQualifiers(const CompilerType &ct);
 
   static clang::TagDecl *GetAsTagDecl(const CompilerType &type);
+
+  /// Returns a textual representation of the given Decl's AST. Does not
+  /// deserialize any child nodes.
+  static std::string DumpDecl(const clang::Decl *d);
+  /// Returns a textual representation of the given type.
+  static std::string ToString(const clang::Type *t);
+  /// Returns a textual representation of the given CompilerType (assuming
+  /// its underlying type is a Clang type).
+  static std::string ToString(const CompilerType &c);
 };
 }
 

diff  --git a/lldb/source/Plugins/ExpressionParser/Clang/ASTDumper.cpp 
b/lldb/source/Plugins/ExpressionParser/Clang/ASTDumper.cpp
deleted file mode 100644
index f33a713cc0b2..
--- a/lldb/source/Plugins/ExpressionParser/Clang/ASTDumper.cpp
+++ /dev/null
@@ -1,104 +0,0 @@
-//===-- ASTDumper.cpp ---*- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "ASTDumper.h"
-
-#include "lldb/Symbol/ClangASTContext.h"
-#include "lldb/Symbol/ClangUtil.h"
-#include "lldb/Symbol/CompilerType.h"
-#include "lldb/Utility/Log.h"
-
-#include "llvm/Support/raw_ostream.h"
-
-using namespace lldb_private;
-
-ASTDumper::ASTDumper(clang::Decl *decl) {
-  clang::DeclContext *decl_ctx = llvm::dyn_cast(decl);
-
-  bool has_external_lexical_storage;
-  bool has_external_visible_storage;
-
-  if (decl_ctx) {
-has_external_lexical_storage = decl_ctx->hasExternalLexicalStorage();
-has_external_visible_storage = decl_ctx->hasExternalVisibleStorage();
-decl_ctx->setHasExternalLexicalStorage(false);
-decl_ctx->setHasExternalVisibleStorage(false);
-  }
-
-  llvm::raw_string_ostream os(m_dump);
-  decl->print(os);
-  os.flush();
-
-  if (decl_ctx) {
-decl_ctx->setHasExternalLexicalStorage(has_external_lexical_storage);
-decl_ctx->setHasExternalVisibleStorage(has_external_visible_storage);
-  }
-}
-
-ASTDumper::ASTDumper(clang::DeclContext *decl_ctx) {
-  bool has_external_lexical_storage = decl_ctx->hasExternalLexicalStorage();
-  bool has_external_visible_storage = decl_ctx->hasExternalVisibleStorage();
-
-  decl_ctx->setHasExternalLexicalStorage(false);
-  decl_ctx->setHasExternalVisibleStorage(false);
-
-  if (clang::Decl *decl = llvm::dyn_cast(decl_ctx)) {
-llvm::raw_string_ostream os(m_dump);
-decl->print(os);
-os.flush();
-  } else {
-m_dump.assign("");
-  }
-
-  decl_ctx->setHasExternalLexicalStorage(has_external_lexical_storage);
-  decl_ctx->setHasExternalVisibleStorage(has_external_visible_storage);
-}
-
-ASTDumper::ASTDumper(cons

[Lldb-commits] [PATCH] D70663: [lldb] Remove lldb's own ASTDumper

2019-11-25 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor updated this revision to Diff 230875.
teemperor added a comment.

- Address feedback (Thanks Pavel)


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

https://reviews.llvm.org/D70663

Files:
  lldb/include/lldb/Symbol/ClangUtil.h
  lldb/source/Plugins/ExpressionParser/Clang/ASTDumper.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ASTDumper.h
  lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
  lldb/source/Symbol/ClangUtil.cpp

Index: lldb/source/Symbol/ClangUtil.cpp
===
--- lldb/source/Symbol/ClangUtil.cpp
+++ lldb/source/Symbol/ClangUtil.cpp
@@ -59,3 +59,24 @@
 
   return qual_type->getAsTagDecl();
 }
+
+std::string ClangUtil::DumpDecl(const clang::Decl *d) {
+  if (!d)
+return "nullptr";
+
+  std::string result;
+  llvm::raw_string_ostream stream(result);
+  bool deserialize = false;
+  d->dump(stream, deserialize);
+
+  stream.flush();
+  return result;
+}
+
+std::string ClangUtil::ToString(const clang::Type *t) {
+  return clang::QualType(t, 0).getAsString();
+}
+
+std::string ClangUtil::ToString(const CompilerType &c) {
+  return ClangUtil::GetQualType(c).getAsString();
+}
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
@@ -8,7 +8,6 @@
 
 #include "AppleObjCDeclVendor.h"
 
-#include "Plugins/ExpressionParser/Clang/ASTDumper.h"
 #include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/ClangExternalASTSourceCommon.h"
@@ -77,24 +76,18 @@
 Log *log(GetLogIfAllCategoriesSet(
 LIBLLDB_LOG_EXPRESSIONS)); // FIXME - a more appropriate log channel?
 
-if (log) {
-  LLDB_LOGF(log,
-"AppleObjCExternalASTSource::CompleteType[%u] on "
-"(ASTContext*)%p Completing (TagDecl*)%p named %s",
-current_id, static_cast(&tag_decl->getASTContext()),
-static_cast(tag_decl),
-tag_decl->getName().str().c_str());
+LLDB_LOGF(log,
+  "AppleObjCExternalASTSource::CompleteType[%u] on "
+  "(ASTContext*)%p Completing (TagDecl*)%p named %s",
+  current_id, static_cast(&tag_decl->getASTContext()),
+  static_cast(tag_decl), tag_decl->getName().str().c_str());
 
-  LLDB_LOGF(log, "  AOEAS::CT[%u] Before:", current_id);
-  ASTDumper dumper((clang::Decl *)tag_decl);
-  dumper.ToLog(log, "[CT] ");
-}
+LLDB_LOG(log, "  AOEAS::CT[{0}] Before:\n{1}", current_id,
+ ClangUtil::DumpDecl(tag_decl));
+
+LLDB_LOG(log, "  AOEAS::CT[{1}] After:{1}", current_id,
+ ClangUtil::DumpDecl(tag_decl));
 
-if (log) {
-  LLDB_LOGF(log, "  AOEAS::CT[%u] After:", current_id);
-  ASTDumper dumper((clang::Decl *)tag_decl);
-  dumper.ToLog(log, "[CT] ");
-}
 return;
   }
 
@@ -115,16 +108,14 @@
 interface_decl->getName().str().c_str());
 
   LLDB_LOGF(log, "  AOEAS::CT[%u] Before:", current_id);
-  ASTDumper dumper((clang::Decl *)interface_decl);
-  dumper.ToLog(log, "[CT] ");
+  LLDB_LOG(log, "[CT] {0}", ClangUtil::DumpDecl(interface_decl));
 }
 
 m_decl_vendor.FinishDecl(interface_decl);
 
 if (log) {
   LLDB_LOGF(log, "  [CT] After:");
-  ASTDumper dumper((clang::Decl *)interface_decl);
-  dumper.ToLog(log, "[CT] ");
+  LLDB_LOG(log, "[CT] {0}", ClangUtil::DumpDecl(interface_decl));
 }
 return;
   }
@@ -526,27 +517,21 @@
 return false;
   };
 
-  if (log) {
-ASTDumper method_dumper((clang::Decl *)interface_decl);
-
-LLDB_LOGF(log,
-  "[AppleObjCDeclVendor::FinishDecl] Finishing Objective-C "
-  "interface for %s",
-  descriptor->GetClassName().AsCString());
-  }
+  LLDB_LOG(log,
+   "[AppleObjCDeclVendor::FinishDecl] Finishing Objective-C "
+   "interface for %s",
+   descriptor->GetClassName().AsCString());
 
   if (!descriptor->Describe(superclass_func, instance_method_func,
 class_method_func, ivar_func))
 return false;
 
   if (log) {
-ASTDumper method_dumper((clang::Decl *)interface_decl);
-
 LLDB_LOGF(
 log,
 "[AppleObjCDeclVendor::FinishDecl] Finished Objective-C interface");
 
-method_dumper.ToLog(log, "  [AOTV::FD] ");
+LLDB_LOG(log, "  [AOTV::FD] {0}", ClangUtil::DumpDecl(interface_decl));
   }
 
   return true;
@@ -590,7 +575,6 @@
 if (log) {

[Lldb-commits] [PATCH] D70663: [lldb] Remove lldb's own ASTDumper

2019-11-25 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7a6588abf8ba: [lldb] Remove lldb's own ASTDumper 
(authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70663

Files:
  lldb/include/lldb/Symbol/ClangUtil.h
  lldb/source/Plugins/ExpressionParser/Clang/ASTDumper.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ASTDumper.h
  lldb/source/Plugins/ExpressionParser/Clang/CMakeLists.txt
  lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
  lldb/source/Symbol/ClangUtil.cpp

Index: lldb/source/Symbol/ClangUtil.cpp
===
--- lldb/source/Symbol/ClangUtil.cpp
+++ lldb/source/Symbol/ClangUtil.cpp
@@ -59,3 +59,24 @@
 
   return qual_type->getAsTagDecl();
 }
+
+std::string ClangUtil::DumpDecl(const clang::Decl *d) {
+  if (!d)
+return "nullptr";
+
+  std::string result;
+  llvm::raw_string_ostream stream(result);
+  bool deserialize = false;
+  d->dump(stream, deserialize);
+
+  stream.flush();
+  return result;
+}
+
+std::string ClangUtil::ToString(const clang::Type *t) {
+  return clang::QualType(t, 0).getAsString();
+}
+
+std::string ClangUtil::ToString(const CompilerType &c) {
+  return ClangUtil::GetQualType(c).getAsString();
+}
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
===
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCDeclVendor.cpp
@@ -8,7 +8,6 @@
 
 #include "AppleObjCDeclVendor.h"
 
-#include "Plugins/ExpressionParser/Clang/ASTDumper.h"
 #include "Plugins/LanguageRuntime/ObjC/ObjCLanguageRuntime.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Symbol/ClangExternalASTSourceCommon.h"
@@ -77,24 +76,18 @@
 Log *log(GetLogIfAllCategoriesSet(
 LIBLLDB_LOG_EXPRESSIONS)); // FIXME - a more appropriate log channel?
 
-if (log) {
-  LLDB_LOGF(log,
-"AppleObjCExternalASTSource::CompleteType[%u] on "
-"(ASTContext*)%p Completing (TagDecl*)%p named %s",
-current_id, static_cast(&tag_decl->getASTContext()),
-static_cast(tag_decl),
-tag_decl->getName().str().c_str());
+LLDB_LOGF(log,
+  "AppleObjCExternalASTSource::CompleteType[%u] on "
+  "(ASTContext*)%p Completing (TagDecl*)%p named %s",
+  current_id, static_cast(&tag_decl->getASTContext()),
+  static_cast(tag_decl), tag_decl->getName().str().c_str());
 
-  LLDB_LOGF(log, "  AOEAS::CT[%u] Before:", current_id);
-  ASTDumper dumper((clang::Decl *)tag_decl);
-  dumper.ToLog(log, "[CT] ");
-}
+LLDB_LOG(log, "  AOEAS::CT[{0}] Before:\n{1}", current_id,
+ ClangUtil::DumpDecl(tag_decl));
+
+LLDB_LOG(log, "  AOEAS::CT[{1}] After:{1}", current_id,
+ ClangUtil::DumpDecl(tag_decl));
 
-if (log) {
-  LLDB_LOGF(log, "  AOEAS::CT[%u] After:", current_id);
-  ASTDumper dumper((clang::Decl *)tag_decl);
-  dumper.ToLog(log, "[CT] ");
-}
 return;
   }
 
@@ -115,16 +108,14 @@
 interface_decl->getName().str().c_str());
 
   LLDB_LOGF(log, "  AOEAS::CT[%u] Before:", current_id);
-  ASTDumper dumper((clang::Decl *)interface_decl);
-  dumper.ToLog(log, "[CT] ");
+  LLDB_LOG(log, "[CT] {0}", ClangUtil::DumpDecl(interface_decl));
 }
 
 m_decl_vendor.FinishDecl(interface_decl);
 
 if (log) {
   LLDB_LOGF(log, "  [CT] After:");
-  ASTDumper dumper((clang::Decl *)interface_decl);
-  dumper.ToLog(log, "[CT] ");
+  LLDB_LOG(log, "[CT] {0}", ClangUtil::DumpDecl(interface_decl));
 }
 return;
   }
@@ -526,27 +517,21 @@
 return false;
   };
 
-  if (log) {
-ASTDumper method_dumper((clang::Decl *)interface_decl);
-
-LLDB_LOGF(log,
-  "[AppleObjCDeclVendor::FinishDecl] Finishing Objective-C "
-  "interface for %s",
-  descriptor->GetClassName().AsCString());
-  }
+  LLDB_LOG(log,
+   "[AppleObjCDeclVendor::FinishDecl] Finishing Objective-C "
+   "interface for %s",
+   descriptor->GetClassName().AsCString());
 
   if (!descriptor->Describe(superclass_func, instance_method_func,
 class_method_func, ivar_func))
 return false;
 
   if (log) {
-ASTDumper method_dumper((clang::Decl *)interface_decl);
-
 LLDB_LOGF(
 log,
 "[AppleObjCDeclVendor::FinishDecl] Finished Objective-C interface");
 
-method_dumper.ToLog(log, "  [AOTV::FD] ");
+LLDB_LOG(log, "  [AOTV::FD] {0}", ClangUti

[Lldb-commits] [PATCH] D70668: [lldb][NFC] Allow range-based for-loops on VariableList

2019-11-25 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor created this revision.
teemperor added a reviewer: labath.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, JDevlieghere.
Herald added a project: LLDB.

Adds support for doing range-based for-loops on LLDB's VariableList and
modernises all the index-based for-loops in LLDB where possible.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D70668

Files:
  lldb/include/lldb/Symbol/VariableList.h
  lldb/source/API/SBFrame.cpp
  lldb/source/API/SBModule.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/Address.cpp
  lldb/source/Core/IOHandler.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Symbol/Block.cpp
  lldb/source/Symbol/Variable.cpp
  lldb/source/Target/StackFrame.cpp

Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -573,8 +573,7 @@
   if (!var_sp && (options & eExpressionPathOptionsInspectAnonymousUnions)) {
 // Check if any anonymous unions are there which contain a variable with
 // the name we need
-for (size_t i = 0; i < variable_list->GetSize(); i++) {
-  VariableSP variable_sp = variable_list->GetVariableAtIndex(i);
+for (VariableSP variable_sp : *variable_list) {
   if (!variable_sp)
 continue;
   if (!variable_sp->GetName().IsEmpty())
@@ -1529,11 +1528,9 @@
  : Instruction::Operand::BuildDereference(
Instruction::Operand::BuildRegister(reg));
 
-  for (size_t vi = 0, ve = variables.GetSize(); vi != ve; ++vi) {
-VariableSP var_sp = variables.GetVariableAtIndex(vi);
-if (var_sp->LocationExpression().MatchesOperand(frame, op)) {
+  for (VariableSP var_sp : variables) {
+if (var_sp->LocationExpression().MatchesOperand(frame, op))
   return frame.GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
-}
   }
 
   const uint32_t current_inst =
Index: lldb/source/Symbol/Variable.cpp
===
--- lldb/source/Symbol/Variable.cpp
+++ lldb/source/Symbol/Variable.cpp
@@ -609,11 +609,8 @@
 VariableList *variable_list = frame->GetVariableList(get_file_globals);
 
 if (variable_list) {
-  const size_t num_variables = variable_list->GetSize();
-  for (size_t i = 0; i < num_variables; ++i) {
-Variable *variable = variable_list->GetVariableAtIndex(i).get();
+  for (VariableSP variable : *variable_list)
 request.AddCompletion(variable->GetName().AsCString());
-  }
 }
   }
 }
@@ -710,9 +707,7 @@
   if (!variable_list)
 break;
 
-  const size_t num_variables = variable_list->GetSize();
-  for (size_t i = 0; i < num_variables; ++i) {
-Variable *variable = variable_list->GetVariableAtIndex(i).get();
+  for (VariableSP variable : *variable_list) {
 
 if (!variable)
   continue;
Index: lldb/source/Symbol/Block.cpp
===
--- lldb/source/Symbol/Block.cpp
+++ lldb/source/Symbol/Block.cpp
@@ -406,8 +406,7 @@
   uint32_t num_variables_added = 0;
   VariableList *block_var_list = GetBlockVariableList(can_create).get();
   if (block_var_list) {
-for (size_t i = 0; i < block_var_list->GetSize(); ++i) {
-  VariableSP variable = block_var_list->GetVariableAtIndex(i);
+for (VariableSP variable : *block_var_list) {
   if (filter(variable.get())) {
 num_variables_added++;
 variable_list->AddVariable(variable);
Index: lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
@@ -2240,8 +2240,7 @@
   // Iterate over all the global variables looking for one with a matching type
   // to the Element. We make the assumption a match exists since there needs to
   // be a global variable to reflect the struct type back into java host code.
-  for (uint32_t i = 0; i < var_list.GetSize(); ++i) {
-const VariableSP var_sp(var_list.GetVariableAtIndex(i));
+  for (const VariableSP var_sp : var_list) {
 if (!var_sp)
   continue;
 
Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
===
--- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
+++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
@@ -648,9 +648,7 @@
 
   if (vars.GetSize()) {
 if (typ

[Lldb-commits] [PATCH] D70668: [lldb][NFC] Allow range-based for-loops on VariableList

2019-11-25 Thread Pavel Labath via Phabricator via lldb-commits

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

Since copying shared pointers isn't completely cheap, I think we should use 
`const shared_ptr &`s whereever possible. Also, the typical lldb naming 
convention for shared_pointer variables is to have them end in `_sp`, so I'd 
try to preserve that.
Besides that, LGTM.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D70668



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

[Lldb-commits] [PATCH] D70668: [lldb][NFC] Allow range-based for-loops on VariableList

2019-11-25 Thread Raphael Isemann via Phabricator via lldb-commits

teemperor updated this revision to Diff 230888.
teemperor added a comment.

- Added `_sp` suffix to variable names.
- Using const-references where possible.


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

https://reviews.llvm.org/D70668

Files:
  lldb/include/lldb/Symbol/VariableList.h
  lldb/source/API/SBFrame.cpp
  lldb/source/API/SBModule.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/Address.cpp
  lldb/source/Core/IOHandler.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Symbol/Block.cpp
  lldb/source/Symbol/Variable.cpp
  lldb/source/Target/StackFrame.cpp

Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -573,8 +573,7 @@
   if (!var_sp && (options & eExpressionPathOptionsInspectAnonymousUnions)) {
 // Check if any anonymous unions are there which contain a variable with
 // the name we need
-for (size_t i = 0; i < variable_list->GetSize(); i++) {
-  VariableSP variable_sp = variable_list->GetVariableAtIndex(i);
+for (const VariableSP &variable_sp : *variable_list) {
   if (!variable_sp)
 continue;
   if (!variable_sp->GetName().IsEmpty())
@@ -1529,11 +1528,9 @@
  : Instruction::Operand::BuildDereference(
Instruction::Operand::BuildRegister(reg));
 
-  for (size_t vi = 0, ve = variables.GetSize(); vi != ve; ++vi) {
-VariableSP var_sp = variables.GetVariableAtIndex(vi);
-if (var_sp->LocationExpression().MatchesOperand(frame, op)) {
+  for (VariableSP var_sp : variables) {
+if (var_sp->LocationExpression().MatchesOperand(frame, op))
   return frame.GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
-}
   }
 
   const uint32_t current_inst =
Index: lldb/source/Symbol/Variable.cpp
===
--- lldb/source/Symbol/Variable.cpp
+++ lldb/source/Symbol/Variable.cpp
@@ -609,11 +609,8 @@
 VariableList *variable_list = frame->GetVariableList(get_file_globals);
 
 if (variable_list) {
-  const size_t num_variables = variable_list->GetSize();
-  for (size_t i = 0; i < num_variables; ++i) {
-Variable *variable = variable_list->GetVariableAtIndex(i).get();
-request.AddCompletion(variable->GetName().AsCString());
-  }
+  for (const VariableSP &var_sp : *variable_list)
+request.AddCompletion(var_sp->GetName().AsCString());
 }
   }
 }
@@ -710,17 +707,15 @@
   if (!variable_list)
 break;
 
-  const size_t num_variables = variable_list->GetSize();
-  for (size_t i = 0; i < num_variables; ++i) {
-Variable *variable = variable_list->GetVariableAtIndex(i).get();
+  for (VariableSP var_sp : *variable_list) {
 
-if (!variable)
+if (!var_sp)
   continue;
 
-const char *variable_name = variable->GetName().AsCString();
+const char *variable_name = var_sp->GetName().AsCString();
 if (strstr(variable_name, token.c_str()) == variable_name) {
   if (strcmp(variable_name, token.c_str()) == 0) {
-Type *variable_type = variable->GetType();
+Type *variable_type = var_sp->GetType();
 if (variable_type) {
   CompilerType variable_compiler_type(
   variable_type->GetForwardCompilerType());
Index: lldb/source/Symbol/Block.cpp
===
--- lldb/source/Symbol/Block.cpp
+++ lldb/source/Symbol/Block.cpp
@@ -406,11 +406,10 @@
   uint32_t num_variables_added = 0;
   VariableList *block_var_list = GetBlockVariableList(can_create).get();
   if (block_var_list) {
-for (size_t i = 0; i < block_var_list->GetSize(); ++i) {
-  VariableSP variable = block_var_list->GetVariableAtIndex(i);
-  if (filter(variable.get())) {
+for (const VariableSP &var_sp : *block_var_list) {
+  if (filter(var_sp.get())) {
 num_variables_added++;
-variable_list->AddVariable(variable);
+variable_list->AddVariable(var_sp);
   }
 }
   }
Index: lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
@@ -2240,8 +2240,7 @@
   // Iterate over all the global variables looking for one with a matching type
   // to the Element. We make the assumption a match exists since t

[Lldb-commits] [lldb] d178213 - [lldb][NFC] Allow range-based for-loops on VariableList

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


Author: Raphael Isemann
Date: 2019-11-25T15:03:46+01:00
New Revision: d1782133d96d316c3bc98e33a191994794a26851

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

LOG: [lldb][NFC] Allow range-based for-loops on VariableList

Summary:
Adds support for doing range-based for-loops on LLDB's VariableList and
modernises all the index-based for-loops in LLDB where possible.

Reviewers: labath, jdoerfert

Reviewed By: labath

Subscribers: JDevlieghere, lldb-commits

Tags: #lldb

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

Added: 


Modified: 
lldb/include/lldb/Symbol/VariableList.h
lldb/source/API/SBFrame.cpp
lldb/source/API/SBModule.cpp
lldb/source/API/SBTarget.cpp
lldb/source/Commands/CommandObjectTarget.cpp
lldb/source/Core/Address.cpp
lldb/source/Core/IOHandler.cpp
lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
lldb/source/Symbol/Block.cpp
lldb/source/Symbol/Variable.cpp
lldb/source/Target/StackFrame.cpp

Removed: 




diff  --git a/lldb/include/lldb/Symbol/VariableList.h 
b/lldb/include/lldb/Symbol/VariableList.h
index 54d27583cd7b..87f98668a8a8 100644
--- a/lldb/include/lldb/Symbol/VariableList.h
+++ b/lldb/include/lldb/Symbol/VariableList.h
@@ -16,6 +16,8 @@
 namespace lldb_private {
 
 class VariableList {
+  typedef std::vector collection;
+
 public:
   // Constructors and Destructors
   //  VariableList(const SymbolContext &symbol_context);
@@ -65,11 +67,15 @@ class VariableList {
   size_t GetSize() const;
   bool Empty() const { return m_variables.empty(); }
 
-protected:
-  typedef std::vector collection;
   typedef collection::iterator iterator;
   typedef collection::const_iterator const_iterator;
 
+  iterator begin() { return m_variables.begin(); }
+  iterator end() { return m_variables.end(); }
+  const_iterator begin() const { return m_variables.begin(); }
+  const_iterator end() const { return m_variables.end(); }
+
+protected:
   collection m_variables;
 
 private:

diff  --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp
index c0e272e1bcd4..af42be9ac75e 100644
--- a/lldb/source/API/SBFrame.cpp
+++ b/lldb/source/API/SBFrame.cpp
@@ -831,14 +831,12 @@ SBValueList SBFrame::GetVariables(const 
lldb::SBVariablesOptions &options) {
 if (stop_locker.TryLock(&process->GetRunLock())) {
   frame = exe_ctx.GetFramePtr();
   if (frame) {
-size_t i;
 VariableList *variable_list = nullptr;
 variable_list = frame->GetVariableList(true);
 if (variable_list) {
   const size_t num_variables = variable_list->GetSize();
   if (num_variables) {
-for (i = 0; i < num_variables; ++i) {
-  VariableSP variable_sp(variable_list->GetVariableAtIndex(i));
+for (const VariableSP &variable_sp : *variable_list) {
   if (variable_sp) {
 bool add_variable = false;
 switch (variable_sp->GetScope()) {

diff  --git a/lldb/source/API/SBModule.cpp b/lldb/source/API/SBModule.cpp
index 6cc6d2628ace..7ac189bb4273 100644
--- a/lldb/source/API/SBModule.cpp
+++ b/lldb/source/API/SBModule.cpp
@@ -419,16 +419,12 @@ SBValueList SBModule::FindGlobalVariables(SBTarget 
&target, const char *name,
 VariableList variable_list;
 module_sp->FindGlobalVariables(ConstString(name), nullptr, max_matches,
variable_list);
-const uint32_t match_count = variable_list.GetSize();
-if (match_count > 0) {
-  for (uint32_t i = 0; i < match_count; ++i) {
-lldb::ValueObjectSP valobj_sp;
-TargetSP target_sp(target.GetSP());
-valobj_sp = ValueObjectVariable::Create(
-target_sp.get(), variable_list.GetVariableAtIndex(i));
-if (valobj_sp)
-  sb_value_list.Append(SBValue(valobj_sp));
-  }
+for (const VariableSP &var_sp : variable_list) {
+  lldb::ValueObjectSP valobj_sp;
+  TargetSP target_sp(target.GetSP());
+  valobj_sp = ValueObjectVariable::Create(target_sp.get(), var_sp);
+  if (valobj_sp)
+sb_value_list.Append(SBValue(valobj_sp));
 }
   }
 

diff  --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index bf444a72278a..7013e2b45e5f 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1929,14 +1929,13 @@ SBValueList SBTarget::FindGlobalVariables(const char 
*name,
 VariableList variable_list;
 target_sp->GetImages().FindGlobalVariables(ConstString(name), max_matches,
variable_list);
-const uint32_t match_count = variable_list.GetSize();
-if (match_count > 0) {
+if

[Lldb-commits] [PATCH] D70668: [lldb][NFC] Allow range-based for-loops on VariableList

2019-11-25 Thread Raphael Isemann via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd1782133d96d: [lldb][NFC] Allow range-based for-loops on 
VariableList (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70668

Files:
  lldb/include/lldb/Symbol/VariableList.h
  lldb/source/API/SBFrame.cpp
  lldb/source/API/SBModule.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/Commands/CommandObjectTarget.cpp
  lldb/source/Core/Address.cpp
  lldb/source/Core/IOHandler.cpp
  lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
  
lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
  lldb/source/Symbol/Block.cpp
  lldb/source/Symbol/Variable.cpp
  lldb/source/Target/StackFrame.cpp

Index: lldb/source/Target/StackFrame.cpp
===
--- lldb/source/Target/StackFrame.cpp
+++ lldb/source/Target/StackFrame.cpp
@@ -573,8 +573,7 @@
   if (!var_sp && (options & eExpressionPathOptionsInspectAnonymousUnions)) {
 // Check if any anonymous unions are there which contain a variable with
 // the name we need
-for (size_t i = 0; i < variable_list->GetSize(); i++) {
-  VariableSP variable_sp = variable_list->GetVariableAtIndex(i);
+for (const VariableSP &variable_sp : *variable_list) {
   if (!variable_sp)
 continue;
   if (!variable_sp->GetName().IsEmpty())
@@ -1529,11 +1528,9 @@
  : Instruction::Operand::BuildDereference(
Instruction::Operand::BuildRegister(reg));
 
-  for (size_t vi = 0, ve = variables.GetSize(); vi != ve; ++vi) {
-VariableSP var_sp = variables.GetVariableAtIndex(vi);
-if (var_sp->LocationExpression().MatchesOperand(frame, op)) {
+  for (VariableSP var_sp : variables) {
+if (var_sp->LocationExpression().MatchesOperand(frame, op))
   return frame.GetValueObjectForFrameVariable(var_sp, eNoDynamicValues);
-}
   }
 
   const uint32_t current_inst =
Index: lldb/source/Symbol/Variable.cpp
===
--- lldb/source/Symbol/Variable.cpp
+++ lldb/source/Symbol/Variable.cpp
@@ -609,11 +609,8 @@
 VariableList *variable_list = frame->GetVariableList(get_file_globals);
 
 if (variable_list) {
-  const size_t num_variables = variable_list->GetSize();
-  for (size_t i = 0; i < num_variables; ++i) {
-Variable *variable = variable_list->GetVariableAtIndex(i).get();
-request.AddCompletion(variable->GetName().AsCString());
-  }
+  for (const VariableSP &var_sp : *variable_list)
+request.AddCompletion(var_sp->GetName().AsCString());
 }
   }
 }
@@ -710,17 +707,15 @@
   if (!variable_list)
 break;
 
-  const size_t num_variables = variable_list->GetSize();
-  for (size_t i = 0; i < num_variables; ++i) {
-Variable *variable = variable_list->GetVariableAtIndex(i).get();
+  for (VariableSP var_sp : *variable_list) {
 
-if (!variable)
+if (!var_sp)
   continue;
 
-const char *variable_name = variable->GetName().AsCString();
+const char *variable_name = var_sp->GetName().AsCString();
 if (strstr(variable_name, token.c_str()) == variable_name) {
   if (strcmp(variable_name, token.c_str()) == 0) {
-Type *variable_type = variable->GetType();
+Type *variable_type = var_sp->GetType();
 if (variable_type) {
   CompilerType variable_compiler_type(
   variable_type->GetForwardCompilerType());
Index: lldb/source/Symbol/Block.cpp
===
--- lldb/source/Symbol/Block.cpp
+++ lldb/source/Symbol/Block.cpp
@@ -406,11 +406,10 @@
   uint32_t num_variables_added = 0;
   VariableList *block_var_list = GetBlockVariableList(can_create).get();
   if (block_var_list) {
-for (size_t i = 0; i < block_var_list->GetSize(); ++i) {
-  VariableSP variable = block_var_list->GetVariableAtIndex(i);
-  if (filter(variable.get())) {
+for (const VariableSP &var_sp : *block_var_list) {
+  if (filter(var_sp.get())) {
 num_variables_added++;
-variable_list->AddVariable(variable);
+variable_list->AddVariable(var_sp);
   }
 }
   }
Index: lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
===
--- lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/RenderScript/RenderScriptRuntime/RenderScriptRuntime.cpp
@@ -2240,8 +2240,7 @@
   // Iterate over all the global variables looking for one with a matching ty

[Lldb-commits] [PATCH] D70025: [lldb] [Process/NetBSD] Fix handling concurrent watchpoint events

2019-11-25 Thread Pavel Labath via Phabricator via lldb-commits

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: dexonsmith.

LGTM, since this seems to be the best we can do given the current netbsd 
behavior.

However, I'd like to repeat what I said on the IRC, that I consider this 
behavior of netbsd to be unreasonable. Serializing signals delivery seems nice 
at a first glance, but when those signals reflect events which are "trapped" 
(== caught only after the relevant thing has happened), as is the case with 
watchpoints on x86, things just start getting weird. This means that if we get 
a "concurrent" watchpoint hits, we will only report the one of the watchpoint 
hits, but the entire state of the program will be already in the "post-hit" 
state for both watchpoints (the memory will contain the new value, PC will 
point after the trapping instruction, etc.). I think this state can be very 
confusing to the user, particularly if he decides to run an expression in this 
state (to figure out what happened?) and then this expression is interrupted by 
the delayed delivery of the second watchpoint signal.


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

https://reviews.llvm.org/D70025



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

[Lldb-commits] [PATCH] D69309: Support template instantiation in the expression evaluator

2019-11-25 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

I wanted to make sure that people understand about how templates are done in 
DWARF and the implications, just for completeness:

1. DWARF represents only specializations of templates (foo) and not the 
generic template definition in terms of type T (foo)
2. DWARF only creates the methods that are used in each compile unit. So we 
might only have `std::vector::erase()` and `std::vector::back()` in 
one compile unit and `std::vector::operator[](size_t)` in another
3. The template type definitions we create in the clang AST context therefore 
have no generic template methods. The definition for foo has no methods. All 
methods for the specialized type (foo) are treated as specializations 
specific to the  type. We have to do this since we item #2 above might 
only have a fraction of the real template definition's methods.
4. #3 means that we must iterate over all instances of foo to find as many 
methods as possible when creating the type since each one might only have a 
fraction of all methods from the original foo definition in the code.

Any fixes we check in should:

- make sure that all tests that were previously passing continue to pass if we 
do any modifications.
- work for all cases (manual indexing, accelerator tables from apple and DWARF)


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

https://reviews.llvm.org/D69309



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

[Lldb-commits] [PATCH] D70532: [lldb] Improve/fix base address selection in location lists

2019-11-25 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

So I am worried by seeing any mention of "load address" and "load bias" in this 
patch. All information that is extracted from the DWARF should be solely in 
terms if "file addresses" IMHO, unless I am not understanding something from 
the new location lists. For mach-o DWARF in .o files, the "file address" will 
be a file address from the .o file (which is fine), but it will need to be 
linked into an executable file address before being handed out. We fix up 
addresses like this for any DW_OP_addr opcodes in location expressions. So 
typically we would have a virtual SymbolFileDWARF method that 
SymbolFileDWARFDebugMap would override and this would use the default 
SymbolFileDWARF::GetLocationList(...) function, and then link up the location 
list or DWARFExpression prior to handing it out. Is there something I am not 
understanding here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70532



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

[Lldb-commits] [PATCH] D70025: [lldb] [Process/NetBSD] Fix handling concurrent watchpoint events

2019-11-25 Thread Kamil Rytarowski via Phabricator via lldb-commits

krytarowski added a comment.

In D70025#1758800 , @labath wrote:

> LGTM, since this seems to be the best we can do given the current netbsd 
> behavior.
>
> However, I'd like to repeat what I said on the IRC, that I consider this 
> behavior of netbsd to be unreasonable.

Thanks for the feedback. Evaluating the options, we will keep using our current 
model. There are no plans for any modifications in this domain.

The end user of a debugger is just expected to see no difference.

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

https://reviews.llvm.org/D70025

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

[Lldb-commits] [PATCH] D70532: [lldb] Improve/fix base address selection in location lists

2019-11-25 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

In D70532#1758907 , @clayborg wrote:

> So I am worried by seeing any mention of "load address" and "load bias" in 
> this patch. All information that is extracted from the DWARF should be solely 
> in terms if "file addresses" IMHO,

This is still true. The information for which goes into the construction of the 
DWARFExpression is based on information from the object file alone -- this is 
the CU base (file) address, and function base (file) address. Essentially, the 
only difference here is that previously we were passing the difference between 
these two values, whereas this patch passes the two values separately.

The "load" address goes in only when the expression is evaluated (and so it can 
be different for each invocation, etc.). This part is not changed at all, 
though I have renamed the value to better reflect how it is now used in the 
patch.

Let's say we have a CU whose base address is 0x1000, and a function which 
starts at 0x1100, and we also have a location list like: (0x142, 0x147) => 
DW_OP_whatever. Let's say that the module is loaded in memory such that the 
function now starts at 0x8100.
What happened previously that we would create the DWARFExpression with the 
"slide" of 0x100. Then, when we wanted to get an actual location we'd pass in 
0x8100 as the "load address", and the code would compute
(0x8100 - 0x100 + 0x142, 0x8100 - 0x100 + 0x147) = (0x8142, 0x8147) to get the 
actual location list range.
This worked fine if you didn't have base address selection entries in the 
location list. Once you have those, you have a problem -- you know that you 
need to change the base address somehow, but it's not possible to compute how. 
If we pass in the "file" address of the function start (0x1100), we can compute 
the load bias (0x8100 - 0x1100 = 0x7000), and add that to any address coming 
out of the location list.

> For mach-o DWARF in .o files, the "file address" will be a file address from 
> the .o file (which is fine), but it will need to be linked into an executable 
> file address before being handed out. We fix up addresses like this for any 
> DW_OP_addr opcodes in location expressions. So typically we would have a 
> virtual SymbolFileDWARF method that SymbolFileDWARFDebugMap would override 
> and this would use the default SymbolFileDWARF::GetLocationList(...) 
> function, and then link up the location list or DWARFExpression prior to 
> handing it out. Is there something I am not understanding here?

The process you're describing is indeed used elsewhere (line tables for 
instance). It's not possible to use it here, because the location lists are 
parsed very late -- basically until you actually start to evaluate the 
expression, the location list is just a blob of bytes, so the relocation needs 
to happen at a later stage. I'm not sure why something similar wasn't done here 
too (I suspect it has something to do with DWARFExpression living separately 
from the rest of the DWARF plugin code), but changing that here seems 
complicated (particularly as I don't know all the details of mach-o linker 
stuff).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70532

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

[Lldb-commits] [PATCH] D69535: build: improve python check for Windows

2019-11-25 Thread Pavel Labath via Phabricator via lldb-commits

labath added a comment.

This looks fine to me, and it's really great to get rid of the that code, but 
it would be good for some some "windows person" to ack this too. I'm 
particularly interested in Stella's opinion, as according to 
http://lab.llvm.org:8011/buildslaves/win-py3-buildbot, her bot still runs cmake 
3.11. @stella.stamenova, would it be possible to upgrade that bot to 
cmake>=3.13 ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69535



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

[Lldb-commits] [PATCH] D70532: [lldb] Improve/fix base address selection in location lists

2019-11-25 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

In D70532#1758962 , @labath wrote:

> In D70532#1758907 , @clayborg wrote:
>
> > So I am worried by seeing any mention of "load address" and "load bias" in 
> > this patch. All information that is extracted from the DWARF should be 
> > solely in terms if "file addresses" IMHO,
>
>
> This is still true. The information for which goes into the construction of 
> the DWARFExpression is based on information from the object file alone -- 
> this is the CU base (file) address, and function base (file) address. 
> Essentially, the only difference here is that previously we were passing the 
> difference between these two values, whereas this patch passes the two values 
> separately.


Gotcha.

> 
> 
>   The "load" address goes in only when the expression is evaluated (and so it 
> can be different for each invocation, etc.). This part is not changed at all, 
> though I have renamed the value to better reflect how it is now used in the 
> patch.
>
> 
> Let's say we have a CU whose base address is 0x1000, and a function which 
> starts at 0x1100, and we also have a location list like: (0x142, 0x147) => 
> DW_OP_whatever. Let's say that the module is loaded in memory such that the 
> function now starts at 0x8100.
>  What happened previously that we would create the DWARFExpression with the 
> "slide" of 0x100. Then, when we wanted to get an actual location we'd pass in 
> 0x8100 as the "load address", and the code would compute
>  (0x8100 - 0x100 + 0x142, 0x8100 - 0x100 + 0x147) = (0x8142, 0x8147) to get 
> the actual location list range.
>  This worked fine if you didn't have base address selection entries in the 
> location list. Once you have those, you have a problem -- you know that you 
> need to change the base address somehow, but it's not possible to compute 
> how. If we pass in the "file" address of the function start (0x1100), we can 
> compute the load bias (0x8100 - 0x1100 = 0x7000), and add that to any address 
> coming out of the location list.

Thanks for explaining.

>> For mach-o DWARF in .o files, the "file address" will be a file address from 
>> the .o file (which is fine), but it will need to be linked into an 
>> executable file address before being handed out. We fix up addresses like 
>> this for any DW_OP_addr opcodes in location expressions. So typically we 
>> would have a virtual SymbolFileDWARF method that SymbolFileDWARFDebugMap 
>> would override and this would use the default 
>> SymbolFileDWARF::GetLocationList(...) function, and then link up the 
>> location list or DWARFExpression prior to handing it out. Is there something 
>> I am not understanding here?
> 
> The process you're describing is indeed used elsewhere (line tables for 
> instance). It's not possible to use it here, because the location lists are 
> parsed very late -- basically until you actually start to evaluate the 
> expression, the location list is just a blob of bytes, so the relocation 
> needs to happen at a later stage. I'm not sure why something similar wasn't 
> done here too (I suspect it has something to do with DWARFExpression living 
> separately from the rest of the DWARF plugin code), but changing that here 
> seems complicated (particularly as I don't know all the details of mach-o 
> linker stuff).

Sounds good.




Comment at: lldb/include/lldb/Expression/DWARFExpression.h:88-89
   ///
-  /// \param[in] process
-  /// The process to use when resolving the load address
+  /// \param[in] load_function_start
+  /// The actual address of the function containing this location list.
   ///

Is this a "file address"? Maybe clarify here in the comment, or rename variable 
appropriately?



Comment at: lldb/include/lldb/Expression/DWARFExpression.h:141-143
+  /// \param[in] base_address
+  /// The base address to use for interpreting relative location list
+  /// entries.

Maybe change name to "cu_file_addr"?



Comment at: lldb/include/lldb/Expression/DWARFExpression.h:144
+  /// entries.
+  /// \param[in] file_function_start
+  /// The file address of the function containing this location list. This

Maybe change name to "func_file_addr"?



Comment at: lldb/include/lldb/Expression/DWARFExpression.h:148
+  /// conjuction with the load_function_start arguments).
+  void SetLocationListSlide(lldb::addr_t base_address,
+lldb::addr_t file_function_start);

Rename from "SetLocationListSlide" to "SetLocationListAddresses"? Slide doesn't 
seem relevant anymore



Comment at: lldb/include/lldb/Expression/DWARFExpression.h:164
   bool Evaluate(ExecutionContextScope *exe_scope,
-lldb::addr_t loclist_base_load_addr,
+lldb::addr_t load_function_start,
 const

[Lldb-commits] [PATCH] D70022: [lldb] [Process/NetBSD] Improve threading support

2019-11-25 Thread Michał Górny via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8d9400b65b97: [lldb] [Process/NetBSD] Improve threading 
support (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70022

Files:
  
lldb/packages/Python/lldbsuite/test/commands/watchpoints/hello_watchlocation/TestWatchLocation.py
  
lldb/packages/Python/lldbsuite/test/commands/watchpoints/watchpoint_set_command/TestWatchLocationWithWatchSet.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentBreakpointOneDelayBreakpointThreads.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentCrashWithBreak.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentCrashWithSignal.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentCrashWithWatchpoint.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentCrashWithWatchpointBreakpointSignal.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentDelaySignalBreak.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentDelayedCrashWithBreakpointSignal.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentDelayedCrashWithBreakpointWatchpoint.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyBreakpoints.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyCrash.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentSignalBreak.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentTwoBreakpointThreads.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/exit_during_break/TestExitDuringBreak.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/exit_during_step/TestExitDuringStep.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/num_threads/TestNumThreads.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_exit/TestThreadExit.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/thread_specific_break/TestThreadSpecificBreakpoint.py
  
lldb/packages/Python/lldbsuite/test/python_api/lldbutil/iter/TestLLDBIterator.py
  
lldb/packages/Python/lldbsuite/test/python_api/watchpoint/watchlocation/TestTargetWatchAddress.py
  
lldb/packages/Python/lldbsuite/test/tools/lldb-server/TestGdbRemote_vContThreads.py
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h

Index: lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
===
--- lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
+++ lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
@@ -48,11 +48,16 @@
 private:
   // Interface for friend classes
 
+  Status Resume();
+  Status SingleStep();
+  Status Suspend();
+
   void SetStoppedBySignal(uint32_t signo, const siginfo_t *info = nullptr);
   void SetStoppedByBreakpoint();
   void SetStoppedByTrace();
   void SetStoppedByExec();
   void SetStoppedByWatchpoint(uint32_t wp_index);
+  void SetStoppedWithNoReason();
   void SetStopped();
   void SetRunning();
   void SetStepping();
Index: lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
@@ -18,6 +18,11 @@
 #include "lldb/Utility/State.h"
 #include "llvm/Support/Errno.h"
 
+// clang-format off
+#include 
+#include 
+// clang-format on
+
 #include 
 
 // clang-format off
@@ -36,6 +41,38 @@
 NativeRegisterContextNetBSD::CreateHostNativeRegisterContextNetBSD(process.GetArchitecture(), *this)
 ), m_stop_description() {}
 
+Status NativeThreadNetBSD::Resume() {
+  Status ret = NativeProcessNetBSD::PtraceWrapper(PT_RESUME, m_process.GetID(),
+  nullptr, GetID());
+  if (!ret.Success())
+return ret;
+  ret = NativeProcessNetBSD::PtraceWrapper(PT_CLEARSTEP, m_process.GetID(),
+   nullptr, GetID());
+  if (ret.Success())
+SetRunning();
+  return ret;
+}
+
+Status NativeThreadNetBSD::SingleStep() {
+  Status ret = NativeProcessNetBSD::PtraceWrapper(PT_RESUME, m_process.GetID(),
+  nullptr, GetID());
+  if (!ret.Success())
+return ret;
+  ret = Nativ

[Lldb-commits] [PATCH] D70023: [lldb] [Process/NetBSD] Copy watchpoints to newly-created threads

2019-11-25 Thread Michał Górny via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd970d4d4aa73: [lldb] [Process/NetBSD] Copy watchpoints to 
newly-created threads (authored by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70023

Files:
  
lldb/packages/Python/lldbsuite/test/commands/watchpoints/multiple_threads/TestWatchpointMultipleThreads.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentBreakpointsDelayedBreakpointOneWatchpoint.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentDelaySignalWatch.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentTwoBreakpointsOneWatchpoint.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentWatchBreakDelay.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentWatchpointDelayWatchpointOneBreakpoint.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentWatchpointWithDelayWatchpointThreads.py
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
  lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
  lldb/test/Shell/Watchpoint/Inputs/thread-dbreg.c
  lldb/test/Shell/Watchpoint/netbsd-nouserdbregs.test
  lldb/test/Shell/lit.cfg.py

Index: lldb/test/Shell/lit.cfg.py
===
--- lldb/test/Shell/lit.cfg.py
+++ lldb/test/Shell/lit.cfg.py
@@ -5,6 +5,7 @@
 import re
 import shutil
 import site
+import subprocess
 import sys
 
 import lit.formats
@@ -103,3 +104,17 @@
 
 if find_executable('xz') != None:
 config.available_features.add('xz')
+
+# NetBSD permits setting dbregs either if one is root
+# or if user_set_dbregs is enabled
+can_set_dbregs = True
+if platform.system() == 'NetBSD' and os.geteuid() != 0:
+try:
+output = subprocess.check_output(["/sbin/sysctl", "-n",
+  "security.models.extensions.user_set_dbregs"]).decode().strip()
+if output != "1":
+can_set_dbregs = False
+except subprocess.CalledProcessError:
+can_set_dbregs = False
+if can_set_dbregs:
+config.available_features.add('dbregs-set')
Index: lldb/test/Shell/Watchpoint/netbsd-nouserdbregs.test
===
--- /dev/null
+++ lldb/test/Shell/Watchpoint/netbsd-nouserdbregs.test
@@ -0,0 +1,22 @@
+# Check that 'watchpoint set' errors out gracefully when we can't set dbregs
+# and that new threads are monitored correctly even though we can't copy dbregs.
+
+# REQUIRES: system-netbsd && (target-x86 || target-x86_64) && !dbregs-set
+# RUN: %clang_host %p/Inputs/thread-dbreg.c -pthread -g -o %t.out
+# RUN: %lldb -b -o 'settings set interpreter.stop-command-source-on-error false' -s %s %t.out 2>&1 | FileCheck %s
+
+settings show interpreter.stop-command-source-on-error
+# CHECK: interpreter.stop-command-source-on-error (boolean) = false
+
+b main
+# CHECK: Breakpoint {{[0-9]+}}: where = {{.*}}`main
+b thread_func
+# CHECK: Breakpoint {{[0-9]+}}: where = {{.*}}`thread_func
+run
+# CHECK: stop reason = breakpoint
+watchpoint set variable g_watchme
+# CHECK: error: Watchpoint creation failed
+cont
+# CHECK: stop reason = breakpoint
+cont
+# CHECK: Process {{[0-9]+}} exited with status = 0
Index: lldb/test/Shell/Watchpoint/Inputs/thread-dbreg.c
===
--- /dev/null
+++ lldb/test/Shell/Watchpoint/Inputs/thread-dbreg.c
@@ -0,0 +1,23 @@
+#include 
+
+int g_watchme = 0;
+
+void *thread_func(void *arg) {
+  /* watchpoint trigger from subthread */
+  g_watchme = 2;
+  return 0;
+}
+
+int main() {
+  pthread_t thread;
+  if (pthread_create(&thread, 0, thread_func, 0))
+return 1;
+
+  /* watchpoint trigger from main thread */
+  g_watchme = 1;
+
+  if (pthread_join(thread, 0))
+return 2;
+
+  return 0;
+}
Index: lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
===
--- lldb/source/Plugins/Process/NetBSD/NativeThreadNetBSD.h
+++ lldb/source/Plugins/Process

[Lldb-commits] [PATCH] D70025: [lldb] [Process/NetBSD] Fix handling concurrent watchpoint events

2019-11-25 Thread Michał Górny via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7644d8ba4dc4: [lldb] [Process/NetBSD] Fix handling 
concurrent watchpoint events (authored by mgorny).
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D70025?vs=228955&id=230946#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70025

Files:
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
  
lldb/packages/Python/lldbsuite/test/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
  
lldb/packages/Python/lldbsuite/test/python_api/watchpoint/watchlocation/TestSetWatchlocation.py
  lldb/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD.h
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
  lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h

Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
===
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.h
@@ -58,6 +58,8 @@
 
   bool ClearHardwareWatchpoint(uint32_t wp_index) override;
 
+  Status ClearWatchpointHit(uint32_t wp_index) override;
+
   Status ClearAllHardwareWatchpoints() override;
 
   Status SetHardwareWatchpointWithIndex(lldb::addr_t addr, size_t size,
Index: lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
===
--- lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
+++ lldb/source/Plugins/Process/NetBSD/NativeRegisterContextNetBSD_x86_64.cpp
@@ -853,10 +853,10 @@
   if (!is_vacant)
 return Status("Watchpoint index not vacant");
 
-  RegisterValue reg_value;
   const RegisterInfo *const reg_info_dr7 =
   GetRegisterInfoAtIndex(lldb_dr7_x86_64);
-  error = ReadRegister(reg_info_dr7, reg_value);
+  RegisterValue dr7_value;
+  error = ReadRegister(reg_info_dr7, dr7_value);
   if (error.Fail())
 return error;
 
@@ -874,16 +874,28 @@
 
   uint64_t bit_mask = (0x3 << (2 * wp_index)) | (0xF << (16 + 4 * wp_index));
 
-  uint64_t control_bits = reg_value.GetAsUInt64() & ~bit_mask;
+  uint64_t control_bits = dr7_value.GetAsUInt64() & ~bit_mask;
 
   control_bits |= enable_bit | rw_bits | size_bits;
 
   const RegisterInfo *const reg_info_drN =
   GetRegisterInfoAtIndex(lldb_dr0_x86_64 + wp_index);
-  error = WriteRegister(reg_info_drN, RegisterValue(addr));
+  RegisterValue drN_value;
+  error = ReadRegister(reg_info_drN, drN_value);
   if (error.Fail())
 return error;
 
+  // clear dr6 if address or bits changed (i.e. we're not reenabling the same
+  // watchpoint)
+  if (drN_value.GetAsUInt64() != addr ||
+  (dr7_value.GetAsUInt64() & bit_mask) != (rw_bits | size_bits)) {
+ClearWatchpointHit(wp_index);
+
+error = WriteRegister(reg_info_drN, RegisterValue(addr));
+if (error.Fail())
+  return error;
+  }
+
   error = WriteRegister(reg_info_dr7, RegisterValue(control_bits));
   if (error.Fail())
 return error;
@@ -897,32 +909,36 @@
   if (wp_index >= NumSupportedHardwareWatchpoints())
 return false;
 
+  // for watchpoints 0, 1, 2, or 3, respectively, clear bits 0-1, 2-3, 4-5
+  // or 6-7 of the debug control register (DR7)
+  const RegisterInfo *const reg_info_dr7 =
+  GetRegisterInfoAtIndex(lldb_dr7_x86_64);
   RegisterValue reg_value;
+  Status error = ReadRegister(reg_info_dr7, reg_value);
+  if (error.Fail())
+return false;
+  uint64_t bit_mask = 0x3 << (2 * wp_index);
+  uint64_t control_bits = reg_value.GetAsUInt64() & ~bit_mask;
+
+  return WriteRegister(reg_info_dr7, RegisterValue(control_bits)).Success();
+}
 
-  // for watchpoints 0, 1, 2, or 3, respectively, clear bits 0, 1, 2, or 3 of
+Status NativeRegisterContextNetBSD_x86_64::ClearWatchpointHit(uint32_t wp_index) {
+  if (wp_index >= NumSupportedHardwareWatchpoints())
+return Status("Watchpoint index out of range");
+
+  // for watchpoints 0, 1, 2, or 3, respectively, check bits 0, 1, 2, or 3 of
   // the debug status register (DR6)
   const RegisterInfo *const reg_info_dr6 =
   GetRegisterInfoAtIndex(lldb_dr6_x86_64);
+  RegisterValue reg_value;
   Status error = ReadRegister(reg_info_dr6, reg_value);
   if (error.Fail())
-return false;
+return error;
+
   uint

[Lldb-commits] [lldb] aa16bf1 - [lldb-vscode] Fix a race in test_extra_launch_commands

[Lldb-commits] [PATCH] D70127: [lldb-vscode] Fix a race in test_extra_launch_commands

[Lldb-commits] [PATCH] D70023: [lldb] [Process/NetBSD] Copy watchpoints to newly-created threads

[Lldb-commits] [PATCH] D70622: [cmake][lit] Follow symlinks when looking for lit tests & reconfigure lldb's tests.

[Lldb-commits] [PATCH] D70646: RFC 2/3: Move non-DWARF code: `DWARFUnit` -> `SymbolFileDWARF`

[Lldb-commits] [PATCH] D70645: RFC 1/3: Unify src<->dst DWARFASTParser for CopyUniqueClassMethodTypes()

[Lldb-commits] [PATCH] D70647: RFC 3/3: Remove DWARFDIE dependency from functions moved by D70646

[Lldb-commits] [PATCH] D70645: RFC 1/3: Unify src<->dst DWARFASTParser for CopyUniqueClassMethodTypes()

[Lldb-commits] [PATCH] D70663: [lldb] Remove lldb's own ASTDumper

[Lldb-commits] [PATCH] D70663: [lldb] Remove lldb's own ASTDumper

[Lldb-commits] [lldb] 7a6588a - [lldb] Remove lldb's own ASTDumper

[Lldb-commits] [PATCH] D70663: [lldb] Remove lldb's own ASTDumper

[Lldb-commits] [PATCH] D70663: [lldb] Remove lldb's own ASTDumper

[Lldb-commits] [PATCH] D70668: [lldb][NFC] Allow range-based for-loops on VariableList

[Lldb-commits] [PATCH] D70668: [lldb][NFC] Allow range-based for-loops on VariableList

[Lldb-commits] [PATCH] D70668: [lldb][NFC] Allow range-based for-loops on VariableList

[Lldb-commits] [lldb] d178213 - [lldb][NFC] Allow range-based for-loops on VariableList

[Lldb-commits] [PATCH] D70668: [lldb][NFC] Allow range-based for-loops on VariableList

[Lldb-commits] [PATCH] D70025: [lldb] [Process/NetBSD] Fix handling concurrent watchpoint events

[Lldb-commits] [PATCH] D69309: Support template instantiation in the expression evaluator

[Lldb-commits] [PATCH] D70532: [lldb] Improve/fix base address selection in location lists

[Lldb-commits] [PATCH] D70025: [lldb] [Process/NetBSD] Fix handling concurrent watchpoint events

[Lldb-commits] [PATCH] D70532: [lldb] Improve/fix base address selection in location lists

[Lldb-commits] [PATCH] D69535: build: improve python check for Windows

[Lldb-commits] [PATCH] D70532: [lldb] Improve/fix base address selection in location lists

[Lldb-commits] [PATCH] D70022: [lldb] [Process/NetBSD] Improve threading support

[Lldb-commits] [PATCH] D70023: [lldb] [Process/NetBSD] Copy watchpoints to newly-created threads

[Lldb-commits] [PATCH] D70025: [lldb] [Process/NetBSD] Fix handling concurrent watchpoint events

28 matches

Site Navigation

Mail list logo

Footer information