[Lldb-commits] [PATCH] D96555: [lldb] Remove the legacy FreeBSD plugin

2021-02-15 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.

In D96555#2558465 , @mgorny wrote:

> @labath, any clue if I'm missing some no-longer-necessary code? I feel like I 
> was supposed to also remove something responsible for launching/attaching 
> processes in the old mode.

We still have windows support around, so I don't think there's any generic code 
that we can remove. However, this is still a big milestone.


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

https://reviews.llvm.org/D96555

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

[Lldb-commits] [PATCH] D96637: Make sure the interpreter module was loaded before making checks against it

2021-02-15 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

I'd like @mgorny to confirm this, but I think this should be fine -- the 
`m_rendezvous.IsValid()` branch does not do any funky module loading, so funky 
**un**loading should also be unnecessary.

From the description is seems like trigerring this bug should be relatively 
simple. All it takes is to attach to a process (after it has finished setting 
up the dynamic section). And indeed, from my simple experiments, it seems that 
we are indeed failing to load modules in that scenarios.

=> We should have a test for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96637

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

[Lldb-commits] [PATCH] D96637: Make sure the interpreter module was loaded before making checks against it

2021-02-15 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

And thanks for catching this. :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96637

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

[Lldb-commits] [PATCH] D95711: [lldb/Interpreter] Add ScriptInterpreter Wrapper for ScriptedProcess

2021-02-15 Thread Pavel Labath via Phabricator via lldb-commits 
labath added inline comments.



Comment at: lldb/unittests/ScriptInterpreter/Python/CMakeLists.txt:6
   LINK_LIBS
+liblldb
 lldbHost

This is a bad idea -- it makes the binary get two copies of everything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95711

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

[Lldb-commits] [PATCH] D95712: [lldb/bindings] Add Python ScriptedProcess base class to lldb module

2021-02-15 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

Unfortunately I don't have the bandwidth to review this, though I feel that 
this should have a wider discussion, given that its destined to become a pretty 
big chunk of our public immutable api surface.
Some questions to seed that discussion:

  get_num_memory_regions() -> int:
  get_memory_region_at_index(idx: int) -> lldb.SBMemoryRegionInfo:

This means that the implementation needs to know the exact count of memory 
regions at any given moment. Elsewhere, we've handled this by having a single 
api like `get_memory_region_containing_address`. This permits (but doesn't 
force) the implementation to be lazy in calculating the memory regions, and the 
caller can still enumerate the memory regions by starting with 
`get_memory_region_containing_address(0)` and continuing by 
`get_memory_region_containing_address(prev_region.base + prev_region+size)`. I 
think we should do the same here.

  get_num_threads() -> int:
  get_thread_at_index(idx: int) -> Dict:

This suffers from the same problem, though the solution is not that simple. But 
given that, not too long ago, we've (me&Jim) had a long discussion about how 
materializing all the threads all the time is prohibitively expensive, it might 
be good to elaborate on how exactly is this supposed to work (must the 
implementation always return all threads, or can it have threads disappear 
temporarily, etc.)

  get_register_for_thread(tid:int) -> Dict:

I guess this should be at least get_register**s**_for_thread

  read_memory_at_address(addr:int) -> lldb.SBData:

How much memory is this supposed to read?

  can_debug() -> bool:

What's the use case for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95712

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

[Lldb-commits] [PATCH] D96634: [lldb][JITLoaderGDB] Test debug support in llvm-jitlink

2021-02-15 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.

sounds reasonable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96634

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

[Lldb-commits] [PATCH] D96558: [lldb] [Process/FreeBSDRemote] Ensure that errors are always handled

2021-02-15 Thread Pavel Labath via Phabricator via lldb-commits 
labath added a comment.

This is precisely the reason we have the LLDB_LOG_ERROR macro :)


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

https://reviews.llvm.org/D96558

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

[Lldb-commits] [PATCH] D96194: Defer the decision whether to use the CU or TU index until after reading the unit header.

2021-02-15 Thread Pavel Labath via Phabricator via lldb-commits 
labath added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp:808-809
 
+  if (cu_index && (header.m_unit_type == llvm::dwarf::DW_UT_compile ||
+   header.m_unit_type == llvm::dwarf::DW_UT_split_compile)) {
+header.m_index_entry = cu_index->getFromOffset(header.m_offset);

jgorbe wrote:
> labath wrote:
> > I guess this could be `header.IsTypeUnit()` (and `!header.IsTypeUnit())`)...
> But `!header.IsTypeUnit` would also treat DW_UT_partial and DW_UT_skeleton as 
> compile units, right?
That's true, but can either of those units legitimately appear in a dwp file?
Even if they do appear for whatever reason, it wouldn't make any sense to use 
them without an index entry, and it would be (somewhat) more reasonable to put 
them in the cu index.

I took this idea from the equivalent llvm code: 



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

https://reviews.llvm.org/D96194

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

[Lldb-commits] [PATCH] D96557: [lldb] Rename FreeBSDRemote to FreeBSD (NFC)

2021-02-15 Thread Michał Górny via Phabricator via lldb-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rGbee4d6efe178: [lldb] Rename FreeBSDRemote to FreeBSD (NFC) 
(authored by mgorny).
Herald added a subscriber: jrtc27.
Herald added a project: LLDB.

Changed prior to commit:
  https://reviews.llvm.org/D96557?vs=323178&id=323709#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96557

Files:
  lldb/source/Plugins/Process/CMakeLists.txt
  lldb/source/Plugins/Process/FreeBSD/CMakeLists.txt
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_mips64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_mips64.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_powerpc.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_powerpc.h
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.h
  lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.h
  lldb/source/Plugins/Process/FreeBSDRemote/CMakeLists.txt
  lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
  lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h
  lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_arm.cpp
  lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_arm.h
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_arm64.h
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_mips64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_mips64.h
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_powerpc.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_powerpc.h
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp
  
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
  lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h
  lldb/tools/lldb-server/CMakeLists.txt
  lldb/tools/lldb-server/lldb-gdbserver.cpp

Index: lldb/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/tools/lldb-server/lldb-gdbserver.cpp
@@ -40,7 +40,7 @@
 #if defined(__linux__)
 #include "Plugins/Process/Linux/NativeProcessLinux.h"
 #elif defined(__FreeBSD__)
-#include "Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h"
+#include "Plugins/Process/FreeBSD/NativeProcessFreeBSD.h"
 #elif defined(__NetBSD__)
 #include "Plugins/Process/NetBSD/NativeProcessNetBSD.h"
 #elif defined(_WIN32)
Index: lldb/tools/lldb-server/CMakeLists.txt
===
--- lldb/tools/lldb-server/CMakeLists.txt
+++ lldb/tools/lldb-server/CMakeLists.txt
@@ -10,7 +10,7 @@
 endif()
 
 if(CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
-  list(APPEND LLDB_PLUGINS lldbPluginProcessFreeBSDRemote)
+  list(APPEND LLDB_PLUGINS lldbPluginProcessFreeBSD)
 endif()
 
 if(CMAKE_SYSTEM_NAME MATCHES "NetBSD")
Index: lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.h
===
--- lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.h
+++ lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.h
@@ -11,7 +11,7 @@
 
 #include "lldb/Host/common/NativeThreadProtocol.h"
 
-#include "Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h"
+#include "Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD.h"
 
 #include 
 #include 
Index: lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.h
===
--- lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.h
+++ lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.h
@@ -20,7 +20,7 @@
 
 #include 
 
-#include "Plugins/Process/FreeBSDRemote/NativeRegiste

[Lldb-commits] [lldb] bee4d6e - [lldb] Rename FreeBSDRemote to FreeBSD (NFC)

2021-02-15 Thread Michał Górny via lldb-commits 

Author: Michał Górny
Date: 2021-02-15T13:04:11+01:00
New Revision: bee4d6efe1781365cb82dcc94ad959309354af6f

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

LOG: [lldb] Rename FreeBSDRemote to FreeBSD (NFC)

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

Added: 
lldb/source/Plugins/Process/FreeBSD/CMakeLists.txt
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD.h
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm.cpp
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm.h
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_mips64.cpp
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_mips64.h
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_powerpc.cpp
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_powerpc.h
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.cpp
lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_x86_64.h
lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSD/NativeThreadFreeBSD.h

Modified: 
lldb/source/Plugins/Process/CMakeLists.txt
lldb/tools/lldb-server/CMakeLists.txt
lldb/tools/lldb-server/lldb-gdbserver.cpp

Removed: 
lldb/source/Plugins/Process/FreeBSDRemote/CMakeLists.txt
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD.h

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_arm.cpp
lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_arm.h

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_arm64.cpp

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_arm64.h

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_mips64.cpp

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_mips64.h

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_powerpc.cpp

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_powerpc.h

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.cpp

lldb/source/Plugins/Process/FreeBSDRemote/NativeRegisterContextFreeBSD_x86_64.h
lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.cpp
lldb/source/Plugins/Process/FreeBSDRemote/NativeThreadFreeBSD.h



diff  --git a/lldb/source/Plugins/Process/CMakeLists.txt 
b/lldb/source/Plugins/Process/CMakeLists.txt
index 55ad58226b19..fdeb211fe7a2 100644
--- a/lldb/source/Plugins/Process/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/CMakeLists.txt
@@ -2,7 +2,7 @@ if (CMAKE_SYSTEM_NAME MATCHES "Linux|Android")
   add_subdirectory(Linux)
   add_subdirectory(POSIX)
 elseif (CMAKE_SYSTEM_NAME MATCHES "FreeBSD")
-  add_subdirectory(FreeBSDRemote)
+  add_subdirectory(FreeBSD)
   add_subdirectory(POSIX)
 elseif (CMAKE_SYSTEM_NAME MATCHES "NetBSD")
   add_subdirectory(NetBSD)

diff  --git a/lldb/source/Plugins/Process/FreeBSDRemote/CMakeLists.txt 
b/lldb/source/Plugins/Process/FreeBSD/CMakeLists.txt
similarity index 90%
rename from lldb/source/Plugins/Process/FreeBSDRemote/CMakeLists.txt
rename to lldb/source/Plugins/Process/FreeBSD/CMakeLists.txt
index 7480154d9998..598911ce8c95 100644
--- a/lldb/source/Plugins/Process/FreeBSDRemote/CMakeLists.txt
+++ b/lldb/source/Plugins/Process/FreeBSD/CMakeLists.txt
@@ -1,4 +1,4 @@
-add_lldb_library(lldbPluginProcessFreeBSDRemote
+add_lldb_library(lldbPluginProcessFreeBSD
   NativeProcessFreeBSD.cpp
   NativeRegisterContextFreeBSD.cpp
   NativeRegisterContextFreeBSD_arm.cpp

diff  --git 
a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp 
b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
similarity index 100%
rename from lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.cpp
rename to lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp

diff  --git a/lldb/source/Plugins/Process/FreeBSDRemote/NativeProcessFreeBSD.h 
b/lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.h
similarity index 100%
rename from lldb/source/Plugins/Process/FreeBSDRemote/Nativ

[Lldb-commits] [PATCH] D96715: [lldb] Decouple IsMasterPlan and OkayToDiscard (NFC)

2021-02-15 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione created this revision.
kastiglione added a reviewer: jingham.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

While learning the semantics of `IsMasterPlan` and `OkayToDiscard`, and guided 
by the
FIXME comment in ThreadPlan.h, this change decouples the two functions.

These two functions have been called in pairs, for example:

  SetIsMasterPlan(true);
  SetOkayToDiscard(false);

With this change, almost all cases turn into a single `SetIsMasterPlan(true)` 
call. Note
that `m_okay_to_discard` now defaults to false.

Most of the decoupling happens in 
`ThreadPlanStack::DiscardConsultingMasterPlans`, which
handles `OkayToDiscard` and `IsMasterPlan` individually. This function could be 
renamed,
but I haven't thought of a good alternative.

Other notes:

- `OkayToDiscard` is no longer virtual
- `ThreadPlanPython` was incorrectly calling `SetOkayToDiscard(true)`
- Reworded some comments


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96715

Files:
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanBase.h
  lldb/source/API/SBThread.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Expression/FunctionCaller.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadPlan.cpp
  lldb/source/Target/ThreadPlanCallFunction.cpp
  lldb/source/Target/ThreadPlanCallUserExpression.cpp
  lldb/source/Target/ThreadPlanPython.cpp
  lldb/source/Target/ThreadPlanStack.cpp
  lldb/source/Target/ThreadPlanStepOut.cpp

Index: lldb/source/Target/ThreadPlanStepOut.cpp
===
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -474,7 +474,6 @@
 m_step_through_inline_plan_sp.get());
 m_step_through_inline_plan_sp->SetPrivate(true);
 
-step_through_inline_plan_ptr->SetOkayToDiscard(true);
 StreamString errors;
 if (!step_through_inline_plan_ptr->ValidatePlan(&errors)) {
   // FIXME: Log this failure.
Index: lldb/source/Target/ThreadPlanStack.cpp
===
--- lldb/source/Target/ThreadPlanStack.cpp
+++ lldb/source/Target/ThreadPlanStack.cpp
@@ -198,32 +198,30 @@
 
 void ThreadPlanStack::DiscardConsultingMasterPlans() {
   while (true) {
-int master_plan_idx;
-bool discard = true;
+int discard_plan_idx;
 
 // Find the first master plan, see if it wants discarding, and if yes
 // discard up to it.
-for (master_plan_idx = m_plans.size() - 1; master_plan_idx >= 0;
- master_plan_idx--) {
-  if (m_plans[master_plan_idx]->IsMasterPlan()) {
-discard = m_plans[master_plan_idx]->OkayToDiscard();
+for (discard_plan_idx = m_plans.size() - 1; discard_plan_idx >= 0;
+ discard_plan_idx--) {
+  auto plan_sp = m_plans[discard_plan_idx];
+  if (plan_sp->OkayToDiscard()) {
 break;
   }
+  if (plan_sp->IsMasterPlan()) {
+return;
+  }
 }
 
-// If the master plan doesn't want to get discarded, then we're done.
-if (!discard)
-  return;
-
 // First pop all the dependent plans:
-for (int i = m_plans.size() - 1; i > master_plan_idx; i--) {
+for (int i = m_plans.size() - 1; i > discard_plan_idx; i--) {
   DiscardPlan();
 }
 
 // Now discard the master plan itself.
 // The bottom-most plan never gets discarded.  "OkayToDiscard" for it
 // means discard it's dependent plans, but not it...
-if (master_plan_idx > 0) {
+if (discard_plan_idx > 0) {
   DiscardPlan();
 }
   }
Index: lldb/source/Target/ThreadPlanPython.cpp
===
--- lldb/source/Target/ThreadPlanPython.cpp
+++ lldb/source/Target/ThreadPlanPython.cpp
@@ -32,7 +32,6 @@
   m_class_name(class_name), m_args_data(args_data), m_did_push(false),
   m_stop_others(false) {
   SetIsMasterPlan(true);
-  SetOkayToDiscard(true);
   SetPrivate(false);
 }
 
Index: lldb/source/Target/ThreadPlanCallUserExpression.cpp
===
--- lldb/source/Target/ThreadPlanCallUserExpression.cpp
+++ lldb/source/Target/ThreadPlanCallUserExpression.cpp
@@ -40,7 +40,6 @@
   // User expressions are generally "User generated" so we should set them up
   // to stop when done.
   SetIsMasterPlan(true);
-  SetOkayToDiscard(false);
 }
 
 ThreadPlanCallUserExpression::~ThreadPlanCallUserExpression() {}
Index: lldb/source/Target/ThreadPlanCallFunction.cpp
===
--- lldb/source/Target/ThreadPlanCallFunction.cpp
+++ lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -34,7 +34,6 @@
 T

[Lldb-commits] [PATCH] D96715: [lldb] Decouple IsMasterPlan and OkayToDiscard (NFC)

2021-02-15 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione updated this revision to Diff 323765.
kastiglione added a comment.
Herald added a subscriber: JDevlieghere.

Restore a comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96715

Files:
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanBase.h
  lldb/source/API/SBThread.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Expression/FunctionCaller.cpp
  
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadPlan.cpp
  lldb/source/Target/ThreadPlanCallFunction.cpp
  lldb/source/Target/ThreadPlanCallUserExpression.cpp
  lldb/source/Target/ThreadPlanPython.cpp
  lldb/source/Target/ThreadPlanStack.cpp
  lldb/source/Target/ThreadPlanStepOut.cpp

Index: lldb/source/Target/ThreadPlanStepOut.cpp
===
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -474,7 +474,6 @@
 m_step_through_inline_plan_sp.get());
 m_step_through_inline_plan_sp->SetPrivate(true);
 
-step_through_inline_plan_ptr->SetOkayToDiscard(true);
 StreamString errors;
 if (!step_through_inline_plan_ptr->ValidatePlan(&errors)) {
   // FIXME: Log this failure.
Index: lldb/source/Target/ThreadPlanStack.cpp
===
--- lldb/source/Target/ThreadPlanStack.cpp
+++ lldb/source/Target/ThreadPlanStack.cpp
@@ -198,32 +198,31 @@
 
 void ThreadPlanStack::DiscardConsultingMasterPlans() {
   while (true) {
-int master_plan_idx;
-bool discard = true;
+int discard_plan_idx;
 
 // Find the first master plan, see if it wants discarding, and if yes
 // discard up to it.
-for (master_plan_idx = m_plans.size() - 1; master_plan_idx >= 0;
- master_plan_idx--) {
-  if (m_plans[master_plan_idx]->IsMasterPlan()) {
-discard = m_plans[master_plan_idx]->OkayToDiscard();
+for (discard_plan_idx = m_plans.size() - 1; discard_plan_idx >= 0;
+ discard_plan_idx--) {
+  auto plan_sp = m_plans[discard_plan_idx];
+  if (plan_sp->OkayToDiscard()) {
 break;
   }
+  // If the master plan doesn't want to get discarded, then we're done.
+  if (plan_sp->IsMasterPlan()) {
+return;
+  }
 }
 
-// If the master plan doesn't want to get discarded, then we're done.
-if (!discard)
-  return;
-
 // First pop all the dependent plans:
-for (int i = m_plans.size() - 1; i > master_plan_idx; i--) {
+for (int i = m_plans.size() - 1; i > discard_plan_idx; i--) {
   DiscardPlan();
 }
 
 // Now discard the master plan itself.
 // The bottom-most plan never gets discarded.  "OkayToDiscard" for it
 // means discard it's dependent plans, but not it...
-if (master_plan_idx > 0) {
+if (discard_plan_idx > 0) {
   DiscardPlan();
 }
   }
Index: lldb/source/Target/ThreadPlanPython.cpp
===
--- lldb/source/Target/ThreadPlanPython.cpp
+++ lldb/source/Target/ThreadPlanPython.cpp
@@ -32,7 +32,6 @@
   m_class_name(class_name), m_args_data(args_data), m_did_push(false),
   m_stop_others(false) {
   SetIsMasterPlan(true);
-  SetOkayToDiscard(true);
   SetPrivate(false);
 }
 
Index: lldb/source/Target/ThreadPlanCallUserExpression.cpp
===
--- lldb/source/Target/ThreadPlanCallUserExpression.cpp
+++ lldb/source/Target/ThreadPlanCallUserExpression.cpp
@@ -40,7 +40,6 @@
   // User expressions are generally "User generated" so we should set them up
   // to stop when done.
   SetIsMasterPlan(true);
-  SetOkayToDiscard(false);
 }
 
 ThreadPlanCallUserExpression::~ThreadPlanCallUserExpression() {}
Index: lldb/source/Target/ThreadPlanCallFunction.cpp
===
--- lldb/source/Target/ThreadPlanCallFunction.cpp
+++ lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -34,7 +34,6 @@
 Thread &thread, ABI *&abi, lldb::addr_t &start_load_addr,
 lldb::addr_t &function_load_addr) {
   SetIsMasterPlan(true);
-  SetOkayToDiscard(false);
   SetPrivate(true);
 
   ProcessSP process_sp(thread.GetProcess());
Index: lldb/source/Target/ThreadPlan.cpp
===
--- lldb/source/Target/ThreadPlan.cpp
+++ lldb/source/Target/ThreadPlan.cpp
@@ -26,7 +26,7 @@
   m_takes_iteration_count(false), m_could_not_resolve_hw_bp(false),
   m_thread(&thread), m_kind(kind), m_name(name), m_plan_complete_mutex(),
   m_cached_plan_explains_stop(eLazyBoolCalculate), m_plan_complete(false),
-  m_plan_private(false), m_okay_to_d

[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add MTE memory tag reading to lldb-server

2021-02-15 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added a comment.

  To get the review going consider further splitting up these patches into 
separate chunks containing, for example MemoryTagHandler API and related unit 
tests, GDB remote addons, ptrace work etc. Also add reference to GDB mailing 
list discussion where design of qMemTag packet has been discussed.

Will do. I erred on the side of larger patches so you could see the pieces 
connect in the same review.

  Soft suggestion to rename MemoryTagHandler, IMO it sounds more like a routine 
rather than a set of helpers used for manipulating memory tags. Also 
lldb/include/lldb/Target doesnt seem like a good place for helper functions as 
MemoryTagHandler is not used as Memory Tag container class. May be consider 
putting it under source/Plugins/Process/Utility

How about "MemoryTagInterface"? It's very similar to a Java "interface" class.

I think my reasoning for putting the header in lldb/Target was because it's 
used in the client and the server, and one of them tends not to include from 
the latter location. I'll double check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95601

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

[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add MTE memory tag reading to lldb-server

2021-02-15 Thread David Spickett via Phabricator via lldb-commits 
DavidSpickett added inline comments.



Comment at: lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp:1518
+  Status error = NativeProcessLinux::PtraceWrapper(
+  details->ptrace_read_req, GetID(),
+  reinterpret_cast(range.GetRangeBase()),

omjavaid wrote:
> Do you think future architectures will have any different 
> ptrace_read_req/ptrace_write_req than PTRACE_PEEKMTETAGS/PTRACE_POKEMTETAGS.
> 
> If not then there is is no advantage of putting them inside 
> MemoryTaggingDetails
Given that "MTE" is in the name, I would think yes. (though they might end up 
being aliases to the same number)
I also want to avoid having AArch64 only (at the current time) constants in 
generic code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95601

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

[Lldb-commits] [PATCH] D96558: [lldb] [Process/FreeBSD] Ensure that errors are always handled

2021-02-15 Thread Michał Górny via Phabricator via lldb-commits 
mgorny added a comment.

In D96558#2563105 , @labath wrote:

> This is precisely the reason we have the LLDB_LOG_ERROR macro :)

Updated.


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

https://reviews.llvm.org/D96558

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

[Lldb-commits] [PATCH] D96558: [lldb] [Process/FreeBSD] Ensure that errors are always handled

2021-02-15 Thread Michał Górny via Phabricator via lldb-commits 
mgorny updated this revision to Diff 323774.
mgorny retitled this revision from "[lldb] [Process/FreeBSDRemote] Ensure that 
errors are always handled" to "[lldb] [Process/FreeBSD] Ensure that errors are 
always handled".
mgorny edited the summary of this revision.

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

https://reviews.llvm.org/D96558

Files:
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp


Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -213,8 +213,9 @@
   llvm::Error error = t.CopyWatchpointsFrom(
   static_cast(*GetCurrentThread()));
   if (error) {
-LLDB_LOG(log, "failed to copy watchpoints to new thread {0}: {1}",
- info.pl_lwpid, llvm::toString(std::move(error)));
+LLDB_LOG_ERROR(log, std::move(error),
+   "failed to copy watchpoints to new thread {1}: {0}",
+   info.pl_lwpid);
 SetState(StateType::eStateInvalid);
 return;
   }


Index: lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
===
--- lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
+++ lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
@@ -213,8 +213,9 @@
   llvm::Error error = t.CopyWatchpointsFrom(
   static_cast(*GetCurrentThread()));
   if (error) {
-LLDB_LOG(log, "failed to copy watchpoints to new thread {0}: {1}",
- info.pl_lwpid, llvm::toString(std::move(error)));
+LLDB_LOG_ERROR(log, std::move(error),
+   "failed to copy watchpoints to new thread {1}: {0}",
+   info.pl_lwpid);
 SetState(StateType::eStateInvalid);
 return;
   }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D96548: [lldb] [Process/FreeBSDRemote] Introduce aarch64 hw break/watchpoint support [WIP]

2021-02-15 Thread Michał Górny via Phabricator via lldb-commits 
mgorny updated this revision to Diff 323779.
mgorny added a comment.

Switch to `LLDB_LOG_ERROR`.


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

https://reviews.llvm.org/D96548

Files:
  lldb/packages/Python/lldbsuite/test/dotest.py
  lldb/source/Plugins/Process/FreeBSD/NativeProcessFreeBSD.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.cpp
  lldb/source/Plugins/Process/FreeBSD/NativeRegisterContextFreeBSD_arm64.h
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.cpp
  lldb/source/Plugins/Process/Linux/NativeRegisterContextLinux_arm64.h
  lldb/source/Plugins/Process/Utility/CMakeLists.txt
  
lldb/source/Plugins/Process/Utility/NativeRegisterContextBreakWatchpoint_arm64.cpp
  
lldb/source/Plugins/Process/Utility/NativeRegisterContextBreakWatchpoint_arm64.h

Index: lldb/source/Plugins/Process/Utility/NativeRegisterContextBreakWatchpoint_arm64.h
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/NativeRegisterContextBreakWatchpoint_arm64.h
@@ -0,0 +1,77 @@
+//===-- NativeRegisterContextBreakWatchpoint_arm64.h *- 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
+//
+//===--===//
+
+#ifndef lldb_NativeRegisterContextBreakWatchpoint_arm64_h
+#define lldb_NativeRegisterContextBreakWatchpoint_arm64_h
+
+#include "Plugins/Process/Utility/NativeRegisterContextRegisterInfo.h"
+
+#include 
+
+namespace lldb_private {
+
+class NativeRegisterContextBreakWatchpoint_arm64
+: public virtual NativeRegisterContextRegisterInfo {
+public:
+  uint32_t NumSupportedHardwareBreakpoints() override;
+
+  uint32_t SetHardwareBreakpoint(lldb::addr_t addr, size_t size) override;
+
+  bool ClearHardwareBreakpoint(uint32_t hw_idx) override;
+
+  Status ClearAllHardwareBreakpoints() override;
+
+  Status GetHardwareBreakHitIndex(uint32_t &bp_index,
+  lldb::addr_t trap_addr) override;
+
+  uint32_t NumSupportedHardwareWatchpoints() override;
+
+  uint32_t SetHardwareWatchpoint(lldb::addr_t addr, size_t size,
+ uint32_t watch_flags) override;
+
+  bool ClearHardwareWatchpoint(uint32_t hw_index) override;
+
+  Status ClearAllHardwareWatchpoints() override;
+
+  Status GetWatchpointHitIndex(uint32_t &wp_index,
+   lldb::addr_t trap_addr) override;
+
+  lldb::addr_t GetWatchpointHitAddress(uint32_t wp_index) override;
+
+  lldb::addr_t GetWatchpointAddress(uint32_t wp_index) override;
+
+  uint32_t GetWatchpointSize(uint32_t wp_index);
+
+  bool WatchpointIsEnabled(uint32_t wp_index);
+
+  // Debug register type select
+  enum DREGType { eDREGTypeWATCH = 0, eDREGTypeBREAK };
+
+protected:
+  // Debug register info for hardware breakpoints and watchpoints management.
+  struct DREG {
+lldb::addr_t address;  // Breakpoint/watchpoint address value.
+lldb::addr_t hit_addr; // Address at which last watchpoint trigger exception
+   // occurred.
+lldb::addr_t real_addr; // Address value that should cause target to stop.
+uint32_t control;   // Breakpoint/watchpoint control value.
+  };
+
+  std::array m_hbr_regs; // hardware breakpoints
+  std::array m_hwp_regs; // hardware watchpoints
+
+  uint32_t m_max_hbp_supported;
+  uint32_t m_max_hwp_supported;
+
+  virtual llvm::Error ReadHardwareDebugInfo() = 0;
+  virtual llvm::Error WriteHardwareDebugRegs(DREGType hwbType) = 0;
+};
+
+} // namespace lldb_private
+
+#endif // #ifndef lldb_NativeRegisterContextBreakWatchpoint_arm64_h
Index: lldb/source/Plugins/Process/Utility/NativeRegisterContextBreakWatchpoint_arm64.cpp
===
--- /dev/null
+++ lldb/source/Plugins/Process/Utility/NativeRegisterContextBreakWatchpoint_arm64.cpp
@@ -0,0 +1,458 @@
+//===-- NativeRegisterContextBreakWatchpoint_arm64.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 "NativeRegisterContextBreakWatchpoint_arm64.h"
+
+#include "lldb/Utility/Log.h"
+#include "lldb/Utility/RegisterValue.h"
+
+using namespace lldb_private;
+
+uint32_t
+NativeRegisterContextBreakWatchpoint_arm64::NumSupportedHardwareBreakpoints() {
+  Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS));
+  llvm::Error error = ReadHardwareDebugInfo();
+  if (error) {
+LLDB_LOG_ERROR(log, std::move(error),
+   "failed to read debug registers: {0}");
+return 0;

[Lldb-commits] [PATCH] D96715: [lldb] Decouple IsMasterPlan and OkayToDiscard (NFC)

2021-02-15 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione added inline comments.



Comment at: 
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleThreadPlanStepThroughObjCTrampoline.cpp:72
+m_func_sp->SetIsMasterPlan(false);
 m_func_sp->SetOkayToDiscard(true);
 PushPlan(m_func_sp);

This is the only remaining `SetOkayToDiscard(true)` call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96715

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

[Lldb-commits] [PATCH] D96689: [lldb] Remove ThreadPlan::ShouldAutoContinue (NFC)

2021-02-15 Thread Dave Lee via Phabricator via lldb-commits 
kastiglione abandoned this revision.
kastiglione added a comment.

Even though the tests pass, I see that my logic was flawed. In theory, if 
autocontinue is true, and `MischiefManaged` returns true, then previous 
(parent) plans will be asked whether to `ShouldStop`. So the semantics of 
`ShouldAutoContinue` is whether a later plan wants to override should stop of 
an earlier plan.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96689

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

[Lldb-commits] [lldb] c2123a8 - [LLDB] Skip TestMultipleTargets.py on Arm/AArch64 Linux

2021-02-15 Thread Muhammad Omair Javaid via lldb-commits 

Author: Muhammad Omair Javaid
Date: 2021-02-16T01:28:16+05:00
New Revision: c2123a82cd0076c548d2b8c2acc14408681d7f89

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

LOG: [LLDB] Skip TestMultipleTargets.py on Arm/AArch64 Linux

TestMultipleTargets.py fails randomly on Arm/AArch64 Linux buildbot with
no reasonable clues. I am marking it skipped for avoiding LLDB buildbot
failures due to this test.

Added: 


Modified: 
lldb/test/API/api/multiple-targets/TestMultipleTargets.py

Removed: 




diff  --git a/lldb/test/API/api/multiple-targets/TestMultipleTargets.py 
b/lldb/test/API/api/multiple-targets/TestMultipleTargets.py
index 7931cc4be050..94b09c71ce2f 100644
--- a/lldb/test/API/api/multiple-targets/TestMultipleTargets.py
+++ b/lldb/test/API/api/multiple-targets/TestMultipleTargets.py
@@ -16,6 +16,7 @@ class TestMultipleTargets(TestBase):
 mydir = TestBase.compute_mydir(__file__)
 NO_DEBUG_INFO_TESTCASE = True
 
+@skipIf(oslist=["linux"], archs=["arm", "aarch64"])
 @skipIfNoSBHeaders
 @skipIfHostIncompatibleWithRemote
 @expectedFailureAll(



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

[Lldb-commits] [PATCH] D96688: [lldb] Minor refinements to ThreadPlan::RestoreThreadState (NFC)

2021-02-15 Thread Jim Ingham via Phabricator via lldb-commits 
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

It is possible that the stub could fail to restore the checkpointed state, 
though apparently it either never has or never has mattered because no one if 
setting or checking the bool return.  If this does end up failing we should 
plumb a Status through, since bool is too low information to be useful.  This 
is fine for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96688

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

[Lldb-commits] [PATCH] D95602: [lldb][AArch64] Add MTE memory tag reading for lldb

2021-02-15 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid added a comment.

This patch also looks quite good. Some minor nits inline and also move gdb* 
changes into a single patch with both client and server side code.




Comment at: lldb/include/lldb/Target/Process.h:1701
+  /// handler that can be used to maniupulate those memory tags.
+  /// Tags present in the addresses given are ignored.
+  ///

Can you explain this line a bit? What i understood we dont include start and 
end address in tag range. right?



Comment at: lldb/include/lldb/Target/Process.h:1724
+  /// \param[in] addr
+  /// Start of memory range to tags for.
+  ///

I guess you meant to say read tags for?



Comment at: lldb/include/lldb/Target/Process.h:2749
 
+  /// Check whether the remote supports memory tagging.
+  ///

By remote you mean gdb-remote process? This probably will be generic routine 
used by other platforms.
in context of lldb we dont have remote rather platforms where gdb-remote is a 
type of platform.



Comment at: lldb/include/lldb/Target/Process.h:2773
+  virtual llvm::Expected>
+  DoReadMemoryTags(lldb::addr_t addr, size_t len, int32_t type) {
+return llvm::createStringError(llvm::inconvertibleErrorCode(),

I guess if there are no restrictions by specs then we should rethink use of 
int32_t for type may be uint32_t if possible.



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:1272
 
-def isAArch64SVE(self):
+def hasLinuxCPUInfoFeature(self, feature):
 triple = self.dbg.GetSelectedPlatform().GetTriple()

This change look good and can be committed outside this patch.



Comment at: lldb/packages/Python/lldbsuite/test/lldbtest.py:1272
 
-def isAArch64SVE(self):
+def hasLinuxCPUInfoFeature(self, feature):
 triple = self.dbg.GetSelectedPlatform().GetTriple()

omjavaid wrote:
> This change look good and can be committed outside this patch.
This change looks fine commit it separately.



Comment at: lldb/source/Target/Process.cpp:6031
+  const MemoryTagHandler *tag_handler =
+  arch ? arch->GetMemoryTagHandler() : nullptr;
+  if (!arch || !tag_handler) {

'arch' may be nullptr so call to GetMemoryTagHandler is not safe.



Comment at: lldb/source/Target/Process.cpp:6058
+  MemoryRegionInfo::RangeType tag_range(untagged_addr, len);
+  tag_range = tag_handler->AlignToGranules(tag_range);
+

Can there be multiple 'granules' defined for an implementation ? if not this 
func may be renamed (AlignToGranule) to reflect that 



Comment at: lldb/test/API/linux/aarch64/mte_tag_read/main.c:44
+  // Tag the original pointer with 9
+  buf = __arm_mte_create_random_tag(buf, ~(1 << 9));
+  // A different tag so that buf_alt_tag > buf if you don't handle the tag

May be add a line or two of comment about mte intrinsics for any future readers 
of these test code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95602

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

[Lldb-commits] [PATCH] D96666: [lldb] Remove unused ThreadPlan tracer utilities (NFC)

2021-02-15 Thread Jim Ingham via Phabricator via lldb-commits 
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

At some point I had the idea that there would be a whole plugin architecture 
where you could add observers on all the stops & steps the lldb's execution 
control did.  But for lldb command line & SB API usages, the scripted thread 
plans add a more natural way to do this.  We probably shouldn't take the 
tracers out altogether.  I used them a couple of times (though admittedly 
pretty far back in the day) for debugging stepping issues.  But we don't need 
the higher level bindings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D9

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

[Lldb-commits] [PATCH] D96687: [lldb] Lower GetRealStopInfo into ThreadPlanCallFunction (NFC)

2021-02-15 Thread Jim Ingham via Phabricator via lldb-commits 
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.

It does seem odd to peek behind the back of a thread plan to figure out how to 
proceed, so making this special to the UserExpression thread plans, which are 
special in a bunch of ways, isn't a bad idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96687

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

[Lldb-commits] [PATCH] D95601: [lldb][AArch64] Add MTE memory tag reading to lldb-server

2021-02-15 Thread Muhammad Omair Javaid via Phabricator via lldb-commits 
omjavaid added a comment.

In D95601#2563613 , @DavidSpickett 
wrote:

>   To get the review going consider further splitting up these patches into 
> separate chunks containing, for example MemoryTagHandler API and related unit 
> tests, GDB remote addons, ptrace work etc. Also add reference to GDB mailing 
> list discussion where design of qMemTag packet has been discussed.
>
> Will do. I erred on the side of larger patches so you could see the pieces 
> connect in the same review.
>
>   Soft suggestion to rename MemoryTagHandler, IMO it sounds more like a 
> routine rather than a set of helpers used for manipulating memory tags. Also 
> lldb/include/lldb/Target doesnt seem like a good place for helper functions 
> as MemoryTagHandler is not used as Memory Tag container class. May be 
> consider putting it under source/Plugins/Process/Utility
>
> How about "MemoryTagInterface"? It's very similar to a Java "interface" class.

MemoryTagHandler class does indeed look like an interface class  example of an 
interface class in LLDB code is RegisterInfosInterface which serves as an 
interface of varies arch dependent register infos. But MemoryTagHandler serves 
more like a provider or manager rather than a container

May be you can use MemoryTagManager or MemoryTagUtils but then this just a soft 
comment. You can keep the existing name or change it to interface or anything 
you feel appropriate.

> I think my reasoning for putting the header in lldb/Target was because it's 
> used in the client and the server, and one of them tends not to include from 
> the latter location. I'll double check.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95601

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

[Lldb-commits] [lldb] 65d91b4 - [lldb] Minor refinements to ThreadPlan::RestoreThreadState (NFC)

2021-02-15 Thread Dave Lee via lldb-commits 

Author: Dave Lee
Date: 2021-02-15T17:57:13-08:00
New Revision: 65d91b40ae1d273586a8265d7cf598f0e56e17a3

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

LOG: [lldb] Minor refinements to ThreadPlan::RestoreThreadState (NFC)

Correct `RestoreThreadState` to a `void` return type. Also, update the 
signature of its
callee, `Thread::RestoreThreadStateFromCheckpoint`, by updating it to a `void` 
return
type, and making it non-`virtual`.

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

Added: 


Modified: 
lldb/include/lldb/Target/Thread.h
lldb/include/lldb/Target/ThreadPlan.h
lldb/include/lldb/Target/ThreadPlanCallFunction.h
lldb/source/Target/Thread.cpp
lldb/source/Target/ThreadPlanCallFunction.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index 4b148063ec6e..04fa210fd4e4 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -1050,8 +1050,7 @@ class Thread : public 
std::enable_shared_from_this,
   virtual bool
   RestoreRegisterStateFromCheckpoint(ThreadStateCheckpoint &saved_state);
 
-  virtual bool
-  RestoreThreadStateFromCheckpoint(ThreadStateCheckpoint &saved_state);
+  void RestoreThreadStateFromCheckpoint(ThreadStateCheckpoint &saved_state);
 
   void EnableTracer(bool value, bool single_step);
 

diff  --git a/lldb/include/lldb/Target/ThreadPlan.h 
b/lldb/include/lldb/Target/ThreadPlan.h
index 17bbfc988f56..74d49ceb0e0a 100644
--- a/lldb/include/lldb/Target/ThreadPlan.h
+++ b/lldb/include/lldb/Target/ThreadPlan.h
@@ -464,10 +464,7 @@ class ThreadPlan : public 
std::enable_shared_from_this,
   // to restore the state when it is done.  This will do that job. This is
   // mostly useful for artificial plans like CallFunction plans.
 
-  virtual bool RestoreThreadState() {
-// Nothing to do in general.
-return true;
-  }
+  virtual void RestoreThreadState() {}
 
   virtual bool IsVirtualStep() { return false; }
 

diff  --git a/lldb/include/lldb/Target/ThreadPlanCallFunction.h 
b/lldb/include/lldb/Target/ThreadPlanCallFunction.h
index 5b432e5e604a..8874c6c6c729 100644
--- a/lldb/include/lldb/Target/ThreadPlanCallFunction.h
+++ b/lldb/include/lldb/Target/ThreadPlanCallFunction.h
@@ -90,7 +90,7 @@ class ThreadPlanCallFunction : public ThreadPlan {
 
   lldb::addr_t GetStopAddress() { return m_stop_address; }
 
-  bool RestoreThreadState() override;
+  void RestoreThreadState() override;
 
   void ThreadDestroyed() override { m_takedown_done = true; }
 

diff  --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 660d5f8908b3..1fb19aea4fd9 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -530,7 +530,7 @@ bool Thread::RestoreRegisterStateFromCheckpoint(
   return false;
 }
 
-bool Thread::RestoreThreadStateFromCheckpoint(
+void Thread::RestoreThreadStateFromCheckpoint(
 ThreadStateCheckpoint &saved_state) {
   if (saved_state.stop_info_sp)
 saved_state.stop_info_sp->MakeStopInfoValid();
@@ -539,7 +539,6 @@ bool Thread::RestoreThreadStateFromCheckpoint(
   saved_state.current_inlined_depth);
   GetPlans().RestoreCompletedPlanCheckpoint(
   saved_state.m_completed_plan_checkpoint);
-  return true;
 }
 
 StateType Thread::GetState() const {

diff  --git a/lldb/source/Target/ThreadPlanCallFunction.cpp 
b/lldb/source/Target/ThreadPlanCallFunction.cpp
index f525173f8a51..3699a507d058 100644
--- a/lldb/source/Target/ThreadPlanCallFunction.cpp
+++ b/lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -454,8 +454,8 @@ void ThreadPlanCallFunction::SetStopOthers(bool new_value) {
   m_subplan_sp->SetStopOthers(new_value);
 }
 
-bool ThreadPlanCallFunction::RestoreThreadState() {
-  return GetThread().RestoreThreadStateFromCheckpoint(m_stored_thread_state);
+void ThreadPlanCallFunction::RestoreThreadState() {
+  GetThread().RestoreThreadStateFromCheckpoint(m_stored_thread_state);
 }
 
 void ThreadPlanCallFunction::SetReturnValue() {



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

[Lldb-commits] [PATCH] D96688: [lldb] Minor refinements to ThreadPlan::RestoreThreadState (NFC)

2021-02-15 Thread Dave Lee 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 rG65d91b40ae1d: [lldb] Minor refinements to 
ThreadPlan::RestoreThreadState (NFC) (authored by kastiglione).

Changed prior to commit:
  https://reviews.llvm.org/D96688?vs=323673&id=323855#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96688

Files:
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanCallFunction.h
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadPlanCallFunction.cpp


Index: lldb/source/Target/ThreadPlanCallFunction.cpp
===
--- lldb/source/Target/ThreadPlanCallFunction.cpp
+++ lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -454,8 +454,8 @@
   m_subplan_sp->SetStopOthers(new_value);
 }
 
-bool ThreadPlanCallFunction::RestoreThreadState() {
-  return GetThread().RestoreThreadStateFromCheckpoint(m_stored_thread_state);
+void ThreadPlanCallFunction::RestoreThreadState() {
+  GetThread().RestoreThreadStateFromCheckpoint(m_stored_thread_state);
 }
 
 void ThreadPlanCallFunction::SetReturnValue() {
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -530,7 +530,7 @@
   return false;
 }
 
-bool Thread::RestoreThreadStateFromCheckpoint(
+void Thread::RestoreThreadStateFromCheckpoint(
 ThreadStateCheckpoint &saved_state) {
   if (saved_state.stop_info_sp)
 saved_state.stop_info_sp->MakeStopInfoValid();
@@ -539,7 +539,6 @@
   saved_state.current_inlined_depth);
   GetPlans().RestoreCompletedPlanCheckpoint(
   saved_state.m_completed_plan_checkpoint);
-  return true;
 }
 
 StateType Thread::GetState() const {
Index: lldb/include/lldb/Target/ThreadPlanCallFunction.h
===
--- lldb/include/lldb/Target/ThreadPlanCallFunction.h
+++ lldb/include/lldb/Target/ThreadPlanCallFunction.h
@@ -90,7 +90,7 @@
 
   lldb::addr_t GetStopAddress() { return m_stop_address; }
 
-  bool RestoreThreadState() override;
+  void RestoreThreadState() override;
 
   void ThreadDestroyed() override { m_takedown_done = true; }
 
Index: lldb/include/lldb/Target/ThreadPlan.h
===
--- lldb/include/lldb/Target/ThreadPlan.h
+++ lldb/include/lldb/Target/ThreadPlan.h
@@ -464,10 +464,7 @@
   // to restore the state when it is done.  This will do that job. This is
   // mostly useful for artificial plans like CallFunction plans.
 
-  virtual bool RestoreThreadState() {
-// Nothing to do in general.
-return true;
-  }
+  virtual void RestoreThreadState() {}
 
   virtual bool IsVirtualStep() { return false; }
 
Index: lldb/include/lldb/Target/Thread.h
===
--- lldb/include/lldb/Target/Thread.h
+++ lldb/include/lldb/Target/Thread.h
@@ -1050,8 +1050,7 @@
   virtual bool
   RestoreRegisterStateFromCheckpoint(ThreadStateCheckpoint &saved_state);
 
-  virtual bool
-  RestoreThreadStateFromCheckpoint(ThreadStateCheckpoint &saved_state);
+  void RestoreThreadStateFromCheckpoint(ThreadStateCheckpoint &saved_state);
 
   void EnableTracer(bool value, bool single_step);
 


Index: lldb/source/Target/ThreadPlanCallFunction.cpp
===
--- lldb/source/Target/ThreadPlanCallFunction.cpp
+++ lldb/source/Target/ThreadPlanCallFunction.cpp
@@ -454,8 +454,8 @@
   m_subplan_sp->SetStopOthers(new_value);
 }
 
-bool ThreadPlanCallFunction::RestoreThreadState() {
-  return GetThread().RestoreThreadStateFromCheckpoint(m_stored_thread_state);
+void ThreadPlanCallFunction::RestoreThreadState() {
+  GetThread().RestoreThreadStateFromCheckpoint(m_stored_thread_state);
 }
 
 void ThreadPlanCallFunction::SetReturnValue() {
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -530,7 +530,7 @@
   return false;
 }
 
-bool Thread::RestoreThreadStateFromCheckpoint(
+void Thread::RestoreThreadStateFromCheckpoint(
 ThreadStateCheckpoint &saved_state) {
   if (saved_state.stop_info_sp)
 saved_state.stop_info_sp->MakeStopInfoValid();
@@ -539,7 +539,6 @@
   saved_state.current_inlined_depth);
   GetPlans().RestoreCompletedPlanCheckpoint(
   saved_state.m_completed_plan_checkpoint);
-  return true;
 }
 
 StateType Thread::GetState() const {
Index: lldb/include/lldb/Target/ThreadPlanCallFunction.h
===
--- lldb/include/lldb/Target/ThreadPlanCallFunction.h
+++ lldb/include/lldb/Target/ThreadPlanCallFunction.h
@@ -90,7 +90,7 @@

[Lldb-commits] [lldb] 2a2464e - [lldb] Lower GetRealStopInfo into ThreadPlanCallFunction (NFC)

2021-02-15 Thread Dave Lee via lldb-commits 

Author: Dave Lee
Date: 2021-02-15T17:59:24-08:00
New Revision: 2a2464eb7651dc89290e9e3bf6928c99453bed99

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

LOG: [lldb] Lower GetRealStopInfo into ThreadPlanCallFunction (NFC)

`GetRealStopInfo` has only one call site, and in that call site a reference to 
the
concrete thread plan is available (`ThreadPlanCallUserExpression`), from which
`GetRealStopInfo` can be called.

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

Added: 


Modified: 
lldb/include/lldb/Target/ThreadPlan.h
lldb/include/lldb/Target/ThreadPlanCallFunction.h
lldb/source/Expression/LLVMUserExpression.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/ThreadPlan.h 
b/lldb/include/lldb/Target/ThreadPlan.h
index 74d49ceb0e0a..caa6c5fe63aa 100644
--- a/lldb/include/lldb/Target/ThreadPlan.h
+++ b/lldb/include/lldb/Target/ThreadPlan.h
@@ -431,15 +431,6 @@ class ThreadPlan : public 
std::enable_shared_from_this,
   m_tracer_sp->Log();
   }
 
-  // Some thread plans hide away the actual stop info which caused any
-  // particular stop.  For instance the ThreadPlanCallFunction restores the
-  // original stop reason so that stopping and calling a few functions won't
-  // lose the history of the run. This call can be implemented to get you back
-  // to the real stop info.
-  virtual lldb::StopInfoSP GetRealStopInfo() { 
-return GetThread().GetStopInfo();
-  }
-
   // If the completion of the thread plan stepped out of a function, the return
   // value of the function might have been captured by the thread plan
   // (currently only ThreadPlanStepOut does this.) If so, the ReturnValueObject

diff  --git a/lldb/include/lldb/Target/ThreadPlanCallFunction.h 
b/lldb/include/lldb/Target/ThreadPlanCallFunction.h
index 8874c6c6c729..24c5736f44c3 100644
--- a/lldb/include/lldb/Target/ThreadPlanCallFunction.h
+++ b/lldb/include/lldb/Target/ThreadPlanCallFunction.h
@@ -81,7 +81,7 @@ class ThreadPlanCallFunction : public ThreadPlan {
   // stop reason. But if something bad goes wrong, it is nice to be able to
   // tell the user what really happened.
 
-  lldb::StopInfoSP GetRealStopInfo() override {
+  virtual lldb::StopInfoSP GetRealStopInfo() {
 if (m_real_stop_info_sp)
   return m_real_stop_info_sp;
 else

diff  --git a/lldb/source/Expression/LLVMUserExpression.cpp 
b/lldb/source/Expression/LLVMUserExpression.cpp
index 187b427e66aa..527dff8bf60f 100644
--- a/lldb/source/Expression/LLVMUserExpression.cpp
+++ b/lldb/source/Expression/LLVMUserExpression.cpp
@@ -188,9 +188,8 @@ LLVMUserExpression::DoExecute(DiagnosticManager 
&diagnostic_manager,
 execution_result == lldb::eExpressionHitBreakpoint) {
   const char *error_desc = nullptr;
 
-  if (call_plan_sp) {
-lldb::StopInfoSP real_stop_info_sp = call_plan_sp->GetRealStopInfo();
-if (real_stop_info_sp)
+  if (user_expression_plan) {
+if (auto real_stop_info_sp = user_expression_plan->GetRealStopInfo())
   error_desc = real_stop_info_sp->GetDescription();
   }
   if (error_desc)



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

[Lldb-commits] [lldb] 22f0aa0 - [lldb] Remove unused ThreadPlan tracer utilities (NFC)

2021-02-15 Thread Dave Lee via lldb-commits 

Author: Dave Lee
Date: 2021-02-15T17:59:25-08:00
New Revision: 22f0aa0d9ebddb004cbf2d0e7c8c015422351d2b

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

LOG: [lldb] Remove unused ThreadPlan tracer utilities (NFC)

Delete unused `EnableTracer()` and `SetTracer()` functions on `Thread`. By 
deleting
these, their `ThreadPlan` counterparts also become unused.

Then, by deleting `ThreadPlanStack::EnableTracer`, `EnableSingleStep` becomes 
unused.
With no more callers to `EnableSingleStep`, the value `m_single_step` is always 
true and
can be removed as well.

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

Added: 


Modified: 
lldb/include/lldb/Target/Thread.h
lldb/include/lldb/Target/ThreadPlanStack.h
lldb/include/lldb/Target/ThreadPlanTracer.h
lldb/source/Target/Thread.cpp
lldb/source/Target/ThreadPlan.cpp
lldb/source/Target/ThreadPlanStack.cpp
lldb/source/Target/ThreadPlanTracer.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index 04fa210fd4e4..b797e4d1ee97 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -1052,10 +1052,6 @@ class Thread : public 
std::enable_shared_from_this,
 
   void RestoreThreadStateFromCheckpoint(ThreadStateCheckpoint &saved_state);
 
-  void EnableTracer(bool value, bool single_step);
-
-  void SetTracer(lldb::ThreadPlanTracerSP &tracer_sp);
-
   // Get the thread index ID. The index ID that is guaranteed to not be re-used
   // by a process. They start at 1 and increase with each new thread. This
   // allows easy command line access by a unique ID that is easier to type than

diff  --git a/lldb/include/lldb/Target/ThreadPlanStack.h 
b/lldb/include/lldb/Target/ThreadPlanStack.h
index 7b2817b2e8fd..a5fcf00242b3 100644
--- a/lldb/include/lldb/Target/ThreadPlanStack.h
+++ b/lldb/include/lldb/Target/ThreadPlanStack.h
@@ -48,10 +48,6 @@ class ThreadPlanStack {
 
   void ThreadDestroyed(Thread *thread);
 
-  void EnableTracer(bool value, bool single_stepping);
-
-  void SetTracer(lldb::ThreadPlanTracerSP &tracer_sp);
-
   void PushPlan(lldb::ThreadPlanSP new_plan_sp);
 
   lldb::ThreadPlanSP PopPlan();

diff  --git a/lldb/include/lldb/Target/ThreadPlanTracer.h 
b/lldb/include/lldb/Target/ThreadPlanTracer.h
index 677a2c0dd93c..a6fd2f031dc2 100644
--- a/lldb/include/lldb/Target/ThreadPlanTracer.h
+++ b/lldb/include/lldb/Target/ThreadPlanTracer.h
@@ -50,14 +50,6 @@ class ThreadPlanTracer {
 
   bool TracingEnabled() { return m_enabled; }
 
-  bool EnableSingleStep(bool value) {
-bool old_value = m_single_step;
-m_single_step = value;
-return old_value;
-  }
-
-  bool SingleStepEnabled() { return m_single_step; }
-  
   Thread &GetThread();
 
 protected:
@@ -71,7 +63,6 @@ class ThreadPlanTracer {
 private:
   bool TracerExplainsStop();
 
-  bool m_single_step;
   bool m_enabled;
   lldb::StreamSP m_stream_sp;
   Thread *m_thread;

diff  --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 1fb19aea4fd9..6cbad2376201 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -1180,14 +1180,6 @@ Status Thread::QueueThreadPlan(ThreadPlanSP 
&thread_plan_sp,
   return status;
 }
 
-void Thread::EnableTracer(bool value, bool single_stepping) {
-  GetPlans().EnableTracer(value, single_stepping);
-}
-
-void Thread::SetTracer(lldb::ThreadPlanTracerSP &tracer_sp) {
-  GetPlans().SetTracer(tracer_sp);
-}
-
 bool Thread::DiscardUserThreadPlansUpToIndex(uint32_t plan_index) {
   // Count the user thread plans from the back end to get the number of the one
   // we want to discard:

diff  --git a/lldb/source/Target/ThreadPlan.cpp 
b/lldb/source/Target/ThreadPlan.cpp
index df59da157319..84fd6b2107b6 100644
--- a/lldb/source/Target/ThreadPlan.cpp
+++ b/lldb/source/Target/ThreadPlan.cpp
@@ -156,8 +156,7 @@ bool ThreadPlan::OkayToDiscard() {
 }
 
 lldb::StateType ThreadPlan::RunState() {
-  if (m_tracer_sp && m_tracer_sp->TracingEnabled() &&
-  m_tracer_sp->SingleStepEnabled())
+  if (m_tracer_sp && m_tracer_sp->TracingEnabled())
 return eStateStepping;
   else
 return GetPlanRunState();

diff  --git a/lldb/source/Target/ThreadPlanStack.cpp 
b/lldb/source/Target/ThreadPlanStack.cpp
index ad37669c31fd..5a4ca8812136 100644
--- a/lldb/source/Target/ThreadPlanStack.cpp
+++ b/lldb/source/Target/ThreadPlanStack.cpp
@@ -124,20 +124,6 @@ void ThreadPlanStack::ThreadDestroyed(Thread *thread) {
   }
 }
 
-void ThreadPlanStack::EnableTracer(bool value, bool single_stepping) {
-  for (ThreadPlanSP plan : m_plans) {
-if (plan->GetThreadPlanTracer()) {
-  plan->GetThreadPlanTracer()->EnableTracing(value);
-  plan->GetThreadPlanTracer()->EnableSingleStep(single_stepping);
-}
-  }
-}
-

[Lldb-commits] [PATCH] D96687: [lldb] Lower GetRealStopInfo into ThreadPlanCallFunction (NFC)

2021-02-15 Thread Dave Lee 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 rG2a2464eb7651: [lldb] Lower GetRealStopInfo into 
ThreadPlanCallFunction (NFC) (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96687

Files:
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanCallFunction.h
  lldb/source/Expression/LLVMUserExpression.cpp


Index: lldb/source/Expression/LLVMUserExpression.cpp
===
--- lldb/source/Expression/LLVMUserExpression.cpp
+++ lldb/source/Expression/LLVMUserExpression.cpp
@@ -188,9 +188,8 @@
 execution_result == lldb::eExpressionHitBreakpoint) {
   const char *error_desc = nullptr;
 
-  if (call_plan_sp) {
-lldb::StopInfoSP real_stop_info_sp = call_plan_sp->GetRealStopInfo();
-if (real_stop_info_sp)
+  if (user_expression_plan) {
+if (auto real_stop_info_sp = user_expression_plan->GetRealStopInfo())
   error_desc = real_stop_info_sp->GetDescription();
   }
   if (error_desc)
Index: lldb/include/lldb/Target/ThreadPlanCallFunction.h
===
--- lldb/include/lldb/Target/ThreadPlanCallFunction.h
+++ lldb/include/lldb/Target/ThreadPlanCallFunction.h
@@ -81,7 +81,7 @@
   // stop reason. But if something bad goes wrong, it is nice to be able to
   // tell the user what really happened.
 
-  lldb::StopInfoSP GetRealStopInfo() override {
+  virtual lldb::StopInfoSP GetRealStopInfo() {
 if (m_real_stop_info_sp)
   return m_real_stop_info_sp;
 else
Index: lldb/include/lldb/Target/ThreadPlan.h
===
--- lldb/include/lldb/Target/ThreadPlan.h
+++ lldb/include/lldb/Target/ThreadPlan.h
@@ -431,15 +431,6 @@
   m_tracer_sp->Log();
   }
 
-  // Some thread plans hide away the actual stop info which caused any
-  // particular stop.  For instance the ThreadPlanCallFunction restores the
-  // original stop reason so that stopping and calling a few functions won't
-  // lose the history of the run. This call can be implemented to get you back
-  // to the real stop info.
-  virtual lldb::StopInfoSP GetRealStopInfo() { 
-return GetThread().GetStopInfo();
-  }
-
   // If the completion of the thread plan stepped out of a function, the return
   // value of the function might have been captured by the thread plan
   // (currently only ThreadPlanStepOut does this.) If so, the ReturnValueObject


Index: lldb/source/Expression/LLVMUserExpression.cpp
===
--- lldb/source/Expression/LLVMUserExpression.cpp
+++ lldb/source/Expression/LLVMUserExpression.cpp
@@ -188,9 +188,8 @@
 execution_result == lldb::eExpressionHitBreakpoint) {
   const char *error_desc = nullptr;
 
-  if (call_plan_sp) {
-lldb::StopInfoSP real_stop_info_sp = call_plan_sp->GetRealStopInfo();
-if (real_stop_info_sp)
+  if (user_expression_plan) {
+if (auto real_stop_info_sp = user_expression_plan->GetRealStopInfo())
   error_desc = real_stop_info_sp->GetDescription();
   }
   if (error_desc)
Index: lldb/include/lldb/Target/ThreadPlanCallFunction.h
===
--- lldb/include/lldb/Target/ThreadPlanCallFunction.h
+++ lldb/include/lldb/Target/ThreadPlanCallFunction.h
@@ -81,7 +81,7 @@
   // stop reason. But if something bad goes wrong, it is nice to be able to
   // tell the user what really happened.
 
-  lldb::StopInfoSP GetRealStopInfo() override {
+  virtual lldb::StopInfoSP GetRealStopInfo() {
 if (m_real_stop_info_sp)
   return m_real_stop_info_sp;
 else
Index: lldb/include/lldb/Target/ThreadPlan.h
===
--- lldb/include/lldb/Target/ThreadPlan.h
+++ lldb/include/lldb/Target/ThreadPlan.h
@@ -431,15 +431,6 @@
   m_tracer_sp->Log();
   }
 
-  // Some thread plans hide away the actual stop info which caused any
-  // particular stop.  For instance the ThreadPlanCallFunction restores the
-  // original stop reason so that stopping and calling a few functions won't
-  // lose the history of the run. This call can be implemented to get you back
-  // to the real stop info.
-  virtual lldb::StopInfoSP GetRealStopInfo() { 
-return GetThread().GetStopInfo();
-  }
-
   // If the completion of the thread plan stepped out of a function, the return
   // value of the function might have been captured by the thread plan
   // (currently only ThreadPlanStepOut does this.) If so, the ReturnValueObject
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo

[Lldb-commits] [PATCH] D96666: [lldb] Remove unused ThreadPlan tracer utilities (NFC)

2021-02-15 Thread Dave Lee 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 rG22f0aa0d9ebd: [lldb] Remove unused ThreadPlan tracer 
utilities (NFC) (authored by kastiglione).

Changed prior to commit:
  https://reviews.llvm.org/D9?vs=323624&id=323857#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D9

Files:
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/Target/ThreadPlanStack.h
  lldb/include/lldb/Target/ThreadPlanTracer.h
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadPlan.cpp
  lldb/source/Target/ThreadPlanStack.cpp
  lldb/source/Target/ThreadPlanTracer.cpp

Index: lldb/source/Target/ThreadPlanTracer.cpp
===
--- lldb/source/Target/ThreadPlanTracer.cpp
+++ lldb/source/Target/ThreadPlanTracer.cpp
@@ -35,11 +35,11 @@
 
 ThreadPlanTracer::ThreadPlanTracer(Thread &thread, lldb::StreamSP &stream_sp)
 : m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
-  m_single_step(true), m_enabled(false), m_stream_sp(stream_sp) {}
+  m_enabled(false), m_stream_sp(stream_sp) {}
 
 ThreadPlanTracer::ThreadPlanTracer(Thread &thread)
 : m_process(*thread.GetProcess().get()), m_tid(thread.GetID()),
-  m_single_step(true), m_enabled(false), m_stream_sp() {}
+  m_enabled(false), m_stream_sp() {}
 
 Stream *ThreadPlanTracer::GetLogStream() {
   if (m_stream_sp)
@@ -75,7 +75,7 @@
 }
 
 bool ThreadPlanTracer::TracerExplainsStop() {
-  if (m_enabled && m_single_step) {
+  if (m_enabled) {
 lldb::StopInfoSP stop_info = GetThread().GetStopInfo();
 return (stop_info->GetStopReason() == eStopReasonTrace);
   } else
Index: lldb/source/Target/ThreadPlanStack.cpp
===
--- lldb/source/Target/ThreadPlanStack.cpp
+++ lldb/source/Target/ThreadPlanStack.cpp
@@ -124,20 +124,6 @@
   }
 }
 
-void ThreadPlanStack::EnableTracer(bool value, bool single_stepping) {
-  for (ThreadPlanSP plan : m_plans) {
-if (plan->GetThreadPlanTracer()) {
-  plan->GetThreadPlanTracer()->EnableTracing(value);
-  plan->GetThreadPlanTracer()->EnableSingleStep(single_stepping);
-}
-  }
-}
-
-void ThreadPlanStack::SetTracer(lldb::ThreadPlanTracerSP &tracer_sp) {
-  for (ThreadPlanSP plan : m_plans)
-plan->SetThreadPlanTracer(tracer_sp);
-}
-
 void ThreadPlanStack::PushPlan(lldb::ThreadPlanSP new_plan_sp) {
   // If the thread plan doesn't already have a tracer, give it its parent's
   // tracer:
Index: lldb/source/Target/ThreadPlan.cpp
===
--- lldb/source/Target/ThreadPlan.cpp
+++ lldb/source/Target/ThreadPlan.cpp
@@ -156,8 +156,7 @@
 }
 
 lldb::StateType ThreadPlan::RunState() {
-  if (m_tracer_sp && m_tracer_sp->TracingEnabled() &&
-  m_tracer_sp->SingleStepEnabled())
+  if (m_tracer_sp && m_tracer_sp->TracingEnabled())
 return eStateStepping;
   else
 return GetPlanRunState();
Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -1180,14 +1180,6 @@
   return status;
 }
 
-void Thread::EnableTracer(bool value, bool single_stepping) {
-  GetPlans().EnableTracer(value, single_stepping);
-}
-
-void Thread::SetTracer(lldb::ThreadPlanTracerSP &tracer_sp) {
-  GetPlans().SetTracer(tracer_sp);
-}
-
 bool Thread::DiscardUserThreadPlansUpToIndex(uint32_t plan_index) {
   // Count the user thread plans from the back end to get the number of the one
   // we want to discard:
Index: lldb/include/lldb/Target/ThreadPlanTracer.h
===
--- lldb/include/lldb/Target/ThreadPlanTracer.h
+++ lldb/include/lldb/Target/ThreadPlanTracer.h
@@ -50,14 +50,6 @@
 
   bool TracingEnabled() { return m_enabled; }
 
-  bool EnableSingleStep(bool value) {
-bool old_value = m_single_step;
-m_single_step = value;
-return old_value;
-  }
-
-  bool SingleStepEnabled() { return m_single_step; }
-  
   Thread &GetThread();
 
 protected:
@@ -71,7 +63,6 @@
 private:
   bool TracerExplainsStop();
 
-  bool m_single_step;
   bool m_enabled;
   lldb::StreamSP m_stream_sp;
   Thread *m_thread;
Index: lldb/include/lldb/Target/ThreadPlanStack.h
===
--- lldb/include/lldb/Target/ThreadPlanStack.h
+++ lldb/include/lldb/Target/ThreadPlanStack.h
@@ -48,10 +48,6 @@
 
   void ThreadDestroyed(Thread *thread);
 
-  void EnableTracer(bool value, bool single_stepping);
-
-  void SetTracer(lldb::ThreadPlanTracerSP &tracer_sp);
-
   void PushPlan(lldb::ThreadPlanSP new_plan_sp);
 
   lldb::ThreadPlanSP PopPlan();
Index: lldb/include/lldb/Target/Thread.h
===
--- lldb/i

[Lldb-commits] [lldb] 4946f37 - [lldb] Remove very old misleading comment (NFC)

2021-02-15 Thread Dave Lee via lldb-commits 

Author: Dave Lee
Date: 2021-02-15T18:14:57-08:00
New Revision: 4946f37d842fe7bca1b4a30962e7b6acdd7a4a9d

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

LOG: [lldb] Remove very old misleading comment (NFC)

Added: 


Modified: 
lldb/source/Target/ThreadPlanStepThrough.cpp

Removed: 




diff  --git a/lldb/source/Target/ThreadPlanStepThrough.cpp 
b/lldb/source/Target/ThreadPlanStepThrough.cpp
index 06b626935aba..6fc031fb 100644
--- a/lldb/source/Target/ThreadPlanStepThrough.cpp
+++ b/lldb/source/Target/ThreadPlanStepThrough.cpp
@@ -22,7 +22,6 @@ using namespace lldb_private;
 // ThreadPlanStepThrough: If the current instruction is a trampoline, step
 // through it If it is the beginning of the prologue of a function, step
 // through that as well.
-// FIXME: At present only handles DYLD trampolines.
 
 ThreadPlanStepThrough::ThreadPlanStepThrough(Thread &thread,
  StackID &m_stack_id,



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

[Lldb-commits] [lldb] 7181104 - [debugserver] Correctly pass argv[0] to execl

2021-02-15 Thread Jonas Devlieghere via lldb-commits 

Author: Jonas Devlieghere
Date: 2021-02-15T18:42:21-08:00
New Revision: 71811048c0061c2ecf9aa6ce14c41f3f6ecf8aa9

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

LOG: [debugserver] Correctly pass argv[0] to execl

The execl function takes both the executable and argv[0].

Added: 


Modified: 
lldb/tools/debugserver/source/DNB.cpp

Removed: 




diff  --git a/lldb/tools/debugserver/source/DNB.cpp 
b/lldb/tools/debugserver/source/DNB.cpp
index a69fc33fc9b0..7048bec4832a 100644
--- a/lldb/tools/debugserver/source/DNB.cpp
+++ b/lldb/tools/debugserver/source/DNB.cpp
@@ -468,8 +468,9 @@ nub_process_t DNBProcessAttach(nub_process_t attach_pid,
 
 snprintf(fdstr, sizeof(fdstr), "--fd=%d", communication_fd);
 snprintf(pidstr, sizeof(pidstr), "--attach=%d", attach_pid);
-execl(translated_debugserver, "--native-regs", "--setsid", fdstr,
-  "--handoff-attach-from-native", pidstr, (char *)0);
+execl(translated_debugserver, translated_debugserver, "--native-regs",
+  "--setsid", fdstr, "--handoff-attach-from-native", pidstr,
+  (char *)0);
 DNBLogThreadedIf(LOG_PROCESS, "Failed to launch debugserver for "
  "translated process: ", errno, strerror(errno));
 __builtin_trap();



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