Re: [Lldb-commits] [PATCH] D18886: Reset continue_after_async only if neither SIGINIT nor SIGSTOP received
labath added a comment. A random thought: will `getchar()` block the inferior on windows (because of missing stdio forwarding, et al.). If it wont then this could be the cause of the flakyness. If that's the case, then we can replace that call with something that will surely halt progress, like an infinite loop with sleep or something. http://reviews.llvm.org/D18886 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18886: Reset continue_after_async only if neither SIGINIT nor SIGSTOP received
labath added a comment.
In http://reviews.llvm.org/D18886#397843, @amccarth wrote:
> It's weird in that, if you run the test independently, it passes. But if you
> run it with the multiprocess test runner (ninja check-lldb), then it fails on
> this line:
>
> self.fail("Setting a breakpoint generated an unexpected event: %s" %
> lldb.SBDebugger.StateAsCString(lldb.SBProcess.GetStateFromEvent(event)))
>
> The state in this case is stopped.
I suspect this test has uncovered a problem in the ProcessWindows
implementation. The same problem we were solving here for ProcessGdbRemote.
Namely, the question here is what happens if we try to set a breakpoint while
the process is running. ProcessGdbRemote (and I expect ProcessWindows as well)
needs to stop the inferior to be able to set the breakpoint. However, since
this stop wasn't requested by the user, it should not generate any externally
visible stops. The process should be silently resumed after you are done with
it (I have no idea how is this supposed to be done with the non-gdb-remote
processes).
I suspect the test is flaky for you because of different timings when running
under heavy system load, but after the issue is fixed the test should pass 100%
of time. Unfortunately, I think this task will be up to you. :)
http://reviews.llvm.org/D18886
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18886: Reset continue_after_async only if neither SIGINIT nor SIGSTOP received
amccarth added a comment.
It's weird in that, if you run the test independently, it passes. But if you
run it with the multiprocess test runner (ninja check-lldb), then it fails on
this line:
self.fail("Setting a breakpoint generated an unexpected event: %s" %
lldb.SBDebugger.StateAsCString(lldb.SBProcess.GetStateFromEvent(event)))
The state in this case is stopped.
http://reviews.llvm.org/D18886
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18886: Reset continue_after_async only if neither SIGINIT nor SIGSTOP received
ovyalov added a comment. In http://reviews.llvm.org/D18886#397637, @amccarth wrote: > FYI: According to git bisect, this patch seems to have introduced a new test > failure on Windows. Thanks for the report - will fix today. http://reviews.llvm.org/D18886 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18886: Reset continue_after_async only if neither SIGINIT nor SIGSTOP received
amccarth added a subscriber: amccarth. amccarth added a comment. FYI: According to git bisect, this patch seems to have introduced a new test failure on Windows. http://reviews.llvm.org/D18886 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18886: Reset continue_after_async only if neither SIGINIT nor SIGSTOP received
ovyalov closed this revision. ovyalov added a comment. Submitted as http://reviews.llvm.org/rL265843 http://reviews.llvm.org/D18886 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18886: Reset continue_after_async only if neither SIGINIT nor SIGSTOP received
ovyalov added a comment.
In http://reviews.llvm.org/D18886#395216, @labath wrote:
> Does this fix an existing test or is a new issue? If it's new (it sounds like
> it is, as I don't see any test failures), could you also add a test for this.
> It shouldn't be too difficult to write one... Would something like this:
>
> dbg.SetAsync(true)
> process.Continue()
> lldbutil.expect_state_changes(self,dbg.GetListener(), [eStateRunning])
> target.BreakpointCreateBySourceRegex("// some code which will not get
> executed by the inferior")
> do_some_assertions_on_the_breakpoint()
> while dbg.GetListener().WaitForEvent(2, event):
> if SBProcess.GetStateFromEvent(event) == eStateStopped and
> SBProcess.GetRestartedFromEvent(state):
> continue
> if SBProcess.GetStateFromEvent(event) == eStateRunning:
> continue
> self.fail("Setting a breakpoint generated an unexpected event:
> %s"%SBDebugger.StateAsCString(SBProcess.GetStateFromEvent(event)));
>
>
> do the trick?
Thanks for suggested test, Pavel.
http://reviews.llvm.org/D18886
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D18886: Reset continue_after_async only if neither SIGINIT nor SIGSTOP received
ovyalov updated this revision to Diff 53054.
ovyalov added a comment.
Added new TestBreakpointSetRestart test to cover the addressed issue.
http://reviews.llvm.org/D18886
Files:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/Makefile
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/TestBreakpointSetRestart.py
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1165,12 +1165,13 @@
// binaries that would send two stop replies
anytime the process
// was interrupted, so we need to also check for
an extra
// stop reply packet if we interrupted the process
-if (m_interrupt_sent || (signo != sigint_signo &&
signo != sigstop_signo))
+const bool received_nonstop_signal = signo !=
sigint_signo && signo != sigstop_signo;
+if (m_interrupt_sent || received_nonstop_signal)
{
-continue_after_async = false;
+if (received_nonstop_signal)
+continue_after_async = false;
-// We didn't get a SIGINT or SIGSTOP, so try
for a
-// very brief time (0.1s) to get another stop
reply
+// Try for a very brief time (0.1s) to get
another stop reply
// packet to make sure it doesn't get in the
way
StringExtractorGDBRemote
extra_stop_reply_packet;
uint32_t timeout_usec = 10;
Index:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp
===
---
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp
+++
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/main.cpp
@@ -0,0 +1,19 @@
+//===-- main.cpp *- C++
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include
+#include
+
+int main(int argc, char const *argv[])
+{
+getchar();
+printf("Set a breakpoint here.\n");
+return 0;
+}
+
Index:
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/TestBreakpointSetRestart.py
===
---
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/TestBreakpointSetRestart.py
+++
packages/Python/lldbsuite/test/functionalities/breakpoint/breakpoint_set_restart/TestBreakpointSetRestart.py
@@ -0,0 +1,37 @@
+"""
+Test inferior restart when breakpoint is set on running target.
+"""
+
+import os
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+class BreakpointSetRestart(TestBase):
+
+mydir = TestBase.compute_mydir(__file__)
+BREAKPOINT_TEXT = 'Set a breakpoint here'
+
+def test_breakpoint_set_restart(self):
+self.build()
+
+cwd = self.get_process_working_directory()
+exe = os.path.join(cwd, "a.out")
+target = self.dbg.CreateTarget(exe)
+
+self.dbg.SetAsync(True)
+process = target.LaunchSimple(None, None, cwd)
+
+lldbutil.expect_state_changes(self, self.dbg.GetListener(),
[lldb.eStateRunning])
+bp = target.BreakpointCreateBySourceRegex(self.BREAKPOINT_TEXT,
+
lldb.SBFileSpec(os.path.join(cwd, 'main.cpp')))
+self.assertTrue(bp.IsValid() and bp.GetNumLocations() == 1,
VALID_BREAKPOINT)
+
+event = lldb.SBEvent()
+while self.dbg.GetListener().WaitForEvent(2, event):
+if lldb.SBProcess.GetStateFromEvent(event) == lldb.eStateStopped
and lldb.SBProcess.GetRestartedFromEvent(event):
+continue
+if lldb.SBProcess.GetStateFromEvent(event) == lldb.eStateRunning:
+continue
+self.fail("Setting a breakpoint generated an unexpected event: %s"
% lldb.SBDebugger.StateAsCString(lldb.SBProcess.GetStateFromEvent(event)))
+
Index:
Re: [Lldb-commits] [PATCH] D18886: Reset continue_after_async only if neither SIGINIT nor SIGSTOP received
clayborg accepted this revision. clayborg added a comment. This revision is now accepted and ready to land. Looks good. http://reviews.llvm.org/D18886 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D18886: Reset continue_after_async only if neither SIGINIT nor SIGSTOP received
ovyalov created this revision.
ovyalov added a reviewer: clayborg.
ovyalov added a subscriber: lldb-commits.
Reset continue_after_async only if neither SIGINIT nor SIGSTOP received -
otherwise it leads to stopped inferior when setting breakpoint (when
m_interrupt_sent == true and signal is SIGSTOP).
http://reviews.llvm.org/D18886
Files:
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1165,12 +1165,13 @@
// binaries that would send two stop replies
anytime the process
// was interrupted, so we need to also check for
an extra
// stop reply packet if we interrupted the process
-if (m_interrupt_sent || (signo != sigint_signo &&
signo != sigstop_signo))
+const bool received_nonstop_signal = signo !=
sigint_signo && signo != sigstop_signo;
+if (m_interrupt_sent || received_nonstop_signal)
{
-continue_after_async = false;
+if (received_nonstop_signal)
+continue_after_async = false;
-// We didn't get a SIGINT or SIGSTOP, so try
for a
-// very brief time (0.1s) to get another stop
reply
+// Try for a very brief time (0.1s) to get
another stop reply
// packet to make sure it doesn't get in the
way
StringExtractorGDBRemote
extra_stop_reply_packet;
uint32_t timeout_usec = 10;
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1165,12 +1165,13 @@
// binaries that would send two stop replies anytime the process
// was interrupted, so we need to also check for an extra
// stop reply packet if we interrupted the process
-if (m_interrupt_sent || (signo != sigint_signo && signo != sigstop_signo))
+const bool received_nonstop_signal = signo != sigint_signo && signo != sigstop_signo;
+if (m_interrupt_sent || received_nonstop_signal)
{
-continue_after_async = false;
+if (received_nonstop_signal)
+continue_after_async = false;
-// We didn't get a SIGINT or SIGSTOP, so try for a
-// very brief time (0.1s) to get another stop reply
+// Try for a very brief time (0.1s) to get another stop reply
// packet to make sure it doesn't get in the way
StringExtractorGDBRemote extra_stop_reply_packet;
uint32_t timeout_usec = 10;
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
