[Lldb-commits] [lldb] d8bd179 - Clear read_fd_set if EINTR received
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
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
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
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
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
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
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
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
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
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
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
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
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