[Lldb-commits] [lldb] r254194 - Re-add an xfail removed by r254163
Author: tberghammer
Date: Fri Nov 27 04:50:33 2015
New Revision: 254194
URL: http://llvm.org/viewvc/llvm-project?rev=254194&view=rev
Log:
Re-add an xfail removed by r254163
The test is flakey but it fails too often with gcc 4.9.2 on x86_64 to
be marked only as expected flakey.
Modified:
lldb/trunk/packages/Python/lldbsuite/test/api/multithreaded/TestMultithreaded.py
Modified:
lldb/trunk/packages/Python/lldbsuite/test/api/multithreaded/TestMultithreaded.py
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/api/multithreaded/TestMultithreaded.py?rev=254194&r1=254193&r2=254194&view=diff
==
---
lldb/trunk/packages/Python/lldbsuite/test/api/multithreaded/TestMultithreaded.py
(original)
+++
lldb/trunk/packages/Python/lldbsuite/test/api/multithreaded/TestMultithreaded.py
Fri Nov 27 04:50:33 2015
@@ -37,6 +37,7 @@ class SBBreakpointCallbackCase(TestBase)
@skipIfWindows # clang-cl does not support throw or catch
(llvm.org/pr24538)
@expectedFlakeyFreeBSD
@expectedFlakeyLinux # Driver occasionally returns '1' as exit status
+@expectedFailureAll("llvm.org/pr23139", oslist=["linux"], compiler="gcc",
compiler_version=[">=","4.9"], archs=["x86_64"])
def test_sb_api_listener_event_process_state(self):
""" Test that a registered SBListener receives events when a process
changes state.
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D14952: Create new "platform process connect" command
labath added a comment.
Thank you for writing the test. I have just a couple of more comments and them
I'm done... :)
Please wait for @clayborg to sign off on this as well.
Comment at:
packages/Python/lldbsuite/test/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py:13
@@ +12,3 @@
+def test_platform_process_connect(self):
+if not lldb.remote_platform:
+self.skipTest("Only supported on remote platforms")
I'm quite opposed to "inline skips", I'd like them to be more declarative.
Let's add a "remote" parameter to the skipIf decorator and use that. Suggested
semantics:
- None: don't care
- True: skip if the target is remote
- False: skip if the target is *not* remote
Comment at:
packages/Python/lldbsuite/test/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py:26
@@ +25,3 @@
+port_file = "%s/port" % working_dir
+commandline_args = ["p", "-L", "*:0", "-f", port_file, "--",
"%s/a.out" % working_dir, "foo"]
+new_platform = self.spawnSubprocess(self.debug_monitor_exe,
commandline_args, install_remote=False)
Please spell out the arguments in full. It makes it much more obvious what is
going to happen.
Comment at:
packages/Python/lldbsuite/test/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py:29
@@ +28,3 @@
+new_port = self.run_shell_cmd("while [ ! -f %s ]; do sleep 0.25; done
&& cat %s" % (port_file, port_file))
+lldb.remote_platform.DisconnectRemote()
+
What will happen when the next test runs? Will we be able to reconnect to the
old platform instance? Please check whether this works when run in
--no-multiprocess mode with other tests...
If it does not work, we might be able to create a brand new SBDebugger instance
and use that to connect...
http://reviews.llvm.org/D14952
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r254200 - [LLGS] Don't forward I/O when process is stopped
Author: labath
Date: Fri Nov 27 07:33:29 2015
New Revision: 254200
URL: http://llvm.org/viewvc/llvm-project?rev=254200&view=rev
Log:
[LLGS] Don't forward I/O when process is stopped
Summary:
This makes sure we do not attempt to send output over the gdb-remote protocol
when the client is
not expecting it (i.e., after sending the stop-reply packet). Normally, this
should not happen
(the process cannot generate output when it is stopped), but due to the fact
that pty
communication is asynchronous in the linux kernel (llvm.org/pr25652), we may
sometimes get this
output too late. Instead, we just hold the output, and send it next time we
resume. This is not
ideal, but at least it makes sure we do not violate the remote protocol. Given
that this happens
extremely rarely it's not worth trying to work around it with sleeps or
something like that.
I also remove the m_stdio_communication_mutex, as all of LLGS is now
single-threaded anyway.
Reviewers: tberghammer, ovyalov
Subscribers: lldb-commits
Differential Revision: http://reviews.llvm.org/D15019
Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
Modified:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp?rev=254200&r1=254199&r2=254200&view=diff
==
---
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
(original)
+++
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
Fri Nov 27 07:33:29 2015
@@ -44,6 +44,7 @@
#include "lldb/Host/common/NativeProcessProtocol.h"
#include "lldb/Host/common/NativeThreadProtocol.h"
#include "lldb/Utility/JSON.h"
+#include "lldb/Utility/LLDBAssert.h"
// Project includes
#include "Utility/StringExtractorGDBRemote.h"
@@ -860,21 +861,30 @@ GDBRemoteCommunicationServerLLGS::Proces
StateAsCString (state));
}
-// Make sure we get all of the pending stdout/stderr from the inferior
-// and send it to the lldb host before we send the state change
-// notification
-SendProcessOutput();
-
switch (state)
{
-case StateType::eStateExited:
-HandleInferiorState_Exited (process);
+case StateType::eStateRunning:
+StartSTDIOForwarding();
break;
case StateType::eStateStopped:
+// Make sure we get all of the pending stdout/stderr from the inferior
+// and send it to the lldb host before we send the state change
+// notification
+SendProcessOutput();
+// Then stop the forwarding, so that any late output (see
llvm.org/pr25652) does not
+// interfere with our protocol.
+StopSTDIOForwarding();
HandleInferiorState_Stopped (process);
break;
+case StateType::eStateExited:
+// Same as above
+SendProcessOutput();
+StopSTDIOForwarding();
+HandleInferiorState_Exited (process);
+break;
+
default:
if (log)
{
@@ -975,7 +985,6 @@ GDBRemoteCommunicationServerLLGS::SetSTD
return error;
}
-Mutex::Locker locker(m_stdio_communication_mutex);
m_stdio_communication.SetCloseOnEOF (false);
m_stdio_communication.SetConnection (conn_up.release());
if (!m_stdio_communication.IsConnected ())
@@ -984,33 +993,42 @@ GDBRemoteCommunicationServerLLGS::SetSTD
return error;
}
+return Error();
+}
+
+void
+GDBRemoteCommunicationServerLLGS::StartSTDIOForwarding()
+{
+// Don't forward if not connected (e.g. when attaching).
+if (! m_stdio_communication.IsConnected())
+return;
+
// llgs local-process debugging may specify PTY paths, which will make
these
// file actions non-null
// process launch -e/o will also make these file actions non-null
// nullptr means that the traffic is expected to flow over gdb-remote
protocol
-if (
-m_process_launch_info.GetFileActionForFD(STDOUT_FILENO) == nullptr ||
-m_process_launch_info.GetFileActionForFD(STDERR_FILENO) == nullptr
-)
-{
-// output from the process must be forwarded over gdb-remote
-m_stdio_handle_up = m_mainloop.RegisterReadObject(
-m_stdio_communication.GetConnection()->GetReadObject(),
-[this] (MainLoopBase &) { SendProcessOutput(); }, error);
-if (! m_stdio_handle_up)
-{
-m_stdio_communication.Disconnect();
-return error;
-}
-}
+if ( m_process_launch_info.GetFileActionForFD(STDOUT_FILENO) &&
+ m_process_launch_info.GetFileActionForFD(STDERR_FILENO))
+return;
-return Error();
+Error error;
+lldbassert(! m_stdio_handl
Re: [Lldb-commits] [PATCH] D15019: [LLGS] Don't forward I/O when process is stopped
This revision was automatically updated to reflect the committed changes.
labath marked an inline comment as done.
Closed by commit rL254200: [LLGS] Don't forward I/O when process is stopped
(authored by labath).
Changed prior to commit:
http://reviews.llvm.org/D15019?vs=41241&id=41298#toc
Repository:
rL LLVM
http://reviews.llvm.org/D15019
Files:
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
Index: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -44,6 +44,7 @@
#include "lldb/Host/common/NativeProcessProtocol.h"
#include "lldb/Host/common/NativeThreadProtocol.h"
#include "lldb/Utility/JSON.h"
+#include "lldb/Utility/LLDBAssert.h"
// Project includes
#include "Utility/StringExtractorGDBRemote.h"
@@ -860,21 +861,30 @@
StateAsCString (state));
}
-// Make sure we get all of the pending stdout/stderr from the inferior
-// and send it to the lldb host before we send the state change
-// notification
-SendProcessOutput();
-
switch (state)
{
-case StateType::eStateExited:
-HandleInferiorState_Exited (process);
+case StateType::eStateRunning:
+StartSTDIOForwarding();
break;
case StateType::eStateStopped:
+// Make sure we get all of the pending stdout/stderr from the inferior
+// and send it to the lldb host before we send the state change
+// notification
+SendProcessOutput();
+// Then stop the forwarding, so that any late output (see llvm.org/pr25652) does not
+// interfere with our protocol.
+StopSTDIOForwarding();
HandleInferiorState_Stopped (process);
break;
+case StateType::eStateExited:
+// Same as above
+SendProcessOutput();
+StopSTDIOForwarding();
+HandleInferiorState_Exited (process);
+break;
+
default:
if (log)
{
@@ -975,42 +985,50 @@
return error;
}
-Mutex::Locker locker(m_stdio_communication_mutex);
m_stdio_communication.SetCloseOnEOF (false);
m_stdio_communication.SetConnection (conn_up.release());
if (!m_stdio_communication.IsConnected ())
{
error.SetErrorString ("failed to set connection for inferior I/O communication");
return error;
}
+return Error();
+}
+
+void
+GDBRemoteCommunicationServerLLGS::StartSTDIOForwarding()
+{
+// Don't forward if not connected (e.g. when attaching).
+if (! m_stdio_communication.IsConnected())
+return;
+
// llgs local-process debugging may specify PTY paths, which will make these
// file actions non-null
// process launch -e/o will also make these file actions non-null
// nullptr means that the traffic is expected to flow over gdb-remote protocol
-if (
-m_process_launch_info.GetFileActionForFD(STDOUT_FILENO) == nullptr ||
-m_process_launch_info.GetFileActionForFD(STDERR_FILENO) == nullptr
-)
-{
-// output from the process must be forwarded over gdb-remote
-m_stdio_handle_up = m_mainloop.RegisterReadObject(
-m_stdio_communication.GetConnection()->GetReadObject(),
-[this] (MainLoopBase &) { SendProcessOutput(); }, error);
-if (! m_stdio_handle_up)
-{
-m_stdio_communication.Disconnect();
-return error;
-}
-}
+if ( m_process_launch_info.GetFileActionForFD(STDOUT_FILENO) &&
+ m_process_launch_info.GetFileActionForFD(STDERR_FILENO))
+return;
-return Error();
+Error error;
+lldbassert(! m_stdio_handle_up);
+m_stdio_handle_up = m_mainloop.RegisterReadObject(
+m_stdio_communication.GetConnection()->GetReadObject(),
+[this] (MainLoopBase &) { SendProcessOutput(); }, error);
+
+if (! m_stdio_handle_up)
+{
+// Not much we can do about the failure. Log it and continue without forwarding.
+if (Log *log = GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS))
+log->Printf("GDBRemoteCommunicationServerLLGS::%s Failed to set up stdio forwarding: %s",
+__FUNCTION__, error.AsCString());
+}
}
void
GDBRemoteCommunicationServerLLGS::StopSTDIOForwarding()
{
-Mutex::Locker locker(m_stdio_communication_mutex);
m_stdio_handle_up.reset();
}
@@ -1020,7 +1038,6 @@
char buffer[1024];
ConnectionStatus status;
Error error;
-Mutex::Locker locker(m_stdio_communication_mutex);
while (true)
{
size_t bytes_read = m_stdio_communication.Read(buffer, sizeof buffer, 0, st
Re: [Lldb-commits] [PATCH] D15042: PTRACE ABI to read FXSAVE area for 32-bit inferior
tberghammer added inline comments. Comment at: source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp:849-850 @@ -844,2 +848,4 @@ m_fpr_type = eFPRTypeFXSAVE; +assert (const_cast(this)->ReadFPR().Success() && +"ptrace APIs to read either XSAVE or FXSAVE area didn't work."); } I think we don't want this assert because of 2 reason: * If we are running on a machine where we can't access to any floating point registers then this assert will fire but lldb is still functional in that case * It is changing the internal state of the object so it will cause a different behavior in debug and in release builds what we should avoid as it can cause hard to diagnose issues http://reviews.llvm.org/D15042 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Re: [Lldb-commits] [PATCH] D14952: Create new "platform process connect" command
tberghammer updated this revision to Diff 41312.
tberghammer marked 4 inline comments as done.
tberghammer added a comment.
Improve the test based on the comments
http://reviews.llvm.org/D14952
Files:
include/lldb/Target/Platform.h
packages/Python/lldbsuite/test/lldbtest.py
packages/Python/lldbsuite/test/tools/lldb-server/gdbremote_testcase.py
packages/Python/lldbsuite/test/tools/lldb-server/platform-process-connect/Makefile
packages/Python/lldbsuite/test/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py
packages/Python/lldbsuite/test/tools/lldb-server/platform-process-connect/main.cpp
source/Commands/CommandObjectPlatform.cpp
source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp
source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h
source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
source/Plugins/Platform/POSIX/PlatformPOSIX.h
source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.h
source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
tools/lldb-server/lldb-platform.cpp
Index: tools/lldb-server/lldb-platform.cpp
===
--- tools/lldb-server/lldb-platform.cpp
+++ tools/lldb-server/lldb-platform.cpp
@@ -285,6 +285,12 @@
exit(option_error);
}
+// Skip any options we consumed with getopt_long_only.
+argc -= optind;
+argv += optind;
+lldb_private::Args inferior_arguments;
+inferior_arguments.SetArguments(argc, const_cast(argv));
+
const bool children_inherit_listen_socket = false;
// the test suite makes many connections in parallel, let's not miss any.
// The highest this should get reasonably is a function of the number
@@ -317,7 +323,9 @@
do {
GDBRemoteCommunicationServerPlatform platform(acceptor_up->GetSocketProtocol(),
acceptor_up->GetSocketScheme());
-
+if (inferior_arguments.GetArgumentCount() > 0)
+platform.SetInferiorArguments(inferior_arguments);
+
if (port_offset > 0)
platform.SetPortOffset(port_offset);
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.h
@@ -66,6 +66,9 @@
void
SetPortOffset (uint16_t port_offset);
+void
+SetInferiorArguments (const lldb_private::Args& args);
+
protected:
const Socket::SocketProtocol m_socket_protocol;
const std::string m_socket_scheme;
@@ -75,6 +78,7 @@
PortMap m_port_map;
uint16_t m_port_offset;
+lldb_private::Args m_inferior_arguments;
PacketResult
Handle_qLaunchGDBServer (StringExtractorGDBRemote &packet);
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -165,7 +165,8 @@
Error error = StartDebugserverProcess (url.str().c_str(),
nullptr,
debugserver_launch_info,
- port_ptr);
+ port_ptr,
+ m_inferior_arguments);
lldb::pid_t debugserver_pid = debugserver_launch_info.GetProcessID();
@@ -553,7 +554,13 @@
}
void
-GDBRemoteCommunicationServerPlatform::SetPortOffset (uint16_t port_offset)
+GDBRemoteCommunicationServerPlatform::SetPortOffset(uint16_t port_offset)
{
m_port_offset = port_offset;
}
+
+void
+GDBRemoteCommunicationServerPlatform::SetInferiorArguments(const Args& args)
+{
+m_inferior_arguments = args;
+}
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h
@@ -25,6 +25,7 @@
#include "lldb/Host/Mutex.h"
#include "lldb/Host/Predicate.h"
#include "lldb/Host/TimeValue.h"
+#include "lldb/Interpreter/Args.h"
#include "Utility/StringExtractorGDBRemote.h"
@@ -168,7 +169,8 @@
StartDebugserverProcess(const char *url,
Platform *platform, // If non nullptr, then check with the platform for the GDB server binary if it can't be located
Re: [Lldb-commits] [PATCH] D14952: Create new "platform process connect" command
tberghammer added inline comments.
Comment at:
packages/Python/lldbsuite/test/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py:13
@@ +12,3 @@
+def test_platform_process_connect(self):
+if not lldb.remote_platform:
+self.skipTest("Only supported on remote platforms")
labath wrote:
> I'm quite opposed to "inline skips", I'd like them to be more declarative.
> Let's add a "remote" parameter to the skipIf decorator and use that.
> Suggested semantics:
> - None: don't care
> - True: skip if the target is remote
> - False: skip if the target is *not* remote
>
Done
Comment at:
packages/Python/lldbsuite/test/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py:26
@@ +25,3 @@
+port_file = "%s/port" % working_dir
+commandline_args = ["p", "-L", "*:0", "-f", port_file, "--",
"%s/a.out" % working_dir, "foo"]
+new_platform = self.spawnSubprocess(self.debug_monitor_exe,
commandline_args, install_remote=False)
labath wrote:
> Please spell out the arguments in full. It makes it much more obvious what is
> going to happen.
Done
Comment at:
packages/Python/lldbsuite/test/tools/lldb-server/platform-process-connect/TestPlatformProcessConnect.py:29
@@ +28,3 @@
+new_port = self.run_shell_cmd("while [ ! -f %s ]; do sleep 0.25; done
&& cat %s" % (port_file, port_file))
+lldb.remote_platform.DisconnectRemote()
+
labath wrote:
> What will happen when the next test runs? Will we be able to reconnect to the
> old platform instance? Please check whether this works when run in
> --no-multiprocess mode with other tests...
> If it does not work, we might be able to create a brand new SBDebugger
> instance and use that to connect...
I changed it to create a new debugger because the previous implementation only
worked if the test wasn't followed by a new one
http://reviews.llvm.org/D14952
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
