date:20220718

[Lldb-commits] [lldb] 83fab8c - Revert "Make hit point counts reliable for architectures that stop before evaluation."

2022-07-18 Thread Jim Ingham via lldb-commits


Author: Jim Ingham
Date: 2022-07-18T17:38:43-07:00
New Revision: 83fab8cee9d6b9fa911195c20325b4512a7a22ef

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

LOG: Revert "Make hit point counts reliable for architectures that stop before 
evaluation."

This reverts commit 5778ada8e54edb2bc2869505b88a959d1915c02f.

The watchpoint tests all stall on aarch64-ubuntu bots.  Reverting till I can
get my hands on an system to test this out.

Added: 


Modified: 
lldb/include/lldb/Breakpoint/Watchpoint.h
lldb/include/lldb/Target/StopInfo.h
lldb/source/Target/StopInfo.cpp

lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatchBreak.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py

Removed: 




diff  --git a/lldb/include/lldb/Breakpoint/Watchpoint.h 
b/lldb/include/lldb/Breakpoint/Watchpoint.h
index 7137b6e4a24c5..41b723a66b6a3 100644
--- a/lldb/include/lldb/Breakpoint/Watchpoint.h
+++ b/lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -157,15 +157,12 @@ class Watchpoint : public 
std::enable_shared_from_this,
 private:
   friend class Target;
   friend class WatchpointList;
-  friend class StopInfoWatchpoint; // This needs to call UndoHitCount()
 
   void ResetHistoricValues() {
 m_old_value_sp.reset();
 m_new_value_sp.reset();
   }
 
-  void UndoHitCount() { m_hit_counter.Decrement(); }
-
   Target _target;
   bool m_enabled;   // Is this watchpoint enabled
   bool m_is_hardware;   // Is this a hardware watchpoint

diff  --git a/lldb/include/lldb/Target/StopInfo.h 
b/lldb/include/lldb/Target/StopInfo.h
index 9527a6ea553e3..cdb906dcd7ede 100644
--- a/lldb/include/lldb/Target/StopInfo.h
+++ b/lldb/include/lldb/Target/StopInfo.h
@@ -17,7 +17,7 @@
 
 namespace lldb_private {
 
-class StopInfo : public std::enable_shared_from_this {
+class StopInfo {
   friend class Process::ProcessEventData;
   friend class ThreadPlanBase;
 

diff  --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 5cf0f760aa249..00d30070c8c9f 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -20,7 +20,6 @@
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadPlan.h"
-#include "lldb/Target/ThreadPlanStepInstruction.h"
 #include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
@@ -691,128 +690,40 @@ class StopInfoWatchpoint : public StopInfo {
   }
 
 protected:
-  using StopInfoWatchpointSP = std::shared_ptr;
-  // This plan is used to orchestrate stepping over the watchpoint for
-  // architectures (e.g. ARM) that report the watch before running the watched
-  // access.  This is the sort of job you have to defer to the thread plans,
-  // if you try to do it directly in the stop info and there are other threads
-  // that needed to process this stop you will have yanked control away from
-  // them and they won't behave correctly.
-  class ThreadPlanStepOverWatchpoint : public ThreadPlanStepInstruction {
-  public:
-ThreadPlanStepOverWatchpoint(Thread , 
- StopInfoWatchpointSP stop_info_sp,
- WatchpointSP watch_sp)
-: ThreadPlanStepInstruction(thread, false, true, eVoteNoOpinion,
-eVoteNoOpinion),
-  m_stop_info_sp(stop_info_sp), m_watch_sp(watch_sp) {
-  assert(watch_sp);
-  m_watch_index = watch_sp->GetHardwareIndex();
-}
-
-bool DoWillResume(lldb::StateType resume_state,
-  bool current_plan) override {
-  if (m_did_reset_watchpoint) {
-GetThread().GetProcess()->DisableWatchpoint(m_watch_sp.get(), false);
-m_did_reset_watchpoint = false;
-  }
-

[Lldb-commits] [lldb] 4f5707e - Revert "This is a followup to https://reviews.llvm.org/D129814"

2022-07-18 Thread Jim Ingham via lldb-commits


Author: Jim Ingham
Date: 2022-07-18T17:37:13-07:00
New Revision: 4f5707e743528a0d40ac154e2e07705a03fd7ad3

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

LOG: Revert "This is a followup to https://reviews.llvm.org/D129814;

This reverts commit 555ae5b8f5aa93ab090af853a8b7a83f815b3f20.

Apparently, there's something different about how Linux ARM handles watchpoints,
as all the watchpoint tests seem to stall on the Ubuntu aarch64 bots.

Reverting till I can get my hands on a linux system and see what is
wrong.

Added: 


Modified: 
lldb/source/Target/StopInfo.cpp

Removed: 




diff  --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 17f6d15059e11..5cf0f760aa249 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -754,22 +754,15 @@ class StopInfoWatchpoint : public StopInfo {
   bool ShouldStopSynchronous(Event *event_ptr) override {
 // If we are running our step-over the watchpoint plan, stop if it's done
 // and continue if it's not:
-if (m_should_stop_is_valid)
-  return m_should_stop;
-
-// If we are running our step over plan, then stop here and let the regular
-// ShouldStop figure out what we should do:  Otherwise, give our plan
-// more time to get run:
 if (m_using_step_over_plan)
   return m_step_over_plan_complete;
 
-Log *log = GetLog(LLDBLog::Process);
 ThreadSP thread_sp(m_thread_wp.lock());
 assert(thread_sp);
 WatchpointSP wp_sp(
 
thread_sp->CalculateTarget()->GetWatchpointList().FindByID(GetValue()));
-// If we can no longer find the watchpoint, we just have to stop:
 if (!wp_sp) {
+  Log *log = GetLog(LLDBLog::Process);
 
   LLDB_LOGF(log,
 "Process::%s could not find watchpoint location id: %" PRId64
@@ -803,42 +796,23 @@ class StopInfoWatchpoint : public StopInfo {
 uint32_t num;
 bool wp_triggers_after;
 
-if (!process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
+if (process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
 .Success()) {
-  m_should_stop_is_valid = true;
-  m_should_stop = true;
-  return m_should_stop;
-}
-
-if (!wp_triggers_after) {
-  // We have to step over the breakpoint before we know what to do:   
+  if (wp_triggers_after)
+return true;
+  
   StopInfoWatchpointSP me_as_siwp_sp 
   = std::static_pointer_cast(shared_from_this());
   ThreadPlanSP step_over_wp_sp(new ThreadPlanStepOverWatchpoint(
   *(thread_sp.get()), me_as_siwp_sp, wp_sp));
   Status error;
   error = thread_sp->QueueThreadPlan(step_over_wp_sp, false);
-  // If we couldn't push the thread plan, just stop here:
-  if (!error.Success()) {
-LLDB_LOGF(log, "Could not push our step over watchpoint plan: %s", 
-error.AsCString());
-
-m_should_stop = true;
-m_should_stop_is_valid = true;
-return true;
-  } else {
-  // Otherwise, don't set m_should_stop, we don't know that yet.  Just 
-  // say we should continue:
-m_using_step_over_plan = true;
-return false;
-  }
-} else {
-  // We didn't have to do anything special here, so just return our value:
-  m_should_stop_is_valid = true;
-  return m_should_stop;
+  m_using_step_over_plan = true;
+  return !error.Success();
 }
-
-return m_should_stop;
+// If we don't have to step over the watchpoint, just let the PerformAction
+// determine what we should do.
+return true;
   }
 
   bool ShouldStop(Event *event_ptr) override {



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

[Lldb-commits] [PATCH] D130054: [trace][intel pt] Introduce wall clock time for each trace item

2022-07-18 Thread walter erquinigo via Phabricator via lldb-commits

wallace updated this revision to Diff 445664.
wallace edited the summary of this revision.
wallace added a comment.
Herald added subscribers: jsji, pengfei.

fix nits


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130054

Files:
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceDumper.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Target/TraceCursor.cpp
  lldb/source/Target/TraceDumper.cpp

Index: lldb/source/Target/TraceDumper.cpp
===
--- lldb/source/Target/TraceDumper.cpp
+++ lldb/source/Target/TraceDumper.cpp
@@ -128,16 +128,26 @@
 
 m_s.Format("{0}: ", item.id);
 
-if (m_options.show_tsc) {
-  m_s.Format("[tsc={0}] ",
- item.tsc ? std::to_string(*item.tsc) : "unavailable");
+if (m_options.show_timestamps) {
+  m_s.Format("[{0}] ", item.timestamp
+   ? (std::to_string(*item.timestamp) + " ns")
+   : "unavailable");
 }
 
 if (item.event) {
   m_s << "(event) " << TraceCursor::EventKindToString(*item.event);
-  if (*item.event == eTraceEventCPUChanged) {
+  switch (*item.event) {
+  case eTraceEventCPUChanged:
 m_s.Format(" [new CPU={0}]",
item.cpu_id ? std::to_string(*item.cpu_id) : "unavailable");
+break;
+  case eTraceEventHWClockTick:
+m_s.Format(" [{0}]", item.hw_clock ? std::to_string(*item.hw_clock)
+   : "unavailable");
+break;
+  case eTraceEventDisabledHW:
+  case eTraceEventDisabledSW:
+break;
   }
 } else if (item.error) {
   m_s << "(error) " << *item.error;
@@ -181,7 +191,8 @@
 | {
   "loadAddress": string decimal,
   "id": decimal,
-  "tsc"?: string decimal,
+  "hwClock"?: string decimal,
+  "timestamp_ns"?: string decimal,
   "module"?: string,
   "symbol"?: string,
   "line"?: decimal,
@@ -202,8 +213,17 @@
 
   void DumpEvent(const TraceDumper::TraceItem ) {
 m_j.attribute("event", TraceCursor::EventKindToString(*item.event));
-if (item.event == eTraceEventCPUChanged)
+switch (*item.event) {
+case eTraceEventCPUChanged:
   m_j.attribute("cpuId", item.cpu_id);
+  break;
+case eTraceEventHWClockTick:
+  m_j.attribute("hwClock", item.hw_clock);
+  break;
+case eTraceEventDisabledHW:
+case eTraceEventDisabledSW:
+  break;
+}
   }
 
   void DumpInstruction(const TraceDumper::TraceItem ) {
@@ -234,10 +254,11 @@
   void TraceItem(const TraceDumper::TraceItem ) override {
 m_j.object([&] {
   m_j.attribute("id", item.id);
-  if (m_options.show_tsc)
-m_j.attribute(
-"tsc",
-item.tsc ? Optional(std::to_string(*item.tsc)) : None);
+  if (m_options.show_timestamps)
+m_j.attribute("timestamp_ns", item.timestamp
+  ? Optional(
+std::to_string(*item.timestamp))
+  : None);
 
   if (item.event) {
 DumpEvent(item);
@@ -289,8 +310,8 @@
   TraceItem item;
   item.id = m_cursor_up->GetId();
 
-  if (m_options.show_tsc)
-item.tsc = m_cursor_up->GetCounter(lldb::eTraceCounterTSC);
+  if (m_options.show_timestamps)
+item.timestamp = m_cursor_up->GetWallClockTime();
   return item;
 }
 
@@ -366,8 +387,17 @@
   if (!m_options.show_events)
 continue;
   item.event = m_cursor_up->GetEventType();
-  if (*item.event == eTraceEventCPUChanged)
+  switch (*item.event) {
+  case eTraceEventCPUChanged:
 item.cpu_id = m_cursor_up->GetCPU();
+break;
+  case eTraceEventHWClockTick:
+item.hw_clock = m_cursor_up->GetHWClock();
+break;
+  case eTraceEventDisabledHW:
+  case eTraceEventDisabledSW:
+break;
+  }
 } else if (m_cursor_up->IsError()) {
   item.error = m_cursor_up->GetError();
 } else {
Index: lldb/source/Target/TraceCursor.cpp
===
--- lldb/source/Target/TraceCursor.cpp
+++ lldb/source/Target/TraceCursor.cpp
@@ -50,5 +50,7 @@
 return "software disabled tracing";
   case lldb::eTraceEventCPUChanged:
 return "CPU core changed";
+  case lldb::eTraceEventHWClockTick:
+return "HW clock tick";
   }
 }
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp

[Lldb-commits] [PATCH] D130054: [trace][intel pt] Introduce wall clock time for each trace item

2022-07-18 Thread walter erquinigo via Phabricator via lldb-commits

wallace created this revision.
Herald added a project: All.
wallace requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

- Decouple TSCs from trace items
- Turn TSCs into events just like CPUs
- Add a GetWallTime that returns the wall time that the trace plug-in can infer 
for each trace item.
- For intel pt, we are doing the following interpolation: if an instruction 
takes less than 1 TSC, we use that duration, otherwise, we assume the 
instruction took 1 TSC. This helps us avoid having to handle context switches, 
changes to kernel, idle times, decoding errors, etc. We are just trying to show 
some approximation and not the real data. For the real data, TSCs are the way 
to go.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130054

Files:
  lldb/include/lldb/Target/TraceCursor.h
  lldb/include/lldb/Target/TraceDumper.h
  lldb/include/lldb/lldb-enumerations.h
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/Options.td
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
  lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
  lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
  lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
  lldb/source/Target/TraceCursor.cpp
  lldb/source/Target/TraceDumper.cpp

Index: lldb/source/Target/TraceDumper.cpp
===
--- lldb/source/Target/TraceDumper.cpp
+++ lldb/source/Target/TraceDumper.cpp
@@ -128,16 +128,27 @@
 
 m_s.Format("{0}: ", item.id);
 
-if (m_options.show_tsc) {
-  m_s.Format("[tsc={0}] ",
- item.tsc ? std::to_string(*item.tsc) : "unavailable");
+if (m_options.show_timestamps) {
+  m_s.Format("[{0}] ", item.timestamp
+   ? (std::to_string(*item.timestamp) + " ns")
+   : "unavailable");
 }
 
 if (item.event) {
   m_s << "(event) " << TraceCursor::EventKindToString(*item.event);
-  if (*item.event == eTraceEventCPUChanged) {
+  switch (*item.event) {
+  case eTraceEventCPUChanged:
 m_s.Format(" [new CPU={0}]",
item.cpu_id ? std::to_string(*item.cpu_id) : "unavailable");
+break;
+  case eTraceEventHWClockTick:
+m_s.Format(" [new HW clock tick={0}",
+   item.hw_clock ? std::to_string(*item.hw_clock)
+ : "unavailable");
+break;
+  case eTraceEventDisabledHW:
+  case eTraceEventDisabledSW:
+break;
   }
 } else if (item.error) {
   m_s << "(error) " << *item.error;
@@ -181,7 +192,8 @@
 | {
   "loadAddress": string decimal,
   "id": decimal,
-  "tsc"?: string decimal,
+  "hwClock"?: string decimal,
+  "timestamp_ns"?: string decimal,
   "module"?: string,
   "symbol"?: string,
   "line"?: decimal,
@@ -202,8 +214,17 @@
 
   void DumpEvent(const TraceDumper::TraceItem ) {
 m_j.attribute("event", TraceCursor::EventKindToString(*item.event));
-if (item.event == eTraceEventCPUChanged)
+switch (*item.event) {
+case eTraceEventCPUChanged:
   m_j.attribute("cpuId", item.cpu_id);
+  break;
+case eTraceEventHWClockTick:
+  m_j.attribute("hwClock", item.hw_clock);
+  break;
+case eTraceEventDisabledHW:
+case eTraceEventDisabledSW:
+  break;
+}
   }
 
   void DumpInstruction(const TraceDumper::TraceItem ) {
@@ -234,10 +255,11 @@
   void TraceItem(const TraceDumper::TraceItem ) override {
 m_j.object([&] {
   m_j.attribute("id", item.id);
-  if (m_options.show_tsc)
-m_j.attribute(
-"tsc",
-item.tsc ? Optional(std::to_string(*item.tsc)) : None);
+  if (m_options.show_timestamps)
+m_j.attribute("timestamp_ns", item.timestamp
+  ? Optional(
+std::to_string(*item.timestamp))
+  : None);
 
   if (item.event) {
 DumpEvent(item);
@@ -289,8 +311,8 @@
   TraceItem item;
   item.id = m_cursor_up->GetId();
 
-  if (m_options.show_tsc)
-item.tsc = m_cursor_up->GetCounter(lldb::eTraceCounterTSC);
+  if (m_options.show_timestamps)
+item.timestamp = m_cursor_up->GetWallClockTime();
   return item;
 }
 
@@ -366,8 +388,17 @@
   if (!m_options.show_events)
 continue;
   item.event = m_cursor_up->GetEventType();
-  if (*item.event == eTraceEventCPUChanged)
+  switch (*item.event) {
+  case eTraceEventCPUChanged:
 item.cpu_id = m_cursor_up->GetCPU();
+break;
+  case eTraceEventHWClockTick:
+item.hw_clock = m_cursor_up->GetHWClock();
+break;
+  case eTraceEventDisabledHW:
+  case eTraceEventDisabledSW:
+break;
+  }
 } else if (m_cursor_up->IsError()) {

[Lldb-commits] [lldb] 555ae5b - This is a followup to https://reviews.llvm.org/D129814

2022-07-18 Thread Jim Ingham via lldb-commits


Author: Jim Ingham
Date: 2022-07-18T16:24:31-07:00
New Revision: 555ae5b8f5aa93ab090af853a8b7a83f815b3f20

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

LOG: This is a followup to https://reviews.llvm.org/D129814

That was causing hit counts to be double-counted on x86_64 Linux.
It looks like StopInfoWatchpoint::ShouldStopSynchronous gets called
twice for a give stop on Linux (not on Darwin).  I had taken out the
"have I been called already" check when I reworked this part of the
code because it didn't seem necessary.  Putting that back in because
it looks like it is on some systems.

Added: 


Modified: 
lldb/source/Target/StopInfo.cpp

Removed: 




diff  --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 5cf0f760aa249..17f6d15059e11 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -754,15 +754,22 @@ class StopInfoWatchpoint : public StopInfo {
   bool ShouldStopSynchronous(Event *event_ptr) override {
 // If we are running our step-over the watchpoint plan, stop if it's done
 // and continue if it's not:
+if (m_should_stop_is_valid)
+  return m_should_stop;
+
+// If we are running our step over plan, then stop here and let the regular
+// ShouldStop figure out what we should do:  Otherwise, give our plan
+// more time to get run:
 if (m_using_step_over_plan)
   return m_step_over_plan_complete;
 
+Log *log = GetLog(LLDBLog::Process);
 ThreadSP thread_sp(m_thread_wp.lock());
 assert(thread_sp);
 WatchpointSP wp_sp(
 
thread_sp->CalculateTarget()->GetWatchpointList().FindByID(GetValue()));
+// If we can no longer find the watchpoint, we just have to stop:
 if (!wp_sp) {
-  Log *log = GetLog(LLDBLog::Process);
 
   LLDB_LOGF(log,
 "Process::%s could not find watchpoint location id: %" PRId64
@@ -796,23 +803,42 @@ class StopInfoWatchpoint : public StopInfo {
 uint32_t num;
 bool wp_triggers_after;
 
-if (process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
+if (!process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
 .Success()) {
-  if (wp_triggers_after)
-return true;
-  
+  m_should_stop_is_valid = true;
+  m_should_stop = true;
+  return m_should_stop;
+}
+
+if (!wp_triggers_after) {
+  // We have to step over the breakpoint before we know what to do:   
   StopInfoWatchpointSP me_as_siwp_sp 
   = std::static_pointer_cast(shared_from_this());
   ThreadPlanSP step_over_wp_sp(new ThreadPlanStepOverWatchpoint(
   *(thread_sp.get()), me_as_siwp_sp, wp_sp));
   Status error;
   error = thread_sp->QueueThreadPlan(step_over_wp_sp, false);
-  m_using_step_over_plan = true;
-  return !error.Success();
+  // If we couldn't push the thread plan, just stop here:
+  if (!error.Success()) {
+LLDB_LOGF(log, "Could not push our step over watchpoint plan: %s", 
+error.AsCString());
+
+m_should_stop = true;
+m_should_stop_is_valid = true;
+return true;
+  } else {
+  // Otherwise, don't set m_should_stop, we don't know that yet.  Just 
+  // say we should continue:
+m_using_step_over_plan = true;
+return false;
+  }
+} else {
+  // We didn't have to do anything special here, so just return our value:
+  m_should_stop_is_valid = true;
+  return m_should_stop;
 }
-// If we don't have to step over the watchpoint, just let the PerformAction
-// determine what we should do.
-return true;
+
+return m_should_stop;
   }
 
   bool ShouldStop(Event *event_ptr) override {



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

[Lldb-commits] [PATCH] D130045: Implement better path matching in FileSpecList::FindFileIndex(...).

2022-07-18 Thread Greg Clayton via Phabricator via lldb-commits

clayborg added a comment.

I added relative/absolute path caching to FileSpec without increasing the size 
of the FileSpec objects.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130045

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

[Lldb-commits] [PATCH] D130045: Implement better path matching in FileSpecList::FindFileIndex(...).

2022-07-18 Thread Greg Clayton via Phabricator via lldb-commits

clayborg updated this revision to Diff 445650.
clayborg added a comment.

Add absolute/relative path caching into FileSpec since we will be using it more 
for breakpoints if this patch gets approved. FileSpecList objects are used by 
the line table classes and the support file lists don't change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130045

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/include/lldb/Utility/FileSpec.h
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Core/FileSpecList.cpp
  lldb/source/Symbol/CompileUnit.cpp
  lldb/source/Utility/FileSpec.cpp
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  lldb/test/API/functionalities/breakpoint/breakpoint_command/relative.yaml
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/FileSpecListTest.cpp

Index: lldb/unittests/Core/FileSpecListTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/FileSpecListTest.cpp
@@ -0,0 +1,58 @@
+//===-- FileSpecListTest.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 "gtest/gtest.h"
+
+#include "lldb/Core/FileSpecList.h"
+
+using namespace lldb_private;
+
+static FileSpec PosixSpec(llvm::StringRef path) {
+  return FileSpec(path, FileSpec::Style::posix);
+}
+
+// static FileSpec WindowsSpec(llvm::StringRef path) {
+//   return FileSpec(path, FileSpec::Style::windows);
+// }
+
+TEST(FileSpecListTest, RelativePathMatches) {
+
+  const FileSpec fullpath = PosixSpec("/build/src/main.cpp");
+  const FileSpec relative = PosixSpec("./src/main.cpp");
+  const FileSpec basename = PosixSpec("./main.cpp");
+  const FileSpec full_wrong = PosixSpec("/other/wrong/main.cpp");
+  const FileSpec rel_wrong = PosixSpec("./wrong/main.cpp");
+
+  FileSpecList files;
+  files.Append(fullpath);
+  files.Append(relative);
+  files.Append(basename);
+  files.Append(full_wrong);
+  files.Append(rel_wrong);
+
+  // Make sure the full path only matches the first entry
+  EXPECT_EQ((size_t)0, files.FindFileIndex(0, fullpath, /*full=*/true));
+  EXPECT_EQ((size_t)1, files.FindFileIndex(1, fullpath, /*full=*/true));
+  EXPECT_EQ((size_t)2, files.FindFileIndex(2, fullpath, /*full=*/true));
+  EXPECT_EQ((size_t)UINT32_MAX,
+files.FindFileIndex(3, fullpath, /*full=*/true));
+  // Make sure the relative path matches the all of the entries that contain
+  // the relative path
+  EXPECT_EQ((size_t)0, files.FindFileIndex(0, relative, /*full=*/true));
+  EXPECT_EQ((size_t)1, files.FindFileIndex(1, relative, /*full=*/true));
+  EXPECT_EQ((size_t)2, files.FindFileIndex(2, relative, /*full=*/true));
+  EXPECT_EQ((size_t)UINT32_MAX,
+files.FindFileIndex(3, relative, /*full=*/true));
+
+  // Make sure looking file a file using the basename matches all entries
+  EXPECT_EQ((size_t)0, files.FindFileIndex(0, basename, /*full=*/true));
+  EXPECT_EQ((size_t)1, files.FindFileIndex(1, basename, /*full=*/true));
+  EXPECT_EQ((size_t)2, files.FindFileIndex(2, basename, /*full=*/true));
+  EXPECT_EQ((size_t)3, files.FindFileIndex(3, basename, /*full=*/true));
+  EXPECT_EQ((size_t)4, files.FindFileIndex(4, basename, /*full=*/true));
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -2,6 +2,7 @@
   CommunicationTest.cpp
   DiagnosticEventTest.cpp
   DumpDataExtractorTest.cpp
+  FileSpecListTest.cpp
   FormatEntityTest.cpp
   MangledTest.cpp
   ModuleSpecTest.cpp
Index: lldb/test/API/functionalities/breakpoint/breakpoint_command/relative.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_command/relative.yaml
@@ -0,0 +1,445 @@
+--- !mach-o
+FileHeader:
+  magic:   0xFEEDFACF
+  cputype: 0x10C
+  cpusubtype:  0x0
+  filetype:0xA
+  ncmds:   7
+  sizeofcmds:  1240
+  flags:   0x0
+  reserved:0x0
+LoadCommands:
+  - cmd: LC_UUID
+cmdsize: 24
+uuid:030921EA-6D76-3A68-B515-386B9AF6D568
+  - cmd: LC_BUILD_VERSION
+cmdsize: 24
+platform:1
+minos:   786432
+sdk: 787200
+ntools:  0
+  - cmd: LC_SYMTAB
+cmdsize: 24
+symoff:  4096
+nsyms:   2
+stroff:  4128
+strsize: 28
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+

[Lldb-commits] [PATCH] D111509: [clang] use getCommonSugar in an assortment of places

2022-07-18 Thread Matheus Izvekov via Phabricator via lldb-commits

mizvekov updated this revision to Diff 445648.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111509

Files:
  clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp
  clang/test/CodeGen/compound-assign-overflow.c
  clang/test/Sema/matrix-type-operators.c
  clang/test/Sema/nullability.c
  clang/test/Sema/sugar-common-types.c
  clang/test/SemaCXX/matrix-type-operators.cpp
  clang/test/SemaCXX/sugar-common-types.cpp
  clang/test/SemaCXX/sugared-auto.cpp
  clang/test/SemaObjC/format-strings-objc.m
  compiler-rt/test/ubsan/TestCases/Integer/add-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/no-recover.cpp
  compiler-rt/test/ubsan/TestCases/Integer/sub-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
  libcxx/DELETE.ME

Index: libcxx/DELETE.ME
===
--- libcxx/DELETE.ME
+++ libcxx/DELETE.ME
@@ -1 +1,2 @@
 D111283
+D111509
Index: compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
@@ -12,12 +12,12 @@
 
 #ifdef SUB_I32
   (void)(uint32_t(1) - uint32_t(2));
-  // CHECK-SUB_I32: usub-overflow.cpp:[[@LINE-1]]:22: runtime error: unsigned integer overflow: 1 - 2 cannot be represented in type 'unsigned int'
+  // CHECK-SUB_I32: usub-overflow.cpp:[[@LINE-1]]:22: runtime error: unsigned integer overflow: 1 - 2 cannot be represented in type '{{uint32_t|unsigned int}}'
 #endif
 
 #ifdef SUB_I64
   (void)(uint64_t(800ll) - uint64_t(900ll));
-  // CHECK-SUB_I64: 800 - 900 cannot be represented in type 'unsigned {{long( long)?}}'
+  // CHECK-SUB_I64: 800 - 900 cannot be represented in type '{{uint64_t|unsigned long( long)?}}'
 #endif
 
 #ifdef SUB_I128
@@ -26,6 +26,6 @@
 # else
   puts("__int128 not supported\n");
 # endif
-  // CHECK-SUB_I128: {{0x4000 - 0x8000 cannot be represented in type 'unsigned __int128'|__int128 not supported}}
+  // CHECK-SUB_I128: {{0x4000 - 0x8000 cannot be represented in type '__uint128_t'|__int128 not supported}}
 #endif
 }
Index: compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
@@ -13,7 +13,7 @@
   (void)(uint16_t(0x) * uint16_t(0x8001));
 
   (void)(uint32_t(0x) * uint32_t(0x2));
-  // CHECK: umul-overflow.cpp:15:31: runtime error: unsigned integer overflow: 4294967295 * 2 cannot be represented in type 'unsigned int'
+  // CHECK: umul-overflow.cpp:15:31: runtime error: unsigned integer overflow: 4294967295 * 2 cannot be represented in type '{{uint32_t|unsigned int}}'
 
   return 0;
 }
Index: compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
@@ -18,7 +18,7 @@
 
 #ifdef ADD_I64
   (void)(uint64_t(1000ull) + uint64_t(900ull));
-  // CHECK-ADD_I64: 1000 + 900 cannot be represented in type 'unsigned {{long( long)?}}'
+  // CHECK-ADD_I64: 1000 + 900 cannot be represented in type '{{uint64_t|unsigned long( long)?}}'
 #endif
 
 #ifdef ADD_I128
@@ -27,6 +27,6 @@
 # else
   puts("__int128 not supported");
 # endif
-  // CHECK-ADD_I128: {{0x8000 \+ 0x8000 cannot be represented in type 'unsigned __int128'|__int128 not supported}}
+  // CHECK-ADD_I128: {{0x8000 \+ 0x8000 cannot be represented in type '__uint128_t'|__int128 not supported}}
 #endif
 }
Index: compiler-rt/test/ubsan/TestCases/Integer/sub-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/sub-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/sub-overflow.cpp
@@ -12,12 +12,12 @@
 
 #ifdef SUB_I32
   (void)(int32_t(-2) - int32_t(0x7fff));
-  // CHECK-SUB_I32: sub-overflow.cpp:[[@LINE-1]]:22: runtime error: signed

[Lldb-commits] [PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-07-18 Thread Matheus Izvekov via Phabricator via lldb-commits

mizvekov updated this revision to Diff 445646.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

Files:
  clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp
  clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.cpp
  clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
  clang-tools-extra/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
  clang-tools-extra/clang-tidy/misc/MisplacedConstCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/DumpASTTests.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/copy-constructor-init.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/shared-ptr-array-mismatch.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-memory-comparison-32bits.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-memory-comparison.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
  clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp
  clang/bindings/python/tests/cindex/test_type.py
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/lib/ARCMigrate/ObjCMT.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/FormatString.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/QualTypeNames.cpp
  clang/lib/AST/ScanfFormatString.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Analysis/RetainSummaryManager.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Sema/TypeLocBuilder.cpp
  clang/lib/Sema/TypeLocBuilder.h
  clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
  clang/test/AST/ast-dump-APValue-anon-union.cpp
  clang/test/AST/ast-dump-APValue-struct.cpp
  clang/test/AST/ast-dump-APValue-union.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-expr-json.cpp
  clang/test/AST/ast-dump-expr.cpp
  clang/test/AST/ast-dump-funcs.cpp
  clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp
  clang/test/AST/ast-dump-overloaded-operators.cpp
  clang/test/AST/ast-dump-records-json.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/AST/ast-dump-stmt-json.cpp
  clang/test/AST/ast-dump-stmt.cpp
  clang/test/AST/ast-dump-template-decls-json.cpp
  clang/test/AST/ast-dump-temporaries-json.cpp
  clang/test/AST/ast-dump-using-template.cpp
  clang/test/AST/ast-dump-using.cpp
  clang/test/AST/coroutine-locals-cleanup-exp-namespace.cpp
  clang/test/AST/coroutine-locals-cleanup.cpp
  clang/test/AST/float16.cpp
  clang/test/AST/sourceranges.cpp
  clang/test/Analysis/Inputs/expected-plists/NewDelete-path-notes.cpp.plist
  clang/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist
  clang/test/Analysis/Inputs/expected-plists/method-call-path-notes.cpp.plist
  clang/test/Analysis/analyzer-display-progress.cpp
  clang/test/Analysis/auto-obj-dtors-cfg-output.cpp
  clang/test/Analysis/blocks.mm
  clang/test/Analysis/bug_hash_test.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/cfg-rich-constructors.cpp
  clang/test/Analysis/cfg-rich-constructors.mm
  clang/test/Analysis/cfg.cpp
  clang/test/Analysis/copy-elision.cpp
  clang/test/Analysis/cxx-uninitialized-object-inheritance.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/exploded-graph-rewriter/dynamic_types.cpp
  clang/test/Analysis/initializers-cfg-output.cpp
  clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist
  clang/test/Analysis/lambdas.cpp
  clang/test/Analysis/lifetime-cfg-output.cpp

[Lldb-commits] [PATCH] D129377: [lldb/Fuzzer] Add fuzzer for expression evaluator

2022-07-18 Thread Jonas Devlieghere via Phabricator via lldb-commits

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

A few nits about naming, but otherwise this LGTM.




Comment at: 
lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp:47
+DEFINE_BINARY_PROTO_FUZZER(const clang_fuzzer::Function ) {
+  auto S = clang_fuzzer::FunctionToString(input);
+





Comment at: 
lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp:51-52
+  // This will be used as the path for the object file to create a target from
+  std::string rawpath = originalargv[2];
+  StringRef objpath = rawpath.erase(0, 2);
+





Comment at: 
lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp:59
+  // Create a breakpoint on the only line in the program
+  SBBreakpoint bp = target.BreakpointCreateByLocation(objpath.str().c_str(), 
1);
+





Comment at: 
lldb/tools/lldb-fuzzer/lldb-expression-fuzzer/lldb-expression-fuzzer.cpp:62
+  // Create launch info and error for launching the process
+  SBLaunchInfo li = target.GetLaunchInfo();
+  SBError error;




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

https://reviews.llvm.org/D129377

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

[Lldb-commits] [PATCH] D111509: [clang] use getCommonSugar in an assortment of places

2022-07-18 Thread Matheus Izvekov via Phabricator via lldb-commits

mizvekov updated this revision to Diff 445643.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111509

Files:
  clang-tools-extra/clangd/unittests/tweaks/ExtractVariableTests.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/narrowing-conversions-ignoreconversionfromtypes-option.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/AST/ast-dump-fpfeatures.cpp
  clang/test/CodeGen/compound-assign-overflow.c
  clang/test/Sema/matrix-type-operators.c
  clang/test/Sema/nullability.c
  clang/test/Sema/sugar-common-types.c
  clang/test/SemaCXX/matrix-type-operators.cpp
  clang/test/SemaCXX/sugar-common-types.cpp
  clang/test/SemaCXX/sugared-auto.cpp
  clang/test/SemaObjC/format-strings-objc.m
  compiler-rt/test/ubsan/TestCases/Integer/add-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/no-recover.cpp
  compiler-rt/test/ubsan/TestCases/Integer/sub-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
  compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
  libcxx/DELETE.ME

Index: libcxx/DELETE.ME
===
--- libcxx/DELETE.ME
+++ libcxx/DELETE.ME
@@ -1 +1,2 @@
 D111283
+D111509
Index: compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/usub-overflow.cpp
@@ -12,12 +12,12 @@
 
 #ifdef SUB_I32
   (void)(uint32_t(1) - uint32_t(2));
-  // CHECK-SUB_I32: usub-overflow.cpp:[[@LINE-1]]:22: runtime error: unsigned integer overflow: 1 - 2 cannot be represented in type 'unsigned int'
+  // CHECK-SUB_I32: usub-overflow.cpp:[[@LINE-1]]:22: runtime error: unsigned integer overflow: 1 - 2 cannot be represented in type '{{uint32_t|unsigned int}}'
 #endif
 
 #ifdef SUB_I64
   (void)(uint64_t(800ll) - uint64_t(900ll));
-  // CHECK-SUB_I64: 800 - 900 cannot be represented in type 'unsigned {{long( long)?}}'
+  // CHECK-SUB_I64: 800 - 900 cannot be represented in type '{{uint64_t|unsigned long( long)?}}'
 #endif
 
 #ifdef SUB_I128
@@ -26,6 +26,6 @@
 # else
   puts("__int128 not supported\n");
 # endif
-  // CHECK-SUB_I128: {{0x4000 - 0x8000 cannot be represented in type 'unsigned __int128'|__int128 not supported}}
+  // CHECK-SUB_I128: {{0x4000 - 0x8000 cannot be represented in type '__uint128_t'|__int128 not supported}}
 #endif
 }
Index: compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/umul-overflow.cpp
@@ -13,7 +13,7 @@
   (void)(uint16_t(0x) * uint16_t(0x8001));
 
   (void)(uint32_t(0x) * uint32_t(0x2));
-  // CHECK: umul-overflow.cpp:15:31: runtime error: unsigned integer overflow: 4294967295 * 2 cannot be represented in type 'unsigned int'
+  // CHECK: umul-overflow.cpp:15:31: runtime error: unsigned integer overflow: 4294967295 * 2 cannot be represented in type '{{uint32_t|unsigned int}}'
 
   return 0;
 }
Index: compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/uadd-overflow.cpp
@@ -18,7 +18,7 @@
 
 #ifdef ADD_I64
   (void)(uint64_t(1000ull) + uint64_t(900ull));
-  // CHECK-ADD_I64: 1000 + 900 cannot be represented in type 'unsigned {{long( long)?}}'
+  // CHECK-ADD_I64: 1000 + 900 cannot be represented in type '{{uint64_t|unsigned long( long)?}}'
 #endif
 
 #ifdef ADD_I128
@@ -27,6 +27,6 @@
 # else
   puts("__int128 not supported");
 # endif
-  // CHECK-ADD_I128: {{0x8000 \+ 0x8000 cannot be represented in type 'unsigned __int128'|__int128 not supported}}
+  // CHECK-ADD_I128: {{0x8000 \+ 0x8000 cannot be represented in type '__uint128_t'|__int128 not supported}}
 #endif
 }
Index: compiler-rt/test/ubsan/TestCases/Integer/sub-overflow.cpp
===
--- compiler-rt/test/ubsan/TestCases/Integer/sub-overflow.cpp
+++ compiler-rt/test/ubsan/TestCases/Integer/sub-overflow.cpp
@@ -12,12 +12,12 @@
 
 #ifdef SUB_I32
   (void)(int32_t(-2) - int32_t(0x7fff));
-  // CHECK-SUB_I32: sub-overflow.cpp:[[@LINE-1]]:22: runtime error: signed

[Lldb-commits] [PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

2022-07-18 Thread Cole Kissane via Phabricator via lldb-commits

ckissane added a comment.

In D128465#3661049 , @MaskRay wrote:

> Does this address the macOS build failure?

I believe so!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465

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

[Lldb-commits] [PATCH] D130045: Implement better path matching in FileSpecList::FindFileIndex(...).

2022-07-18 Thread Greg Clayton via Phabricator via lldb-commits

clayborg updated this revision to Diff 445638.
clayborg added a comment.
Herald added a subscriber: mgorny.

Didn't submit all changes at first because I had two commits. Squashed the 
commits to fix the diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130045

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Core/FileSpecList.cpp
  lldb/source/Symbol/CompileUnit.cpp
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  lldb/test/API/functionalities/breakpoint/breakpoint_command/relative.yaml
  lldb/unittests/Core/CMakeLists.txt
  lldb/unittests/Core/FileSpecListTest.cpp

Index: lldb/unittests/Core/FileSpecListTest.cpp
===
--- /dev/null
+++ lldb/unittests/Core/FileSpecListTest.cpp
@@ -0,0 +1,58 @@
+//===-- FileSpecListTest.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 "gtest/gtest.h"
+
+#include "lldb/Core/FileSpecList.h"
+
+using namespace lldb_private;
+
+static FileSpec PosixSpec(llvm::StringRef path) {
+  return FileSpec(path, FileSpec::Style::posix);
+}
+
+// static FileSpec WindowsSpec(llvm::StringRef path) {
+//   return FileSpec(path, FileSpec::Style::windows);
+// }
+
+TEST(FileSpecListTest, RelativePathMatches) {
+
+  const FileSpec fullpath = PosixSpec("/build/src/main.cpp");
+  const FileSpec relative = PosixSpec("./src/main.cpp");
+  const FileSpec basename = PosixSpec("./main.cpp");
+  const FileSpec full_wrong = PosixSpec("/other/wrong/main.cpp");
+  const FileSpec rel_wrong = PosixSpec("./wrong/main.cpp");
+
+  FileSpecList files;
+  files.Append(fullpath);
+  files.Append(relative);
+  files.Append(basename);
+  files.Append(full_wrong);
+  files.Append(rel_wrong);
+
+  // Make sure the full path only matches the first entry
+  EXPECT_EQ((size_t)0, files.FindFileIndex(0, fullpath, /*full=*/true));
+  EXPECT_EQ((size_t)1, files.FindFileIndex(1, fullpath, /*full=*/true));
+  EXPECT_EQ((size_t)2, files.FindFileIndex(2, fullpath, /*full=*/true));
+  EXPECT_EQ((size_t)UINT32_MAX,
+files.FindFileIndex(3, fullpath, /*full=*/true));
+  // Make sure the relative path matches the all of the entries that contain
+  // the relative path
+  EXPECT_EQ((size_t)0, files.FindFileIndex(0, relative, /*full=*/true));
+  EXPECT_EQ((size_t)1, files.FindFileIndex(1, relative, /*full=*/true));
+  EXPECT_EQ((size_t)2, files.FindFileIndex(2, relative, /*full=*/true));
+  EXPECT_EQ((size_t)UINT32_MAX,
+files.FindFileIndex(3, relative, /*full=*/true));
+
+  // Make sure looking file a file using the basename matches all entries
+  EXPECT_EQ((size_t)0, files.FindFileIndex(0, basename, /*full=*/true));
+  EXPECT_EQ((size_t)1, files.FindFileIndex(1, basename, /*full=*/true));
+  EXPECT_EQ((size_t)2, files.FindFileIndex(2, basename, /*full=*/true));
+  EXPECT_EQ((size_t)3, files.FindFileIndex(3, basename, /*full=*/true));
+  EXPECT_EQ((size_t)4, files.FindFileIndex(4, basename, /*full=*/true));
+}
Index: lldb/unittests/Core/CMakeLists.txt
===
--- lldb/unittests/Core/CMakeLists.txt
+++ lldb/unittests/Core/CMakeLists.txt
@@ -2,6 +2,7 @@
   CommunicationTest.cpp
   DiagnosticEventTest.cpp
   DumpDataExtractorTest.cpp
+  FileSpecListTest.cpp
   FormatEntityTest.cpp
   MangledTest.cpp
   ModuleSpecTest.cpp
Index: lldb/test/API/functionalities/breakpoint/breakpoint_command/relative.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_command/relative.yaml
@@ -0,0 +1,445 @@
+--- !mach-o
+FileHeader:
+  magic:   0xFEEDFACF
+  cputype: 0x10C
+  cpusubtype:  0x0
+  filetype:0xA
+  ncmds:   7
+  sizeofcmds:  1240
+  flags:   0x0
+  reserved:0x0
+LoadCommands:
+  - cmd: LC_UUID
+cmdsize: 24
+uuid:030921EA-6D76-3A68-B515-386B9AF6D568
+  - cmd: LC_BUILD_VERSION
+cmdsize: 24
+platform:1
+minos:   786432
+sdk: 787200
+ntools:  0
+  - cmd: LC_SYMTAB
+cmdsize: 24
+symoff:  4096
+nsyms:   2
+stroff:  4128
+strsize: 28
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __PAGEZERO
+vmaddr:  0
+vmsize:  4294967296
+fileoff: 0
+filesize:0
+maxprot: 0
+

[Lldb-commits] [PATCH] D130045: Implement better path matching in FileSpecList::FindFileIndex(...).

2022-07-18 Thread Greg Clayton via Phabricator via lldb-commits

clayborg created this revision.
clayborg added reviewers: labath, JDevlieghere, jingham, aadsm, yinghuitan.
Herald added a subscriber: arphaman.
Herald added a project: All.
clayborg requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: lldb-commits, sstefan1.
Herald added a project: LLDB.

Currently a FileSpecList will only match the specified FileSpec if:

- it has filename and directory and both match exactly
- if has a filename only and any filename in the list matches

Because of this, we modify our breakpoint resolving so it can handle relative 
paths by doing some extra code that removes the directory from the FileSpec 
when searching if the path is relative.

This patch is intended to fix breakpoints so they work as users expect them to 
by adding the following features:

- allow matches to relative paths in the file list to match as long as the 
relative path is at the end of the specified path
- allow matches to paths to match if the specified path is relative and shorter 
than the file paths in the list

This allows us to remove the extra logic from BreakpointResolverFileLine.cpp 
that added support for setting breakpoints with relative paths.

This means we can still set breakpoints with relative paths when the debug info 
contains full paths. We add the ability to set breakpoints with full paths when 
the debug info contains relative paths.

Debug info contains "./a/b/c/main.cpp", the following will set breakpoints 
successfully:

- /build/a/b/c/main.cpp
- a/b/c/main.cpp
- b/c/main.cpp
- c/main.cpp
- main.cpp
- ./c/main.cpp
- ./a/b/c/main.cpp
- ./b/c/main.cpp
- ./main.cpp

This also means that all users of FileSpecList::FindFileIndex(...) will get 
this extra matching, which currently is used for matching modules, but I 
believe we want this extra matching ability there as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130045

Files:
  lldb/include/lldb/Breakpoint/BreakpointResolverFileLine.h
  lldb/source/Breakpoint/BreakpointResolverFileLine.cpp
  lldb/source/Core/FileSpecList.cpp
  
lldb/test/API/functionalities/breakpoint/breakpoint_command/TestBreakpointCommand.py
  lldb/test/API/functionalities/breakpoint/breakpoint_command/relative.yaml

Index: lldb/test/API/functionalities/breakpoint/breakpoint_command/relative.yaml
===
--- /dev/null
+++ lldb/test/API/functionalities/breakpoint/breakpoint_command/relative.yaml
@@ -0,0 +1,445 @@
+--- !mach-o
+FileHeader:
+  magic:   0xFEEDFACF
+  cputype: 0x10C
+  cpusubtype:  0x0
+  filetype:0xA
+  ncmds:   7
+  sizeofcmds:  1240
+  flags:   0x0
+  reserved:0x0
+LoadCommands:
+  - cmd: LC_UUID
+cmdsize: 24
+uuid:030921EA-6D76-3A68-B515-386B9AF6D568
+  - cmd: LC_BUILD_VERSION
+cmdsize: 24
+platform:1
+minos:   786432
+sdk: 787200
+ntools:  0
+  - cmd: LC_SYMTAB
+cmdsize: 24
+symoff:  4096
+nsyms:   2
+stroff:  4128
+strsize: 28
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __PAGEZERO
+vmaddr:  0
+vmsize:  4294967296
+fileoff: 0
+filesize:0
+maxprot: 0
+initprot:0
+nsects:  0
+flags:   0
+  - cmd: LC_SEGMENT_64
+cmdsize: 232
+segname: __TEXT
+vmaddr:  4294967296
+vmsize:  16384
+fileoff: 0
+filesize:0
+maxprot: 5
+initprot:5
+nsects:  2
+flags:   0
+Sections:
+  - sectname:__text
+segname: __TEXT
+addr:0x13F94
+size:36
+offset:  0x0
+align:   2
+reloff:  0x0
+nreloc:  0
+flags:   0x8400
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+content: CFFAEDFE0C010A000700D8041B00
+  - sectname:__unwind_info
+segname: __TEXT
+addr:0x13FB8
+size:72
+offset:  0x0
+align:   2
+reloff:  0x0
+nreloc:  0
+flags:   0x0
+reserved1:   0x0
+reserved2:   0x0
+reserved3:   0x0
+content: CFFAEDFE0C010A000700D8041B001800030921EA6D763A68B515386B9AF6D5683200180001000C00
+  - cmd: LC_SEGMENT_64
+cmdsize: 72
+segname: __LINKEDIT
+vmaddr:  4294983680
+vmsize:  4096
+

[Lldb-commits] [PATCH] D129338: Tell the user which pathname was invalid...

2022-07-18 Thread Jim Ingham via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe83d47f6b7bd: When the module path for `command script 
import` is invalid, echo the path. (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129338

Files:
  lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
  lldb/test/API/commands/command/script/import/TestImport.py


Index: lldb/test/API/commands/command/script/import/TestImport.py
===
--- lldb/test/API/commands/command/script/import/TestImport.py
+++ lldb/test/API/commands/command/script/import/TestImport.py
@@ -38,12 +38,13 @@
 self.runCmd("command script import ./foo/bar/foobar.py --allow-reload")
 self.runCmd("command script import ./bar/bar.py --allow-reload")
 
+self.expect("command script import ''",
+error=True, startstr="error: module importing failed: 
empty path")
 self.expect("command script import ./nosuchfile.py",
-error=True, startstr='error: module importing failed')
+error=True, startstr="error: module importing failed: 
invalid pathname './nosuchfile.py'")
 self.expect("command script import ./nosuchfolder/",
-error=True, startstr='error: module importing failed')
+error=True, startstr="error: module importing failed: 
invalid pathname './nosuchfolder/'")
 self.expect("command script import ./foo/foo.py", error=False)
-
 self.runCmd("command script import --allow-reload ./thepackage")
 self.expect("TPcommandA", substrs=["hello world A"])
 self.expect("TPcommandB", substrs=["hello world B"])
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2637,7 +2637,7 @@
  .SetSetLLDBGlobals(false);
 
   if (!pathname || !pathname[0]) {
-error.SetErrorString("invalid pathname");
+error.SetErrorString("empty path");
 return false;
   }
 
@@ -2707,14 +2707,14 @@
   // if not a valid file of any sort, check if it might be a filename still
   // dot can't be used but / and \ can, and if either is found, reject
   if (strchr(pathname, '\\') || strchr(pathname, '/')) {
-error.SetErrorString("invalid pathname");
+error.SetErrorStringWithFormatv("invalid pathname '{0}'", pathname);
 return false;
   }
   // Not a filename, probably a package of some sort, let it go through.
   possible_package = true;
 } else if (is_directory(st) || is_regular_file(st)) {
   if (module_file.GetDirectory().IsEmpty()) {
-error.SetErrorString("invalid directory name");
+error.SetErrorStringWithFormatv("invalid directory name '{0}'", 
pathname);
 return false;
   }
   if (llvm::Error e =


Index: lldb/test/API/commands/command/script/import/TestImport.py
===
--- lldb/test/API/commands/command/script/import/TestImport.py
+++ lldb/test/API/commands/command/script/import/TestImport.py
@@ -38,12 +38,13 @@
 self.runCmd("command script import ./foo/bar/foobar.py --allow-reload")
 self.runCmd("command script import ./bar/bar.py --allow-reload")
 
+self.expect("command script import ''",
+error=True, startstr="error: module importing failed: empty path")
 self.expect("command script import ./nosuchfile.py",
-error=True, startstr='error: module importing failed')
+error=True, startstr="error: module importing failed: invalid pathname './nosuchfile.py'")
 self.expect("command script import ./nosuchfolder/",
-error=True, startstr='error: module importing failed')
+error=True, startstr="error: module importing failed: invalid pathname './nosuchfolder/'")
 self.expect("command script import ./foo/foo.py", error=False)
-
 self.runCmd("command script import --allow-reload ./thepackage")
 self.expect("TPcommandA", substrs=["hello world A"])
 self.expect("TPcommandB", substrs=["hello world B"])
Index: lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===
--- lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2637,7 +2637,7 @@
  .SetSetLLDBGlobals(false);
 
   if (!pathname || !pathname[0]) {
-

[Lldb-commits] [lldb] e83d47f - When the module path for `command script import` is invalid, echo the path.

2022-07-18 Thread Jim Ingham via lldb-commits


Author: Jim Ingham
Date: 2022-07-18T14:49:07-07:00
New Revision: e83d47f6b7bd2f8dcf419e03b3a00d5db5bd61c4

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

LOG: When the module path for `command script import` is invalid, echo the path.

We were just emitting "invalid module" w/o saying which module.  That's
not particularly helpful.

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

Added: 


Modified: 
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/test/API/commands/command/script/import/TestImport.py

Removed: 




diff  --git 
a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp 
b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 53e2bcaccafbf..a21adcfbdbd60 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2637,7 +2637,7 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule(
  .SetSetLLDBGlobals(false);
 
   if (!pathname || !pathname[0]) {
-error.SetErrorString("invalid pathname");
+error.SetErrorString("empty path");
 return false;
   }
 
@@ -2707,14 +2707,14 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule(
   // if not a valid file of any sort, check if it might be a filename still
   // dot can't be used but / and \ can, and if either is found, reject
   if (strchr(pathname, '\\') || strchr(pathname, '/')) {
-error.SetErrorString("invalid pathname");
+error.SetErrorStringWithFormatv("invalid pathname '{0}'", pathname);
 return false;
   }
   // Not a filename, probably a package of some sort, let it go through.
   possible_package = true;
 } else if (is_directory(st) || is_regular_file(st)) {
   if (module_file.GetDirectory().IsEmpty()) {
-error.SetErrorString("invalid directory name");
+error.SetErrorStringWithFormatv("invalid directory name '{0}'", 
pathname);
 return false;
   }
   if (llvm::Error e =

diff  --git a/lldb/test/API/commands/command/script/import/TestImport.py 
b/lldb/test/API/commands/command/script/import/TestImport.py
index ff1fd145ce395..6f3e61b0b45ea 100644
--- a/lldb/test/API/commands/command/script/import/TestImport.py
+++ b/lldb/test/API/commands/command/script/import/TestImport.py
@@ -38,12 +38,13 @@ def cleanup():
 self.runCmd("command script import ./foo/bar/foobar.py --allow-reload")
 self.runCmd("command script import ./bar/bar.py --allow-reload")
 
+self.expect("command script import ''",
+error=True, startstr="error: module importing failed: 
empty path")
 self.expect("command script import ./nosuchfile.py",
-error=True, startstr='error: module importing failed')
+error=True, startstr="error: module importing failed: 
invalid pathname './nosuchfile.py'")
 self.expect("command script import ./nosuchfolder/",
-error=True, startstr='error: module importing failed')
+error=True, startstr="error: module importing failed: 
invalid pathname './nosuchfolder/'")
 self.expect("command script import ./foo/foo.py", error=False)
-
 self.runCmd("command script import --allow-reload ./thepackage")
 self.expect("TPcommandA", substrs=["hello world A"])
 self.expect("TPcommandB", substrs=["hello world B"])



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

[Lldb-commits] [PATCH] D129814: Fix stepping over watchpoints in architectures that raise the exception before executing the instruction

2022-07-18 Thread Jim Ingham via Phabricator via lldb-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rG5778ada8e54e: Make hit point counts reliable for 
architectures that stop before evaluation. (authored by jingham).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129814

Files:
  lldb/include/lldb/Breakpoint/Watchpoint.h
  lldb/include/lldb/Target/StopInfo.h
  lldb/source/Target/StopInfo.cpp
  
lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatchBreak.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py

Index: lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py
===
--- lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py
+++ lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py
@@ -12,10 +12,6 @@
 # Atomic sequences are not supported yet for MIPS in LLDB.
 @skipIf(triple='^mips')
 @add_test_categories(["watchpoint"])
-@skipIf(
-oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-bugnumber="rdar://93863107")
 
 def test(self):
 """Test watchpoint and a breakpoint in multiple threads."""
Index: lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
===
--- lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
+++ lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
@@ -12,10 +12,6 @@
 # Atomic sequences are not supported yet for MIPS in LLDB.
 @skipIf(triple='^mips')
 @expectedFailureNetBSD
-@skipIf(
-oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-bugnumber="rdar://93863107")
 @add_test_categories(["watchpoint"])
 def test(self):
 """Test two threads that trigger a watchpoint and one signal thread. """
Index: lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
===
--- lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
+++ lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
@@ -11,10 +11,6 @@
 
 # Atomic sequences are not supported yet for MIPS in LLDB.
 @skipIf(triple='^mips')
-@skipIf(
-oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-bugnumber="rdar://93863107")
 @add_test_categories(["watchpoint"])
 def test(self):
 """Test two threads that trigger a watchpoint and one (1 second delay) breakpoint thread. """
Index: lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
===
--- lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
+++ lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
@@ -11,10 +11,6 @@
 
 # Atomic sequences are not supported yet for MIPS in LLDB.
 @skipIf(triple='^mips')
-@skipIf(
-oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-bugnumber="rdar://93863107")
 @add_test_categories(["watchpoint"])
 def test(self):
 """Test two threads that trigger a watchpoint and one breakpoint thread. """
Index: lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py

[Lldb-commits] [lldb] 5778ada - Make hit point counts reliable for architectures that stop before evaluation.

2022-07-18 Thread Jim Ingham via lldb-commits


Author: Jim Ingham
Date: 2022-07-18T14:36:32-07:00
New Revision: 5778ada8e54edb2bc2869505b88a959d1915c02f

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

LOG: Make hit point counts reliable for architectures that stop before 
evaluation.

Since we want to present the "new & old" values for watchpoint hits, on 
architectures,
including the ARM family, that stop before the triggering instruction is run, 
we need
to single step over the instruction before stopping for realz.  This was 
incorrectly
done directly in the StopInfoWatchpoint::ShouldStop.  That causes problems if 
more than
one thread stops "for a reason" at the same time as the watchpoint, since the 
other actions
didn't expect the process to make progress in this part of the execution 
control machinery.

The correct way to do this is to schedule the step over using ThreadPlans, and 
then to restore
the stop info after that plan stops, so that the rest of the stop info actions 
can happen when
all the other threads have handled their immediate actions as well.

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

Added: 


Modified: 
lldb/include/lldb/Breakpoint/Watchpoint.h
lldb/include/lldb/Target/StopInfo.h
lldb/source/Target/StopInfo.cpp

lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatchBreak.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py

lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py

Removed: 




diff  --git a/lldb/include/lldb/Breakpoint/Watchpoint.h 
b/lldb/include/lldb/Breakpoint/Watchpoint.h
index 41b723a66b6a3..7137b6e4a24c5 100644
--- a/lldb/include/lldb/Breakpoint/Watchpoint.h
+++ b/lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -157,12 +157,15 @@ class Watchpoint : public 
std::enable_shared_from_this,
 private:
   friend class Target;
   friend class WatchpointList;
+  friend class StopInfoWatchpoint; // This needs to call UndoHitCount()
 
   void ResetHistoricValues() {
 m_old_value_sp.reset();
 m_new_value_sp.reset();
   }
 
+  void UndoHitCount() { m_hit_counter.Decrement(); }
+
   Target _target;
   bool m_enabled;   // Is this watchpoint enabled
   bool m_is_hardware;   // Is this a hardware watchpoint

diff  --git a/lldb/include/lldb/Target/StopInfo.h 
b/lldb/include/lldb/Target/StopInfo.h
index cdb906dcd7ede..9527a6ea553e3 100644
--- a/lldb/include/lldb/Target/StopInfo.h
+++ b/lldb/include/lldb/Target/StopInfo.h
@@ -17,7 +17,7 @@
 
 namespace lldb_private {
 
-class StopInfo {
+class StopInfo : public std::enable_shared_from_this {
   friend class Process::ProcessEventData;
   friend class ThreadPlanBase;
 

diff  --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 00d30070c8c9f..5cf0f760aa249 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -20,6 +20,7 @@
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadPlan.h"
+#include "lldb/Target/ThreadPlanStepInstruction.h"
 #include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
@@ -690,40 +691,128 @@ class StopInfoWatchpoint : public StopInfo {
   }
 
 protected:
+  using StopInfoWatchpointSP = std::shared_ptr;
+  // This plan is used to orchestrate stepping over the watchpoint for
+  // architectures (e.g. ARM) that report the watch before running the watched
+  // access.  This is the sort of job you have to defer to the thread plans,
+  // if you try to do it directly in the stop info and there are other threads
+  // that needed to process this stop you will have yanked control away from
+  // them and they won't behave correctly.
+  class ThreadPlanStepOverWatchpoint : public ThreadPlanStepInstruction {
+  public:
+ThreadPlanStepOverWatchpoint(Thread , 
+

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

2022-07-18 Thread Alex Langford via Phabricator via lldb-commits

bulbazord updated this revision to Diff 445629.
bulbazord edited the summary of this revision.
bulbazord added a comment.

I have factored out part of `Module::LookupInfo::Prune` into its own method 
`Module::LookupInfo::NameMatchesLookupInfo` which I re-use in 
`ProcessFunctionDIE` instead of copying code.

I also have switched to taking the mangled name of a DIE and demangled it to 
get the fully qualified name. This doesn't seem to have regressed performance 
at all, so I'm happy to go with this moving forward. I've deleted the 
`GetQualifiedNameWithParams` I had written previously since it shouldn't be 
needed anymore.


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

https://reviews.llvm.org/D129682

Files:
  lldb/include/lldb/Core/Module.h
  lldb/include/lldb/Symbol/SymbolFile.h
  lldb/include/lldb/Symbol/SymbolFileOnDemand.h
  lldb/source/Breakpoint/BreakpointResolverName.cpp
  lldb/source/Core/Module.cpp
  lldb/source/Core/ModuleList.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp
  lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
  lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
  lldb/source/Symbol/SymbolFile.cpp
  lldb/source/Symbol/SymbolFileOnDemand.cpp
  lldb/tools/lldb-test/lldb-test.cpp

Index: lldb/tools/lldb-test/lldb-test.cpp
===
--- lldb/tools/lldb-test/lldb-test.cpp
+++ lldb/tools/lldb-test/lldb-test.cpp
@@ -459,8 +459,9 @@
 ContextOr->IsValid() ? *ContextOr : CompilerDeclContext();
 
 List.Clear();
-Symfile.FindFunctions(ConstString(Name), ContextPtr, getFunctionNameFlags(),
- true, List);
+Module::LookupInfo lookup_info(ConstString(Name), getFunctionNameFlags(),
+   eLanguageTypeUnknown);
+Symfile.FindFunctions(lookup_info, ContextPtr, true, List);
   }
   outs() << formatv("Found {0} functions:\n", List.GetSize());
   StreamString Stream;
Index: lldb/source/Symbol/SymbolFileOnDemand.cpp
===
--- lldb/source/Symbol/SymbolFileOnDemand.cpp
+++ lldb/source/Symbol/SymbolFileOnDemand.cpp
@@ -375,9 +375,11 @@
 }
 
 void SymbolFileOnDemand::FindFunctions(
-ConstString name, const CompilerDeclContext _decl_ctx,
-FunctionNameType name_type_mask, bool include_inlines,
+const Module::LookupInfo _info,
+const CompilerDeclContext _decl_ctx, bool include_inlines,
 SymbolContextList _list) {
+  ConstString name = lookup_info.GetLookupName();
+  FunctionNameType name_type_mask = lookup_info.GetNameTypeMask();
   if (!m_debug_info_enabled) {
 Log *log = GetLog();
 
@@ -402,7 +404,7 @@
 // allow the FindFucntions to go through.
 SetLoadDebugInfoEnabled();
   }
-  return m_sym_file_impl->FindFunctions(name, parent_decl_ctx, name_type_mask,
+  return m_sym_file_impl->FindFunctions(lookup_info, parent_decl_ctx,
 include_inlines, sc_list);
 }
 
Index: lldb/source/Symbol/SymbolFile.cpp
===
--- lldb/source/Symbol/SymbolFile.cpp
+++ lldb/source/Symbol/SymbolFile.cpp
@@ -120,9 +120,8 @@
  uint32_t max_matches,
  VariableList ) {}
 
-void SymbolFile::FindFunctions(ConstString name,
+void SymbolFile::FindFunctions(const Module::LookupInfo _info,
const CompilerDeclContext _decl_ctx,
-   lldb::FunctionNameType name_type_mask,
bool include_inlines,
SymbolContextList _list) {}
 
Index: lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
===
--- lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
+++ lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.h
@@ -119,9 +119,8 @@
uint32_t max_matches,

[Lldb-commits] [PATCH] D130037: [lldb] [gdb-remote] Fix process ID after following forked child

2022-07-18 Thread Michał Górny via Phabricator via lldb-commits

mgorny created this revision.
mgorny added reviewers: labath, krytarowski, emaste, jingham.
Herald added a subscriber: arichardson.
Herald added a project: All.
mgorny requested review of this revision.

Update the process ID after handling fork/vfork to ensure that
the process plugin reports the correct PID immediately.

Sponsored by: The FreeBSD Foundation


https://reviews.llvm.org/D130037

Files:
  lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  lldb/test/API/functionalities/gdb_remote_client/TestFork.py


Index: lldb/test/API/functionalities/gdb_remote_client/TestFork.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestFork.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestFork.py
@@ -9,7 +9,7 @@
 
 class TestMultiprocess(GDBRemoteTestBase):
 
-def base_test(self, variant):
+def base_test(self, variant, follow_child=False):
 class MyResponder(MockGDBServerResponder):
 def __init__(self):
 super().__init__()
@@ -43,12 +43,24 @@
   self.runCmd("log enable gdb-remote packets")
   self.addTearDownHook(
 lambda: self.runCmd("log disable gdb-remote packets"))
+if follow_child:
+self.runCmd("settings set target.process.follow-fork-mode child")
 process = self.connect(target)
+self.assertEqual(process.GetProcessID(), 1024)
 process.Continue()
-self.assertRegex(self.server.responder.detached, r"D;0*401")
+self.assertRegex(self.server.responder.detached,
+ r"D;0*400" if follow_child else r"D;0*401")
+self.assertEqual(process.GetProcessID(),
+ 1025 if follow_child else 1024)
 
 def test_fork(self):
 self.base_test("fork")
 
 def test_vfork(self):
 self.base_test("vfork")
+
+def test_fork_follow_child(self):
+self.base_test("fork", follow_child=True)
+
+def test_vfork_follow_child(self):
+self.base_test("vfork", follow_child=True)
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -5319,8 +5319,11 @@
 
   // Hardware breakpoints/watchpoints are not inherited implicitly,
   // so we need to readd them if we're following child.
-  if (GetFollowForkMode() == eFollowChild)
+  if (GetFollowForkMode() == eFollowChild) {
 DidForkSwitchHardwareTraps(true);
+// Update our PID
+SetID(child_pid);
+  }
 }
 
 void ProcessGDBRemote::DidVFork(lldb::pid_t child_pid, lldb::tid_t child_tid) {
@@ -5373,6 +5376,11 @@
 error.AsCString() ? error.AsCString() : "");
   return;
   }
+
+  if (GetFollowForkMode() == eFollowChild) {
+// Update our PID
+SetID(child_pid);
+  }
 }
 
 void ProcessGDBRemote::DidVForkDone() {


Index: lldb/test/API/functionalities/gdb_remote_client/TestFork.py
===
--- lldb/test/API/functionalities/gdb_remote_client/TestFork.py
+++ lldb/test/API/functionalities/gdb_remote_client/TestFork.py
@@ -9,7 +9,7 @@
 
 class TestMultiprocess(GDBRemoteTestBase):
 
-def base_test(self, variant):
+def base_test(self, variant, follow_child=False):
 class MyResponder(MockGDBServerResponder):
 def __init__(self):
 super().__init__()
@@ -43,12 +43,24 @@
   self.runCmd("log enable gdb-remote packets")
   self.addTearDownHook(
 lambda: self.runCmd("log disable gdb-remote packets"))
+if follow_child:
+self.runCmd("settings set target.process.follow-fork-mode child")
 process = self.connect(target)
+self.assertEqual(process.GetProcessID(), 1024)
 process.Continue()
-self.assertRegex(self.server.responder.detached, r"D;0*401")
+self.assertRegex(self.server.responder.detached,
+ r"D;0*400" if follow_child else r"D;0*401")
+self.assertEqual(process.GetProcessID(),
+ 1025 if follow_child else 1024)
 
 def test_fork(self):
 self.base_test("fork")
 
 def test_vfork(self):
 self.base_test("vfork")
+
+def test_fork_follow_child(self):
+self.base_test("fork", follow_child=True)
+
+def test_vfork_follow_child(self):
+self.base_test("vfork", follow_child=True)
Index: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===
--- lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -5319,8 +5319,11 @@
 
   // Hardware breakpoints/watchpoints are not inherited implicitly,
   // so we need to readd them if we're following

[Lldb-commits] [PATCH] D128465: [OLD] [llvm] add zstd to `llvm::compression` namespace

2022-07-18 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay added a comment.

In D128465#3660733 , @ckissane wrote:

> In D128465#3660714 , @MaskRay wrote:
>
>> Instead of marking a differential `[OLD] ` (which may confuse readers 
>> whether this is to be abandoned), you can mark a differential `Request 
>> Reviews`/`Request Changes`. Then it will not be in the green state.
>>
>> I clicked "Reopen" to make it clear the patch hasn't relanded.
>
> Oh, sorry about that!

That's fine :) We need to learn the web UI.

> I had actually created https://reviews.llvm.org/D129786 instead of reopening 
> this one because at the time I tried, reopening didn't seem to work.

A reverted patch by default is still in the "Closed" state. You can "reopen" it 
so that people know it hasn't relanded.
When repairing a differential, just reuse the existing patch. It keeps all 
discussions (including past suggestions and issues a previous commit did not 
address) in one thread.
If the new patch is very similar to this one, you probably need to abandon it 
in favor of this one.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465

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

[Lldb-commits] [PATCH] D128465: [OLD] [llvm] add zstd to `llvm::compression` namespace

2022-07-18 Thread Cole Kissane via Phabricator via lldb-commits

ckissane added a comment.

In D128465#3660714 , @MaskRay wrote:

> Instead of marking a differential `[OLD] ` (which may confuse readers whether 
> this is to be abandoned), you can mark a differential `Request 
> Reviews`/`Request Changes`. Then it will not be in the green state.
>
> I clicked "Reopen" to make it clear the patch hasn't relanded.

Oh, sorry about that!
I had actually created https://reviews.llvm.org/D129786 instead of reopening 
this one because at the time I tried, reopening didn't seem to work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465

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

[Lldb-commits] [PATCH] D128465: [OLD] [llvm] add zstd to `llvm::compression` namespace

2022-07-18 Thread Fangrui Song via Phabricator via lldb-commits

MaskRay added a comment.

Instead of marking a differential `[OLD] ` (which may confuse readers whether 
this is to be abandoned), you can mark a differential `Request 
Reviews`/`Request Changes`. Then it will not be in the green state.

I clicked "Reopen" to make it clear the patch hasn't relanded.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465

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

[Lldb-commits] [PATCH] D129814: Fix stepping over watchpoints in architectures that raise the exception before executing the instruction

2022-07-18 Thread Jim Ingham via Phabricator via lldb-commits

jingham marked an inline comment as done.
jingham added a comment.

In D129814#3658998 , @labath wrote:

> In D129814#3655878 , @jingham wrote:
>
>> In D129814#3654368 , @labath wrote:
>>
>>> In D129814#3654276 , 
>>> @jasonmolenda wrote:
>>>
 In D129814#3654230 , @labath 
 wrote:

> Generally, this makes sense to me, but I do have one question (not 
> necessarily for Jim). These tests work on (arm) linux, and I am wondering 
> why is that the case. Could it be related to the fact that lldb-server 
> preserves the stop reason for threads that are not running? I.e. if we 
> have two threads hit a breakpoint (or whatever), and then step one of 
> them, then the other thread will still report a stop_reason=breakpoint. 
> Do you know if this is supposed to happen (e.g. does debugserver do that)?

 I'd have to go back and test it to be 100% sure but from the behavior I 
 see on Darwin systems, when we receive a watchpoint hit on two threads at 
 the same time, and instruction-step the first thread, when we ask 
 debugserver for the current stop-reason on thread two, it says there is no 
 reason.  The watchpoint exception on the second thread is lost.  I could 
 imagine lldb-server behaving differently. (I think when we fetch the mach 
 exceptions for the threads, they've now been delivered, and when we ask 
 the kernel for any pending mach exceptions for the threads a second time 
 -- even if those threads haven't been allowed to run, I think the state is 
 lost, and debugserver didn't save that initial state it received.)
>>>
>>> Yes, that sounds plausible. It lldb-server (on linux) it happens 
>>> differently because we store each stop reason inside the thread object, and 
>>> since we can control each thread independently, there's no reason to touch 
>>> it if we're not running the thread. Although it also wouldn't be hard to 
>>> clear in on every resume.
>>>
>>> So which one of these behaviors would you say is the correct one?
>>
>> Darwin has always worked in such a way that by the time it stops for an 
>> exception it will have given other threads enough chance to run that if they 
>> were likely to, other threads will hit the exception before the first 
>> exception is delivered to lldb.  In gdb we tried to hide that fact by only 
>> publishing one exception, but that was awkward since you when you tried to 
>> resume to do some piece of work, you'd immediately stop again without 
>> getting to run.  That just lead to confusing interactions (and was a little 
>> harder to deal with in the debugger as well).  I think it's clearer to stop 
>> and see 5 threads have hit your breakpoint, then decide what to do, than to 
>> have one breakpoint reported then when you "continue" immediately turn 
>> around and get another without anybody really seeming to make progress, 
>> rinse, repeat...
>>
>> Rereading this I wasn't sure what question you were asking.  I answered 
>> "what should you tell lldb if lldb-server sees 5 threads stopped for reasons 
>> at one stop point."  But the other question is "what to do with stop info's 
>> if you only run one thread".  If lldb knows it has stopped a thread from 
>> running when it resumes the target, it also preserves the StopInfo for that 
>> thread.  That seems to me the most useful behavior.
>
> Given your latest comment, I think you've understood my question, but to 
> avoid confusion, the situation I was referring to is when lldb**-server** 
> sees 5 threads stopped for some reason, and then lldb steps one of them. The 
> question was whether lldb-server should still report the original stop reason 
> of those threads (if asked -- theoretically lldb should know that the reason 
> hasn't changed and might decide not to ask).
>
> I agree that preserving the stop reason is the most useful behavior for 
> **lldb** (the threads haven't run, so why should their state change?), but 
> then one could apply the same logic to the lldb-/debugserver component as 
> well.

I think the useful part to lldb is that if IT knows that it was the one 
responsible for a thread not continuing (it told the stub to stop it) then it 
should preserve the stop info.  If the stub mirrors this, then that's okay but 
lldb shouldn't rely on it.  OTOH, if lldb told the stub to continue all 
threads, but the stub for some reason of it's own didn't continue one of the 
threads, and then it re-reported the stop info, that would be confusing to 
lldb.  I can't think of any reason why the stub would do this, but that's the 
only case where having the stub also do this work is anything but redundant.

> In D129814#3656658 , @jingham wrote:
>
>> In D129814#3655750

[Lldb-commits] [PATCH] D129814: Fix stepping over watchpoints in architectures that raise the exception before executing the instruction

2022-07-18 Thread Jim Ingham via Phabricator via lldb-commits

jingham marked 3 inline comments as done.
jingham added inline comments.



Comment at: lldb/source/Target/StopInfo.cpp:730
+StopInfoWatchpoint *as_wp_stop_info 
+= reinterpret_cast(m_stop_info_sp.get());
+as_wp_stop_info->SetStepOverPlanComplete();

labath wrote:
> Change type of `m_stop_info_sp` (and the relevant constructor argument) to 
> `shared_ptr` to avoid the cast here.
> 
> (btw, the right cast for upcasts is `static_cast`. reinterpret_cast can 
> return wrong results in more [[ https://godbolt.org/z/5W7rz3fKK | complicated 
> scenarios ]])
> Change type of `m_stop_info_sp` (and the relevant constructor argument) to 
> `shared_ptr` to avoid the cast here.

Cute, I didn't know the voodoo to change the type of the pointer inside a 
shared pointer while retaining its reference counts.  

> 
> (btw, the right cast for upcasts is `static_cast`. reinterpret_cast can 
> return wrong results in more [[ https://godbolt.org/z/5W7rz3fKK | complicated 
> scenarios ]])

Huh, interesting.  The names seem backwards to me, static_cast sounds like it 
takes the pointer you gave it and casts it to the target type, whereas 
reinterpret sounds like the one that figures out what the class hierarchy 
actually is and does the right thing, but it looks like it's the other way 
round.



Comment at: lldb/source/Target/StopInfo.cpp:805
+  ThreadPlanSP step_over_wp_sp(new ThreadPlanStepOverWatchpoint(
+  *(thread_sp.get()), shared_from_this(), wp_sp));
+  Status error;

labath wrote:
> And use `static_pointer_cast(shared_from_this())` here. 
> That way the casting remains confined within the origin class.
I didn't know that voodoo, looks like we use it in other places, however, so I 
switched to doing it that way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129814

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

[Lldb-commits] [PATCH] D129814: Fix stepping over watchpoints in architectures that raise the exception before executing the instruction

2022-07-18 Thread Jim Ingham via Phabricator via lldb-commits

jingham updated this revision to Diff 445591.
jingham added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129814

Files:
  lldb/include/lldb/Breakpoint/Watchpoint.h
  lldb/include/lldb/Target/StopInfo.h
  lldb/source/Target/StopInfo.cpp
  
lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatchBreak.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
  
lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py

Index: lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py
===
--- lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py
+++ lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py
@@ -12,10 +12,6 @@
 # Atomic sequences are not supported yet for MIPS in LLDB.
 @skipIf(triple='^mips')
 @add_test_categories(["watchpoint"])
-@skipIf(
-oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-bugnumber="rdar://93863107")
 
 def test(self):
 """Test watchpoint and a breakpoint in multiple threads."""
Index: lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
===
--- lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
+++ lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
@@ -12,10 +12,6 @@
 # Atomic sequences are not supported yet for MIPS in LLDB.
 @skipIf(triple='^mips')
 @expectedFailureNetBSD
-@skipIf(
-oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-bugnumber="rdar://93863107")
 @add_test_categories(["watchpoint"])
 def test(self):
 """Test two threads that trigger a watchpoint and one signal thread. """
Index: lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
===
--- lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
+++ lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
@@ -11,10 +11,6 @@
 
 # Atomic sequences are not supported yet for MIPS in LLDB.
 @skipIf(triple='^mips')
-@skipIf(
-oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-bugnumber="rdar://93863107")
 @add_test_categories(["watchpoint"])
 def test(self):
 """Test two threads that trigger a watchpoint and one (1 second delay) breakpoint thread. """
Index: lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
===
--- lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
+++ lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
@@ -11,10 +11,6 @@
 
 # Atomic sequences are not supported yet for MIPS in LLDB.
 @skipIf(triple='^mips')
-@skipIf(
-oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-bugnumber="rdar://93863107")
 @add_test_categories(["watchpoint"])
 def test(self):
 """Test two threads that trigger a watchpoint and one breakpoint thread. """
Index: lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py
===
--- lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py

[Lldb-commits] [PATCH] D129807: [LLDB][NativePDB] Add MSInheritanceAttr when completing CXXRecordDecl

2022-07-18 Thread Reid Kleckner via Phabricator via lldb-commits

rnk added inline comments.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:492
   if (pr.isPointerToMember()) {
 MemberPointerInfo mpi = pr.getMemberInfo();
 GetOrCreateType(mpi.ContainingType);

I think you want to make code changes here to look at `mpi.Representation`.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp:762
 
 VariableSP SymbolFileNativePDB::CreateGlobalVariable(PdbGlobalSymId var_id) {
   CVSymbol sym = m_index->symrecords().readRecord(var_id.offset);

This codepath seems specific to global variable creation. Are there other ways 
to hit this issue through other codepaths, such as local variables or non-type 
template parameters?

I would consider moving this logic closer to the logic which creates the AST 
member pointer type. Any time LLDB loads a member pointer type, it should check 
the inheritance kind, and then check if the class already has an 
MSInheritanceAttr, and add one if it is missing.

If the existing attribute doesn't match the attribute you want to add, that 
represents an ODR violation in the user program. I'm not sure what LLDB should 
do in that case.



Comment at: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp:319
+  spelling = clang::MSInheritanceAttr::Keyword_virtual_inheritance;
+else if (bases.size() > 1)
+  spelling = clang::MSInheritanceAttr::Keyword_multiple_inheritance;

We shouldn't try to reimplement the frontend calculation logic, we should just 
trust the debug info. Users can use the [pointers_to_members 
pragma](https://docs.microsoft.com/en-us/cpp/preprocessor/pointers-to-members?view=msvc-170)
 to force the compiler to use different representations, so this calculation 
won't always be right.

Also, I don't think this case handles multiple inheritance via indirect bases. 
Consider:
```
struct A { int a; };
struct B { int b; };
struct C : A, B { int c; };
struct D : C { int d; };
```

If we need this logic in LLDB, try calling 
`CXXRecordDecl::calculateInheritanceModel`.


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

https://reviews.llvm.org/D129807

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

[Lldb-commits] [PATCH] D126615: [lldb] Add an integration test for non-stop protocol

2022-07-18 Thread Michał Górny via Phabricator via lldb-commits

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

Update again ;-).


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

https://reviews.llvm.org/D126615

Files:
  lldb/test/Shell/Process/Inputs/signal.c
  lldb/test/Shell/Process/TestNonStop.test


Index: lldb/test/Shell/Process/TestNonStop.test
===
--- /dev/null
+++ lldb/test/Shell/Process/TestNonStop.test
@@ -0,0 +1,35 @@
+# UNSUPPORTED: lldb-repro
+# REQUIRES: native
+# RUN: %clang_host %p/Inputs/signal.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+settings set plugin.process.gdb-remote.use-non-stop-protocol true
+log enable gdb-remote packets
+process launch
+
+# CHECK:  lldb <  14> send packet: $QNonStop:1#8d
+# CHECK-NEXT: lldb <   6> read packet: $OK#9a
+
+# CHECK:  b-remote.async>  <   5> send packet: $c#63
+# CHECK-NEXT: b-remote.async>  <   6> read packet: $OK#9a
+# CHECK-NEXT: b-remote.async>  <{{[ 0-9]+}}> read packet: %Stop:T{{.*}}
+# CHECK-NEXT: b-remote.async>  <  12> send packet: $vStopped#55
+# CHECK-NEXT: b-remote.async>  <   6> read packet: $OK#9a
+# CHECK-NEXT: b-remote.async>  <  11> send packet: $vCont;t#b9
+# CHECK-NEXT: b-remote.async>  <   6> read packet: $OK#9a
+# CHECK-NEXT: b-remote.async>  <   5> send packet: $?#3f
+# CHECK-NEXT: b-remote.async>  <{{[ 0-9]+}}> read packet: $T{{.*}}
+# CHECK-NEXT: b-remote.async>  <  12> send packet: $vStopped#55
+# CHECK-NEXT: b-remote.async>  <   6> read packet: $OK#9a
+
+# CHECK:  * thread #1, name = '{{.*}}', stop reason = signal SIGINT
+
+process continue
+
+# CHECK:  b-remote.async>  <   5> send packet: $c#63
+# CHECK-NEXT: b-remote.async>  <   6> read packet: $OK#9a
+# CHECK-NEXT: b-remote.async>  <{{[ 0-9]+}}> read packet: 
%Stop:W2a;process:{{.*}}
+# CHECK-NEXT: b-remote.async>  <  12> send packet: $vStopped#55
+# CHECK-NEXT: b-remote.async>  <   6> read packet: $OK#9a
+# CHECK-NOT:  b-remote.async>  <  11> send packet: $vCont;t#b9
+
+# CHECK:  Process {{[0-9]+}} exited with status = 42 (0x002a)
Index: lldb/test/Shell/Process/Inputs/signal.c
===
--- /dev/null
+++ lldb/test/Shell/Process/Inputs/signal.c
@@ -0,0 +1,13 @@
+#include 
+#include 
+#include 
+
+void handler() {
+  printf("got SIGINT\n");
+}
+
+int main() {
+  assert(signal(SIGINT, handler) != SIG_ERR);
+  raise(SIGINT);
+  return 42;
+}


Index: lldb/test/Shell/Process/TestNonStop.test
===
--- /dev/null
+++ lldb/test/Shell/Process/TestNonStop.test
@@ -0,0 +1,35 @@
+# UNSUPPORTED: lldb-repro
+# REQUIRES: native
+# RUN: %clang_host %p/Inputs/signal.c -o %t
+# RUN: %lldb -b -s %s %t | FileCheck %s
+settings set plugin.process.gdb-remote.use-non-stop-protocol true
+log enable gdb-remote packets
+process launch
+
+# CHECK:  lldb <  14> send packet: $QNonStop:1#8d
+# CHECK-NEXT: lldb <   6> read packet: $OK#9a
+
+# CHECK:  b-remote.async>  <   5> send packet: $c#63
+# CHECK-NEXT: b-remote.async>  <   6> read packet: $OK#9a
+# CHECK-NEXT: b-remote.async>  <{{[ 0-9]+}}> read packet: %Stop:T{{.*}}
+# CHECK-NEXT: b-remote.async>  <  12> send packet: $vStopped#55
+# CHECK-NEXT: b-remote.async>  <   6> read packet: $OK#9a
+# CHECK-NEXT: b-remote.async>  <  11> send packet: $vCont;t#b9
+# CHECK-NEXT: b-remote.async>  <   6> read packet: $OK#9a
+# CHECK-NEXT: b-remote.async>  <   5> send packet: $?#3f
+# CHECK-NEXT: b-remote.async>  <{{[ 0-9]+}}> read packet: $T{{.*}}
+# CHECK-NEXT: b-remote.async>  <  12> send packet: $vStopped#55
+# CHECK-NEXT: b-remote.async>  <   6> read packet: $OK#9a
+
+# CHECK:  * thread #1, name = '{{.*}}', stop reason = signal SIGINT
+
+process continue
+
+# CHECK:  b-remote.async>  <   5> send packet: $c#63
+# CHECK-NEXT: b-remote.async>  <   6> read packet: $OK#9a
+# CHECK-NEXT: b-remote.async>  <{{[ 0-9]+}}> read packet: %Stop:W2a;process:{{.*}}
+# CHECK-NEXT: b-remote.async>  <  12> send packet: $vStopped#55
+# CHECK-NEXT: b-remote.async>  <   6> read packet: $OK#9a
+# CHECK-NOT:  b-remote.async>  <  11> send packet: $vCont;t#b9
+
+# CHECK:  Process {{[0-9]+}} exited with status = 42 (0x002a)
Index: lldb/test/Shell/Process/Inputs/signal.c
===
--- /dev/null
+++ lldb/test/Shell/Process/Inputs/signal.c
@@ -0,0 +1,13 @@
+#include 
+#include 
+#include 
+
+void handler() {
+  printf("got SIGINT\n");
+}
+
+int main() {
+  assert(signal(SIGINT, handler) != SIG_ERR);
+  raise(SIGINT);
+  return 42;
+}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-07-18 Thread Matheus Izvekov via Phabricator via lldb-commits

mizvekov added a comment.

In D112374#3659597 , @nikic wrote:

> Given that LLVM 15 branches off in one week, maybe it would be better to wait 
> for that before relanding the change, as it seems to have significant impact 
> on plugins?

I think it would make sense to try LLVM15.

So this change in this MR is in effect a sanitizer for some incorrect use of a 
bunch of APIs, it turns a bunch of bugs from something that needs a specific 
set of circumstances to happen, into problems that happen reliably.

And those incorrect uses, for all examples I have seen so far, and there are 
many, are pretty easy to fix.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[Lldb-commits] [PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-07-18 Thread Nikita Popov via Phabricator via lldb-commits

nikic added a comment.

Given that LLVM 15 branches off in one week, maybe it would be better to wait 
for that before relanding the change, as it seems to have significant impact on 
plugins?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[Lldb-commits] [PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-07-18 Thread Matheus Izvekov via Phabricator via lldb-commits

mizvekov marked an inline comment as done.
mizvekov added a comment.

@JDevlieghere @teemperor ping.

Sorry for the urgency here, but I have a lot of work built on top of this patch.

And this patch fixes a bunch of bugs that other people are unaware and 
duplicating effort, see github issues.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[Lldb-commits] [PATCH] D129814: Fix stepping over watchpoints in architectures that raise the exception before executing the instruction

2022-07-18 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 D129814#3655878 , @jingham wrote:

> In D129814#3654368 , @labath wrote:
>
>> In D129814#3654276 , @jasonmolenda 
>> wrote:
>>
>>> In D129814#3654230 , @labath 
>>> wrote:
>>>
 Generally, this makes sense to me, but I do have one question (not 
 necessarily for Jim). These tests work on (arm) linux, and I am wondering 
 why is that the case. Could it be related to the fact that lldb-server 
 preserves the stop reason for threads that are not running? I.e. if we 
 have two threads hit a breakpoint (or whatever), and then step one of 
 them, then the other thread will still report a stop_reason=breakpoint. Do 
 you know if this is supposed to happen (e.g. does debugserver do that)?
>>>
>>> I'd have to go back and test it to be 100% sure but from the behavior I see 
>>> on Darwin systems, when we receive a watchpoint hit on two threads at the 
>>> same time, and instruction-step the first thread, when we ask debugserver 
>>> for the current stop-reason on thread two, it says there is no reason.  The 
>>> watchpoint exception on the second thread is lost.  I could imagine 
>>> lldb-server behaving differently. (I think when we fetch the mach 
>>> exceptions for the threads, they've now been delivered, and when we ask the 
>>> kernel for any pending mach exceptions for the threads a second time -- 
>>> even if those threads haven't been allowed to run, I think the state is 
>>> lost, and debugserver didn't save that initial state it received.)
>>
>> Yes, that sounds plausible. It lldb-server (on linux) it happens differently 
>> because we store each stop reason inside the thread object, and since we can 
>> control each thread independently, there's no reason to touch it if we're 
>> not running the thread. Although it also wouldn't be hard to clear in on 
>> every resume.
>>
>> So which one of these behaviors would you say is the correct one?
>
> Darwin has always worked in such a way that by the time it stops for an 
> exception it will have given other threads enough chance to run that if they 
> were likely to, other threads will hit the exception before the first 
> exception is delivered to lldb.  In gdb we tried to hide that fact by only 
> publishing one exception, but that was awkward since you when you tried to 
> resume to do some piece of work, you'd immediately stop again without getting 
> to run.  That just lead to confusing interactions (and was a little harder to 
> deal with in the debugger as well).  I think it's clearer to stop and see 5 
> threads have hit your breakpoint, then decide what to do, than to have one 
> breakpoint reported then when you "continue" immediately turn around and get 
> another without anybody really seeming to make progress, rinse, repeat...
>
> Rereading this I wasn't sure what question you were asking.  I answered "what 
> should you tell lldb if lldb-server sees 5 threads stopped for reasons at one 
> stop point."  But the other question is "what to do with stop info's if you 
> only run one thread".  If lldb knows it has stopped a thread from running 
> when it resumes the target, it also preserves the StopInfo for that thread.  
> That seems to me the most useful behavior.

Given your latest comment, I think you've understood my question, but to avoid 
confusion, the situation I was referring to is when lldb**-server** sees 5 
threads stopped for some reason, and then lldb steps one of them. The question 
was whether lldb-server should still report the original stop reason of those 
threads (if asked -- theoretically lldb should know that the reason hasn't 
changed and might decide not to ask).

I agree that preserving the stop reason is the most useful behavior for 
**lldb** (the threads haven't run, so why should their state change?), but then 
one could apply the same logic to the lldb-/debugserver component as well.

In D129814#3656658 , @jingham wrote:

> In D129814#3655750 , @jasonmolenda 
> wrote:
>
>> In D129814#3654368 , @labath wrote:
>>
>>> In D129814#3654276 , 
>>> @jasonmolenda wrote:
>>>
 In D129814#3654230 , @labath 
 wrote:

> Generally, this makes sense to me, but I do have one question (not 
> necessarily for Jim). These tests work on (arm) linux, and I am wondering 
> why is that the case. Could it be related to the fact that lldb-server 
> preserves the stop reason for threads that are not running? I.e. if we 
> have two threads hit a breakpoint (or whatever), and then step one of 
> them, then the

[Lldb-commits] [lldb] 83fab8c - Revert "Make hit point counts reliable for architectures that stop before evaluation."

[Lldb-commits] [lldb] 4f5707e - Revert "This is a followup to https://reviews.llvm.org/D129814"

[Lldb-commits] [PATCH] D130054: [trace][intel pt] Introduce wall clock time for each trace item

[Lldb-commits] [PATCH] D130054: [trace][intel pt] Introduce wall clock time for each trace item

[Lldb-commits] [lldb] 555ae5b - This is a followup to https://reviews.llvm.org/D129814

[Lldb-commits] [PATCH] D130045: Implement better path matching in FileSpecList::FindFileIndex(...).

[Lldb-commits] [PATCH] D130045: Implement better path matching in FileSpecList::FindFileIndex(...).

[Lldb-commits] [PATCH] D111509: [clang] use getCommonSugar in an assortment of places

[Lldb-commits] [PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Lldb-commits] [PATCH] D129377: [lldb/Fuzzer] Add fuzzer for expression evaluator

[Lldb-commits] [PATCH] D111509: [clang] use getCommonSugar in an assortment of places

[Lldb-commits] [PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

[Lldb-commits] [PATCH] D130045: Implement better path matching in FileSpecList::FindFileIndex(...).

[Lldb-commits] [PATCH] D130045: Implement better path matching in FileSpecList::FindFileIndex(...).

[Lldb-commits] [PATCH] D129338: Tell the user which pathname was invalid...

[Lldb-commits] [lldb] e83d47f - When the module path for `command script import` is invalid, echo the path.

[Lldb-commits] [PATCH] D129814: Fix stepping over watchpoints in architectures that raise the exception before executing the instruction

[Lldb-commits] [lldb] 5778ada - Make hit point counts reliable for architectures that stop before evaluation.

[Lldb-commits] [PATCH] D129682: [lldb] Filter DIEs based on qualified name when possible

[Lldb-commits] [PATCH] D130037: [lldb] [gdb-remote] Fix process ID after following forked child

[Lldb-commits] [PATCH] D128465: [OLD] [llvm] add zstd to `llvm::compression` namespace

[Lldb-commits] [PATCH] D128465: [OLD] [llvm] add zstd to `llvm::compression` namespace

[Lldb-commits] [PATCH] D128465: [OLD] [llvm] add zstd to `llvm::compression` namespace

[Lldb-commits] [PATCH] D129814: Fix stepping over watchpoints in architectures that raise the exception before executing the instruction

[Lldb-commits] [PATCH] D129814: Fix stepping over watchpoints in architectures that raise the exception before executing the instruction

[Lldb-commits] [PATCH] D129814: Fix stepping over watchpoints in architectures that raise the exception before executing the instruction

[Lldb-commits] [PATCH] D129807: [LLDB][NativePDB] Add MSInheritanceAttr when completing CXXRecordDecl

[Lldb-commits] [PATCH] D126615: [lldb] Add an integration test for non-stop protocol

[Lldb-commits] [PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Lldb-commits] [PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Lldb-commits] [PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Lldb-commits] [PATCH] D129814: Fix stepping over watchpoints in architectures that raise the exception before executing the instruction

32 matches

Site Navigation

Mail list logo

Footer information