date:20210219

[Lldb-commits] [PATCH] D97017: [lldb-server] Exit the DataAvailableCallback loop when `done` or `interrupt` are set

2021-02-19 Thread Muhammad Omair Javaid via Phabricator via lldb-commits

omjavaid added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:950
   // We are ready to exit the debug monitor.
   m_exit_now = true;
   m_mainloop.RequestTermination();

May be we can set m_exit_now instead of setting done, or interrupt. if we have 
to exit on empty response.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1064
+
+if (done || interrupt) {
+  m_mainloop.RequestTermination();

GDBRemoteCommunicationServer::GetPacketAndSendResponse sets done on 
eServerPacketType_invalid (empty packet) wouldnt termination be too strict for 
that case. 



Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D97017

___
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

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

mgorny added a comment.

In D96548#2573954 , @omjavaid wrote:

> In D96548#2573245 , @mgorny wrote:
>
>> In D96548#2572770 , @omjavaid wrote:
>>
>>> I am wondering Whats the need for version checks? you are excluding 
>>> hardware breakpoint mangement code from the build but then that lldb-server 
>>> executable may well run on FreeBSD version which actually supports HW 
>>> breakpoints. Isnt there a dynamic way of checking support for hardware 
>>> break/watch points. May be query hardware debug registers and if it fails 
>>> just disable hardware breakpoints for that thread.
>>
>> The code relies on `struct` members that are only present with this version. 
>> I'd like to avoid copying implementation details like that into the code.
>
> Only concern here is that someone might be cross-compiling lldb-server on an 
> older system but eventually using it on a target which supports HW 
> break/watchpoints. What chances do you think we have for such a scenario? if 
> this is rare then this should be ok.

I don't think it's very likely. @emaste, any opinion on this?

>>> Moreover, can we shrink class name 
>>> NativeRegisterContextBreakWatchpoint_arm64 to may be 
>>> NativeDebugRegisterContext_arm64.
>>
>> I generally leave naming decisions to @labath ;-).
>
> Lets see what @labath has to say... IMO Debug Register is a general term used 
> in other architecture specs for referring to hardware debug capabilities like 
> breakpoints watchpoints etc.

I agree. For consistency with the x86 class, maybe 
`NativeRegisterContextDBReg_arm64`? I wonder if we should rename the x86 class 
too.


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

https://reviews.llvm.org/D96548

___
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

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

mgorny added a comment.

In D96548#2571489 , @mhorne wrote:

> I should also note that it is my intention to merge the FreeBSD watchpoint 
> patches into the 13.0 branch, in about a week's time. So your 
> `__FreeBSD_version` checks will need to be updated after that point.

Do you have any suggestion how we could check for kernel version from Python at 
runtime? (to determine whether we should expect watchpoint tests to work)


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

https://reviews.llvm.org/D96548

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

[Lldb-commits] [PATCH] D96840: [LLDB] [docs] Update the list of supported architectures on Windows

2021-02-19 Thread Martin Storsjö via Phabricator via lldb-commits

mstorsjo added a comment.

Ping, does this form look acceptable?


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

https://reviews.llvm.org/D96840

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

[Lldb-commits] [lldb] 0833128 - [lldb/Commands] Fix help text typo for 'breakpoint set' -a|--address.

2021-02-19 Thread Jan Kratochvil via lldb-commits


Author: Jan Kratochvil
Date: 2021-02-19T14:33:42+01:00
New Revision: 08331281af7bebf76d541cfb33d01dca22ed4d79

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

LOG: [lldb/Commands] Fix help text typo for 'breakpoint set' -a|--address.

Added: 


Modified: 
lldb/source/Commands/Options.td

Removed: 




diff  --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index d3329078893a..5dc451b8df6c 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -125,7 +125,7 @@ let Command = "breakpoint set" in {
   def breakpoint_set_address : Option<"address", "a">, Group<2>,
 Arg<"AddressOrExpression">, Required,
 Desc<"Set the breakpoint at the specified address.  If the address maps "
-"uniquely toa particular binary, then the address will be converted to "
+"uniquely to a particular binary, then the address will be converted to "
 "a \"file\"address, so that the breakpoint will track that binary+offset "
 "no matter where the binary eventually loads.  Alternately, if you also "
 "specify the module - with the -s option - then the address will be "



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

[Lldb-commits] [PATCH] D96840: [LLDB] [docs] Update the list of supported architectures on Windows

2021-02-19 Thread Adrian McCarthy via Phabricator via lldb-commits

amccarth added a comment.

I'm in no position to evaluate the functionality of lldb on Windows.  I've 
spent the last few months trying to understand why 900+ tests spontaneously 
started failing on Windows.


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

https://reviews.llvm.org/D96840

___
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

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

mgorny marked 5 inline comments as done.
mgorny added inline comments.



Comment at: 
lldb/source/Plugins/Process/Utility/NativeRegisterContextBreakWatchpoint_arm64.cpp:248
+  // non 8-byte aligned addresses as well.
+  if (addr & 0x07) {
+uint8_t watch_mask = (addr & 0x07) + size;

That's something to consider for a followup work but I'm thinking that instead 
of changing `size` here we could set bit-mask appropriately to watch only the 
exact bytes we need. If I read the manual correctly 
(https://developer.arm.com/documentation/ddi0595/2020-12/AArch64-Registers/DBGWCR-n--EL1--Debug-Watchpoint-Control-Registers?lang=en#fieldset_0-12_5),
 this should be doable.

e.g. if `addr = 0x2002` and `size = 2` ,
we currently set `addr = 0x2000` and `size = 4` and therefore watch 
`0x2000`..`0x2003`
but we could instead set BAS to `0b1100` to watch `0x2002`..`0x2003`.

Am I grasping this correctly?


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

https://reviews.llvm.org/D96548

___
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

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

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

- fixed broken FreeBSD release check in lldb
- fixed UB in FreeBSD's bp/wp impl
- implemented changes requested in comments
- refactored bp/wp code to use a few helper constants and utility functions

Please let me know if you think I should change more stuff here.


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,79 @@
+//===-- 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;
+
+  bool BreakpointIsEnabled(uint32_t bp_index);
+
+  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_hbp_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,474 @@
+//===-- 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;
+
+// E (bit 0), used to enable breakpoint/watchpoint
+constexpr

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

2021-02-19 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

In D96637#2573758 , @aadsm wrote:

>> I don't think this does what you think it does. The $() doesn't give you the 
>> process id of anything -- it substitutes a string by the result of running 
>> that string as a shell command. So, the PID variable would get the (entire) 
>> stdout of %s.out
>
> I'm confused here, "the PID variable would get the (entire) stdout of %s.out" 
> is exactly what I'm expecting to happen, the stdout of the program is its pid.
>
> I was finally able to figure out what the issue was. I thought `pause()` 
> would continue once the debugger attached because it sends a signal, but that 
> doesn't seem to be the case?

It does on mac , and I don't think it does on linux.


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] D97076: [lldb] Refine ThreadPlan::ShouldAutoContinue

2021-02-19 Thread Dave Lee via Phabricator via lldb-commits

kastiglione created this revision.
kastiglione added reviewers: jingham, aprantl.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Adjust `ShouldAutoContinue` to be available to any thread plan previous to the 
plan that
explains a stop, not limited to the parent to the plan that explains the stop.

Before this change, `Thread::ShouldStop` did the following:

1. find the plan that explains the stop
2. if it's not a master plan, continue processing previous (aka parent) plans
3. first, call `ShouldAutoContinue` on the immediate parent of the explaining 
plan
4. then loop over previous plans, calling `ShouldStop` and `MischiefManaged`

Of note, the iteration in step 4 does not call `ShouldAutoContinue`, so again 
only the
plan just prior to the explaining plan is given the opportunity to override 
whether to
continue or stop.

This commit changes the loop call `ShouldAutoContinue`, giving each plan the 
opportunity
to override `ShouldStop` of previous plans.

Why? This allows a plan to do the following:

1. mark itself done and be popped off the stack
2. allow parent plans to finish their work, and to also be popped off the stack
3. and finally, have the thread continue, not stop

This is useful for stepping into async functions. A plan will would step far 
enough
enough to set a breakpoint on the async target, and then use 
`ShouldAutoContinue` to
unwind the necessary stepping, and then have the calling thread continue.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97076

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


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -829,6 +829,8 @@
   ThreadPlan *plan_ptr = current_plan;
   while ((plan_ptr = GetPreviousPlan(plan_ptr)) != nullptr) {
 if (plan_ptr->PlanExplainsStop(event_ptr)) {
+  LLDB_LOGF(log, "Plan %s explains stop.", plan_ptr->GetName());
+
   should_stop = plan_ptr->ShouldStop(event_ptr);
 
   // plan_ptr explains the stop, next check whether plan_ptr is done,
@@ -858,10 +860,7 @@
   }
 
   if (!done_processing_current_plan) {
-bool over_ride_stop = current_plan->ShouldAutoContinue(event_ptr);
-
-LLDB_LOGF(log, "Plan %s explains stop, auto-continue %i.",
-  current_plan->GetName(), over_ride_stop);
+bool override_stop = false;
 
 // We're starting from the base plan, so just let it decide;
 if (current_plan->IsBasePlan()) {
@@ -882,19 +881,24 @@
   if (should_stop)
 current_plan->WillStop();
 
+  PopPlan();
+
+  if (current_plan->ShouldAutoContinue(event_ptr)) {
+override_stop = true;
+LLDB_LOGF(log, "Plan %s auto-continue: true.",
+  current_plan->GetName());
+  }
+
   // If a Master Plan wants to stop, we let it. Otherwise, see if the
   // plan's parent wants to stop.
 
   if (should_stop && current_plan->IsMasterPlan()) {
-PopPlan();
 break;
-  } else {
-PopPlan();
+  }
 
-current_plan = GetCurrentPlan();
-if (current_plan == nullptr) {
-  break;
-}
+  current_plan = GetCurrentPlan();
+  if (current_plan == nullptr) {
+break;
   }
 } else {
   break;
@@ -902,7 +906,7 @@
   }
 }
 
-if (over_ride_stop)
+if (override_stop)
   should_stop = false;
   }
 
Index: lldb/include/lldb/Target/ThreadPlan.h
===
--- lldb/include/lldb/Target/ThreadPlan.h
+++ lldb/include/lldb/Target/ThreadPlan.h
@@ -351,6 +351,12 @@
 
   virtual bool ShouldStop(Event *event_ptr) = 0;
 
+  /// Returns whether this thread plan overrides the `ShouldStop` of previous
+  /// plans.
+  ///
+  /// When processing the thread plan stack, this function gives plans the
+  /// ability to continue - even when previous plans subsequently return false
+  /// from `ShouldStop`. \see Thread::ShouldStop
   virtual bool ShouldAutoContinue(Event *event_ptr) { return false; }
 
   // Whether a "stop class" event should be reported to the "outside world".


Index: lldb/source/Target/Thread.cpp
===
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -829,6 +829,8 @@
   ThreadPlan *plan_ptr = current_plan;
   while ((plan_ptr = GetPreviousPlan(plan_ptr)) != nullptr) {
 if (plan_ptr->PlanExplainsStop(event_ptr)) {
+  LLDB_LOGF(log, "Plan %s explains stop.", plan_ptr->GetName());
+
   should_stop = plan_ptr->ShouldStop(event_ptr);
 
   // plan_ptr explains the stop, next check whether plan_ptr is done,
@@ -858,10 +860,7 @@

[Lldb-commits] [PATCH] D97076: [lldb] Refine ThreadPlan::ShouldAutoContinue

2021-02-19 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added inline comments.



Comment at: lldb/include/lldb/Target/ThreadPlan.h:354-359
+  /// Returns whether this thread plan overrides the `ShouldStop` of previous
+  /// plans.
+  ///
+  /// When processing the thread plan stack, this function gives plans the
+  /// ability to continue - even when previous plans subsequently return false
+  /// from `ShouldStop`. \see Thread::ShouldStop

Happy to iterate on this. I wanted to take the opportunity to explain this, 
since by name it may seem unclear how exactly it relates to `ShouldStop`.



Comment at: lldb/source/Target/Thread.cpp:832
 if (plan_ptr->PlanExplainsStop(event_ptr)) {
+  LLDB_LOGF(log, "Plan %s explains stop.", plan_ptr->GetName());
+

This log was previously incorrect about which plan explains the stop. I moved 
it here and fixed it.



Comment at: lldb/source/Target/Thread.cpp:884
 
+  PopPlan();
+

This is a drive by change. It was called from both branches of an `if`/`else`, 
so I lifted it out to be clear that it happens either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97076

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

[Lldb-commits] [lldb] 9d3b9e5 - [lldb] Rename {stop, run}_vote to report_{stop, run}_vote

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


Author: Dave Lee
Date: 2021-02-19T13:04:53-08:00
New Revision: 9d3b9e5799f6b1ac3869df977c82304e0121d256

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

LOG: [lldb] Rename {stop,run}_vote to report_{stop,run}_vote

Rename `stop_vote` and `run_vote` to `report_stop_vote` and `report_run_vote`
respectively. These variables are limited to logic involving (event) reporting 
only.
This naming is intended to make their context more clear.

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

Added: 


Modified: 
lldb/include/lldb/Target/Thread.h
lldb/include/lldb/Target/ThreadPlan.h
lldb/include/lldb/Target/ThreadPlanStepInstruction.h
lldb/include/lldb/Target/ThreadPlanStepOut.h
lldb/source/Target/Process.cpp
lldb/source/Target/Thread.cpp
lldb/source/Target/ThreadPlan.cpp
lldb/source/Target/ThreadPlanBase.cpp
lldb/source/Target/ThreadPlanStepInstruction.cpp
lldb/source/Target/ThreadPlanStepOut.cpp

Removed: 




diff  --git a/lldb/include/lldb/Target/Thread.h 
b/lldb/include/lldb/Target/Thread.h
index b797e4d1ee97..916493c061bb 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -781,10 +781,10 @@ class Thread : public 
std::enable_shared_from_this,
   /// \param[in] stop_other_threads
   ///\b true if we will stop other threads while we single step this one.
   ///
-  /// \param[in] stop_vote
+  /// \param[in] report_stop_vote
   ///See standard meanings for the stop & run votes in ThreadPlan.h.
   ///
-  /// \param[in] run_vote
+  /// \param[in] report_run_vote
   ///See standard meanings for the stop & run votes in ThreadPlan.h.
   ///
   /// \param[out] status
@@ -800,7 +800,7 @@ class Thread : public std::enable_shared_from_this,
   /// plan could not be queued.
   virtual lldb::ThreadPlanSP QueueThreadPlanForStepOut(
   bool abort_other_plans, SymbolContext *addr_context, bool first_insn,
-  bool stop_other_threads, Vote stop_vote, Vote run_vote,
+  bool stop_other_threads, Vote report_stop_vote, Vote report_run_vote,
   uint32_t frame_idx, Status &status,
   LazyBool step_out_avoids_code_without_debug_info = eLazyBoolCalculate);
 
@@ -830,10 +830,10 @@ class Thread : public 
std::enable_shared_from_this,
   /// \param[in] stop_other_threads
   ///\b true if we will stop other threads while we single step this one.
   ///
-  /// \param[in] stop_vote
+  /// \param[in] report_stop_vote
   ///See standard meanings for the stop & run votes in ThreadPlan.h.
   ///
-  /// \param[in] run_vote
+  /// \param[in] report_run_vote
   ///See standard meanings for the stop & run votes in ThreadPlan.h.
   ///
   /// \param[in] frame_idx
@@ -864,7 +864,7 @@ class Thread : public std::enable_shared_from_this,
   /// plan could not be queued.
   virtual lldb::ThreadPlanSP QueueThreadPlanForStepOutNoShouldStop(
   bool abort_other_plans, SymbolContext *addr_context, bool first_insn,
-  bool stop_other_threads, Vote stop_vote, Vote run_vote,
+  bool stop_other_threads, Vote report_stop_vote, Vote report_run_vote,
   uint32_t frame_idx, Status &status, bool continue_to_next_branch = 
false);
 
   /// Gets the plan used to step through the code that steps from a function

diff  --git a/lldb/include/lldb/Target/ThreadPlan.h 
b/lldb/include/lldb/Target/ThreadPlan.h
index caa6c5fe63aa..30e81f6e3050 100644
--- a/lldb/include/lldb/Target/ThreadPlan.h
+++ b/lldb/include/lldb/Target/ThreadPlan.h
@@ -260,8 +260,8 @@ namespace lldb_private {
 //  One other little detail here, sometimes a plan will push another plan onto
 //  the plan stack to do some part of the first plan's job, and it would be
 //  convenient to tell that plan how it should respond to ShouldReportStop.
-//  You can do that by setting the stop_vote in the child plan when you create
-//  it.
+//  You can do that by setting the report_stop_vote in the child plan when you
+//  create it.
 //
 //  Suppressing the initial eStateRunning event:
 //
@@ -275,8 +275,9 @@ namespace lldb_private {
 //  eVoteNo from ShouldReportStop, to force a running event to be reported
 //  return eVoteYes, in general though you should return eVoteNoOpinion which
 //  will allow the ThreadList to figure out the right thing to do.  The
-//  run_vote argument to the constructor works like stop_vote, and is a way for
-//  a plan to instruct a sub-plan on how to respond to ShouldReportStop.
+//  report_run_vote argument to the constructor works like report_stop_vote, 
and
+//  is a way for a plan to instruct a sub-plan on how to respond to
+//  ShouldReportStop.
 
 class ThreadPlan : public std::enable_shared_from_this,
public UserID {
@@ -472,7 +473,7 @@ class ThreadPlan : public 
std::enable_shared_from_t

[Lldb-commits] [PATCH] D96917: [lldb] Rename {stop, run}_vote to report_{stop, run}_vote (NFC)

2021-02-19 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 rG9d3b9e5799f6: [lldb] Rename {stop,run}_vote to 
report_{stop,run}_vote (authored by kastiglione).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96917

Files:
  lldb/include/lldb/Target/Thread.h
  lldb/include/lldb/Target/ThreadPlan.h
  lldb/include/lldb/Target/ThreadPlanStepInstruction.h
  lldb/include/lldb/Target/ThreadPlanStepOut.h
  lldb/source/Target/Process.cpp
  lldb/source/Target/Thread.cpp
  lldb/source/Target/ThreadPlan.cpp
  lldb/source/Target/ThreadPlanBase.cpp
  lldb/source/Target/ThreadPlanStepInstruction.cpp
  lldb/source/Target/ThreadPlanStepOut.cpp

Index: lldb/source/Target/ThreadPlanStepOut.cpp
===
--- lldb/source/Target/ThreadPlanStepOut.cpp
+++ lldb/source/Target/ThreadPlanStepOut.cpp
@@ -33,11 +33,11 @@
 // ThreadPlanStepOut: Step out of the current frame
 ThreadPlanStepOut::ThreadPlanStepOut(
 Thread &thread, SymbolContext *context, bool first_insn, bool stop_others,
-Vote stop_vote, Vote run_vote, uint32_t frame_idx,
+Vote report_stop_vote, Vote report_run_vote, uint32_t frame_idx,
 LazyBool step_out_avoids_code_without_debug_info,
 bool continue_to_next_branch, bool gather_return_value)
-: ThreadPlan(ThreadPlan::eKindStepOut, "Step out", thread, stop_vote,
- run_vote),
+: ThreadPlan(ThreadPlan::eKindStepOut, "Step out", thread, report_stop_vote,
+ report_run_vote),
   ThreadPlanShouldStopHere(this), m_step_from_insn(LLDB_INVALID_ADDRESS),
   m_return_bp_id(LLDB_INVALID_BREAK_ID),
   m_return_addr(LLDB_INVALID_ADDRESS), m_stop_others(stop_others),
Index: lldb/source/Target/ThreadPlanStepInstruction.cpp
===
--- lldb/source/Target/ThreadPlanStepInstruction.cpp
+++ lldb/source/Target/ThreadPlanStepInstruction.cpp
@@ -23,10 +23,11 @@
 ThreadPlanStepInstruction::ThreadPlanStepInstruction(Thread &thread,
  bool step_over,
  bool stop_other_threads,
- Vote stop_vote,
- Vote run_vote)
+ Vote report_stop_vote,
+ Vote report_run_vote)
 : ThreadPlan(ThreadPlan::eKindStepInstruction,
- "Step over single instruction", thread, stop_vote, run_vote),
+ "Step over single instruction", thread, report_stop_vote,
+ report_run_vote),
   m_instruction_addr(0), m_stop_other_threads(stop_other_threads),
   m_step_over(step_over) {
   m_takes_iteration_count = true;
Index: lldb/source/Target/ThreadPlanBase.cpp
===
--- lldb/source/Target/ThreadPlanBase.cpp
+++ lldb/source/Target/ThreadPlanBase.cpp
@@ -70,8 +70,8 @@
 }
 
 bool ThreadPlanBase::ShouldStop(Event *event_ptr) {
-  m_stop_vote = eVoteYes;
-  m_run_vote = eVoteYes;
+  m_report_stop_vote = eVoteYes;
+  m_report_run_vote = eVoteYes;
 
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_STEP));
 
@@ -82,8 +82,8 @@
 case eStopReasonInvalid:
 case eStopReasonNone:
   // This
-  m_run_vote = eVoteNoOpinion;
-  m_stop_vote = eVoteNo;
+  m_report_run_vote = eVoteNoOpinion;
+  m_report_stop_vote = eVoteNo;
   return false;
 
 case eStopReasonBreakpoint:
@@ -106,11 +106,11 @@
   // with "restarted" so the UI will know to wait and expect the consequent
   // "running".
   if (stop_info_sp->ShouldNotify(event_ptr)) {
-m_stop_vote = eVoteYes;
-m_run_vote = eVoteYes;
+m_report_stop_vote = eVoteYes;
+m_report_run_vote = eVoteYes;
   } else {
-m_stop_vote = eVoteNo;
-m_run_vote = eVoteNo;
+m_report_stop_vote = eVoteNo;
+m_report_run_vote = eVoteNo;
   }
   return false;
 
@@ -156,9 +156,9 @@
 // We're not going to stop, but while we are here, let's figure out
 // whether to report this.
 if (stop_info_sp->ShouldNotify(event_ptr))
-  m_stop_vote = eVoteYes;
+  m_report_stop_vote = eVoteYes;
 else
-  m_stop_vote = eVoteNo;
+  m_report_stop_vote = eVoteNo;
   }
   return false;
 
@@ -167,8 +167,8 @@
 }
 
   } else {
-m_run_vote = eVoteNoOpinion;
-m_stop_vote = eVoteNo;
+m_report_run_vote = eVoteNoOpinion;
+m_report_stop_vote = eVoteNo;
   }
 
   // If there's no explicit reason to stop, then we will continue.
@@ -185,8 +185,8 @@
   bool current_plan) {
   // Re

[Lldb-commits] [PATCH] D96947: [lldb] Prevent double new lines behind errors/warning/messages from LLDB commands

2021-02-19 Thread Jonas Devlieghere via Phabricator via lldb-commits

JDevlieghere added inline comments.



Comment at: lldb/source/Interpreter/CommandReturnObject.cpp:93
 return;
-  GetOutputStream() << in_string << "\n";
+  GetOutputStream() << in_string;
+  if (!in_string.endswith("\n"))

How about rstripping any trailing `\n` and then always adding one? 


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

https://reviews.llvm.org/D96947

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

[Lldb-commits] [PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

2021-02-19 Thread Shafik Yaghmour via Phabricator via lldb-commits

shafik updated this revision to Diff 325100.
shafik added a comment.
Herald added subscribers: usaxena95, kadircet.

Fixing tests that I missed before.


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

https://reviews.llvm.org/D96807

Files:
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/test/AST/ast-dump-decl-json.c
  clang/test/AST/ast-dump-enum-json.cpp
  clang/test/AST/ast-dump-openmp-cancel.c
  clang/test/AST/ast-dump-openmp-cancellation-point.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-distribute-simd.c
  clang/test/AST/ast-dump-openmp-distribute.c
  clang/test/AST/ast-dump-openmp-for-simd.c
  clang/test/AST/ast-dump-openmp-for.c
  clang/test/AST/ast-dump-openmp-ordered.c
  clang/test/AST/ast-dump-openmp-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-parallel-for.c
  clang/test/AST/ast-dump-openmp-parallel-sections.c
  clang/test/AST/ast-dump-openmp-parallel.c
  clang/test/AST/ast-dump-openmp-section.c
  clang/test/AST/ast-dump-openmp-sections.c
  clang/test/AST/ast-dump-openmp-simd.c
  clang/test/AST/ast-dump-openmp-single.c
  clang/test/AST/ast-dump-openmp-target-data.c
  clang/test/AST/ast-dump-openmp-target-enter-data.c
  clang/test/AST/ast-dump-openmp-target-exit-data.c
  clang/test/AST/ast-dump-openmp-target-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-parallel.c
  clang/test/AST/ast-dump-openmp-target-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute-simd.c
  clang/test/AST/ast-dump-openmp-target-teams-distribute.c
  clang/test/AST/ast-dump-openmp-target-teams.c
  clang/test/AST/ast-dump-openmp-target-update.c
  clang/test/AST/ast-dump-openmp-target.c
  clang/test/AST/ast-dump-openmp-task.c
  clang/test/AST/ast-dump-openmp-taskgroup.c
  clang/test/AST/ast-dump-openmp-taskloop-simd.c
  clang/test/AST/ast-dump-openmp-taskloop.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute-parallel-for.c
  clang/test/AST/ast-dump-openmp-teams-distribute-simd.c
  clang/test/AST/ast-dump-openmp-teams-distribute.c
  clang/test/AST/ast-dump-openmp-teams.c
  clang/test/AST/ast-dump-records-json.cpp
  clang/test/AST/ast-dump-records.c
  clang/test/AST/ast-dump-records.cpp
  clang/test/AST/ast-dump-stmt-json.m
  clang/test/ASTMerge/struct/test.c
  clang/test/Analysis/cfg.cpp
  clang/test/Analysis/padding_c.c
  clang/test/Index/print-type.c
  clang/test/Index/print-type.cpp
  clang/test/Layout/ms-x86-alias-avoidance-padding.cpp
  clang/test/PCH/stmt-openmp_structured_block-bit.cpp
  clang/test/Sema/assign.c
  clang/test/Sema/switch.c
  clang/test/SemaCXX/condition.cpp
  clang/test/SemaCXX/enum.cpp
  clang/test/SemaCXX/warn-sign-conversion.cpp
  
lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp
  lldb/test/Shell/SymbolFile/DWARF/debug-types-missing-signature.test

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

[Lldb-commits] [PATCH] D97076: [lldb] Refine ThreadPlan::ShouldAutoContinue

2021-02-19 Thread Jim Ingham via Phabricator via lldb-commits

jingham added a comment.

This seems like a fine improvement.  One little nit, I would ask the current 
plan ShouldAutoContinue before popping it.  Popping the plan does call WillPop, 
so the plan does have a chance to react to being popped, and you don't know 
what it will do.  So to be on the safe side it would be better to ask questions 
of it as an active plan before you pop it.




Comment at: lldb/include/lldb/Target/ThreadPlan.h:354-359
+  /// Returns whether this thread plan overrides the `ShouldStop` of previous
+  /// plans.
+  ///
+  /// When processing the thread plan stack, this function gives plans the
+  /// ability to continue - even when previous plans subsequently return false
+  /// from `ShouldStop`. \see Thread::ShouldStop

kastiglione wrote:
> Happy to iterate on this. I wanted to take the opportunity to explain this, 
> since by name it may seem unclear how exactly it relates to `ShouldStop`.
The use of "previous plans" is a little confusing, since it not clear "previous 
to what".   To me the obvious meaning is plans previously dealt with in this 
negotiation, which is not what you mean, and then sounds really odd when you 
hit the "subsequently".  I don't think you need the "previous" at all.  You 
could just say "subsequently processed plans".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97076

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

[Lldb-commits] [PATCH] D97076: [lldb] Refine ThreadPlan::ShouldAutoContinue

2021-02-19 Thread Dave Lee via Phabricator via lldb-commits

kastiglione added a comment.

Good call on calling ShouldAutoContinue before Pop.




Comment at: lldb/include/lldb/Target/ThreadPlan.h:354-359
+  /// Returns whether this thread plan overrides the `ShouldStop` of previous
+  /// plans.
+  ///
+  /// When processing the thread plan stack, this function gives plans the
+  /// ability to continue - even when previous plans subsequently return false
+  /// from `ShouldStop`. \see Thread::ShouldStop

jingham wrote:
> kastiglione wrote:
> > Happy to iterate on this. I wanted to take the opportunity to explain this, 
> > since by name it may seem unclear how exactly it relates to `ShouldStop`.
> The use of "previous plans" is a little confusing, since it not clear 
> "previous to what".   To me the obvious meaning is plans previously dealt 
> with in this negotiation, which is not what you mean, and then sounds really 
> odd when you hit the "subsequently".  I don't think you need the "previous" 
> at all.  You could just say "subsequently processed plans".
Thanks I'll update it.

I agree that "previous" can be ambiguous, especially in the context of 
`Thread::ShouldStop`. What do you think of renaming `GetPreviousPlan` to 
`GetParentPlan` (and related variables and documentation too)? I can do this in 
a follow up if you agree.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97076

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

[Lldb-commits] [PATCH] D96680: [lldb-vscode] Emit the breakpoint changed event on location resolved

2021-02-19 Thread António Afonso via Phabricator via lldb-commits

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

Add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96680

Files:
  lldb/test/API/tools/lldb-vscode/breakpoint-events/Makefile
  
lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointLocationResolvedEvent.py
  lldb/test/API/tools/lldb-vscode/breakpoint-events/dylib.c
  lldb/test/API/tools/lldb-vscode/breakpoint-events/dylib_loader.c
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -417,7 +417,8 @@
   // of wether the locations were added or removed, the breakpoint
   // ins't going away, so we the reason is always "changed".
   if ((event_type & lldb::eBreakpointEventTypeLocationsAdded ||
-   event_type & lldb::eBreakpointEventTypeLocationsRemoved) &&
+   event_type & lldb::eBreakpointEventTypeLocationsRemoved ||
+   event_type & lldb::eBreakpointEventTypeLocationsResolved) &&
   bp.MatchesName(BreakpointBase::GetBreakpointLabel())) {
 auto bp_event = CreateEventObject("breakpoint");
 llvm::json::Object body;
Index: lldb/test/API/tools/lldb-vscode/breakpoint-events/dylib_loader.c
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/breakpoint-events/dylib_loader.c
@@ -0,0 +1,15 @@
+#include 
+#include 
+#include 
+
+volatile int flip_to_1_to_continue = 0;
+
+int main() {
+  lldb_enable_attach();
+  while (! flip_to_1_to_continue) // Wait for debugger to attach
+sleep(1);
+  // dlopen the feature
+  void *dylib = dlopen("libdylib.so", RTLD_LAZY);
+  assert(dylib && "dlopen failed?");
+  return 0; // break after dlopen
+}
Index: lldb/test/API/tools/lldb-vscode/breakpoint-events/dylib.c
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/breakpoint-events/dylib.c
@@ -0,0 +1,3 @@
+extern void foo() {
+// breakpoint dylib
+}
Index: lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointLocationResolvedEvent.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointLocationResolvedEvent.py
@@ -0,0 +1,71 @@
+"""
+Test lldb-vscode setBreakpoints request
+"""
+
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+import os
+
+
+class TestVSCode_breakpointLocationResolvedEvent(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def build_launch_and_attach(self):
+self.build_and_create_debug_adaptor()
+# launch externally
+exe = self.getBuildArtifact("dylib_loader")
+popen = self.spawnSubprocess(exe)
+# attach
+self.attach(exe, popen.pid)
+
+def set_breakpoint(self, filename, comment):
+source_basename = filename
+source_path = os.path.join(os.getcwd(), source_basename)
+bp_line = line_number(filename, comment)
+return self.vscode.request_setBreakpoints(source_path,
+  [bp_line])
+
+@skipUnlessPlatform(["linux"])
+def test_breakpoint_location_resolved_event(self):
+'''
+This test sets a breakpoint in a shared library before it's loaded.
+This will make the client receive a breakpoint notification of
+unresolved location. Once the library is loaded the client should
+receive another change event indicating the location is resolved.
+'''
+self.build_launch_and_attach()
+self.set_breakpoint('dylib_loader.c', 'break after dlopen')
+response = self.set_breakpoint('dylib.c', 'breakpoint dylib')
+if response:
+breakpoints = response['body']['breakpoints']
+for breakpoint in breakpoints:
+bp_id = breakpoint['id']
+self.assertFalse(breakpoint['verified'],
+"expect dylib breakpoint to be unverified")
+break
+response = self.vscode.request_evaluate("flip_to_1_to_continue = 1")
+self.assertTrue(response['success'])
+
+self.continue_to_next_stop()
+self.assertTrue(len(self.vscode.breakpoint_events) > 1,
+"make sure we got a breakpoint event")
+
+# find the last breakpoint event for bp_id
+for event in reversed(self.vscode.breakpoint_events):
+if event['body']['breakpoint']['id'] == bp_id:
+break
+body = eve

[Lldb-commits] [PATCH] D96680: [lldb-vscode] Emit the breakpoint changed event on location resolved

2021-02-19 Thread António Afonso via Phabricator via lldb-commits

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

Removed unnecessary comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96680

Files:
  lldb/test/API/tools/lldb-vscode/breakpoint-events/Makefile
  
lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointLocationResolvedEvent.py
  lldb/test/API/tools/lldb-vscode/breakpoint-events/dylib.c
  lldb/test/API/tools/lldb-vscode/breakpoint-events/dylib_loader.c
  lldb/tools/lldb-vscode/lldb-vscode.cpp

Index: lldb/tools/lldb-vscode/lldb-vscode.cpp
===
--- lldb/tools/lldb-vscode/lldb-vscode.cpp
+++ lldb/tools/lldb-vscode/lldb-vscode.cpp
@@ -417,7 +417,8 @@
   // of wether the locations were added or removed, the breakpoint
   // ins't going away, so we the reason is always "changed".
   if ((event_type & lldb::eBreakpointEventTypeLocationsAdded ||
-   event_type & lldb::eBreakpointEventTypeLocationsRemoved) &&
+   event_type & lldb::eBreakpointEventTypeLocationsRemoved ||
+   event_type & lldb::eBreakpointEventTypeLocationsResolved) &&
   bp.MatchesName(BreakpointBase::GetBreakpointLabel())) {
 auto bp_event = CreateEventObject("breakpoint");
 llvm::json::Object body;
Index: lldb/test/API/tools/lldb-vscode/breakpoint-events/dylib_loader.c
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/breakpoint-events/dylib_loader.c
@@ -0,0 +1,14 @@
+#include 
+#include 
+#include 
+
+volatile int flip_to_1_to_continue = 0;
+
+int main() {
+  lldb_enable_attach();
+  while (! flip_to_1_to_continue) // Wait for debugger to attach
+sleep(1);
+  void *dylib = dlopen("libdylib.so", RTLD_LAZY);
+  assert(dylib && "dlopen failed?");
+  return 0; // break after dlopen
+}
Index: lldb/test/API/tools/lldb-vscode/breakpoint-events/dylib.c
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/breakpoint-events/dylib.c
@@ -0,0 +1,3 @@
+extern void foo() {
+// breakpoint dylib
+}
Index: lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointLocationResolvedEvent.py
===
--- /dev/null
+++ lldb/test/API/tools/lldb-vscode/breakpoint-events/TestVSCode_breakpointLocationResolvedEvent.py
@@ -0,0 +1,71 @@
+"""
+Test lldb-vscode setBreakpoints request
+"""
+
+
+import unittest2
+import vscode
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+import lldbvscode_testcase
+import os
+
+
+class TestVSCode_breakpointLocationResolvedEvent(lldbvscode_testcase.VSCodeTestCaseBase):
+
+mydir = TestBase.compute_mydir(__file__)
+
+def build_launch_and_attach(self):
+self.build_and_create_debug_adaptor()
+# launch externally
+exe = self.getBuildArtifact("dylib_loader")
+popen = self.spawnSubprocess(exe)
+# attach
+self.attach(exe, popen.pid)
+
+def set_breakpoint(self, filename, comment):
+source_basename = filename
+source_path = os.path.join(os.getcwd(), source_basename)
+bp_line = line_number(filename, comment)
+return self.vscode.request_setBreakpoints(source_path,
+  [bp_line])
+
+@skipUnlessPlatform(["linux"])
+def test_breakpoint_location_resolved_event(self):
+'''
+This test sets a breakpoint in a shared library before it's loaded.
+This will make the client receive a breakpoint notification of
+unresolved location. Once the library is loaded the client should
+receive another change event indicating the location is resolved.
+'''
+self.build_launch_and_attach()
+self.set_breakpoint('dylib_loader.c', 'break after dlopen')
+response = self.set_breakpoint('dylib.c', 'breakpoint dylib')
+if response:
+breakpoints = response['body']['breakpoints']
+for breakpoint in breakpoints:
+bp_id = breakpoint['id']
+self.assertFalse(breakpoint['verified'],
+"expect dylib breakpoint to be unverified")
+break
+response = self.vscode.request_evaluate("flip_to_1_to_continue = 1")
+self.assertTrue(response['success'])
+
+self.continue_to_next_stop()
+self.assertTrue(len(self.vscode.breakpoint_events) > 1,
+"make sure we got a breakpoint event")
+
+# find the last breakpoint event for bp_id
+for event in reversed(self.vscode.breakpoint_events):
+if event['body']['breakpoint']['id'] == bp_id:
+break
+body = event['bo

[Lldb-commits] [PATCH] D97076: [lldb] Refine ThreadPlan::ShouldAutoContinue

2021-02-19 Thread Jim Ingham via Phabricator via lldb-commits

jingham added inline comments.



Comment at: lldb/include/lldb/Target/ThreadPlan.h:354-359
+  /// Returns whether this thread plan overrides the `ShouldStop` of previous
+  /// plans.
+  ///
+  /// When processing the thread plan stack, this function gives plans the
+  /// ability to continue - even when previous plans subsequently return false
+  /// from `ShouldStop`. \see Thread::ShouldStop

kastiglione wrote:
> jingham wrote:
> > kastiglione wrote:
> > > Happy to iterate on this. I wanted to take the opportunity to explain 
> > > this, since by name it may seem unclear how exactly it relates to 
> > > `ShouldStop`.
> > The use of "previous plans" is a little confusing, since it not clear 
> > "previous to what".   To me the obvious meaning is plans previously dealt 
> > with in this negotiation, which is not what you mean, and then sounds 
> > really odd when you hit the "subsequently".  I don't think you need the 
> > "previous" at all.  You could just say "subsequently processed plans".
> Thanks I'll update it.
> 
> I agree that "previous" can be ambiguous, especially in the context of 
> `Thread::ShouldStop`. What do you think of renaming `GetPreviousPlan` to 
> `GetParentPlan` (and related variables and documentation too)? I can do this 
> in a follow up if you agree.
Changing away from previous would be great! 

Parent isn't quite right anyway because there's no necessary relationship 
between plans on the stack.  You could stop at a breakpoint while using one 
plan, and then do a "step" and the plan that was working before the breakpoint 
has no knowledge or relationship to the new plan.

I've been trying recently to be consistent about using Older and Younger for 
things pushed onto a stack. That seems clear to me since you are describing the 
process of pushing things onto a stack, so the order in the stack is governed 
by the time at which they were pushed.  I've been trying to speak of stack 
frames this way as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97076

___
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-19 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added inline comments.



Comment at: 
lldb/test/API/functionalities/module_load_attach/TestModuleLoadAttach.py:28
+def test_x(self):
+process = self.build_launch_and_attach()
+thread = process.GetSelectedThread()

Is this racy? What happens on a really slow system? Can we fail to attach? If 
we do attach, are we guaranteed to be at a place where we can set 
"flip_to_1_to_continue = 1"? The nice thing is it is a global variable that we 
should be able to set no matter where we stop. 



Comment at: 
lldb/test/API/functionalities/module_load_attach/TestModuleLoadAttach.py:32-33
+# Continue so that dlopen is called.
+breakpoint = self.target().BreakpointCreateBySourceRegex(
+"// break after dlopen", lldb.SBFileSpec("main.c"))
+self.assertNotEqual(breakpoint.GetNumResolvedLocations(), 0)

Don't we need to break before the dlopen and make sure we don't have a 
libfeature.so in our module list, then run over the dlopen and verify we do see 
it afterwards? Wasn't this bug that we will see shared libraries correctly one 
time when we attach, but just not get any updates after 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] D97017: [lldb-server] Exit the DataAvailableCallback loop when `done` or `interrupt` are set

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

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

[Lldb-commits] [PATCH] D96840: [LLDB] [docs] Update the list of supported architectures on Windows

[Lldb-commits] [lldb] 0833128 - [lldb/Commands] Fix help text typo for 'breakpoint set' -a|--address.

[Lldb-commits] [PATCH] D96840: [LLDB] [docs] Update the list of supported architectures on Windows

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

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

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

[Lldb-commits] [PATCH] D97076: [lldb] Refine ThreadPlan::ShouldAutoContinue

[Lldb-commits] [PATCH] D97076: [lldb] Refine ThreadPlan::ShouldAutoContinue

[Lldb-commits] [lldb] 9d3b9e5 - [lldb] Rename {stop, run}_vote to report_{stop, run}_vote

[Lldb-commits] [PATCH] D96917: [lldb] Rename {stop, run}_vote to report_{stop, run}_vote (NFC)

[Lldb-commits] [PATCH] D96947: [lldb] Prevent double new lines behind errors/warning/messages from LLDB commands

[Lldb-commits] [PATCH] D96807: Modify TypePrinter to differentiate between anonymous struct and unnamed struct

[Lldb-commits] [PATCH] D97076: [lldb] Refine ThreadPlan::ShouldAutoContinue

[Lldb-commits] [PATCH] D97076: [lldb] Refine ThreadPlan::ShouldAutoContinue

[Lldb-commits] [PATCH] D96680: [lldb-vscode] Emit the breakpoint changed event on location resolved

[Lldb-commits] [PATCH] D96680: [lldb-vscode] Emit the breakpoint changed event on location resolved

[Lldb-commits] [PATCH] D97076: [lldb] Refine ThreadPlan::ShouldAutoContinue

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

21 matches

Site Navigation

Mail list logo

Footer information