[Lldb-commits] [PATCH] D41008: MainLoop: avoid infinite loop when pty slave gets closed
labath created this revision.
For ptys (at least on Linux), the end-of-file (closing of the slave FD)
is signalled by the POLLHUP flag. We were ignoring this flag, which
meant that when this happened, we would spin in a loop, continuously
calling poll(2) and not making any progress.
This makes sure we treat POLLHUP as a read event (reading will return
0), and we call the registered callback when it happens. This is the
behavior our clients expect (and is consistent with how select(2)
works).
https://reviews.llvm.org/D41008
Files:
source/Host/common/MainLoop.cpp
unittests/Host/MainLoopTest.cpp
Index: unittests/Host/MainLoopTest.cpp
===
--- unittests/Host/MainLoopTest.cpp
+++ unittests/Host/MainLoopTest.cpp
@@ -8,6 +8,8 @@
//===--===//
#include "lldb/Host/MainLoop.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
+#include "lldb/Host/PseudoTerminal.h"
#include "lldb/Host/common/TCPSocket.h"
#include "gtest/gtest.h"
#include
@@ -107,6 +109,24 @@
}
#ifdef LLVM_ON_UNIX
+TEST_F(MainLoopTest, DetectsEOF) {
+ lldb_utility::PseudoTerminal term;
+ ASSERT_TRUE(term.OpenFirstAvailableMaster(O_RDWR, nullptr, 0));
+ ASSERT_TRUE(term.OpenSlave(O_RDWR | O_NOCTTY, nullptr, 0));
+ auto conn = llvm::make_unique(
+ term.ReleaseMasterFileDescriptor(), true);
+
+ Status error;
+ MainLoop loop;
+ auto handle =
+ loop.RegisterReadObject(conn->GetReadObject(), make_callback(), error);
+ ASSERT_TRUE(error.Success());
+ term.CloseSlaveFileDescriptor();
+
+ ASSERT_TRUE(loop.Run().Success());
+ ASSERT_EQ(1u, callback_count);
+}
+
TEST_F(MainLoopTest, Signal) {
MainLoop loop;
Status error;
Index: source/Host/common/MainLoop.cpp
===
--- source/Host/common/MainLoop.cpp
+++ source/Host/common/MainLoop.cpp
@@ -221,7 +221,7 @@
for (const auto &handle : fds) {
#else
for (const auto &fd : read_fds) {
-if ((fd.revents & POLLIN) == 0)
+if ((fd.revents & (POLLIN | POLLHUP)) == 0)
continue;
IOObject::WaitableHandle handle = fd.fd;
#endif
Index: unittests/Host/MainLoopTest.cpp
===
--- unittests/Host/MainLoopTest.cpp
+++ unittests/Host/MainLoopTest.cpp
@@ -8,6 +8,8 @@
//===--===//
#include "lldb/Host/MainLoop.h"
+#include "lldb/Host/ConnectionFileDescriptor.h"
+#include "lldb/Host/PseudoTerminal.h"
#include "lldb/Host/common/TCPSocket.h"
#include "gtest/gtest.h"
#include
@@ -107,6 +109,24 @@
}
#ifdef LLVM_ON_UNIX
+TEST_F(MainLoopTest, DetectsEOF) {
+ lldb_utility::PseudoTerminal term;
+ ASSERT_TRUE(term.OpenFirstAvailableMaster(O_RDWR, nullptr, 0));
+ ASSERT_TRUE(term.OpenSlave(O_RDWR | O_NOCTTY, nullptr, 0));
+ auto conn = llvm::make_unique(
+ term.ReleaseMasterFileDescriptor(), true);
+
+ Status error;
+ MainLoop loop;
+ auto handle =
+ loop.RegisterReadObject(conn->GetReadObject(), make_callback(), error);
+ ASSERT_TRUE(error.Success());
+ term.CloseSlaveFileDescriptor();
+
+ ASSERT_TRUE(loop.Run().Success());
+ ASSERT_EQ(1u, callback_count);
+}
+
TEST_F(MainLoopTest, Signal) {
MainLoop loop;
Status error;
Index: source/Host/common/MainLoop.cpp
===
--- source/Host/common/MainLoop.cpp
+++ source/Host/common/MainLoop.cpp
@@ -221,7 +221,7 @@
for (const auto &handle : fds) {
#else
for (const auto &fd : read_fds) {
-if ((fd.revents & POLLIN) == 0)
+if ((fd.revents & (POLLIN | POLLHUP)) == 0)
continue;
IOObject::WaitableHandle handle = fd.fd;
#endif
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41008: MainLoop: avoid infinite loop when pty slave gets closed
davide added subscribers: jingham, davide. davide accepted this revision. davide added a comment. This revision is now accepted and ready to land. LGTM. @jingham ? I'll try this change on macOS to make sure it won't break anything. https://reviews.llvm.org/D41008 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D41008: MainLoop: avoid infinite loop when pty slave gets closed
labath added a comment. Darwin uses a kqueue-based implementation of this, so it definitely won't *break* anything, although I'd appreciate it if you can check whether the test passes there. My cursory reading of the kqueue man page makes me believe it should not be affected by this. https://reviews.llvm.org/D41008 ___ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r320240 - Update PlatformDarwin::GetDeveloperDir to handle the two
Author: jmolenda
Date: Fri Dec 8 19:06:19 2017
New Revision: 320240
URL: http://llvm.org/viewvc/llvm-project?rev=320240&view=rev
Log:
Update PlatformDarwin::GetDeveloperDir to handle the two
most common cases where the Xcode.app bundle puts lldb -
either as a default part of the bundle, or in a toolchain
subdirectory, so the platform subclasses can find files
relative to this directory.
Dropped support for handling the case where the lldb
framework was in /Library/PrivateFrameworks. I think
this was intended to handle the case where lldb is installed
in / (outside the Xcode.app bundle) - but in that case, we
can look in the raw directory file paths to find anything.
Modified:
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp?rev=320240&r1=320239&r2=320240&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp Fri Dec 8
19:06:19 2017
@@ -1132,28 +1132,33 @@ bool PlatformDarwin::ARMGetSupportedArch
return false;
}
+// Return a directory path like /Applications/Xcode.app/Contents/Developer
const char *PlatformDarwin::GetDeveloperDirectory() {
std::lock_guard guard(m_mutex);
if (m_developer_directory.empty()) {
bool developer_dir_path_valid = false;
char developer_dir_path[PATH_MAX];
FileSpec temp_file_spec;
+
+// Get the lldb framework's file path, and if it exists, truncate some
+// components to only the developer directory path.
if (HostInfo::GetLLDBPath(ePathTypeLLDBShlibDir, temp_file_spec)) {
if (temp_file_spec.GetPath(developer_dir_path,
sizeof(developer_dir_path))) {
+// e.g.
/Applications/Xcode.app/Contents/SharedFrameworks/LLDB.framework
char *shared_frameworks =
strstr(developer_dir_path, "/SharedFrameworks/LLDB.framework");
if (shared_frameworks) {
- ::snprintf(shared_frameworks,
- sizeof(developer_dir_path) -
- (shared_frameworks - developer_dir_path),
- "/Developer");
+ shared_frameworks[0] = '\0'; // truncate developer_dir_path at this
point
+ strncat (developer_dir_path, "/Developer", sizeof
(developer_dir_path) - 1); // add /Developer on
developer_dir_path_valid = true;
} else {
- char *lib_priv_frameworks = strstr(
- developer_dir_path, "/Library/PrivateFrameworks/LLDB.framework");
- if (lib_priv_frameworks) {
-*lib_priv_frameworks = '\0';
+ // e.g.
/Applications/Xcode.app/Contents/Developer/Toolchains/iOS11.2.xctoolchain/System/Library/PrivateFrameworks/LLDB.framework
+ char *developer_toolchains =
+strstr(developer_dir_path, "/Contents/Developer/Toolchains/");
+ if (developer_toolchains) {
+developer_toolchains += sizeof ("/Contents/Developer") - 1;
+developer_toolchains[0] = '\0'; // truncate developer_dir_path at
this point
developer_dir_path_valid = true;
}
}
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r320241 - Change the ordering that we search for kexts and kernels on the local
Author: jmolenda
Date: Fri Dec 8 19:28:15 2017
New Revision: 320241
URL: http://llvm.org/viewvc/llvm-project?rev=320241&view=rev
Log:
Change the ordering that we search for kexts and kernels on the local
computer. When doing kernel debugging, lldb scrapes around a few
well-known locations to find kexts and kernels. It builds up two
lists - kexts and kernels with dSYM, and kexts and kernels without dSYMs.
After both lists have failed to provide a file, then we'll call out
to things like the DebugSymbols framework to find a kext/kernel.
This meant that when you had a kext/kernel on the local computer that
did not have debug information, lldb wouldn't consult DebugSymbols etc
once it'd locked on to one of these no-debug-info binaries on the local
computer.
Reorder this so we give DebugSymbols etc a shot at finding a debug-info
file before we use any of the no-debug-info binaries that were found on
the system.
Modified:
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp?rev=320241&r1=320240&r2=320241&view=diff
==
--- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
(original)
+++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp Fri Dec
8 19:28:15 2017
@@ -694,7 +694,16 @@ Status PlatformDarwinKernel::GetSharedMo
}
}
-// Second look through the kext binarys without dSYMs
+// Give the generic methods, including possibly calling into
+// DebugSymbols framework on macOS systems, a chance.
+error = PlatformDarwin::GetSharedModule(module_spec, process, module_sp,
+ module_search_paths_ptr,
+ old_module_sp_ptr, did_create_ptr);
+if (error.Success() && module_sp.get()) {
+ return error;
+}
+
+// Lastly, look through the kext binarys without dSYMs
if (m_name_to_kext_path_map_without_dsyms.count(kext_bundle_cs) > 0) {
for (BundleIDToKextIterator it =
m_name_to_kext_path_map_without_dsyms.begin();
@@ -739,7 +748,17 @@ Status PlatformDarwinKernel::GetSharedMo
}
}
}
-// Second try all kernel binaries that don't have a dSYM
+
+// Give the generic methods, including possibly calling into
+// DebugSymbols framework on macOS systems, a chance.
+error = PlatformDarwin::GetSharedModule(module_spec, process, module_sp,
+module_search_paths_ptr,
+old_module_sp_ptr, did_create_ptr);
+if (error.Success() && module_sp.get()) {
+ return error;
+}
+
+// Next try all kernel binaries that don't have a dSYM
for (auto possible_kernel : m_kernel_binaries_without_dsyms) {
if (possible_kernel.Exists()) {
ModuleSpec kern_spec(possible_kernel);
@@ -767,11 +786,7 @@ Status PlatformDarwinKernel::GetSharedMo
}
}
- // Else fall back to treating the file's path as an actual file path - defer
- // to PlatformDarwin's GetSharedModule.
- return PlatformDarwin::GetSharedModule(module_spec, process, module_sp,
- module_search_paths_ptr,
- old_module_sp_ptr, did_create_ptr);
+ return error;
}
Status PlatformDarwinKernel::ExamineKextForMatchingUUID(
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] r320242 - Change uses of strncpy in debugserver to strlcpy
Author: jmolenda
Date: Fri Dec 8 19:37:09 2017
New Revision: 320242
URL: http://llvm.org/viewvc/llvm-project?rev=320242&view=rev
Log:
Change uses of strncpy in debugserver to strlcpy
for better safety.
Modified:
lldb/trunk/tools/debugserver/source/DNB.cpp
lldb/trunk/tools/debugserver/source/DNBRegisterInfo.cpp
lldb/trunk/tools/debugserver/source/MacOSX/MachThread.cpp
lldb/trunk/tools/debugserver/source/RNBRemote.cpp
lldb/trunk/tools/debugserver/source/debugserver.cpp
Modified: lldb/trunk/tools/debugserver/source/DNB.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/DNB.cpp?rev=320242&r1=320241&r2=320242&view=diff
==
--- lldb/trunk/tools/debugserver/source/DNB.cpp (original)
+++ lldb/trunk/tools/debugserver/source/DNB.cpp Fri Dec 8 19:37:09 2017
@@ -368,7 +368,7 @@ nub_process_t DNBProcessLaunch(
if (launch_err.Fail()) {
const char *launch_err_str = launch_err.AsString();
if (launch_err_str) {
- strncpy(err_str, launch_err_str, err_len - 1);
+ strlcpy(err_str, launch_err_str, err_len - 1);
err_str[err_len - 1] =
'\0'; // Make sure the error string is terminated
}
@@ -1698,7 +1698,7 @@ nub_bool_t DNBResolveExecutablePath(cons
if (realpath(path, max_path)) {
// Found the path relatively...
-::strncpy(resolved_path, max_path, resolved_path_size);
+::strlcpy(resolved_path, max_path, resolved_path_size);
return strlen(resolved_path) + 1 < resolved_path_size;
} else {
// Not a relative path, check the PATH environment variable if the
@@ -1722,7 +1722,7 @@ nub_bool_t DNBResolveExecutablePath(cons
result += path;
struct stat s;
if (stat(result.c_str(), &s) == 0) {
- ::strncpy(resolved_path, result.c_str(), resolved_path_size);
+ ::strlcpy(resolved_path, result.c_str(), resolved_path_size);
return result.size() + 1 < resolved_path_size;
}
}
Modified: lldb/trunk/tools/debugserver/source/DNBRegisterInfo.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/DNBRegisterInfo.cpp?rev=320242&r1=320241&r2=320242&view=diff
==
--- lldb/trunk/tools/debugserver/source/DNBRegisterInfo.cpp (original)
+++ lldb/trunk/tools/debugserver/source/DNBRegisterInfo.cpp Fri Dec 8 19:37:09
2017
@@ -35,7 +35,7 @@ bool DNBRegisterValueClass::IsValid() co
do {
\
if (pos < end) {
\
if (i > 0) {
\
-strncpy(pos, ", ", end - pos);
\
+strlcpy(pos, ", ", end - pos);
\
pos += 2;
\
}
\
}
\
@@ -69,7 +69,7 @@ void DNBRegisterValueClass::Dump(const c
value.v_uint64[1]);
break;
default:
-strncpy(str, "0x", 3);
+strlcpy(str, "0x", 3);
pos = str + 2;
for (uint32_t i = 0; i < info.size; ++i) {
if (pos < end)
Modified: lldb/trunk/tools/debugserver/source/MacOSX/MachThread.cpp
URL:
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/source/MacOSX/MachThread.cpp?rev=320242&r1=320241&r2=320242&view=diff
==
--- lldb/trunk/tools/debugserver/source/MacOSX/MachThread.cpp (original)
+++ lldb/trunk/tools/debugserver/source/MacOSX/MachThread.cpp Fri Dec 8
19:37:09 2017
@@ -164,15 +164,15 @@ const char *MachThread::GetBasicInfoAsSt
//size_t run_state_str_size = sizeof(run_state_str);
//switch (basicInfo.run_state)
//{
-//case TH_STATE_RUNNING: strncpy(run_state_str, "running",
+//case TH_STATE_RUNNING: strlcpy(run_state_str, "running",
//run_state_str_size); break;
-//case TH_STATE_STOPPED: strncpy(run_state_str, "stopped",
+//case TH_STATE_STOPPED: strlcpy(run_state_str, "stopped",
//run_state_str_size); break;
-//case TH_STATE_WAITING: strncpy(run_state_str, "waiting",
+//case TH_STATE_WAITING: strlcpy(run_state_str, "waiting",
//run_state_str_size); break;
-//case TH_STATE_UNINTERRUPTIBLE: strncpy(run_state_str,
+//case TH_STATE_UNINTERRUPTIBLE: strlcpy(run_state_str,
//"uninterruptible", run_state_str_size); break;
-
