[Lldb-commits] [PATCH] D106226: [lldb] Improve error message when "lldb attach" fails

2021-07-19 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

Thanks for looking at this, I've been tripped up by this myself.

> Is AppendMessage() really the right function to use in an error scenario ? or 
> the additional message should just be appended to AppendErrorWithFormat() ?

It changes whether the output goes to stdout or stderr and whether we set the 
return status to failed. So I would merge it into one call to 
`AppendErrorWithFormat`.

> The error object is being returned by the Attach() call on line:399 so, 
> should this message be handled by the Attach() function and not by 
> DoExecute() ?

Go to https://lldb.llvm.org/python_reference/index.html, search for `SBTarget` 
and you'll see a few attach methods there. They call Target Attach so if you 
added it there they'd also get it. Which seems like a good thing to me. (but 
see my other comment about platforms)




Comment at: lldb/source/Commands/CommandObjectProcess.cpp:416
+  "check the setting of /proc/sys/kernel/yama/ptrace_scope, \n"
+  "or try again as the root user. \n");
 }

While this is useful for Linux/Unixish things, lldb also runs on Windows as 
well where this message isn't going to make a lot of sense. Adding a `if linux 
...` at this level violates the separation of concerns though.
(yes an experienced user could ignore it but they'll probably at least wonder 
if lldb is confused)

Would it be possible to move this message further down into `Target::Attach` to 
a place where we know the target kind? (that would also answer your question 
about what level it should be at)

If you could return a status from that level with this message then you'd only 
see it where it makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106226

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

[Lldb-commits] [PATCH] D106192: [LLDB][GUI] Add Create Target form

2021-07-19 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

> This doesn't support remote files yet, I am still having trouble testing 
> those.

If you want to set that up we have some docs for it:
https://lldb.llvm.org/use/remote.html
https://lldb.llvm.org/use/qemu-testing.html

This is mostly how I work on AArch64 changes. That said there are some tricky 
bits getting the networking setup so I'm happy to try this out for you if you 
want.

I'll keep an eye on this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106192

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

[Lldb-commits] [PATCH] D105788: [LLDB] Silence warnings from ScriptedProcessPythonInterface.cpp

2021-07-19 Thread Med Ismail Bennani via Phabricator via lldb-commits 
mib commandeered this revision.
mib edited reviewers, added: omjavaid; removed: mib.
mib added a comment.

Hello @omjavaid, apologies for only looking at this now but I was on vacation 
last week. This changes make sense but I'd rather conform to the Python 
interface types. Also, I'm still working on `Scripted Processes` and I'm 
planning to land a patch with these changes soon. Thanks for letting me know 
about this oversight.




Comment at: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:175
   if (py_return.get()) {
 auto size = py_return.AsUnsignedLongLong();
+return (size) ? *size : gen_int_val;

I'd use to `unsigned long long` to match the interface.


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

https://reviews.llvm.org/D105788

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

[Lldb-commits] [PATCH] D106263: [lldb] Make WatchpointList iterable

2021-07-19 Thread Michał Górny via Phabricator via lldb-commits 
mgorny created this revision.
mgorny added reviewers: JDevlieghere, jingham.
mgorny requested review of this revision.

Based on de448c0a9e5088979526e2e67152fe547ae4ccf0 
.


https://reviews.llvm.org/D106263

Files:
  lldb/include/lldb/Breakpoint/WatchpointList.h
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1159,9 +1159,7 @@
   if (!ProcessIsValid())
 return false;
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1191,8 +1189,7 @@
 return false;
 
   size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1219,9 +1216,7 @@
   if (!ProcessIsValid())
 return false;
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1237,9 +1232,7 @@
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
   LLDB_LOGF(log, "Target::%s\n", __FUNCTION__);
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1253,9 +1246,7 @@
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
   LLDB_LOGF(log, "Target::%s\n", __FUNCTION__);
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1273,9 +1264,7 @@
   if (!ProcessIsValid())
 return false;
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
Index: lldb/source/Commands/CommandCompletions.cpp
===
--- lldb/source/Commands/CommandCompletions.cpp
+++ lldb/source/Commands/CommandCompletions.cpp
@@ -774,9 +774,7 @@
 return;
 
   const WatchpointList &wp_list = exe_ctx.GetTargetPtr()->GetWatchpointList();
-  const size_t wp_num = wp_list.GetSize();
-  for (size_t idx = 0; idx < wp_num; ++idx) {
-const lldb::WatchpointSP wp_sp = wp_list.GetByIndex(idx);
+  for (lldb::WatchpointSP wp_sp : wp_list.Watchpoints()) {
 StreamString strm;
 wp_sp->Dump(&strm);
 request.TryCompleteCurrentArg(std::to_string(wp_sp->GetID()),
Index: lldb/include/lldb/Breakpoint/WatchpointList.h
===
--- lldb/include/lldb/Breakpoint/WatchpointList.h
+++ lldb/include/lldb/Breakpoint/WatchpointList.h
@@ -14,6 +14,7 @@
 #include 
 
 #include "lldb/Core/Address.h"
+#include "lldb/Utility/Iterable.h"
 #include "lldb/lldb-private.h"
 
 namespace lldb_private {
@@ -37,6 +38,11 @@
   /// Destructor, currently does nothing.
   ~WatchpointList();
 
+  typedef std::list wp_collection;
+  typedef LockingAdaptedIterable
+  WatchpointIterable;
+
   /// Add a Watchpoint to the list.
   ///
   /// \param[in] wp_sp
@@ -184,8 +190,11 @@
   ///   The locker object that is set.
   void GetListMutex(std::unique_lock &lock);
 
+  WatchpointIterable Watchpoints() const {
+return WatchpointIterable(m_watchpoints, m_mutex);
+  }
+
 protected:
-  typedef std::list wp_collection;
   typedef std::vector id_vector;
 
   id_vector GetWatchpointIDs() const;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D106263: [lldb] Make WatchpointList iterable

2021-07-19 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added inline comments.



Comment at: lldb/include/lldb/Breakpoint/WatchpointList.h:193
 
+  WatchpointIterable Watchpoints() const {
+return WatchpointIterable(m_watchpoints, m_mutex);

NB: I needed to make a `const` variant since it's used in some `const` context 
(below). However, it doesn't seem that a non-`const` variant is needed at all 
but this is above my pay grade ;-).


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

https://reviews.llvm.org/D106263

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

[Lldb-commits] [PATCH] D100503: [lldb] [client] Implement follow-fork-mode

2021-07-19 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:5504-5513
+  WatchpointList &wps = GetTarget().GetWatchpointList();
+  size_t wp_count = wps.GetSize();
+  for (size_t i = 0; i < wp_count; ++i) {
+WatchpointSP wp = wps.GetByIndex(i);
+if (wp->IsEnabled()) {
+  GDBStoppointType type = GetGDBStoppointType(wp.get());
+  m_gdb_comm.SendGDBStoppointTypePacket(type, enable, wp->GetLoadAddress(),

JDevlieghere wrote:
> Should we lock the `WatchpointList` while iterating over it? If so I think 
> that class could benefit from an Iterable (like D105914) that will lock 
> during iteration, which would also simplify this code a bit. 
Indeed that sounds like a good idea. Opened D106263 for it. I'll update this 
one once that one is merged.


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

https://reviews.llvm.org/D100503

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

[Lldb-commits] [PATCH] D106266: [C++4OpenCL] Add run line standard aliases clc++1.0 and CLC++1.0

2021-07-19 Thread Justas Janickas via Phabricator via lldb-commits 
Topotuna created this revision.
Topotuna added a reviewer: Anastasia.
Herald added subscribers: jeroen.dobbelaere, ldrumm, dexonsmith, dang, yaxunl.
Topotuna requested review of this revision.
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

The commit renames language standard from openclcpp to openclcpp10.
It also enables run line arguments -cl-std=clc++1.0 and
-cl-std=CLC++1.0.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106266

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/LangStandards.def
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/autocomplete.c
  clang/test/Driver/unknown-std.cl
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
===
--- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
+++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp
@@ -478,7 +478,7 @@
   LangStd = LangStandard::lang_opencl10;
   break;
 case clang::Language::OpenCLCXX:
-  LangStd = LangStandard::lang_openclcpp;
+  LangStd = LangStandard::lang_openclcpp10;
   break;
 case clang::Language::CUDA:
   LangStd = LangStandard::lang_cuda;
Index: clang/test/Driver/unknown-std.cl
===
--- clang/test/Driver/unknown-std.cl
+++ clang/test/Driver/unknown-std.cl
@@ -11,7 +11,7 @@
 // CHECK-NEXT: note: use 'cl1.2' for 'OpenCL 1.2' standard
 // CHECK-NEXT: note: use 'cl2.0' for 'OpenCL 2.0' standard
 // CHECK-NEXT: note: use 'cl3.0' for 'OpenCL 3.0' standard
-// CHECK-NEXT: note: use 'clc++' for 'C++ for OpenCL' standard
+// CHECK-NEXT: note: use 'clc++1.0' for 'C++ for OpenCL 1.0' standard
 
 // Make sure that no other output is present.
 // CHECK-NOT: {{^.+$}}
Index: clang/test/Driver/autocomplete.c
===
--- clang/test/Driver/autocomplete.c
+++ clang/test/Driver/autocomplete.c
@@ -49,6 +49,8 @@
 // CLSTDALL-NEXT: CL3.0
 // CLSTDALL-NEXT: clc++
 // CLSTDALL-NEXT: CLC++
+// CLSTDALL-NEXT: clc++1.0
+// CLSTDALL-NEXT: CLC++1.0
 // RUN: %clang --autocomplete=-fno-sanitize-coverage=,f | FileCheck %s -check-prefix=FNOSANICOVER
 // FNOSANICOVER: func
 // RUN: %clang --autocomplete=-fno-sanitize-coverage= | FileCheck %s -check-prefix=FNOSANICOVERALL
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3088,7 +3088,7 @@
   LangStd = LangStandard::lang_opencl10;
   break;
 case Language::OpenCLCXX:
-  LangStd = LangStandard::lang_openclcpp;
+  LangStd = LangStandard::lang_openclcpp10;
   break;
 case Language::CUDA:
   LangStd = LangStandard::lang_cuda;
@@ -3161,7 +3161,7 @@
 Opts.OpenCLVersion = 200;
   else if (LangStd == LangStandard::lang_opencl30)
 Opts.OpenCLVersion = 300;
-  else if (LangStd == LangStandard::lang_openclcpp)
+  else if (LangStd == LangStandard::lang_openclcpp10)
 Opts.OpenCLCPlusPlusVersion = 100;
 
   // OpenCL has some additional defaults.
@@ -3311,7 +3311,7 @@
   case LangStandard::lang_opencl12:
   case LangStandard::lang_opencl20:
   case LangStandard::lang_opencl30:
-  case LangStandard::lang_openclcpp:
+  case LangStandard::lang_openclcpp10:
 StdOpt = OPT_cl_std_EQ;
 break;
   default:
@@ -3605,7 +3605,8 @@
 .Cases("cl1.2", "CL1.2", LangStandard::lang_opencl12)
 .Cases("cl2.0", "CL2.0", LangStandard::lang_opencl20)
 .Cases("cl3.0", "CL3.0", LangStandard::lang_opencl30)
-.Cases("clc++", "CLC++", LangStandard::lang_openclcpp)
+.Cases("clc++", "CLC++", LangStandard::lang_openclcpp10)
+.Cases("clc++1.0", "CLC++1.0", LangStandard::lang_openclcpp10)
 .Default(LangStandard::lang_unspecified);
 
 if (OpenCLLangStd == LangStandard::lang_unspecified) {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -843,7 +843,7 @@
   HelpText<"OpenCL only. Allow use of less precise no signed zeros computations in the generated binary.">,
   MarshallingInfoFlag>;
 def cl_std_EQ : Joined<["-"], "cl-std=">, Group, Flags<[CC1Option]>,
-  HelpText<"OpenCL language standard to compile for.">, Values<"cl,CL,cl1.0,CL1.0,cl1.1,CL1.1,cl1.2,CL1.2,cl2.0,CL2.0,cl3.0,CL3.0,clc++,CLC++">;
+  HelpText<"OpenCL language standard to compile for.">, Values<"cl,CL,cl1.0,CL1.0,cl1.1,CL1.1,cl1.2,CL1.2,cl2.0,CL2.0,cl3.0,CL3.0,clc++,CLC++,clc++1.0,CLC++1.0">;
 def cl_denorms_are_zero : Flag<["-"], "cl-denorms-are-zero">, Group,
   HelpText<"OpenCL only. Allow denormals to be flushed to zero.">;
 def cl_fp32_corr

[Lldb-commits] [PATCH] D106270: [DWARF5] Fix offset check when using .debug_names

2021-07-19 Thread Kim-Anh Tran via Phabricator via lldb-commits 
kimanh created this revision.
Herald added a subscriber: arphaman.
kimanh added a subscriber: pfaffe.
kimanh edited the summary of this revision.
kimanh added a reviewer: labath.
kimanh published this revision for review.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

When going through the CU entries in the name index,
make sure to compare against the name entry's CU
offset against the skeleton CU's offset.

Previously there would be a mismatch, since the
wrong offset was compared, and thus no suitable
entry was found.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106270

Files:
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp

Index: lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp
===
--- lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp
+++ lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp
@@ -27,6 +27,17 @@
 // RUN: lldb-test symbols --file=find-variable-file-2.cpp --find=variable %t | \
 // RUN:   FileCheck --check-prefix=TWO %s
 
+// Run the same test with split dwarf and pubnames to check whether we can find
+// the compile unit using the name index if it is split.
+// RUN: %clang -c -o %t-1.o --target=x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -gpubnames %s
+// RUN: %clang -c -o %t-2.o --target=x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -gpubnames %S/Inputs/find-variable-file-2.cpp
+// RUN: ld.lld %t-1.o %t-2.o -o %t
+// RUN: llvm-readobj --sections %t | FileCheck %s --check-prefix NAMES
+// RUN: lldb-test symbols --file=find-variable-file.cpp --find=variable %t | \
+// RUN:   FileCheck --check-prefix=ONE %s
+// RUN: lldb-test symbols --file=find-variable-file-2.cpp --find=variable %t | \
+// RUN:   FileCheck --check-prefix=TWO %s
+
 // NAMES: Name: .debug_names
 
 // ONE: Found 1 variables:
Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -3020,16 +3020,14 @@
 variables = std::make_shared();
 sc.comp_unit->SetVariableList(variables);
 
-m_index->GetGlobalVariables(
-dwarf_cu->GetNonSkeletonUnit(), [&](DWARFDIE die) {
-  VariableSP var_sp(
-  ParseVariableDIE(sc, die, LLDB_INVALID_ADDRESS));
-  if (var_sp) {
-variables->AddVariableIfUnique(var_sp);
-++vars_added;
-  }
-  return true;
-});
+m_index->GetGlobalVariables(*dwarf_cu, [&](DWARFDIE die) {
+  VariableSP var_sp(ParseVariableDIE(sc, die, LLDB_INVALID_ADDRESS));
+  if (var_sp) {
+variables->AddVariableIfUnique(var_sp);
+++vars_added;
+  }
+  return true;
+});
   }
   return vars_added;
 }
Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
@@ -33,7 +33,7 @@
   GetGlobalVariables(const RegularExpression ®ex,
  llvm::function_ref callback) override;
   void
-  GetGlobalVariables(const DWARFUnit &unit,
+  GetGlobalVariables(DWARFUnit &unit,
  llvm::function_ref callback) override;
   void GetObjCMethods(ConstString class_name,
   llvm::function_ref callback) override;
Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -358,9 +358,10 @@
 }
 
 void ManualDWARFIndex::GetGlobalVariables(
-const DWARFUnit &unit, llvm::function_ref callback) {
+DWARFUnit &unit, llvm::function_ref callback) {
   Index();
-  m_set.globals.FindAllEntriesForUnit(unit, DIERefCallback(callback));
+  m_set.globals.FindAllEntriesForUnit(unit.GetNonSkeletonUnit(),
+  DIERefCallback(callback));
 }
 
 void ManualDWARFIndex::GetObjCMethods(
Index: lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
===
--- lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ lld

[Lldb-commits] [PATCH] D106270: [DWARF5] Fix offset check when using .debug_names

2021-07-19 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:82
 
+  const DWARFUnit &non_skeleton_cu = cu.GetNonSkeletonUnit();
   DWARFMappedHash::DIEInfoArray hash_data;

I assume the `const` of the `DWARFUnit` is being dropped because 
`GetNonSkeletonUnit`? Rather than doing that, can't that function be const 
instead? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106270

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

[Lldb-commits] [PATCH] D106263: [lldb] Make WatchpointList iterable

2021-07-19 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM. Thanks!




Comment at: lldb/include/lldb/Breakpoint/WatchpointList.h:193
 
+  WatchpointIterable Watchpoints() const {
+return WatchpointIterable(m_watchpoints, m_mutex);

mgorny wrote:
> NB: I needed to make a `const` variant since it's used in some `const` 
> context (below). However, it doesn't seem that a non-`const` variant is 
> needed at all but this is above my pay grade ;-).
Haha :-D 


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

https://reviews.llvm.org/D106263

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

[Lldb-commits] [PATCH] D105741: [trace] Add `thread trace export` command for Intel PT trace visualization

2021-07-19 Thread Jakob Johnson via Phabricator via lldb-commits 
jj10306 updated this revision to Diff 359848.
jj10306 retitled this revision from "[trace] Add `thread trace dump ctf -f 
` command" to "[trace] Add `thread trace export` command for Intel PT 
trace visualization".
jj10306 edited the summary of this revision.
jj10306 added a comment.

- Add HTR documentation to `lldb/docs/htr.rst`
- Change new command to `thread trace export --format  --outfile 
`
- Distinguish between the instruction layer and all other layers - add 
`IHTRLayer` interface that `HTRInstructionLayer` and `HTRBlockLayer` implement
- Remove unnecessary templates
- Add accessor methods to HTR classes and remove all uses of `friend`


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

https://reviews.llvm.org/D105741

Files:
  lldb/docs/htr.rst
  lldb/docs/media/basic_super_block_pass.png
  lldb/docs/media/htr_example.png
  lldb/include/lldb/Target/Trace.h
  lldb/include/lldb/Target/TraceHTR.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Target/CMakeLists.txt
  lldb/source/Target/Trace.cpp
  lldb/source/Target/TraceCursor.cpp
  lldb/source/Target/TraceHTR.cpp
  lldb/test/API/commands/trace/TestTraceExport.py

Index: lldb/test/API/commands/trace/TestTraceExport.py
===
--- /dev/null
+++ lldb/test/API/commands/trace/TestTraceExport.py
@@ -0,0 +1,60 @@
+import lldb
+from intelpt_testcase import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+from lldbsuite.test.decorators import *
+import os
+
+class TestTraceExport(TraceIntelPTTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def testErrorMessages(self):
+ctf_test_file = self.getBuildArtifact("ctf-test.json")
+# We first check the output when there are no targets
+self.expect(f"thread trace export --format ctf --outfile {ctf_test_file}",
+substrs=["error: invalid target, create a target using the 'target create' command"],
+error=True)
+
+# We now check the output when there's a non-running target
+self.expect("target create " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "a.out"))
+
+self.expect(f"thread trace export --format ctf --outfile {ctf_test_file}",
+substrs=["error: invalid process"],
+error=True)
+
+# Now we check the output when there's a running target without a trace
+self.expect("b main")
+self.expect("run")
+
+self.expect(f"thread trace export --format ctf --outfile {ctf_test_file}",
+substrs=["error: Process is not being traced"],
+error=True)
+
+def testExportChromeTraceFormat(self):
+self.expect("trace load -v " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
+substrs=["intel-pt"])
+
+ctf_test_file = self.getBuildArtifact("ctf-test.json")
+
+# file name, no count
+if os.path.exists(ctf_test_file):
+remove_file(ctf_test_file)
+self.expect(f"thread trace export --format ctf --outfile {ctf_test_file}",
+substrs=["Success", f"{ctf_test_file}"])
+self.assertTrue(os.path.exists(ctf_test_file))
+
+def testInvalidExportFormat(self):
+self.expect("trace load -v " +
+os.path.join(self.getSourceDir(), "intelpt-trace", "trace.json"),
+substrs=["intel-pt"])
+
+ctf_test_file = self.getBuildArtifact("ctf-test.json")
+unknown_format = "DEADBEEF"
+
+self.expect(f"thread trace export --format {unknown_format} --outfile {ctf_test_file}",
+substrs=[f"error: unknown format '{unknown_format}' specified."],
+error=True)
+
Index: lldb/source/Target/TraceHTR.cpp
===
--- /dev/null
+++ lldb/source/Target/TraceHTR.cpp
@@ -0,0 +1,468 @@
+//===-- TraceHTR.cpp --===//
+//
+// 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 "lldb/Target/TraceHTR.h"
+
+#include "lldb/Symbol/Function.h"
+#include "lldb/Target/Process.h"
+#include "lldb/Target/Target.h"
+#include "llvm/Support/JSON.h"
+#include 
+#include 
+
+using namespace lldb_private;
+using namespace lldb;
+
+size_t HTRBlockMetadata::GetNumInstructions() const {
+  return m_num_instructions;
+}
+
+llvm::Optional
+HTRBlockMetadata::GetMostFrequentylyCalledFunction() const {
+  size_t max_ncalls = 0;
+  llvm::Optional max_name = llvm::None;
+  for (const auto &[name, ncalls] : m_func_calls) {
+if (ncalls > max_ncalls) {
+  max_ncalls = ncalls;
+

[Lldb-commits] [PATCH] D105741: [trace] Add `thread trace export` command for Intel PT trace visualization

2021-07-19 Thread Jakob Johnson via Phabricator via lldb-commits 
jj10306 marked 19 inline comments as done.
jj10306 added inline comments.



Comment at: lldb/docs/htr.rst:1
+Hierarchical Trace Representation (HTR)
+==

@clayborg @wallace Is there a way to view the "rendered" rst to ensure the the 
format is as I intended and that the image links are working?



Comment at: lldb/include/lldb/Target/Trace.h:65-68
+  ///
+  /// \param[in] count
+  /// The number of trace instructions to include in the CTF dump
+  ///

Update docs - remove count and add export_format


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

https://reviews.llvm.org/D105741

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

[Lldb-commits] [lldb] 2656af9 - Don't use !eStateRunning when you mean eStateStopped in DestroyImpl.

2021-07-19 Thread Jim Ingham via lldb-commits 

Author: Jim Ingham
Date: 2021-07-19T14:30:04-07:00
New Revision: 2656af95eb8e67364db7b8dc4a95c3b65c286b2d

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

LOG: Don't use !eStateRunning when you mean eStateStopped in DestroyImpl.

When we go to destroy the process, we first try to halt it, if
we succeeded and the target stopped, we want to clear out the
thread plans and breakpoints in case we still need to resume to complete
killing the process.  If the target was exited or detached, it's
pointless but harmless to do this.  But if the state is eStateInvalid -
for instance if we tried to interrupt the target to Halt it and that
fails - we don't want to keep trying to interact with the inferior,
so we shouldn't do this work.

This change explicitly checks eStateStopped, and only does the pre-resume
cleanup if we did manage to stop the process.

Added: 


Modified: 
lldb/source/Target/Process.cpp

Removed: 




diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index 2907c71054f08..05a361ba0ab7c 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -3228,7 +3228,7 @@ Status Process::DestroyImpl(bool force_kill) {
   error = StopForDestroyOrDetach(exit_event_sp);
 }
 
-if (m_public_state.GetValue() != eStateRunning) {
+if (m_public_state.GetValue() == eStateStopped) {
   // Ditch all thread plans, and remove all our breakpoints: in case we
   // have to restart the target to kill it, we don't want it hitting a
   // breakpoint... Only do this if we've stopped, however, since if we



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

[Lldb-commits] [PATCH] D106266: [C++4OpenCL] Add run line standard aliases clc++1.0 and CLC++1.0

2021-07-19 Thread Anastasia Stulova via Phabricator via lldb-commits 
Anastasia added inline comments.



Comment at: clang/include/clang/Basic/LangStandards.def:187
  Digraphs | HexFloat | OpenCL)
+LANGSTANDARD_ALIAS_DEPR(openclcpp10, "clc++")
 

I am not sure we should move it into the deprecated category yet... for now 
let's just use `LANGSTANDARD_ALIAS`.



Comment at: clang/test/Driver/unknown-std.cl:14
 // CHECK-NEXT: note: use 'cl3.0' for 'OpenCL 3.0' standard
-// CHECK-NEXT: note: use 'clc++' for 'C++ for OpenCL' standard
+// CHECK-NEXT: note: use 'clc++1.0' for 'C++ for OpenCL 1.0' standard
 

does it not print `clc++` anymore?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106266

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

[Lldb-commits] [PATCH] D105717: [trace] [intel pt] Create a "thread trace dump stats" command

2021-07-19 Thread walter erquinigo via Phabricator via lldb-commits 
wallace added inline comments.



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:113
   Optional raw_size = GetRawTraceSize(thread);
-  s.Printf("Tracing technology: %s\nthread #%u: tid = %" PRIu64, 
plugin_name.AsCString(), thread.GetIndexID(), thread.GetID());
+  s.Printf("thread #%u: tid = %" PRIu64, thread.GetIndexID(), thread.GetID());
   if (!raw_size) {

add an empty line before this to clearly separate the output of each thread



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:118
   }
   s.Printf("\nRaw trace size: %zu bytes\n", *raw_size);
   return;

Add 2 empty spaces before the text to have some sort of indentation


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

https://reviews.llvm.org/D105717

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

[Lldb-commits] [PATCH] D106328: [trace][intel pt] Add TSC timestamps

2021-07-19 Thread walter erquinigo via Phabricator via lldb-commits 
wallace created this revision.
Herald added a subscriber: dang.
wallace updated this revision to Diff 359972.
wallace added a comment.
wallace updated this revision to Diff 359973.
wallace retitled this revision from "[intel pt] Add TSC timestamps" to 
"[trace][intel pt] Add TSC timestamps".
wallace edited the summary of this revision.
wallace added a reviewer: clayborg.
Herald added a subscriber: pengfei.
wallace published this revision for review.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

nits


wallace added a comment.

nits


I've just added the GetTimestampCounter method to Trace.h, which returns an 
Optional of a timestamp counter. Major CPU architectures support timestamp 
counters, which fortunately are invariant in modern chips (i.e. they are 
synchronized between logical CPUs and increase at a constant rate). This helps 
with having accurate timestamps even when the the thread is jumping around CPUs.

A followup diff would be to add a GetRealTime method, which would ask the trace 
plugin to convert a TSC to any of ms/ns/us or just to return the real time if 
available in the actual trace.

Tools could be built upon any of those two APIs, being GetTimestampCounter way 
more accurate in terms of keeping relative times.

I also implemented the specific bits for Intel PT. Now the "trace start" 
command accepts two params: --tsc which enables the generation of timestamp 
counters, and --psb_period, which is the simplest way to tweak how often these 
timestamps are generated. There are other ways to increase the resolution of 
timestamps: CYC and MTC packets, but that can be left for a follow up diff to 
keep this one short.

  wallace@devbig418 ~/llvm-sand/build/Release/fbcode-x86_64/toolchain ▶ 
./bin/lldb ~/a.out 
  (lldb) target create "/home/wallace/a.out"
  Current executable set to '/home/wallace/a.out' (x86_64).
  (lldb) b main
  Breakpoint 1: where = a.out`main + 9 at main.cpp:6:15, address = 
0x00400c5f
  (lldb) r
  Process 1130363 launched: '/home/wallace/a.out' (x86_64)
  Process 1130363 stopped
  * thread #1, name = 'a.out', stop reason = breakpoint 1.1
  frame #0: 0x00400c5f a.out`main at main.cpp:6:15
 3using namespace std;
 4   
 5int main() {
  -> 6  vector s;
 7  s.push_back(1);
 8  s.push_back(2);
 9  cout << s.size() << endl;
  (lldb) thread trace start --tsc
  (lldb) n
  Process 1130363 stopped
  * thread #1, name = 'a.out', stop reason = step over
  frame #0: 0x00400c6b a.out`main at main.cpp:7:15
 4   
 5int main() {
 6  vector s;
  -> 7  s.push_back(1);
 8  s.push_back(2);
 9  cout << s.size() << endl;
 10 int x = 1;
  (lldb) thread trace dump instructions --tsc -f
  thread #1: tid = 1130363
a.out`main + 9 at main.cpp:6:15
  [ 0] [tsc=0x005a8eadd08764ba] 0x00400c5fleaq   -0x30(%rbp), 
%rax
  [ 1] [tsc=0x005a8eadd09356a8] 0x00400c63movq   %rax, %rdi
  [ 2] [tsc=0x005a8eadd09356a8] 0x00400c66callq  0x400d9c   
   ; std::vector >::vector at 
stl_vector.h:391:7
...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106328

Files:
  lldb/docs/lldb-gdb-remote.txt
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceInstructionDumper.h
  lldb/include/lldb/Utility/TraceIntelPTGDBRemotePackets.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Process/Linux/IntelPTManager.cpp
  lldb/source/Plugins/Process/Linux/IntelPTManager.h
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/CommandObjectTraceStartIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/IntelPTDecoder.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTConstants.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPTOptions.td
  lldb/source/Target/TraceInstructionDumper.cpp
  lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp

Index: lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
===
--- lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
+++ lldb/source/Utility/TraceIntelPTGDBRemotePackets.cpp
@@ -17,6 +17,7 @@
   Path path) {
   ObjectMapper o(value, path);
   if (!o || !fromJSON(value, (TraceStartRequest &)packet, path) ||
+  !o.map("tsc", packet.tsc) || !o.map("psbPeriod", packet.psb_period) ||
   !o.map("threadBufferSize", packet.threadBufferSize) ||
   !o.map("processBufferSizeLimit", packet.processBufferSizeLimit))
 return false;
@@

[Lldb-commits] [PATCH] D106328: [trace][intel pt] Add TSC timestamps

2021-07-19 Thread Greg Clayton via Phabricator via lldb-commits 
clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
Herald added a subscriber: JDevlieghere.

Man cases of having variables named "tsc" or "m_tsc" when it might be clear to 
have them named "enable_tsc" and "m_enable_tsc".




Comment at: lldb/source/Commands/Options.td:1071
+Desc<"For each instruction, print the corresponding timestamp counter if 
available.">;
 }
 

You mentioned "--psb_period" in your description, but that isn't here. Just 
wanted to make sure that is on purpose. And if you do add it it should be 
--psb-period with - and not a _



Comment at: lldb/source/Plugins/Process/Linux/IntelPTManager.cpp:58
+IntelPTConfigFileType type) {
+  auto stream = llvm::MemoryBuffer::getFileAsStream(file);
+

Don't use auto here? More readable



Comment at: lldb/source/Plugins/Process/Linux/IntelPTManager.cpp:87
+
+  if (buffer.trim().consumeInteger(getRadix(), value)) {
+std::ostringstream error;

Is "buffer" binary or text? If text, then this is fine.



Comment at: lldb/source/Plugins/Process/Linux/IntelPTManager.cpp:104
+return createStringError(inconvertibleErrorCode(), error.str().c_str());
+  }
+  return value;

Do you want an else clause here to error check "ZeroOne" in case you get 
something like 12 back?



Comment at: lldb/source/Plugins/Process/Linux/IntelPTManager.h:85
+  llvm::Error StartTrace(lldb::pid_t pid, lldb::tid_t tid, uint64_t 
buffer_size,
+ bool tsc, llvm::Optional psb_period);
 

rename "tsc" to "enable_tsc"?



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTConstants.h:21
 const size_t kProcessBufferSizeLimit = 5 * 1024 * 1024; // 500MB
+const bool kTSC = false;
+const llvm::Optional kPSBPeriod = llvm::None;

"kDefaultTSCValue"? The name above doesn't really convey what this variable is



Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPTConstants.h:22
+const bool kTSC = false;
+const llvm::Optional kPSBPeriod = llvm::None;
 

kDefaultPSBPeriod?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106328

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

[Lldb-commits] [PATCH] D105717: [trace] [intel pt] Create a "thread trace dump stats" command

2021-07-19 Thread hanbing wang via Phabricator via lldb-commits 
hanbingwang updated this revision to Diff 359994.
hanbingwang added a comment.

{F18030885 }add two spaces before the 
string "Raw trace size : xxx"; add new line between each thread.


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

https://reviews.llvm.org/D105717

Files:
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp


Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -110,12 +110,12 @@
 
 void TraceIntelPT::DumpTraceInfo(Thread &thread, Stream &s, bool verbose) {
   Optional raw_size = GetRawTraceSize(thread);
-  s.Printf("thread #%u: tid = %" PRIu64, thread.GetIndexID(), thread.GetID());
+  s.Printf("\nthread #%u: tid = %" PRIu64, thread.GetIndexID(), 
thread.GetID());
   if (!raw_size) {
 s.Printf(", not traced\n");
 return;
   }
-  s.Printf("\nRaw trace size: %zu bytes\n", *raw_size);
+  s.Printf("\n  Raw trace size: %zu bytes\n", *raw_size);
   return;
 }
 


Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -110,12 +110,12 @@
 
 void TraceIntelPT::DumpTraceInfo(Thread &thread, Stream &s, bool verbose) {
   Optional raw_size = GetRawTraceSize(thread);
-  s.Printf("thread #%u: tid = %" PRIu64, thread.GetIndexID(), thread.GetID());
+  s.Printf("\nthread #%u: tid = %" PRIu64, thread.GetIndexID(), thread.GetID());
   if (!raw_size) {
 s.Printf(", not traced\n");
 return;
   }
-  s.Printf("\nRaw trace size: %zu bytes\n", *raw_size);
+  s.Printf("\n  Raw trace size: %zu bytes\n", *raw_size);
   return;
 }
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D106324: Remove debugserver support for the DarwinLog StructuredData streaming service, lldb tests for it

2021-07-19 Thread Jonas Devlieghere via Phabricator via lldb-commits 
JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106324

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

[Lldb-commits] [lldb] 7b54b1c - [lldb] Make WatchpointList iterable

2021-07-19 Thread Michał Górny via lldb-commits 

Author: Michał Górny
Date: 2021-07-20T07:47:48+02:00
New Revision: 7b54b1cdafbcaa5721bcf8ae78e8390a74d580bf

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

LOG: [lldb] Make WatchpointList iterable

Based on de448c0a9e5088979526e2e67152fe547ae4ccf0.

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

Added: 


Modified: 
lldb/include/lldb/Breakpoint/WatchpointList.h
lldb/source/Commands/CommandCompletions.cpp
lldb/source/Target/Target.cpp

Removed: 




diff  --git a/lldb/include/lldb/Breakpoint/WatchpointList.h 
b/lldb/include/lldb/Breakpoint/WatchpointList.h
index 6f224b641396c..bf87495d79dba 100644
--- a/lldb/include/lldb/Breakpoint/WatchpointList.h
+++ b/lldb/include/lldb/Breakpoint/WatchpointList.h
@@ -14,6 +14,7 @@
 #include 
 
 #include "lldb/Core/Address.h"
+#include "lldb/Utility/Iterable.h"
 #include "lldb/lldb-private.h"
 
 namespace lldb_private {
@@ -37,6 +38,11 @@ class WatchpointList {
   /// Destructor, currently does nothing.
   ~WatchpointList();
 
+  typedef std::list wp_collection;
+  typedef LockingAdaptedIterable
+  WatchpointIterable;
+
   /// Add a Watchpoint to the list.
   ///
   /// \param[in] wp_sp
@@ -184,8 +190,11 @@ class WatchpointList {
   ///   The locker object that is set.
   void GetListMutex(std::unique_lock &lock);
 
+  WatchpointIterable Watchpoints() const {
+return WatchpointIterable(m_watchpoints, m_mutex);
+  }
+
 protected:
-  typedef std::list wp_collection;
   typedef std::vector id_vector;
 
   id_vector GetWatchpointIDs() const;

diff  --git a/lldb/source/Commands/CommandCompletions.cpp 
b/lldb/source/Commands/CommandCompletions.cpp
index 857b2a05b91c1..55018cef57d4e 100644
--- a/lldb/source/Commands/CommandCompletions.cpp
+++ b/lldb/source/Commands/CommandCompletions.cpp
@@ -774,9 +774,7 @@ void CommandCompletions::WatchPointIDs(CommandInterpreter 
&interpreter,
 return;
 
   const WatchpointList &wp_list = exe_ctx.GetTargetPtr()->GetWatchpointList();
-  const size_t wp_num = wp_list.GetSize();
-  for (size_t idx = 0; idx < wp_num; ++idx) {
-const lldb::WatchpointSP wp_sp = wp_list.GetByIndex(idx);
+  for (lldb::WatchpointSP wp_sp : wp_list.Watchpoints()) {
 StreamString strm;
 wp_sp->Dump(&strm);
 request.TryCompleteCurrentArg(std::to_string(wp_sp->GetID()),

diff  --git a/lldb/source/Target/Target.cpp b/lldb/source/Target/Target.cpp
index 6cb7a9942a5c7..2a0dcaa4ce6bd 100644
--- a/lldb/source/Target/Target.cpp
+++ b/lldb/source/Target/Target.cpp
@@ -1159,9 +1159,7 @@ bool Target::RemoveAllWatchpoints(bool end_to_end) {
   if (!ProcessIsValid())
 return false;
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1191,8 +1189,7 @@ bool Target::DisableAllWatchpoints(bool end_to_end) {
 return false;
 
   size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1219,9 +1216,7 @@ bool Target::EnableAllWatchpoints(bool end_to_end) {
   if (!ProcessIsValid())
 return false;
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1237,9 +1232,7 @@ bool Target::ClearAllWatchpointHitCounts() {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
   LLDB_LOGF(log, "Target::%s\n", __FUNCTION__);
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1253,9 +1246,7 @@ bool Target::ClearAllWatchpointHistoricValues() {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
   LLDB_LOGF(log, "Target::%s\n", __FUNCTION__);
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1273,9 +1264,7 @@ bool Target::IgnoreAllWatchpoints(uint32_t ignore_count) {
   if (!ProcessIsValid())
 return false;
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (siz

[Lldb-commits] [PATCH] D106263: [lldb] Make WatchpointList iterable

2021-07-19 Thread Michał Górny via Phabricator via lldb-commits 
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7b54b1cdafbc: [lldb] Make WatchpointList iterable (authored 
by mgorny).
Herald added a project: LLDB.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106263

Files:
  lldb/include/lldb/Breakpoint/WatchpointList.h
  lldb/source/Commands/CommandCompletions.cpp
  lldb/source/Target/Target.cpp

Index: lldb/source/Target/Target.cpp
===
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -1159,9 +1159,7 @@
   if (!ProcessIsValid())
 return false;
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1191,8 +1189,7 @@
 return false;
 
   size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1219,9 +1216,7 @@
   if (!ProcessIsValid())
 return false;
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1237,9 +1232,7 @@
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
   LLDB_LOGF(log, "Target::%s\n", __FUNCTION__);
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1253,9 +1246,7 @@
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
   LLDB_LOGF(log, "Target::%s\n", __FUNCTION__);
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
@@ -1273,9 +1264,7 @@
   if (!ProcessIsValid())
 return false;
 
-  size_t num_watchpoints = m_watchpoint_list.GetSize();
-  for (size_t i = 0; i < num_watchpoints; ++i) {
-WatchpointSP wp_sp = m_watchpoint_list.GetByIndex(i);
+  for (WatchpointSP wp_sp : m_watchpoint_list.Watchpoints()) {
 if (!wp_sp)
   return false;
 
Index: lldb/source/Commands/CommandCompletions.cpp
===
--- lldb/source/Commands/CommandCompletions.cpp
+++ lldb/source/Commands/CommandCompletions.cpp
@@ -774,9 +774,7 @@
 return;
 
   const WatchpointList &wp_list = exe_ctx.GetTargetPtr()->GetWatchpointList();
-  const size_t wp_num = wp_list.GetSize();
-  for (size_t idx = 0; idx < wp_num; ++idx) {
-const lldb::WatchpointSP wp_sp = wp_list.GetByIndex(idx);
+  for (lldb::WatchpointSP wp_sp : wp_list.Watchpoints()) {
 StreamString strm;
 wp_sp->Dump(&strm);
 request.TryCompleteCurrentArg(std::to_string(wp_sp->GetID()),
Index: lldb/include/lldb/Breakpoint/WatchpointList.h
===
--- lldb/include/lldb/Breakpoint/WatchpointList.h
+++ lldb/include/lldb/Breakpoint/WatchpointList.h
@@ -14,6 +14,7 @@
 #include 
 
 #include "lldb/Core/Address.h"
+#include "lldb/Utility/Iterable.h"
 #include "lldb/lldb-private.h"
 
 namespace lldb_private {
@@ -37,6 +38,11 @@
   /// Destructor, currently does nothing.
   ~WatchpointList();
 
+  typedef std::list wp_collection;
+  typedef LockingAdaptedIterable
+  WatchpointIterable;
+
   /// Add a Watchpoint to the list.
   ///
   /// \param[in] wp_sp
@@ -184,8 +190,11 @@
   ///   The locker object that is set.
   void GetListMutex(std::unique_lock &lock);
 
+  WatchpointIterable Watchpoints() const {
+return WatchpointIterable(m_watchpoints, m_mutex);
+  }
+
 protected:
-  typedef std::list wp_collection;
   typedef std::vector id_vector;
 
   id_vector GetWatchpointIDs() const;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D106270: [DWARF5] Fix offset check when using .debug_names

2021-07-19 Thread Kim-Anh Tran via Phabricator via lldb-commits 
kimanh added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp:82
 
+  const DWARFUnit &non_skeleton_cu = cu.GetNonSkeletonUnit();
   DWARFMappedHash::DIEInfoArray hash_data;

JDevlieghere wrote:
> I assume the `const` of the `DWARFUnit` is being dropped because 
> `GetNonSkeletonUnit`? Rather than doing that, can't that function be const 
> instead? 
Yes correct. I agree with your point that dropping `const` here is not very 
nice, but I'm not sure whether we can make that function const easily. 
As is `GetNonSkeletonUnit` is updating itself in `ExtractUnitDIEIfNeeded()`, 
which kicks off many non-const operations for updating itself. As far as I 
understand, if we would want to make it const, we'd need to change the 
architectural logic in how the skeleton and non-skeleton units are connected. 
I'm not very familiar with the code base, so any advice here is welcome!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106270

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