[Lldb-commits] [lldb] d8bd179 - Clear read_fd_set if EINTR received

2023-02-23 Thread Pavel Labath via lldb-commits

Author: Emre Kultursay
Date: 2023-02-23T13:27:19+01:00
New Revision: d8bd179a173876a7a9ee11828b63efffe145356c

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

LOG: Clear read_fd_set if EINTR received

Leaving bits uncleared set causes callbacks to be triggered even
though there are no events to process. Starting with D131160
we have a callback that makes blocking read calls over pipe which
was causing the lldb-server main loop to become unresponsive / blocked
on Android.

Reviewed By: labath

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

Added: 


Modified: 
lldb/source/Host/posix/MainLoopPosix.cpp

Removed: 




diff  --git a/lldb/source/Host/posix/MainLoopPosix.cpp 
b/lldb/source/Host/posix/MainLoopPosix.cpp
index b185c3d3b7076..5b50b450433ea 100644
--- a/lldb/source/Host/posix/MainLoopPosix.cpp
+++ b/lldb/source/Host/posix/MainLoopPosix.cpp
@@ -156,9 +156,12 @@ Status MainLoopPosix::RunImpl::Poll() {
 size_t sigset_len;
   } extra_data = {&kernel_sigset, sizeof(kernel_sigset)};
   if (syscall(__NR_pselect6, nfds, &read_fd_set, nullptr, nullptr, nullptr,
-  &extra_data) == -1 &&
-  errno != EINTR)
-return Status(errno, eErrorTypePOSIX);
+  &extra_data) == -1) {
+if (errno != EINTR)
+  return Status(errno, eErrorTypePOSIX);
+else
+  FD_ZERO(&read_fd_set);
+  }
 
   return Status();
 }



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


[Lldb-commits] [PATCH] D144240: Clear read_fd_set if EINTR received

2023-02-23 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd8bd179a1738: Clear read_fd_set if EINTR received (authored 
by emrekultursay, committed by labath).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144240

Files:
  lldb/source/Host/posix/MainLoopPosix.cpp


Index: lldb/source/Host/posix/MainLoopPosix.cpp
===
--- lldb/source/Host/posix/MainLoopPosix.cpp
+++ lldb/source/Host/posix/MainLoopPosix.cpp
@@ -156,9 +156,12 @@
 size_t sigset_len;
   } extra_data = {&kernel_sigset, sizeof(kernel_sigset)};
   if (syscall(__NR_pselect6, nfds, &read_fd_set, nullptr, nullptr, nullptr,
-  &extra_data) == -1 &&
-  errno != EINTR)
-return Status(errno, eErrorTypePOSIX);
+  &extra_data) == -1) {
+if (errno != EINTR)
+  return Status(errno, eErrorTypePOSIX);
+else
+  FD_ZERO(&read_fd_set);
+  }
 
   return Status();
 }


Index: lldb/source/Host/posix/MainLoopPosix.cpp
===
--- lldb/source/Host/posix/MainLoopPosix.cpp
+++ lldb/source/Host/posix/MainLoopPosix.cpp
@@ -156,9 +156,12 @@
 size_t sigset_len;
   } extra_data = {&kernel_sigset, sizeof(kernel_sigset)};
   if (syscall(__NR_pselect6, nfds, &read_fd_set, nullptr, nullptr, nullptr,
-  &extra_data) == -1 &&
-  errno != EINTR)
-return Status(errno, eErrorTypePOSIX);
+  &extra_data) == -1) {
+if (errno != EINTR)
+  return Status(errno, eErrorTypePOSIX);
+else
+  FD_ZERO(&read_fd_set);
+  }
 
   return Status();
 }
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [lldb] dc2d2ca - [LLDB] Mark test_stop_reply_contains_thread_pcs as an expected failure on Windows

2023-02-23 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2023-02-23T14:10:07Z
New Revision: dc2d2ca0603a5aaaea68515ff7c0605049268778

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

LOG: [LLDB] Mark test_stop_reply_contains_thread_pcs as an expected failure on 
Windows

This has been flaky on the Windows on Arm LLDB bot.

https://lab.llvm.org/buildbot/#/builders/219/builds/826

Given that test_stop_reply_reports_multiple_threads is already expected
to fail on Windows, this is not suprising.

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py

Removed: 




diff  --git 
a/lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
index c41446f96b8d7..e8805a68d33aa 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
@@ -171,6 +171,7 @@ def test_stop_reply_reports_correct_threads(self):
 self.assertIn(tid, stop_reply_threads)
 
 @skipIfNetBSD
+@expectedFailureAll(oslist=["windows"]) # Extra threads present
 def test_stop_reply_contains_thread_pcs(self):
 self.build()
 self.set_inferior_startup_launch()



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


[Lldb-commits] [lldb] bd56c8d - [lldb] Skip test_stop_reply_contains_thread_pcs on Windows

2023-02-23 Thread David Spickett via lldb-commits

Author: David Spickett
Date: 2023-02-23T16:08:44Z
New Revision: bd56c8d5f59a22b6c0ab64328b87d831a0c4468c

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

LOG: [lldb] Skip test_stop_reply_contains_thread_pcs on Windows

I marked this as expected to fail, but it doesn't always fail,
and an unexpected success is a failure.

Skip it instead.

Added: 


Modified: 
lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py

Removed: 




diff  --git 
a/lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py 
b/lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
index e8805a68d33a..c81bd34c2f6c 100644
--- a/lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
+++ b/lldb/test/API/tools/lldb-server/TestGdbRemoteThreadsInStopReply.py
@@ -171,7 +171,7 @@ def test_stop_reply_reports_correct_threads(self):
 self.assertIn(tid, stop_reply_threads)
 
 @skipIfNetBSD
-@expectedFailureAll(oslist=["windows"]) # Extra threads present
+@skipIfWindows # Flaky on Windows
 def test_stop_reply_contains_thread_pcs(self):
 self.build()
 self.set_inferior_startup_launch()



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


[Lldb-commits] [PATCH] D144311: [debugserver] Add one additional sleep before attaching after waiting

2023-02-23 Thread Jason Molenda via Phabricator via lldb-commits
jasonmolenda accepted this revision.
jasonmolenda added a comment.
This revision is now accepted and ready to land.

Ah, sorry I missed this one, LGTM.  Attaching to a process very early in 
startup during this transition between dyld's is especially difficult to 
recover from; letting the process run a little extra to get past this is a good 
workaround.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144311

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


[Lldb-commits] [lldb] 97b579d - [debugserver] Add one additional sleep before attaching after waiting

2023-02-23 Thread Alex Langford via lldb-commits

Author: Alex Langford
Date: 2023-02-23T11:22:22-08:00
New Revision: 97b579d3140476cd32d94a6e052f0908e2c21228

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

LOG: [debugserver] Add one additional sleep before attaching after waiting

It's possible for debugserver to attach to a process during the handoff
between /usr/lib/dyld and the dyld in the shared cache. When that
happens, we may end up in a state where there is no dyld in the process
and our debugging session is doomed. To make that scenario a lot less
likely, we can insert a sleep right before attaching after waiting to
find the right pid.

rdar://105513180

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

Added: 


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

Removed: 




diff  --git a/lldb/tools/debugserver/source/DNB.cpp 
b/lldb/tools/debugserver/source/DNB.cpp
index a95238c6279e1..07423ea56e6e2 100644
--- a/lldb/tools/debugserver/source/DNB.cpp
+++ b/lldb/tools/debugserver/source/DNB.cpp
@@ -793,6 +793,14 @@ DNBProcessAttachWait(RNBContext *ctx, const char 
*waitfor_process_name,
   if (waitfor_pid != INVALID_NUB_PROCESS) {
 DNBLogThreadedIf(LOG_PROCESS, "Attaching to %s with pid %i...\n",
  waitfor_process_name, waitfor_pid);
+// In some cases, we attempt to attach during the transition from
+// /usr/lib/dyld to the dyld in the shared cache. If that happens, we may
+// end up in a state where there is no dyld in the process and from there
+// the debugging session is doomed.
+// In an attempt to make this scenario much less likely, we sleep
+// for an additional `waitfor_interval` number of microseconds before
+// attaching.
+::usleep(waitfor_interval);
 waitfor_pid = DNBProcessAttach(waitfor_pid, timeout_abstime,
ctx->GetIgnoredExceptions(), err_str, 
err_len);



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


[Lldb-commits] [PATCH] D144311: [debugserver] Add one additional sleep before attaching after waiting

2023-02-23 Thread Alex Langford via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG97b579d31404: [debugserver] Add one additional sleep before 
attaching after waiting (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144311

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


Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -793,6 +793,14 @@
   if (waitfor_pid != INVALID_NUB_PROCESS) {
 DNBLogThreadedIf(LOG_PROCESS, "Attaching to %s with pid %i...\n",
  waitfor_process_name, waitfor_pid);
+// In some cases, we attempt to attach during the transition from
+// /usr/lib/dyld to the dyld in the shared cache. If that happens, we may
+// end up in a state where there is no dyld in the process and from there
+// the debugging session is doomed.
+// In an attempt to make this scenario much less likely, we sleep
+// for an additional `waitfor_interval` number of microseconds before
+// attaching.
+::usleep(waitfor_interval);
 waitfor_pid = DNBProcessAttach(waitfor_pid, timeout_abstime,
ctx->GetIgnoredExceptions(), err_str, 
err_len);


Index: lldb/tools/debugserver/source/DNB.cpp
===
--- lldb/tools/debugserver/source/DNB.cpp
+++ lldb/tools/debugserver/source/DNB.cpp
@@ -793,6 +793,14 @@
   if (waitfor_pid != INVALID_NUB_PROCESS) {
 DNBLogThreadedIf(LOG_PROCESS, "Attaching to %s with pid %i...\n",
  waitfor_process_name, waitfor_pid);
+// In some cases, we attempt to attach during the transition from
+// /usr/lib/dyld to the dyld in the shared cache. If that happens, we may
+// end up in a state where there is no dyld in the process and from there
+// the debugging session is doomed.
+// In an attempt to make this scenario much less likely, we sleep
+// for an additional `waitfor_interval` number of microseconds before
+// attaching.
+::usleep(waitfor_interval);
 waitfor_pid = DNBProcessAttach(waitfor_pid, timeout_abstime,
ctx->GetIgnoredExceptions(), err_str, 
err_len);
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D144664: Preserve SBValue's that only have an Error state

2023-02-23 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision.
jingham added reviewers: JDevlieghere, clayborg, labath, jasonmolenda.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

SBValues carry their own errors with them (SBValue::GetError).  That means that 
even if an SBValue was made in a situation where we couldn't actually make a 
value (e.g. you called `lldb.frame.EvaluateExpression` while the target is 
running) the error is still useful.  We were using the IsValid check to decide 
whether to actually return the underlying ValueObject to the SBValue.

This could have been implemented two ways.  One is to add "Has an error" to the 
conditions that would cause IsValid to return true.  I chose not to do that 
because previously you could use IsValid to mean "has a valid Value" and so 
this would have been a semantic change and could break scripts.

Instead I just add the few "or has error" checks needed to preserve "error only 
SBValues" in the SBValue interface, and fixed up a couple places in ValueObject 
printing where we needed to print the error if the ValueObject had no other 
useful state.

This patch doesn't have a test.  I have a follow-on commit which is what 
motivated this change and the test for that was what necessitated, and will 
verify, this change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144664

Files:
  lldb/source/API/SBValue.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/DataFormatters/ValueObjectPrinter.cpp


Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -71,6 +71,18 @@
 }
 
 bool ValueObjectPrinter::PrintValueObject() {
+  if (!m_orig_valobj)
+return false;
+
+  // If the incoming ValueObject is in an error state, the best we're going to 
+  // get out of it is its type.  But if we don't even have that, just print
+  // the error and exit early.
+  if (m_orig_valobj->GetError().Fail() 
+  && !m_orig_valobj->GetCompilerType().IsValid()) {
+m_stream->Printf("Error: '%s'", m_orig_valobj->GetError().AsCString());
+return true;
+  }
+   
   if (!GetMostSpecializedValue() || m_valobj == nullptr)
 return false;
 
Index: lldb/source/Core/ValueObject.cpp
===
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -1173,6 +1173,15 @@
 Stream &s, ValueObjectRepresentationStyle val_obj_display,
 Format custom_format, PrintableRepresentationSpecialCases special,
 bool do_dump_error) {
+
+  // If the ValueObject has an error, we might end up dumping the type, which
+  // is useful, but if we don't even have a type, then don't examine the object
+  // further as that's not meaningful, only the error is.
+  if (m_error.Fail() && !GetCompilerType().IsValid()) {
+if (do_dump_error)
+  s.Printf("<%s>", m_error.AsCString());
+return false;
+  }
 
   Flags flags(GetTypeInfo());
 
@@ -1374,6 +1383,8 @@
 if (!str.empty())
   s << str;
 else {
+  // We checked for errors at the start, but do it again here in case
+  // realizing the value for dumping produced an error.
   if (m_error.Fail()) {
 if (do_dump_error)
   s.Printf("<%s>", m_error.AsCString());
Index: lldb/source/API/SBValue.cpp
===
--- lldb/source/API/SBValue.cpp
+++ lldb/source/API/SBValue.cpp
@@ -114,6 +114,10 @@
 lldb::ValueObjectSP value_sp = m_valobj_sp;
 
 Target *target = value_sp->GetTargetSP().get();
+// If this ValueObject holds an error, then it is valuable for that.
+if (value_sp->GetError().Fail()) 
+  return value_sp;
+
 if (!target)
   return ValueObjectSP();
 
@@ -1047,7 +1051,12 @@
 }
 
 lldb::ValueObjectSP SBValue::GetSP(ValueLocker &locker) const {
-  if (!m_opaque_sp || !m_opaque_sp->IsValid()) {
+  // IsValid means that the SBValue has a value in it.  But that's not the
+  // only time that ValueObjects are useful.  We also want to return the value
+  // if there's an error state in it.
+  if (!m_opaque_sp || (!m_opaque_sp->IsValid() 
+  && (m_opaque_sp->GetRootSP() 
+  && !m_opaque_sp->GetRootSP()->GetError().Fail( {
 locker.GetError().SetErrorString("No value");
 return ValueObjectSP();
   }


Index: lldb/source/DataFormatters/ValueObjectPrinter.cpp
===
--- lldb/source/DataFormatters/ValueObjectPrinter.cpp
+++ lldb/source/DataFormatters/ValueObjectPrinter.cpp
@@ -71,6 +71,18 @@
 }
 
 bool ValueObjectPrinter::PrintValueObject() {
+  if (!m_orig_valobj)
+return false;
+
+  // If the incoming ValueObject is in an error state, the best we're going to 
+  // get out of it is its type.  But 

[Lldb-commits] [PATCH] D144665: Use Resume not PrivateResume when asynchronously continuing after the start at stop

2023-02-23 Thread Jim Ingham via Phabricator via lldb-commits
jingham created this revision.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

In the auto-continue after we stop at the start point when running the debugger 
asynchronously, we use PrivateResume to continue.  But that doesn't update the 
stop locker state, which is currently at "stopped".  That means the stop locker 
wasn't doing its job to prevent you from running expressions while the target 
is running.

I fixed this, and added a test for this case to make sure we get a proper error 
back.  This patch requires (and adds the test for):

https://reviews.llvm.org/D144664


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144665

Files:
  lldb/source/API/SBFrame.cpp
  lldb/source/API/SBTarget.cpp
  lldb/source/Target/Target.cpp
  lldb/test/API/python_api/run_locker/Makefile
  lldb/test/API/python_api/run_locker/TestRunLocker.py
  lldb/test/API/python_api/run_locker/main.c

Index: lldb/test/API/python_api/run_locker/main.c
===
--- /dev/null
+++ lldb/test/API/python_api/run_locker/main.c
@@ -0,0 +1,15 @@
+#include 
+
+int
+SomethingToCall() {
+  return 100;
+}
+
+int
+main()
+{
+  while (1) {
+sleep(1);
+  }
+  return SomethingToCall();
+}
Index: lldb/test/API/python_api/run_locker/TestRunLocker.py
===
--- /dev/null
+++ lldb/test/API/python_api/run_locker/TestRunLocker.py
@@ -0,0 +1,89 @@
+"""
+Test that the run locker really does work to keep
+us from running SB API that should only be run
+while stopped.  This test is mostly concerned with
+what happens between launch and first stop.
+"""
+
+import lldb
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+class TestRunLocker(TestBase):
+
+NO_DEBUG_INFO_TESTCASE = True
+
+def test_run_locker(self):
+"""Test that the run locker is set correctly when we launch"""
+self.build()
+self.runlocker_test(False)
+
+def test_run_locker_stop_at_entry(self):
+"""Test that the run locker is set correctly when we launch"""
+self.build()
+self.runlocker_test(False)
+
+def setUp(self):
+# Call super's setUp().
+TestBase.setUp(self)
+self.main_source_file = lldb.SBFileSpec("main.c")
+
+def runlocker_test(self, stop_at_entry):
+"""The code to stop at entry handles events slightly differently, so 
+   we test both versions of process launch."""
+
+target = lldbutil.run_to_breakpoint_make_target(self)
+
+launch_info = target.GetLaunchInfo()
+if stop_at_entry:
+flags = launch_info.GetFlags()
+launch_info.SetFlags(flags | lldb.eLaunchFlagStopAtEntry)
+
+error = lldb.SBError()
+# We are trying to do things when the process is running, so
+# we have to run the debugger asynchronously.
+self.dbg.SetAsync(True)
+
+listener = lldb.SBListener("test-run-lock-listener")
+launch_info.SetListener(listener)
+process = target.Launch(launch_info, error)
+self.assertSuccess(error, "Launched the process")
+
+event = lldb.SBEvent()
+
+event_result = listener.WaitForEvent(10, event)
+self.assertTrue(event_result, "timed out waiting for launch")
+state_type = lldb.SBProcess.GetStateFromEvent(event)
+# We don't always see a launching...
+if state_type == lldb.eStateLaunching:
+event_result = listener.WaitForEvent(10, event)
+self.assertTrue(event_result, "Timed out waiting for running after launching")
+state_type = lldb.SBProcess.GetStateFromEvent(event)
+
+self.assertState(state_type, lldb.eStateRunning, "Didn't get a running event")
+
+# We aren't checking the entry state, but just making sure
+# the running state is set properly if we continue in this state.
+
+if stop_at_entry:
+event_result = listener.WaitForEvent(10, event)
+self.assertTrue(event_result, "Timed out waiting for stop at entry stop")
+state_type = lldb.SBProcess.GetStateFromEvent(event)
+self.assertState(state_type, eStateStopped, "Stop at entry stopped")
+process.Continue()
+
+# Okay, now the process is running, make sure we can't do things
+# you aren't supposed to do while running, and that we get some
+# actual error:
+val = target.EvaluateExpression("SomethingToCall()")
+error = val.GetError()
+self.assertTrue(error.Fail(), "Failed to run expression")
+print(f"Got Error: {error.GetCString()}")
+self.assertIn("can't evaluate expressions when the process is running", error.GetCString(), "Stopped by stop locker")
+
+# This should als

[Lldb-commits] [PATCH] D143695: [lldb] Make repeat commands work for regex commands

2023-02-23 Thread Jim Ingham via Phabricator via lldb-commits
jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

This change didn't immediately make sense to me, which means it's a bit tricky 
and tricky changes should get comments or they will confuse us later on.  Plus, 
I wasn't sure how it worked, so I'm looking forward to reading the comment as 
part of the review...




Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2005
 // stored "repeat command" which we should give a chance to produce it's
 // repeat command, even though we don't add repeat commands to the history.
+Args command_args(command_string);

You at least need to fix this comment, since you aren't checking empty_command 
anymore.

It would also be good to explain the logic here, since I'm not entirely sure 
why this patch fixes the problem.  It should just mean that we are setting the 
"previous repeat command" twice, once above and once here, in the case where we 
didn't set "empty_command", so it's not clear why that helps.

I also think it's wrong to set a repeat command when add_to_history is false.  
That generally happens if some code, like a breakpoint command, uses 
HandleCommand, i.e. that's a command the user didn't directly execute.  So it 
doesn't make sense that that non-user command should set the user repeat 
command.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143695

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


[Lldb-commits] [PATCH] D140630: [lldb-vscode] Add data breakpoint support

2023-02-23 Thread Greg Clayton via Phabricator via lldb-commits
clayborg added a comment.

Sorry for delay, I have been on vacation for the last two weeks. I will be back 
next Monday and get to this soon. Feel free to ping again next week if I don't 
get to it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140630

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


[Lldb-commits] [PATCH] D144224: [lldb] Extend SWIG SBProcess interface with WriteMemoryAsCString method

2023-02-23 Thread Jim Ingham via Phabricator via lldb-commits
jingham added a comment.

I see no tests.  If there were tests, one of them could be for "" and then you 
would know whether Pavel was right or not...


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

https://reviews.llvm.org/D144224

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


[Lldb-commits] [PATCH] D144688: [lldb] Fix watchpoint command function stopping behavior

2023-02-23 Thread Med Ismail Bennani via Phabricator via lldb-commits
mib created this revision.
mib added a reviewer: bulbazord.
mib added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
mib requested review of this revision.
Herald added a subscriber: lldb-commits.

This patch fixes an issue where we would discard the return value of the
user-provided function for watchpoint commands.

That causes lldb to not to take into account the function expected
stopping behavior. To prevent that, this patch prepends  a return
statement to the function call, to ensure we behave as the user wanted.

By doing so, watchpoint commands now behave the same way was breakpoint
commands.

rdar://105461140

Signed-off-by: Med Ismail Bennani 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144688

Files:
  lldb/source/Commands/CommandObjectWatchpointCommand.cpp
  
lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
  lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
  
lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py


Index: 
lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py
===
--- 
lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py
+++ 
lldb/test/API/commands/watchpoints/watchpoint_commands/command/watchpoint_command.py
@@ -9,7 +9,8 @@
 print ("I stopped the first time")
 frame.EvaluateExpression("cookie = 888")
 num_hits += 1
-frame.thread.process.Continue()
+return True
 else:
 print ("I stopped the %d time" % (num_hits))
 frame.EvaluateExpression("cookie = 999")
+return False # This cause the process to continue.
Index: lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
===
--- lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
+++ lldb/test/API/commands/watchpoints/watchpoint_commands/command/main.cpp
@@ -16,5 +16,5 @@
 modify(global);
 
 printf("global=%d\n", global);
-printf("cookie=%d\n", cookie);
+printf("cookie=%d\n", cookie); // Set another breakpoint here.
 }
Index: 
lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
===
--- 
lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
+++ 
lldb/test/API/commands/watchpoints/watchpoint_commands/command/TestWatchpointCommandPython.py
@@ -1,4 +1,4 @@
-"""
+"""
 Test 'watchpoint command'.
 """
 
@@ -22,6 +22,8 @@
 # Find the line number to break inside main().
 self.line = line_number(
 self.source, '// Set break point at this line.')
+self.second_line = line_number(
+self.source, '// Set another breakpoint here.')
 # And the watchpoint variable declaration line number.
 self.decl = line_number(self.source,
 '// Watchpoint variable declaration.')
@@ -143,6 +145,20 @@
 self.expect("thread backtrace", STOPPED_DUE_TO_WATCHPOINT,
 substrs=['stop reason = watchpoint'])
 
+# We should have hit the watchpoint once, set cookie to 888, then 
continued to the
+# second hit and set it to 999
+self.expect("frame variable --show-globals cookie",
+substrs=['(int32_t)', 'cookie = 888'])
+
+# Add a breakpoint to set a watchpoint when stopped on the breakpoint.
+lldbutil.run_break_set_by_file_and_line(
+self, None, self.second_line, num_expected_locations=1)
+
+self.runCmd("process continue")
+
+self.expect("thread backtrace", STOPPED_DUE_TO_BREAKPOINT,
+substrs=['stop reason = breakpoint'])
+
 # We should have hit the watchpoint once, set cookie to 888, then 
continued to the
 # second hit and set it to 999
 self.expect("frame variable --show-globals cookie",
Index: lldb/source/Commands/CommandObjectWatchpointCommand.cpp
===
--- lldb/source/Commands/CommandObjectWatchpointCommand.cpp
+++ lldb/source/Commands/CommandObjectWatchpointCommand.cpp
@@ -422,7 +422,8 @@
   // automatize what the user would do manually: make their watchpoint
   // command be a function call
   else if (!m_options.m_function_name.empty()) {
-std::string oneliner(m_options.m_function_name);
+std::string oneliner("return ");
+oneliner += m_options.m_function_name;
 oneliner += "(frame, wp, internal_dict)";
 script_interp->SetWatchpointCommandCallback(
 wp_options, oneliner.c_str());


Index: lldb/test/API/commands/watchpoints/watchpoint_commands/comman