[Lldb-commits] [lldb] r320345 - MainLoop: avoid infinite loop when pty slave gets closed

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Dec 11 01:33:18 2017
New Revision: 320345

URL: http://llvm.org/viewvc/llvm-project?rev=320345&view=rev
Log:
MainLoop: avoid infinite loop when pty slave gets closed

Summary:
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).

Reviewers: eugene, beanz

Subscribers: lldb-commits

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

Modified:
lldb/trunk/source/Host/common/MainLoop.cpp
lldb/trunk/unittests/Host/MainLoopTest.cpp

Modified: lldb/trunk/source/Host/common/MainLoop.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/MainLoop.cpp?rev=320345&r1=320344&r2=320345&view=diff
==
--- lldb/trunk/source/Host/common/MainLoop.cpp (original)
+++ lldb/trunk/source/Host/common/MainLoop.cpp Mon Dec 11 01:33:18 2017
@@ -221,7 +221,7 @@ void MainLoop::RunImpl::ProcessEvents()
   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

Modified: lldb/trunk/unittests/Host/MainLoopTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Host/MainLoopTest.cpp?rev=320345&r1=320344&r2=320345&view=diff
==
--- lldb/trunk/unittests/Host/MainLoopTest.cpp (original)
+++ lldb/trunk/unittests/Host/MainLoopTest.cpp Mon Dec 11 01:33:18 2017
@@ -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 @@ TEST_F(MainLoopTest, TerminatesImmediate
 }
 
 #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;


___
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

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320345: MainLoop: avoid infinite loop when pty slave gets 
closed (authored by labath).

Repository:
  rL LLVM

https://reviews.llvm.org/D41008

Files:
  lldb/trunk/source/Host/common/MainLoop.cpp
  lldb/trunk/unittests/Host/MainLoopTest.cpp


Index: lldb/trunk/unittests/Host/MainLoopTest.cpp
===
--- lldb/trunk/unittests/Host/MainLoopTest.cpp
+++ lldb/trunk/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: lldb/trunk/source/Host/common/MainLoop.cpp
===
--- lldb/trunk/source/Host/common/MainLoop.cpp
+++ lldb/trunk/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: lldb/trunk/unittests/Host/MainLoopTest.cpp
===
--- lldb/trunk/unittests/Host/MainLoopTest.cpp
+++ lldb/trunk/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: lldb/trunk/source/Host/common/MainLoop.cpp
===
--- lldb/trunk/source/Host/common/MainLoop.cpp
+++ lldb/trunk/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] [lldb] r320346 - Move PseudoTerminal to the lldb_private namespace

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Dec 11 02:09:14 2017
New Revision: 320346

URL: http://llvm.org/viewvc/llvm-project?rev=320346&view=rev
Log:
Move PseudoTerminal to the lldb_private namespace

lldb_utility doesn't make sense, as it is no longer even living in the
"utility" module.

Modified:
lldb/trunk/include/lldb/Host/PseudoTerminal.h
lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h
lldb/trunk/source/Host/common/PseudoTerminal.cpp
lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
lldb/trunk/source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp
lldb/trunk/source/Plugins/Process/Darwin/DarwinProcessLauncher.cpp
lldb/trunk/source/Plugins/Process/FreeBSD/ProcessMonitor.cpp
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Target/Platform.cpp
lldb/trunk/source/Target/ProcessLaunchInfo.cpp
lldb/trunk/unittests/Editline/EditlineTest.cpp
lldb/trunk/unittests/Host/MainLoopTest.cpp

Modified: lldb/trunk/include/lldb/Host/PseudoTerminal.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/PseudoTerminal.h?rev=320346&r1=320345&r2=320346&view=diff
==
--- lldb/trunk/include/lldb/Host/PseudoTerminal.h (original)
+++ lldb/trunk/include/lldb/Host/PseudoTerminal.h Mon Dec 11 02:09:14 2017
@@ -9,14 +9,13 @@
 
 #ifndef LLDB_HOST_PSEUDOTERMINAL_H
 #define LLDB_HOST_PSEUDOTERMINAL_H
-#if defined(__cplusplus)
 
 #include 
 #include 
 
 #include "lldb/lldb-defines.h"
 
-namespace lldb_utility {
+namespace lldb_private {
 
 //--
 /// @class PseudoTerminal PseudoTerminal.h "lldb/Host/PseudoTerminal.h"
@@ -246,7 +245,6 @@ private:
   DISALLOW_COPY_AND_ASSIGN(PseudoTerminal);
 };
 
-} // namespace lldb_utility
+} // namespace lldb_private
 
-#endif // #if defined(__cplusplus)
 #endif // #ifndef liblldb_PseudoTerminal_h_

Modified: lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h?rev=320346&r1=320345&r2=320346&view=diff
==
--- lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h (original)
+++ lldb/trunk/include/lldb/Target/ProcessLaunchInfo.h Mon Dec 11 02:09:14 2017
@@ -117,7 +117,7 @@ public:
 
   bool MonitorProcess() const;
 
-  lldb_utility::PseudoTerminal &GetPTY() { return *m_pty; }
+  PseudoTerminal &GetPTY() { return *m_pty; }
 
   // Get and set the actual listener that will be used for the process events
   lldb::ListenerSP GetListener() const { return m_listener_sp; }
@@ -150,7 +150,7 @@ protected:
   FileSpec m_shell;
   Flags m_flags; // Bitwise OR of bits from lldb::LaunchFlags
   std::vector m_file_actions; // File actions for any other files
-  std::shared_ptr m_pty;
+  std::shared_ptr m_pty;
   uint32_t m_resume_count; // How many times do we resume after launching
   Host::MonitorChildProcessCallback m_monitor_callback;
   void *m_monitor_callback_baton;

Modified: lldb/trunk/source/Host/common/PseudoTerminal.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Host/common/PseudoTerminal.cpp?rev=320346&r1=320345&r2=320346&view=diff
==
--- lldb/trunk/source/Host/common/PseudoTerminal.cpp (original)
+++ lldb/trunk/source/Host/common/PseudoTerminal.cpp Mon Dec 11 02:09:14 2017
@@ -24,7 +24,7 @@
 int posix_openpt(int flags);
 #endif
 
-using namespace lldb_utility;
+using namespace lldb_private;
 
 //--
 // PseudoTerminal constructor

Modified: lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp?rev=320346&r1=320345&r2=320346&view=diff
==
--- lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Platform/Linux/PlatformLinux.cpp Mon Dec 11 
02:09:14 2017
@@ -372,7 +372,7 @@ PlatformLinux::DebugProcess(ProcessLaunc
 // Hook up process PTY if we have one (which we should for local debugging
 // with llgs).
 int pty_fd = launch_info.GetPTY().ReleaseMasterFileDescriptor();
-if (pty_fd != lldb_utility::PseudoTerminal::invalid_fd) {
+if (pty_fd != PseudoTerminal::invalid_fd) {
   process_sp->SetSTDIOFileDescriptor(pty_fd);
   LLDB_LOG(log, "hooked up STDIO pty to process");
 } else

Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformAppleSimulator.cpp?rev=320346&r1=320345&r2=320346&view=diff

[Lldb-commits] [lldb] r320349 - Fix osx build broken in r320346

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Dec 11 03:12:22 2017
New Revision: 320349

URL: http://llvm.org/viewvc/llvm-project?rev=320349&view=rev
Log:
Fix osx build broken in r320346

Modified:

lldb/trunk/source/Plugins/Platform/MacOSX/PlatformiOSSimulatorCoreSimulatorSupport.mm

Modified: 
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformiOSSimulatorCoreSimulatorSupport.mm
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformiOSSimulatorCoreSimulatorSupport.mm?rev=320349&r1=320348&r2=320349&view=diff
==
--- 
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformiOSSimulatorCoreSimulatorSupport.mm
 (original)
+++ 
lldb/trunk/source/Plugins/Platform/MacOSX/PlatformiOSSimulatorCoreSimulatorSupport.mm
 Mon Dec 11 03:12:22 2017
@@ -22,7 +22,6 @@
 #include "llvm/ADT/StringRef.h"
 
 using namespace lldb_private;
-using namespace lldb_utility;
 // CoreSimulator lives as part of Xcode, which means we can't really link
 // against it, so we dlopen()
 // it at runtime, and error out nicely if that fails


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

[Lldb-commits] [PATCH] D41066: llgs-tests: Make addition of new tests easier

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
Herald added a subscriber: mgorny.

Adding a new test would require one to duplicate a significant part of
the existing test that we have. This attempts to reduce that by moving
some part of that code to the test fixture. The StandardStartupTest
fixture automatically starts up the server and connects it to the
client. I also add a more low-level TestBase fixture, which allows one
to start up the client and server in a custom way (I am going to need
this for the test I am writing).


https://reviews.llvm.org/D41066

Files:
  unittests/tools/lldb-server/CMakeLists.txt
  unittests/tools/lldb-server/tests/CMakeLists.txt
  unittests/tools/lldb-server/tests/MessageObjects.h
  unittests/tools/lldb-server/tests/TestBase.cpp
  unittests/tools/lldb-server/tests/TestBase.h
  unittests/tools/lldb-server/tests/TestClient.cpp
  unittests/tools/lldb-server/tests/TestClient.h
  unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp

Index: unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
===
--- unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
+++ unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
@@ -7,38 +7,31 @@
 //
 //===--===//
 
+#include "TestBase.h"
 #include "TestClient.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 #include 
 
 using namespace llgs_tests;
+using namespace llvm;
 
-class ThreadsInJstopinfoTest : public ::testing::Test {
-protected:
-  virtual void SetUp() { TestClient::Initialize(); }
-};
+TEST_F(StandardStartupTest, TestStopReplyContainsThreadPcs) {
+  // This inferior spawns 4 threads, then forces a break.
+  ASSERT_THAT_ERROR(
+  Client->SetInferior({getInferiorPath("thread_inferior"), "4"}),
+  Succeeded());
 
-TEST_F(ThreadsInJstopinfoTest, TestStopReplyContainsThreadPcsLlgs) {
-  std::vector inferior_args;
-  // This inferior spawns N threads, then forces a break.
-  inferior_args.push_back(THREAD_INFERIOR);
-  inferior_args.push_back("4");
-
-  auto test_info = ::testing::UnitTest::GetInstance()->current_test_info();
-
-  TestClient client(test_info->name(), test_info->test_case_name());
-  ASSERT_THAT_ERROR(client.StartDebugger(), llvm::Succeeded());
-  ASSERT_THAT_ERROR(client.SetInferior(inferior_args), llvm::Succeeded());
-  ASSERT_THAT_ERROR(client.ListThreadsInStopReply(), llvm::Succeeded());
-  ASSERT_THAT_ERROR(client.ContinueAll(), llvm::Succeeded());
-  unsigned int pc_reg = client.GetPcRegisterId();
+  ASSERT_THAT_ERROR(Client->ListThreadsInStopReply(), Succeeded());
+  ASSERT_THAT_ERROR(Client->ContinueAll(), Succeeded());
+  unsigned int pc_reg = Client->GetPcRegisterId();
   ASSERT_NE(pc_reg, UINT_MAX);
 
-  auto jthreads_info = client.GetJThreadsInfo();
+  auto jthreads_info = Client->GetJThreadsInfo();
   ASSERT_TRUE(jthreads_info);
 
-  auto stop_reply = client.GetLatestStopReply();
+  auto stop_reply = Client->GetLatestStopReply();
   auto stop_reply_pcs = stop_reply.GetThreadPcs();
   auto thread_infos = jthreads_info->GetThreadInfos();
   ASSERT_EQ(stop_reply_pcs.size(), thread_infos.size())
@@ -49,10 +42,8 @@
 ASSERT_TRUE(thread_infos.find(tid) != thread_infos.end())
 << "Thread ID: " << tid << " not in JThreadsInfo.";
 auto pc_value = thread_infos[tid].ReadRegisterAsUint64(pc_reg);
-ASSERT_THAT_EXPECTED(pc_value, llvm::Succeeded());
+ASSERT_THAT_EXPECTED(pc_value, Succeeded());
 ASSERT_EQ(stop_reply_pcs[tid], *pc_value)
 << "Mismatched PC for thread: " << tid;
   }
-
-  ASSERT_THAT_ERROR(client.StopDebugger(), llvm::Succeeded());
 }
Index: unittests/tools/lldb-server/tests/TestClient.h
===
--- unittests/tools/lldb-server/tests/TestClient.h
+++ unittests/tools/lldb-server/tests/TestClient.h
@@ -7,28 +7,35 @@
 //
 //===--===//
 
+#ifndef LLDB_SERVER_TESTS_TESTCLIENT_H
+#define LLDB_SERVER_TESTS_TESTCLIENT_H
+
 #include "MessageObjects.h"
 #include "Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h"
 #include "lldb/Target/ProcessLaunchInfo.h"
 #include "lldb/Utility/ArchSpec.h"
+#include "lldb/Utility/Connection.h"
 #include "llvm/ADT/Optional.h"
 #include 
 #include 
 
 namespace llgs_tests {
-// TODO: Make the test client an abstract base class, with different children
-// for different types of connections: llgs v. debugserver
 class TestClient
 : public lldb_private::process_gdb_remote::GDBRemoteCommunicationClient {
 public:
-  static void Initialize();
   static bool IsDebugServer();
   static bool IsLldbServer();
 
-  TestClient(const std::string &test_name, const std::string &test_case_name);
-  virtual ~TestClient();
-  llvm::Error StartDebugger();
-  llvm::Error StopDebugger();
+  /// Launches the server, connects i

[Lldb-commits] [PATCH] D41067: llgs-tests: Add support for "exit" stop-reply packets

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.

This makes StopReply class abstract, so that we can represent different
types of stop replies such as StopReplyStop and StopReplyExit (there
should also be a StopReplySignal, but I don't need that right now so I
haven't implemented it yet).

This prepares the ground for a new test I'm writing.


https://reviews.llvm.org/D41067

Files:
  include/lldb/Host/Host.h
  unittests/tools/lldb-server/tests/MessageObjects.cpp
  unittests/tools/lldb-server/tests/MessageObjects.h
  unittests/tools/lldb-server/tests/TestClient.cpp
  unittests/tools/lldb-server/tests/TestClient.h
  unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp

Index: unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
===
--- unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
+++ unittests/tools/lldb-server/tests/ThreadIdsInJstopinfoTest.cpp
@@ -31,8 +31,9 @@
   auto jthreads_info = Client->GetJThreadsInfo();
   ASSERT_TRUE(jthreads_info);
 
-  auto stop_reply = Client->GetLatestStopReply();
-  auto stop_reply_pcs = stop_reply.GetThreadPcs();
+  auto stop_reply = Client->GetLatestStopReplyAs();
+  ASSERT_THAT_EXPECTED(stop_reply, Succeeded());
+  auto stop_reply_pcs = stop_reply->getThreadPcs();
   auto thread_infos = jthreads_info->GetThreadInfos();
   ASSERT_EQ(stop_reply_pcs.size(), thread_infos.size())
   << "Thread count mismatch.";
Index: unittests/tools/lldb-server/tests/TestClient.h
===
--- unittests/tools/lldb-server/tests/TestClient.h
+++ unittests/tools/lldb-server/tests/TestClient.h
@@ -16,6 +16,8 @@
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/Connection.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/FormatVariadic.h"
 #include 
 #include 
 
@@ -44,6 +46,14 @@
   const ProcessInfo &GetProcessInfo();
   llvm::Optional GetJThreadsInfo();
   const StopReply &GetLatestStopReply();
+  template  llvm::Expected GetLatestStopReplyAs() {
+assert(m_stop_reply);
+if (const auto *Reply = llvm::dyn_cast(m_stop_reply.get()))
+  return *Reply;
+return llvm::make_error(
+llvm::formatv("Unexpected Stop Reply {0}", m_stop_reply->getKind()),
+llvm::inconvertibleErrorCode());
+  }
   llvm::Error SendMessage(llvm::StringRef message);
   llvm::Error SendMessage(llvm::StringRef message,
   std::string &response_string);
@@ -62,7 +72,7 @@
   result);
 
   llvm::Optional m_process_info;
-  llvm::Optional m_stop_reply;
+  std::unique_ptr m_stop_reply;
   unsigned int m_pc_register = UINT_MAX;
 };
 
Index: unittests/tools/lldb-server/tests/TestClient.cpp
===
--- unittests/tools/lldb-server/tests/TestClient.cpp
+++ unittests/tools/lldb-server/tests/TestClient.cpp
@@ -154,7 +154,8 @@
 }
 
 const StopReply &TestClient::GetLatestStopReply() {
-  return m_stop_reply.getValue();
+  assert(m_stop_reply);
+  return *m_stop_reply;
 }
 
 Error TestClient::SendMessage(StringRef message) {
@@ -236,7 +237,7 @@
   std::string response;
   if (Error E = SendMessage(message, response))
 return E;
-  auto creation = StopReply::Create(response, m_process_info->GetEndian());
+  auto creation = StopReply::create(response, m_process_info->GetEndian());
   if (Error E = creation.takeError())
 return E;
 
Index: unittests/tools/lldb-server/tests/MessageObjects.h
===
--- unittests/tools/lldb-server/tests/MessageObjects.h
+++ unittests/tools/lldb-server/tests/MessageObjects.h
@@ -10,6 +10,7 @@
 #ifndef LLDB_SERVER_TESTS_MESSAGEOBJECTS_H
 #define LLDB_SERVER_TESTS_MESSAGEOBJECTS_H
 
+#include "lldb/Host/Host.h"
 #include "lldb/lldb-types.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallString.h"
@@ -74,20 +75,67 @@
 
 class StopReply {
 public:
-  static llvm::Expected Create(llvm::StringRef response,
-  llvm::support::endianness endian);
-  const U64Map &GetThreadPcs() const;
+  StopReply() = default;
+  virtual ~StopReply() = default;
+
+  static llvm::Expected>
+  create(llvm::StringRef response, llvm::support::endianness endian);
+
+  // for llvm::cast<>
+  virtual lldb_private::WaitStatus getKind() const = 0;
+
+  StopReply(const StopReply &) = delete;
+  void operator=(const StopReply &) = delete;
+};
+
+class StopReplyStop : public StopReply {
+public:
+  StopReplyStop(uint8_t Signal, lldb::tid_t ThreadId, llvm::StringRef Name,
+U64Map ThreadPcs, RegisterMap Registers, llvm::StringRef Reason)
+  : Signal(Signal), ThreadId(ThreadId), Name(Name),
+ThreadPcs(std::move(ThreadPcs)), Registers(std::move(Registers)),
+Reason(Reason) {}
+
+  static llvm::Expected>
+  create(llvm::StringRef response, llvm::support::endianness endian);
+
+  const U64Map 

[Lldb-commits] [lldb] r320366 - Add a StringList constructor to Args class

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Dec 11 06:22:30 2017
New Revision: 320366

URL: http://llvm.org/viewvc/llvm-project?rev=320366&view=rev
Log:
Add a StringList constructor to Args class

Host::GetEnvironment returns a StringList, but the interface for
launching a process takes Args. The fact that we use two classes for
representing an environment is not ideal, but for now we should at least
have an easy way to convert between the two.

Modified:
lldb/trunk/include/lldb/Interpreter/Args.h
lldb/trunk/include/lldb/Utility/StringList.h
lldb/trunk/source/Interpreter/Args.cpp
lldb/trunk/unittests/Interpreter/TestArgs.cpp

Modified: lldb/trunk/include/lldb/Interpreter/Args.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Interpreter/Args.h?rev=320366&r1=320365&r2=320366&view=diff
==
--- lldb/trunk/include/lldb/Interpreter/Args.h (original)
+++ lldb/trunk/include/lldb/Interpreter/Args.h Mon Dec 11 06:22:30 2017
@@ -86,6 +86,7 @@ public:
   Args(llvm::StringRef command = llvm::StringRef());
 
   Args(const Args &rhs);
+  explicit Args(const StringList &list);
 
   Args &operator=(const Args &rhs);
 

Modified: lldb/trunk/include/lldb/Utility/StringList.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/StringList.h?rev=320366&r1=320365&r2=320366&view=diff
==
--- lldb/trunk/include/lldb/Utility/StringList.h (original)
+++ lldb/trunk/include/lldb/Utility/StringList.h Mon Dec 11 06:22:30 2017
@@ -29,7 +29,7 @@ class StringList {
 public:
   StringList();
 
-  StringList(const char *str);
+  explicit StringList(const char *str);
 
   StringList(const char **strv, int strc);
 

Modified: lldb/trunk/source/Interpreter/Args.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Interpreter/Args.cpp?rev=320366&r1=320365&r2=320366&view=diff
==
--- lldb/trunk/source/Interpreter/Args.cpp (original)
+++ lldb/trunk/source/Interpreter/Args.cpp Mon Dec 11 06:22:30 2017
@@ -185,6 +185,11 @@ Args::Args(llvm::StringRef command) { Se
 
 Args::Args(const Args &rhs) { *this = rhs; }
 
+Args::Args(const StringList &list) : Args() {
+  for(size_t i = 0; i < list.GetSize(); ++i)
+AppendArgument(list[i]);
+}
+
 Args &Args::operator=(const Args &rhs) {
   Clear();
 

Modified: lldb/trunk/unittests/Interpreter/TestArgs.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Interpreter/TestArgs.cpp?rev=320366&r1=320365&r2=320366&view=diff
==
--- lldb/trunk/unittests/Interpreter/TestArgs.cpp (original)
+++ lldb/trunk/unittests/Interpreter/TestArgs.cpp Mon Dec 11 06:22:30 2017
@@ -10,6 +10,7 @@
 #include "gtest/gtest.h"
 
 #include "lldb/Interpreter/Args.h"
+#include "lldb/Utility/StringList.h"
 
 #include 
 #include 
@@ -117,6 +118,16 @@ TEST(ArgsTest, TestArgv) {
   EXPECT_EQ(nullptr, args.GetArgumentVector()[5]);
 }
 
+TEST(ArgsTest, StringListConstructor) {
+  StringList list;
+  list << "foo" << "bar" << "baz";
+  Args args(list);
+  ASSERT_EQ(3u, args.GetArgumentCount());
+  EXPECT_EQ("foo", args[0].ref);
+  EXPECT_EQ("bar", args[1].ref);
+  EXPECT_EQ("baz", args[2].ref);
+}
+
 TEST(ArgsTest, GetQuotedCommandString) {
   Args args;
   const char *str = "process launch -o stdout.txt -- \"a b c\"";


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

[Lldb-commits] [lldb] r320377 - dotest.py: Correctly annotate lldbinline tests with debug info categories

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Dec 11 07:28:21 2017
New Revision: 320377

URL: http://llvm.org/viewvc/llvm-project?rev=320377&view=rev
Log:
dotest.py: Correctly annotate lldbinline tests with debug info categories

This enables one to run all dwo tests with dotest.py --category dwo, or
skip them with --skip-category.

Modified:
lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py?rev=320377&r1=320376&r2=320377&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py Mon Dec 11 07:28:21 
2017
@@ -134,25 +134,28 @@ class InlineTest(TestBase):
 makefile.flush()
 makefile.close()
 
-@skipUnlessDarwin
+@add_test_categories(["dsym"])
 def __test_with_dsym(self):
 self.using_dsym = True
 self.BuildMakefile()
 self.buildDsym()
 self.do_test()
 
+@add_test_categories(["dwarf"])
 def __test_with_dwarf(self):
 self.using_dsym = False
 self.BuildMakefile()
 self.buildDwarf()
 self.do_test()
 
+@add_test_categories(["dwo"])
 def __test_with_dwo(self):
 self.using_dsym = False
 self.BuildMakefile()
 self.buildDwo()
 self.do_test()
 
+@add_test_categories(["gmodules"])
 def __test_with_gmodules(self):
 self.using_dsym = False
 self.BuildMakefile()


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

[Lldb-commits] [PATCH] D41069: NPL: Clean up handling of inferior exit

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.

lldb-server was sending the "exit" packet (W??) twice. This happened
because it was handling both the pre-exit (PTRACE_EVENT_EXIT) and
post-exit (WIFEXITED) as exit events. We had some code which was trying
to detect when we've already sent the exit packet, but this stopped
working quite a while ago.

This never really caused any problems in practice because the client
automatically closes the connection after receiving the first packet, so
the only effect of this was some warning messages about extra packets
from the lldb-server test suite, which were ignored because they didn't
fail the test.

The new test suite will be stricter about this, so I fix this issue
ignoring the first event. I think this is the correct behavior, as the
inferior is not really dead at that point, so it's premature to send the
exit packet.

There isn't an actual test yet which would verify the exit behavior, but
in my next patch I will add a test which will also test this
functionality.


https://reviews.llvm.org/D41069

Files:
  source/Plugins/Process/Linux/NativeProcessLinux.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  unittests/tools/lldb-server/tests/TestClient.cpp

Index: unittests/tools/lldb-server/tests/TestClient.cpp
===
--- unittests/tools/lldb-server/tests/TestClient.cpp
+++ unittests/tools/lldb-server/tests/TestClient.cpp
@@ -40,6 +40,9 @@
 }
 
 TestClient::~TestClient() {
+  if( !IsConnected())
+return;
+
   std::string response;
   // Debugserver (non-conformingly?) sends a reply to the k packet instead of
   // simply closing the connection.
@@ -242,6 +245,18 @@
 return E;
 
   m_stop_reply = std::move(*creation);
+  if (!isa(m_stop_reply)) {
+StringExtractorGDBRemote R;
+PacketResult result = ReadPacket(R, GetPacketTimeout(), false);
+if (result != PacketResult::ErrorDisconnected) {
+  return make_error(
+  formatv("Expected connection close after receiving {0}. Got {1}/{2} "
+  "instead.",
+  response, result, R.GetStringRef())
+  .str(),
+  inconvertibleErrorCode());
+}
+  }
   return Error::success();
 }
 
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -825,6 +825,7 @@
 
   // We are ready to exit the debug monitor.
   m_exit_now = true;
+  m_mainloop.RequestTermination();
 }
 
 void GDBRemoteCommunicationServerLLGS::HandleInferiorState_Stopped(
Index: source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -412,46 +412,20 @@
 
   // Handle when the thread exits.
   if (exited) {
-LLDB_LOG(log, "got exit signal({0}) , tid = {1} ({2} main thread)", signal,
- pid, is_main_thread ? "is" : "is not");
+LLDB_LOG(log,
+ "got exit signal({0}) , tid = {1} ({2} main thread), process "
+ "state = {3}",
+ signal, pid, is_main_thread ? "is" : "is not", GetState());
 
 // This is a thread that exited.  Ensure we're not tracking it anymore.
-const bool thread_found = StopTrackingThread(pid);
+StopTrackingThread(pid);
 
 if (is_main_thread) {
-  // We only set the exit status and notify the delegate if we haven't
-  // already set the process
-  // state to an exited state.  We normally should have received a SIGTRAP |
-  // (PTRACE_EVENT_EXIT << 8)
-  // for the main thread.
-  const bool already_notified = (GetState() == StateType::eStateExited) ||
-(GetState() == StateType::eStateCrashed);
-  if (!already_notified) {
-LLDB_LOG(
-log,
-"tid = {0} handling main thread exit ({1}), expected exit state "
-"already set but state was {2} instead, setting exit state now",
-pid,
-thread_found ? "stopped tracking thread metadata"
- : "thread metadata not found",
-GetState());
-// The main thread exited.  We're done monitoring.  Report to delegate.
-SetExitStatus(status, true);
+  // The main thread exited.  We're done monitoring.  Report to delegate.
+  SetExitStatus(status, true);
 
-// Notify delegate that our process has exited.
-SetState(StateType::eStateExited, true);
-  } else
-LLDB_LOG(log, "tid = {0} main thread now exited (%s)", pid,
- thread_found ? "stopped tracking thread metadata"
-  : "thread metadata not found");
-} else {
-  // Do we want to repo

[Lldb-commits] [PATCH] D41070: llgs: Propagate the environment when launching the inferior from command line

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
Herald added a subscriber: mgorny.

We were failing to propagate the environment when lldb-server was
started with a pre-loaded process
(e.g.: lldb-server gdbserver -- inferior --inferior_args)

This patch makes sure the environment is propagated. Instead of adding a
new GDBRemoteCommunicationServerLLGS::SetLaunchEnvironment function to
complement SetLaunchArgs and SetLaunchFlags, I replace these with a
more generic SetLaunchInfo, which can be used to set any launch-related
property.

The accompanying test also verifies that the server correctly terminates
the connection after sending the exit packet (specifically, that it does
not send the exit packet twice).


https://reviews.llvm.org/D41070

Files:
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
  tools/lldb-server/lldb-gdbserver.cpp
  unittests/tools/lldb-server/CMakeLists.txt
  unittests/tools/lldb-server/inferior/environment_check.cpp
  unittests/tools/lldb-server/tests/CMakeLists.txt
  unittests/tools/lldb-server/tests/LLGSTest.cpp
  unittests/tools/lldb-server/tests/TestClient.cpp

Index: unittests/tools/lldb-server/tests/TestClient.cpp
===
--- unittests/tools/lldb-server/tests/TestClient.cpp
+++ unittests/tools/lldb-server/tests/TestClient.cpp
@@ -89,14 +89,26 @@
   ProcessLaunchInfo Info;
   Info.SetArchitecture(arch_spec);
   Info.SetArguments(args, true);
+
+  StringList Env;
+  Host::GetEnvironment(Env);
+  Info.GetEnvironmentEntries() = Args(Env);
+
   status = Host::LaunchProcess(Info);
   if (status.Fail())
 return status.ToError();
 
   Socket *accept_socket;
   listen_socket.Accept(accept_socket);
   auto Conn = llvm::make_unique(accept_socket);
-  return std::unique_ptr(new TestClient(std::move(Conn)));
+  auto Client = std::unique_ptr(new TestClient(std::move(Conn)));
+
+  if (!InferiorArgs.empty()) {
+if (Error E = Client->QueryProcessInfo())
+  return std::move(E);
+  }
+
+  return std::move(Client);
 }
 
 Error TestClient::SetInferior(llvm::ArrayRef inferior_args) {
Index: unittests/tools/lldb-server/tests/LLGSTest.cpp
===
--- /dev/null
+++ unittests/tools/lldb-server/tests/LLGSTest.cpp
@@ -0,0 +1,31 @@
+//===-- LLGSTest.cpp *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "TestBase.h"
+#include "lldb/Host/Host.h"
+#include "llvm/Testing/Support/Error.h"
+
+using namespace llgs_tests;
+using namespace lldb_private;
+using namespace llvm;
+
+TEST_F(TestBase, LaunchModePreservesEnvironment) {
+  putenv(const_cast("LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE"));
+
+  auto ClientOr = TestClient::launch(getLogFileName(),
+ {getInferiorPath("environment_check")});
+  ASSERT_THAT_EXPECTED(ClientOr, Succeeded());
+  auto &Client = **ClientOr;
+
+  ASSERT_THAT_ERROR(Client.ContinueAll(), Succeeded());
+  ASSERT_THAT_EXPECTED(
+  Client.GetLatestStopReplyAs(),
+  HasValue(testing::Property(&StopReply::getKind,
+ WaitStatus{WaitStatus::Exit, 0})));
+}
Index: unittests/tools/lldb-server/tests/CMakeLists.txt
===
--- unittests/tools/lldb-server/tests/CMakeLists.txt
+++ unittests/tools/lldb-server/tests/CMakeLists.txt
@@ -1,7 +1,8 @@
 add_lldb_unittest(LLDBServerTests
+  LLGSTest.cpp
+  MessageObjects.cpp
   TestBase.cpp
   TestClient.cpp
-  MessageObjects.cpp
   ThreadIdsInJstopinfoTest.cpp
 
   LINK_LIBS
Index: unittests/tools/lldb-server/inferior/environment_check.cpp
===
--- /dev/null
+++ unittests/tools/lldb-server/inferior/environment_check.cpp
@@ -0,0 +1,20 @@
+//===-- thread_inferior.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() {
+  const char *value = std::getenv("LLDB_TEST_MAGIC_VARIABLE");
+  if (!value)
+return 1;
+  if (std::string(value) != "LLDB_TEST_MAGIC_VALUE")
+return 2;
+  return 0;
+}
Index: unittests/tools/lldb-server/CMakeLists.txt
===
--- unittests/tools/lldb-server/CMakeLists.txt
+++ unittests/tools/lldb-server/CMakeLists.txt
@@ -10,6 +10,7 @@
 endfunction()
 
 add_lldb_test_executable(thread_i

Re: [Lldb-commits] [lldb] r320242 - Change uses of strncpy in debugserver to strlcpy

[Adrian McCarthy via lldb-commits]

I have some concerns about this change.

1.  strlcpy is not a C++ standard function, so it's not available for
non-POSIX targets.  As far as I can tell, this is the first use of strlcpy
in LLVM.

2.  In some of the changed calls, the buffer size argument still has a -1,
which is redundant with what strlcpy is going to do, so this could cause
strings to be truncated a char too soon.

3.  At some of the call sites, there remains now-redundant code to
guarantee termination.. Since that's the point of changing these calls, we
should probably eliminate that.

4.  I'm not familiar with this part of the code base.  Is it necessary for
the APIs to work with pointer+length rather than a std::string (or other
class) that would give us safety and portability?

On Sun, Dec 10, 2017 at 3:52 PM, Davide Italiano via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> On Fri, Dec 8, 2017 at 7:37 PM, Jason Molenda via lldb-commits
>  wrote:
> > 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.
> >
> > 
> >
>
> Thanks!
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D41070: llgs: Propagate the environment when launching the inferior from command line

[Greg Clayton via Phabricator via lldb-commits]

clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Anything that launches a process should use the ProcessLaunchInfo. Nice patch.




Comment at: 
source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h:46
 
-  //--
-  /// Specify the program to launch and its arguments.
-  ///
-  /// @param[in] args
-  /// The command line to launch.
-  ///
-  /// @param[in] argc
-  /// The number of elements in the args array of cstring pointers.
-  ///
-  /// @return
-  /// An Status object indicating the success or failure of making
-  /// the setting.
-  //--
-  Status SetLaunchArguments(const char *const args[], int argc);
-
-  //--
-  /// Specify the launch flags for the process.
-  ///
-  /// @param[in] launch_flags
-  /// The launch flags to use when launching this process.
-  ///
-  /// @return
-  /// An Status object indicating the success or failure of making
-  /// the setting.
-  //--
-  Status SetLaunchFlags(unsigned int launch_flags);
+  void SetLaunchInfo(const ProcessLaunchInfo &info);
 

We might want a "ProcessLaunchInfo &" accessor so we can modify the flags after 
they have initially been set?

```
ProcessLaunchInfo &GetLaunchInfo() { return m_process_launch_info; }
```

If we don't need it, we shouldn't add it yet, just thinking out loud here.



Comment at: tools/lldb-server/lldb-gdbserver.cpp:181-182
+  ProcessLaunchInfo info;
+  info.GetFlags().Set(eLaunchFlagStopAtEntry | eLaunchFlagDebug |
+  eLaunchFlagDisableASLR);
+  info.SetArguments(const_cast(argv), true);

How many places do we set these flags like this? Wondering if we should add a 
method like:

```
info.SetFlagsForDebugging();
```

That way if we add new flags that must be set later, it will be easier than 
updating all sites that modify these flags for debugging a process.


https://reviews.llvm.org/D41070



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

[Lldb-commits] [PATCH] D41069: NPL: Clean up handling of inferior exit

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

Is the lldb_private::Process we have an exit status in the class itself. The 
first person to set the exit status wins and no one can set it twice. Doesn't 
look like what we are doing here. I am not able to tell what actually fixes 
things here?




Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:43
 TestClient::~TestClient() {
+  if( !IsConnected())
+return;

space after if and not after (


https://reviews.llvm.org/D41069



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

[Lldb-commits] [PATCH] D41069: NPL: Clean up handling of inferior exit

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D41069#951208, @clayborg wrote:

> Is the lldb_private::Process we have an exit status in the class itself. The 
> first person to set the exit status wins and no one can set it twice. Doesn't 
> look like what we are doing here. I am not able to tell what actually fixes 
> things here?


The `Process` class is in the client. The problem I am fixing is in server 
code, which was sending the `Wxx` packet multiple times. The happened because 
on linux we get one ptrace stop before the process dies (in this state its 
memory still exists, so we could still inspect it, if we wanted to) and then 
another one after its death (this happens after zombification of the process), 
and we were sending the packet after both.




Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:439
-// The main thread exited.  We're done monitoring.  Report to delegate.
-SetExitStatus(status, true);
 

This is where the second packet got sent. Now it's the only place that sends 
death packets.



Comment at: source/Plugins/Process/Linux/NativeProcessLinux.cpp:681
-if (is_main_thread)
-  SetExitStatus(WaitStatus::Decode(data), true);
 

This is what sent the first packet (it eventually trickles down into 
GDBRemoteCommunicationServerLLGS::HandleInferiorState_Exited, which is what 
does the sending). Deleting it fixes things.


https://reviews.llvm.org/D41069



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

[Lldb-commits] [PATCH] D41069: NPL: Clean up handling of inferior exit

[Pavel Labath via Phabricator via lldb-commits]

labath updated this revision to Diff 126389.
labath added a comment.

apply clang-format


https://reviews.llvm.org/D41069

Files:
  source/Plugins/Process/Linux/NativeProcessLinux.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  unittests/tools/lldb-server/tests/TestClient.cpp

Index: unittests/tools/lldb-server/tests/TestClient.cpp
===
--- unittests/tools/lldb-server/tests/TestClient.cpp
+++ unittests/tools/lldb-server/tests/TestClient.cpp
@@ -40,6 +40,9 @@
 }
 
 TestClient::~TestClient() {
+  if (!IsConnected())
+return;
+
   std::string response;
   // Debugserver (non-conformingly?) sends a reply to the k packet instead of
   // simply closing the connection.
@@ -242,6 +245,18 @@
 return E;
 
   m_stop_reply = std::move(*creation);
+  if (!isa(m_stop_reply)) {
+StringExtractorGDBRemote R;
+PacketResult result = ReadPacket(R, GetPacketTimeout(), false);
+if (result != PacketResult::ErrorDisconnected) {
+  return make_error(
+  formatv("Expected connection close after receiving {0}. Got {1}/{2} "
+  "instead.",
+  response, result, R.GetStringRef())
+  .str(),
+  inconvertibleErrorCode());
+}
+  }
   return Error::success();
 }
 
Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -825,6 +825,7 @@
 
   // We are ready to exit the debug monitor.
   m_exit_now = true;
+  m_mainloop.RequestTermination();
 }
 
 void GDBRemoteCommunicationServerLLGS::HandleInferiorState_Stopped(
Index: source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ source/Plugins/Process/Linux/NativeProcessLinux.cpp
@@ -412,46 +412,20 @@
 
   // Handle when the thread exits.
   if (exited) {
-LLDB_LOG(log, "got exit signal({0}) , tid = {1} ({2} main thread)", signal,
- pid, is_main_thread ? "is" : "is not");
+LLDB_LOG(log,
+ "got exit signal({0}) , tid = {1} ({2} main thread), process "
+ "state = {3}",
+ signal, pid, is_main_thread ? "is" : "is not", GetState());
 
 // This is a thread that exited.  Ensure we're not tracking it anymore.
-const bool thread_found = StopTrackingThread(pid);
+StopTrackingThread(pid);
 
 if (is_main_thread) {
-  // We only set the exit status and notify the delegate if we haven't
-  // already set the process
-  // state to an exited state.  We normally should have received a SIGTRAP |
-  // (PTRACE_EVENT_EXIT << 8)
-  // for the main thread.
-  const bool already_notified = (GetState() == StateType::eStateExited) ||
-(GetState() == StateType::eStateCrashed);
-  if (!already_notified) {
-LLDB_LOG(
-log,
-"tid = {0} handling main thread exit ({1}), expected exit state "
-"already set but state was {2} instead, setting exit state now",
-pid,
-thread_found ? "stopped tracking thread metadata"
- : "thread metadata not found",
-GetState());
-// The main thread exited.  We're done monitoring.  Report to delegate.
-SetExitStatus(status, true);
+  // The main thread exited.  We're done monitoring.  Report to delegate.
+  SetExitStatus(status, true);
 
-// Notify delegate that our process has exited.
-SetState(StateType::eStateExited, true);
-  } else
-LLDB_LOG(log, "tid = {0} main thread now exited (%s)", pid,
- thread_found ? "stopped tracking thread metadata"
-  : "thread metadata not found");
-} else {
-  // Do we want to report to the delegate in this case?  I think not.  If
-  // this was an orderly thread exit, we would already have received the
-  // SIGTRAP | (PTRACE_EVENT_EXIT << 8) signal, and we would have done an
-  // all-stop then.
-  LLDB_LOG(log, "tid = {0} handling non-main thread exit (%s)", pid,
-   thread_found ? "stopped tracking thread metadata"
-: "thread metadata not found");
+  // Notify delegate that our process has exited.
+  SetState(StateType::eStateExited, true);
 }
 return;
   }
@@ -662,10 +636,8 @@
   case (SIGTRAP | (PTRACE_EVENT_EXIT << 8)): {
 // The inferior process or one of its threads is about to exit.
 // We don't want to do anything with the thread so we just resume it. In
-// case we
-// want to implement "break on thread exit" functionality, we would need to
-// stop
-// here.
+// case we wa

[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

@davide: Any thoughts on `.yaml` as a test file suffix?

@clayborg: What do you think about my comment about GetFileSize() of compressed 
sections

In https://reviews.llvm.org/D40616#940408, @labath wrote:

> This rewrites the test in terms on the new lldb-test utility. It should be 
> applied on top of https://reviews.llvm.org/D40636.
>
> While doing that, I noticed a discrepancy in the data presented by the object
>  file interface -- for GetFileSize(), it would return the compressed size, 
> but,
>  when reading the data, it would return the decompressed size. This seemed odd
>  and unwanted.
>
> So now I fetch the decompressed size when constructing the Section object, and
>  make sure GetFileSize result matches what the GetSectionData returns. This is
>  slightly odd as well, because now if someone looks at individual section file
>  offsets and sizes, it will seem that multiple sections overlap. While
>  unfortunate, this is a situation that can arise in without the presence of
>  compressed sections (no linker will produce a file like that, but you can
>  certainly hand-craft one), and our elf parser will hapily accept these files.





https://reviews.llvm.org/D40616



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

[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I think GetFileSize() should remain the number of bytes of the section on disk 
and we should add new API if we need to figure out the decompressed size. Or 
maybe when we get bytes from a compressed section we are expected to always 
just get the raw bytes, then we check of the section is compressed, and if so, 
then we call another API on ObjectFile to decompress the data. So I would 
prefer GetFileSize() to return the file size of the section size in the file 
and not the decompressed size. Is there a way to make this work?


https://reviews.llvm.org/D40616



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

Re: [Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

[Zachary Turner via lldb-commits]

What about adding GetMemorySize?
On Mon, Dec 11, 2017 at 10:23 AM Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:

> clayborg added a comment.
>
> I think GetFileSize() should remain the number of bytes of the section on
> disk and we should add new API if we need to figure out the decompressed
> size. Or maybe when we get bytes from a compressed section we are expected
> to always just get the raw bytes, then we check of the section is
> compressed, and if so, then we call another API on ObjectFile to decompress
> the data. So I would prefer GetFileSize() to return the file size of the
> section size in the file and not the decompressed size. Is there a way to
> make this work?
>
>
> https://reviews.llvm.org/D40616
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

[Greg Clayton via lldb-commits]

> On Dec 11, 2017, at 10:25 AM, Zachary Turner  wrote:
> 
> What about adding GetMemorySize?

We already have that as Section::GetByteSize().


> On Mon, Dec 11, 2017 at 10:23 AM Greg Clayton via Phabricator 
> mailto:revi...@reviews.llvm.org>> wrote:
> clayborg added a comment.
> 
> I think GetFileSize() should remain the number of bytes of the section on 
> disk and we should add new API if we need to figure out the decompressed 
> size. Or maybe when we get bytes from a compressed section we are expected to 
> always just get the raw bytes, then we check of the section is compressed, 
> and if so, then we call another API on ObjectFile to decompress the data. So 
> I would prefer GetFileSize() to return the file size of the section size in 
> the file and not the decompressed size. Is there a way to make this work?
> 
> 
> https://reviews.llvm.org/D40616 
> 
> 
> 

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

Re: [Lldb-commits] [lldb] r320242 - Change uses of strncpy in debugserver to strlcpy

[Joerg Sonnenberger via lldb-commits]

On Sat, Dec 09, 2017 at 03:37:09AM -, Jason Molenda via lldb-commits wrote:
> Log:
> Change uses of strncpy in debugserver to strlcpy
> for better safety.

I don't think this is an improvement really. Many of those should be
plain memcpy or protected against truncation.

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

[Lldb-commits] [PATCH] D40616: ObjectFileELF: Add support for compressed sections

[Davide Italiano via Phabricator via lldb-commits]

davide added a comment.

In https://reviews.llvm.org/D40616#951256, @labath wrote:

> @davide: Any thoughts on `.yaml` as a test file suffix?


I think this is fine.


https://reviews.llvm.org/D40616



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

Re: [Lldb-commits] [lldb] r320242 - Change uses of strncpy in debugserver to strlcpy

[Jason Molenda via lldb-commits]

Hi Adrian, 

These are good points, thanks for the feedback.  debugserver is unique in the 
code base in that it only builds on darwin targets.  Originally it was intended 
to be a generic standalone gdb-remote protocol stub implementation, but it was 
only ever implemented for macos and for that matter, we have several 
Objective-C++ source files in the MacOSX subdir -- I don't think it'll build 
anywhere but on a mac already.

We have some strncpy's in the main lldb source, I remember going to change some 
of those to strlcpy's years ago & found that it was not as portable as I'd 
assumed.

Agreed, using std::string's would be better for most of these.  This was a 
quick fix to get off of the strncpy's.

Long term I know we're hoping to add our darwin-specific changes to lldb-server 
and move to that.  But that may not happen for a while.

J

> On Dec 11, 2017, at 8:26 AM, Adrian McCarthy  wrote:
> 
> I have some concerns about this change.
> 
> 1.  strlcpy is not a C++ standard function, so it's not available for 
> non-POSIX targets.  As far as I can tell, this is the first use of strlcpy in 
> LLVM.
> 
> 2.  In some of the changed calls, the buffer size argument still has a -1, 
> which is redundant with what strlcpy is going to do, so this could cause 
> strings to be truncated a char too soon.
> 
> 3.  At some of the call sites, there remains now-redundant code to guarantee 
> termination.. Since that's the point of changing these calls, we should 
> probably eliminate that.
> 
> 4.  I'm not familiar with this part of the code base.  Is it necessary for 
> the APIs to work with pointer+length rather than a std::string (or other 
> class) that would give us safety and portability?
> 
> On Sun, Dec 10, 2017 at 3:52 PM, Davide Italiano via lldb-commits 
>  wrote:
> On Fri, Dec 8, 2017 at 7:37 PM, Jason Molenda via lldb-commits
>  wrote:
> > 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.
> >
> > 
> >
> 
> Thanks!
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 

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

[Lldb-commits] [PATCH] D41086: [lldb] Check that a regex is valid before searching by regex for a symbol in a pdb.

[Aaron Smith via Phabricator via lldb-commits]

asmith created this revision.

Check that a regex is valid before searching by regex for a symbol in a pdb.
This avoids throwing an exception on an invalid regex.


Repository:
  rL LLVM

https://reviews.llvm.org/D41086

Files:
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Index: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
===
--- unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -161,8 +161,7 @@
 
 TEST_F(SymbolFilePDBTests, TestResolveSymbolContextBasename) {
   // Test that attempting to call ResolveSymbolContext with only a basename
-  // finds all full paths
-  // with the same basename
+  // finds all full paths with the same basename
   FileSpec fspec(m_pdb_test_exe.c_str(), false);
   ArchSpec aspec("i686-pc-windows");
   lldb::ModuleSP module = std::make_shared(fspec, aspec);
@@ -181,8 +180,7 @@
 
 TEST_F(SymbolFilePDBTests, TestResolveSymbolContextFullPath) {
   // Test that attempting to call ResolveSymbolContext with a full path only
-  // finds the one source
-  // file that matches the full path.
+  // finds the one source file that matches the full path.
   FileSpec fspec(m_pdb_test_exe.c_str(), false);
   ArchSpec aspec("i686-pc-windows");
   lldb::ModuleSP module = std::make_shared(fspec, aspec);
@@ -203,10 +201,9 @@
 
 TEST_F(SymbolFilePDBTests,
TestLookupOfHeaderFileWithInlines) {
-  // Test that when looking up a header file via ResolveSymbolContext (i.e. a
-  // file that was not by itself
-  // compiled, but only contributes to the combined code of other source files),
-  // a SymbolContext is returned
+  // Test that when looking up a header file via ResolveSymbolContext
+  // (i.e. a file that was not by itself compiled, but only contributes
+  // to the combined code of other source files), a SymbolContext is returned
   // for each compiland which has line contributions from the requested header.
   FileSpec fspec(m_pdb_test_exe.c_str(), false);
   ArchSpec aspec("i686-pc-windows");
@@ -231,10 +228,9 @@
 }
 
 TEST_F(SymbolFilePDBTests, TestLookupOfHeaderFileWithNoInlines) {
-  // Test that when looking up a header file via ResolveSymbolContext (i.e. a
-  // file that was not by itself
-  // compiled, but only contributes to the combined code of other source files),
-  // that if check_inlines
+  // Test that when looking up a header file via ResolveSymbolContext
+  // (i.e. a file that was not by itself compiled, but only contributes
+  // to the combined code of other source files), that if check_inlines
   // is false, no SymbolContexts are returned.
   FileSpec fspec(m_pdb_test_exe.c_str(), false);
   ArchSpec aspec("i686-pc-windows");
@@ -256,8 +252,7 @@
 
 TEST_F(SymbolFilePDBTests, TestLineTablesMatchAll) {
   // Test that when calling ResolveSymbolContext with a line number of 0, all
-  // line entries from
-  // the specified files are returned.
+  // line entries from the specified files are returned.
   FileSpec fspec(m_pdb_test_exe.c_str(), false);
   ArchSpec aspec("i686-pc-windows");
   lldb::ModuleSP module = std::make_shared(fspec, aspec);
@@ -305,8 +300,7 @@
 
 TEST_F(SymbolFilePDBTests, TestLineTablesMatchSpecific) {
   // Test that when calling ResolveSymbolContext with a specific line number,
-  // only line entries
-  // which match the requested line are returned.
+  // only line entries which match the requested line are returned.
   FileSpec fspec(m_pdb_test_exe.c_str(), false);
   ArchSpec aspec("i686-pc-windows");
   lldb::ModuleSP module = std::make_shared(fspec, aspec);
@@ -459,16 +453,14 @@
 
 TEST_F(SymbolFilePDBTests, TestArrayTypes) {
   // In order to get this test working, we need to support lookup by symbol
-  // name.  Because array
-  // types themselves do not have names, only the symbols have names (i.e. the
-  // name of the array).
+  // name.  Because array types themselves do not have names, only the
+  // symbols have names (i.e. the name of the array).
 }
 
 TEST_F(SymbolFilePDBTests, TestFunctionTypes) {
   // In order to get this test working, we need to support lookup by symbol
-  // name.  Because array
-  // types themselves do not have names, only the symbols have names (i.e. the
-  // name of the array).
+  // name.  Because array types themselves do not have names, only the
+  // symbols have names (i.e. the name of the array).
 }
 
 TEST_F(SymbolFilePDBTests, TestTypedefs) {
@@ -520,6 +512,13 @@
 false, 0, searched_files, results);
   EXPECT_GT(num_results, 1u);
   EXPECT_EQ(num_results, results.GetSize());
+
+  // We expect no exception thrown if the given regex can't be compiled
+  results.Clear();
+  num_results = symfile->FindTypes(sc, ConstString("**"), nullptr,
+   false, 0, searched_files, results);
+  EXPECT_EQ(num_results, 0u);
+  EXPECT_EQ(num_results, results.Get

[Lldb-commits] [PATCH] D41086: [lldb] Check that a regex is valid before searching by regex for a symbol in a pdb.

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:392-399
+  bool is_regex = false;
+  if (name_str.find_first_of("[]?*.-+\\") != std::string::npos) {
+// Trying to compile an invalid regex could throw an exception.
+// Only search by regex when it's valid.
+lldb_private::RegularExpression name_regex(name_str);
+is_regex = name_regex.IsValid();
+  }

I can see two possible problems here.

1. Now if it is a valid regex, it's going to compile it twice, hurting 
performance (not sure by how much, it could be negligible)

2. Whether or not it's valid is determined by asking LLDB's regex engine, but 
it's then compiled with C++ STL's engine.  It's possible they won't agree on 
whether or not a regex is valid.

You mentioned that it throws an exception on an invalid regex.  What actually 
happens though?  Because we compile with exception support disabled.  Does it 
terminate the program?

At a minimum, the validity check and the find function should agree on the 
regex engine.  If the only way to make that work is to change from `std::regex` 
to `lldb_private::RegularExpression` as the matching engine, then I guess we 
have to do that.


Repository:
  rL LLVM

https://reviews.llvm.org/D41086



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

[Lldb-commits] [PATCH] D41086: [lldb] Check that a regex is valid before searching by regex for a symbol in a pdb.

[Aaron Smith via Phabricator via lldb-commits]

asmith added inline comments.



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:393
+  bool is_regex = false;
+  if (name_str.find_first_of("[]?*.-+\\") != std::string::npos) {
+// Trying to compile an invalid regex could throw an exception.

The find_first could be removed depending on the cost of 
lldb_private::RegularExpression()


Repository:
  rL LLVM

https://reviews.llvm.org/D41086



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

[Lldb-commits] [lldb] r320422 - [Testsuite] Rename this file from *m -> *mm.

[Davide Italiano via lldb-commits]

Author: davide
Date: Mon Dec 11 13:21:53 2017
New Revision: 320422

URL: http://llvm.org/viewvc/llvm-project?rev=320422&view=rev
Log:
[Testsuite] Rename this file from *m -> *mm.

Should hopefully bring the bots back.



Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.mm
  - copied, changed from r320377, 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.m
Removed:
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.m

Removed: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.m
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.m?rev=320421&view=auto
==
--- lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.m 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.m 
(removed)
@@ -1,31 +0,0 @@
-//===-- main.m ---*- 
Objective-C-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#import 
-
-@interface VarClass : NSObject
-- (id) lottaArgs: (id) first, ...;
-@end
-
-@implementation VarClass
-- (id) lottaArgs: (id) first, ...
-{
-  return first;
-}
-@end
-
-int
-main()
-{
-  VarClass *my_var = [[VarClass alloc] init];
-  id something = [my_var lottaArgs: @"111", @"222", nil];
-  NSLog (@"%@ - %@", my_var, something); //% self.expect("expression -O -- 
[my_var lottaArgs:@\"111\", @\"222\", nil]", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs = ["111"])
-  return 0;
-}
-

Copied: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.mm 
(from r320377, 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.m)
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.mm?p2=lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.mm&p1=lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.m&r1=320377&r2=320422&rev=320422&view=diff
==
(empty)


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

[Lldb-commits] [PATCH] D41086: [lldb] Check that a regex is valid before searching by regex for a symbol in a pdb.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

We should have a dedicated API that actually searches for types using a 
lldb_private::RegularExpression. Why do we even try to create a regular 
expression in SymbolFilePDB::FindTypes()? Was it used in testing? No API in 
LLDB currently expects this and all other API in LLDB has a separate function 
call that uses lldb_private::RegularExpression. We can add one if needed to the 
lldb_private::SymbolFile API. I don't like API that magically tries to look at 
something and tries to compile a regular expression on each invocation. Can we 
change this?


Repository:
  rL LLVM

https://reviews.llvm.org/D41086



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

[Lldb-commits] [PATCH] D41092: Enable more abilities in SymbolFilePDB

[Aaron Smith via Phabricator via lldb-commits]

asmith created this revision.

1. Finding symbols through --symfile
2. More abilities: Functions, Blocks, GlobalVariables, LocalVariables, 
VariableTypes


Repository:
  rL LLVM

https://reviews.llvm.org/D41092

Files:
  source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
  unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

Index: unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
===
--- unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
+++ unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp
@@ -154,8 +154,7 @@
   EXPECT_NE(nullptr, symfile);
   EXPECT_EQ(symfile->GetPluginName(), SymbolFilePDB::GetPluginNameStatic());
 
-  uint32_t expected_abilities =
-  SymbolFile::CompileUnits | SymbolFile::LineTables;
+  uint32_t expected_abilities = SymbolFile::kAllAbilities;
   EXPECT_EQ(expected_abilities, symfile->CalculateAbilities());
 }
 
Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
===
--- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -21,9 +21,11 @@
 #include "lldb/Symbol/TypeMap.h"
 
 #include "llvm/DebugInfo/PDB/GenericError.h"
+#include "llvm/DebugInfo/PDB/IPDBDataStream.h"
 #include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
 #include "llvm/DebugInfo/PDB/IPDBLineNumber.h"
 #include "llvm/DebugInfo/PDB/IPDBSourceFile.h"
+#include "llvm/DebugInfo/PDB/IPDBTable.h"
 #include "llvm/DebugInfo/PDB/PDBSymbol.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolCompiland.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolCompilandDetails.h"
@@ -93,17 +95,81 @@
 SymbolFilePDB::~SymbolFilePDB() {}
 
 uint32_t SymbolFilePDB::CalculateAbilities() {
+  uint32_t abilities = 0;
+  if (!m_obj_file)
+return 0;
   if (!m_session_up) {
 // Lazily load and match the PDB file, but only do this once.
 std::string exePath = m_obj_file->GetFileSpec().GetPath();
 auto error = loadDataForEXE(PDB_ReaderType::DIA, llvm::StringRef(exePath),
 m_session_up);
 if (error) {
   llvm::consumeError(std::move(error));
-  return 0;
+  if (auto module_sp = m_obj_file->GetModule()) {
+// See if a symbol file was specified through the `--symfile` option.
+FileSpec symfile = module_sp->GetSymbolFileFileSpec();
+if (symfile) {
+  error = loadDataForPDB(PDB_ReaderType::DIA,
+ llvm::StringRef(symfile.GetPath()),
+ m_session_up);
+  if (error) {
+llvm::consumeError(std::move(error));
+return 0;
+  }
+} else {
+  return 0;
+}
+  } else {
+return 0;
+  }
+}
+  }
+  lldbassert(m_session_up.get());
+  if (auto enum_tables_up = m_session_up->getEnumTables()) {
+while (auto table_up = enum_tables_up->getNext()) {
+  if (table_up->getItemCount()) {
+auto type = table_up->getTableType();
+switch (type) {
+case PDB_TableType::SectionContribs:
+  // This table contains module/compiland information.
+  abilities |= CompileUnits;
+  break;
+case PDB_TableType::Symbols:
+  abilities |= (Functions | Blocks);
+  // Check if there are sections of raw data for any variables
+  // in the `SECTIONHEADERS` data stream.
+  if (auto enum_dbg_streams_up = m_session_up->getDebugStreams()) {
+while (auto dbg_stream_up = enum_dbg_streams_up->getNext()) {
+  auto stream_name = dbg_stream_up->getName();
+  if (stream_name == "SECTIONHEADERS") {
+for (int rec_idx = 0;
+ rec_idx < dbg_stream_up->getRecordCount();
+ rec_idx++) {
+  auto record = dbg_stream_up->getItemAtIndex(rec_idx);
+  auto record_pointer =
+  reinterpret_cast(record->data());
+  if (strncmp(record_pointer, ".data", 5) == 0 ||
+  strncmp(record_pointer, ".bss", 4) == 0 ||
+  strncmp(record_pointer, ".rdata", 6) == 0) {
+abilities |= (GlobalVariables | LocalVariables |
+  VariableTypes);
+break;
+  }
+}
+  }
+}
+  }
+  break;
+case PDB_TableType::LineNumbers:
+  abilities |= LineTables;
+  break;
+default:
+  break;
+}
+  }
 }
   }
-  return CompileUnits | LineTables;
+  return abilities;
 }
 
 void SymbolFilePDB::InitializeObject() {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D41086: [lldb] Check that a regex is valid before searching by regex for a symbol in a pdb.

[Zachary Turner via lldb-commits]

SymbolFile does not have a method for searching by regex.  There is a
FindFunctions and FindGlobalVariables that allows searching by regex, but
not one for types.  That said, one could be added to the base class, yes.
That does seem like a better long term solution.

On Mon, Dec 11, 2017 at 1:37 PM Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:

> clayborg added a comment.
>
> We should have a dedicated API that actually searches for types using a
> lldb_private::RegularExpression. Why do we even try to create a regular
> expression in SymbolFilePDB::FindTypes()? Was it used in testing? No API in
> LLDB currently expects this and all other API in LLDB has a separate
> function call that uses lldb_private::RegularExpression. We can add one if
> needed to the lldb_private::SymbolFile API. I don't like API that magically
> tries to look at something and tries to compile a regular expression on
> each invocation. Can we change this?
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D41086
>
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [PATCH] D41086: [lldb] Check that a regex is valid before searching by regex for a symbol in a pdb.

[Greg Clayton via lldb-commits]

Please do add this as that is the best solution. If no other plug-ins currently 
support it, it is fine to just stub out the virtual function in 
SymbolFile::FindTypesByRegex(...) to return Error("unsupported") and we can 
implement it in the regex search later for DWARF and other plug-ins.

Greg

> On Dec 11, 2017, at 1:40 PM, Zachary Turner  wrote:
> 
> SymbolFile does not have a method for searching by regex.  There is a 
> FindFunctions and FindGlobalVariables that allows searching by regex, but not 
> one for types.  That said, one could be added to the base class, yes.  That 
> does seem like a better long term solution.
> 
> On Mon, Dec 11, 2017 at 1:37 PM Greg Clayton via Phabricator 
> mailto:revi...@reviews.llvm.org>> wrote:
> clayborg added a comment.
> 
> We should have a dedicated API that actually searches for types using a 
> lldb_private::RegularExpression. Why do we even try to create a regular 
> expression in SymbolFilePDB::FindTypes()? Was it used in testing? No API in 
> LLDB currently expects this and all other API in LLDB has a separate function 
> call that uses lldb_private::RegularExpression. We can add one if needed to 
> the lldb_private::SymbolFile API. I don't like API that magically tries to 
> look at something and tries to compile a regular expression on each 
> invocation. Can we change this?
> 
> 
> Repository:
>   rL LLVM
> 
> https://reviews.llvm.org/D41086 
> 
> 
> 

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

[Lldb-commits] [PATCH] D41092: Enable more abilities in SymbolFilePDB

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:127
+  }
+  lldbassert(m_session_up.get());
+  if (auto enum_tables_up = m_session_up->getEnumTables()) {

I am assuming this assert won't fire if we give this an ELF or MachO file or 
any file that doesn't contain PDB info? Every SymbolFile subclass gets to 
calculate its abilities on each file until on of them returns that they can 
handle all abilities, or until all plug-ins have had a chance to answer and 
then the best one is picked. Seems like this shouldn't be here? I can't 
remember what checks get run before SymbolFile::CalculateAbilities() is 
called...


Repository:
  rL LLVM

https://reviews.llvm.org/D41092



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

[Lldb-commits] [lldb] r320425 - [test-suite] Un'XFAIL a test that's not failing anymore.

[Davide Italiano via lldb-commits]

Author: davide
Date: Mon Dec 11 13:52:02 2017
New Revision: 320425

URL: http://llvm.org/viewvc/llvm-project?rev=320425&view=rev
Log:
[test-suite] Un'XFAIL a test that's not failing anymore.

This is the first of a series of commits aiming to improve
overall LLDB's hygiene. Feel free to shout at me in case
I break something.



Modified:
lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py

Modified: lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py?rev=320425&r1=320424&r2=320425&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/macosx/queues/TestQueues.py Mon 
Dec 11 13:52:02 2017
@@ -18,7 +18,6 @@ class TestQueues(TestBase):
 
 @skipUnlessDarwin
 @add_test_categories(['pyapi'])
-@expectedFailureAll(bugnumber="rdar://30915340")
 def test_with_python_api(self):
 """Test queues inspection SB APIs."""
 self.build()


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

[Lldb-commits] [PATCH] D41092: Enable more abilities in SymbolFilePDB

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:108-125
+  if (auto module_sp = m_obj_file->GetModule()) {
+// See if a symbol file was specified through the `--symfile` option.
+FileSpec symfile = module_sp->GetSymbolFileFileSpec();
+if (symfile) {
+  error = loadDataForPDB(PDB_ReaderType::DIA,
+ llvm::StringRef(symfile.GetPath()),
+ m_session_up);

LLVM coding style dictates the use of early returns whenever possible.  So this 
block should be changed to something like.

```
auto module_sp = m_obj_file->GetModule();
if (!module_sp)
  return 0;

FileSpec symfile = module_sp->GetSymbolFileSpec();
if (!symfile)
  return 0;

error = loadDataForPDB(...);
if (error) {
  llvm::consumeError(error);
  return 0;
}

// etc
```



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:141
+  // in the `SECTIONHEADERS` data stream.
+  if (auto enum_dbg_streams_up = m_session_up->getDebugStreams()) {
+while (auto dbg_stream_up = enum_dbg_streams_up->getNext()) {

Similar thing here.  Assign first, and then add:

```
if (!enum_dbg_streams_up)
  break;
```



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:144
+  auto stream_name = dbg_stream_up->getName();
+  if (stream_name == "SECTIONHEADERS") {
+for (int rec_idx = 0;

```
if (stream_name != "SECTIONHEADERS")
  continue;
```



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:151-157
+  if (strncmp(record_pointer, ".data", 5) == 0 ||
+  strncmp(record_pointer, ".bss", 4) == 0 ||
+  strncmp(record_pointer, ".rdata", 6) == 0) {
+abilities |= (GlobalVariables | LocalVariables |
+  VariableTypes);
+break;
+  }

Is this correct?  If *any* of these 3 sections of present, then *all* of these 
capabilities are present?

Shouldn't we be actually trying to query DIA for a global, local, or variable 
and then seeing what it returns?

(Incidentally, it's easy to figure this out with the native PDB reader, since 
you can just look for the presence of a module symbol stream, globals stream, 
and/or TPI stream).


Repository:
  rL LLVM

https://reviews.llvm.org/D41092



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

[Lldb-commits] [PATCH] D41086: [lldb] Check that a regex is valid before searching by regex for a symbol in a pdb.

[Aaron Smith via Phabricator via lldb-commits]

asmith added inline comments.



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:392-399
+  bool is_regex = false;
+  if (name_str.find_first_of("[]?*.-+\\") != std::string::npos) {
+// Trying to compile an invalid regex could throw an exception.
+// Only search by regex when it's valid.
+lldb_private::RegularExpression name_regex(name_str);
+is_regex = name_regex.IsValid();
+  }

zturner wrote:
> I can see two possible problems here.
> 
> 1. Now if it is a valid regex, it's going to compile it twice, hurting 
> performance (not sure by how much, it could be negligible)
> 
> 2. Whether or not it's valid is determined by asking LLDB's regex engine, but 
> it's then compiled with C++ STL's engine.  It's possible they won't agree on 
> whether or not a regex is valid.
> 
> You mentioned that it throws an exception on an invalid regex.  What actually 
> happens though?  Because we compile with exception support disabled.  Does it 
> terminate the program?
> 
> At a minimum, the validity check and the find function should agree on the 
> regex engine.  If the only way to make that work is to change from 
> `std::regex` to `lldb_private::RegularExpression` as the matching engine, 
> then I guess we have to do that.
How about this?

```
  lldb_private::RegularExpression name_regex(name_str);
  if (name_regex.IsValid())
FindTypesByRegex(name_regex, max_matches, types); // pass regex here
  else
FindTypesByName(name_str, max_matches, types);
  return types.GetSize();
}

void SymbolFilePDB::FindTypesByRegex(lldb_private::RegularExpression ®ex,
 uint32_t max_matches,
 lldb_private::TypeMap &types) {
```


Repository:
  rL LLVM

https://reviews.llvm.org/D41086



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

[Lldb-commits] [PATCH] D41099: [lldb] Switch to add_llvm_install_targets

[Petr Hosek via Phabricator via lldb-commits]

phosek accepted this revision.
phosek added a comment.
This revision is now accepted and ready to land.

LGTM


https://reviews.llvm.org/D41099



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

[Lldb-commits] [PATCH] D41099: [lldb] Switch to add_llvm_install_targets

[Shoaib Meenai via Phabricator via lldb-commits]

smeenai created this revision.
Herald added a subscriber: mgorny.

This adds the install-*-stripped targets to LLDB, which are required for
the install-distribution-stripped option. We also need to create some
install-*-stripped targets manually, which are modeled after their
corresponding install-* targets.


https://reviews.llvm.org/D41099

Files:
  cmake/modules/AddLLDB.cmake


Index: cmake/modules/AddLLDB.cmake
===
--- cmake/modules/AddLLDB.cmake
+++ cmake/modules/AddLLDB.cmake
@@ -66,11 +66,8 @@
   ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX})
   endif()
   if (NOT CMAKE_CONFIGURATION_TYPES)
-add_custom_target(install-${name}
-  DEPENDS ${name}
-  COMMAND "${CMAKE_COMMAND}"
-  -DCMAKE_INSTALL_COMPONENT=${name}
-  -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+add_llvm_install_targets(install-${name}
+ DEPENDS ${name})
   endif()
 endif()
   endif()
@@ -121,6 +118,8 @@
   if(ARG_GENERATE_INSTALL)
 add_custom_target(install-${name} DEPENDS ${name})
 add_dependencies(install-liblldb ${name})
+add_custom_target(install-${name}-stripped DEPENDS ${name})
+add_dependencies(install-liblldb-stripped ${name})
   endif()
 else()
   set_target_properties(${name} PROPERTIES
@@ -134,11 +133,8 @@
   COMPONENT ${name}
   RUNTIME DESTINATION bin)
 if (NOT CMAKE_CONFIGURATION_TYPES)
-  add_custom_target(install-${name}
-DEPENDS ${name}
-COMMAND "${CMAKE_COMMAND}"
--DCMAKE_INSTALL_COMPONENT=${name}
--P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+  add_llvm_install_targets(install-${name}
+   DEPENDS ${name})
 endif()
   endif()
 


Index: cmake/modules/AddLLDB.cmake
===
--- cmake/modules/AddLLDB.cmake
+++ cmake/modules/AddLLDB.cmake
@@ -66,11 +66,8 @@
   ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX})
   endif()
   if (NOT CMAKE_CONFIGURATION_TYPES)
-add_custom_target(install-${name}
-  DEPENDS ${name}
-  COMMAND "${CMAKE_COMMAND}"
-  -DCMAKE_INSTALL_COMPONENT=${name}
-  -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+add_llvm_install_targets(install-${name}
+ DEPENDS ${name})
   endif()
 endif()
   endif()
@@ -121,6 +118,8 @@
   if(ARG_GENERATE_INSTALL)
 add_custom_target(install-${name} DEPENDS ${name})
 add_dependencies(install-liblldb ${name})
+add_custom_target(install-${name}-stripped DEPENDS ${name})
+add_dependencies(install-liblldb-stripped ${name})
   endif()
 else()
   set_target_properties(${name} PROPERTIES
@@ -134,11 +133,8 @@
   COMPONENT ${name}
   RUNTIME DESTINATION bin)
 if (NOT CMAKE_CONFIGURATION_TYPES)
-  add_custom_target(install-${name}
-DEPENDS ${name}
-COMMAND "${CMAKE_COMMAND}"
--DCMAKE_INSTALL_COMPONENT=${name}
--P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+  add_llvm_install_targets(install-${name}
+   DEPENDS ${name})
 endif()
   endif()
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r320443 - [lldb] Switch to add_llvm_install_targets

[Shoaib Meenai via lldb-commits]

Author: smeenai
Date: Mon Dec 11 16:47:07 2017
New Revision: 320443

URL: http://llvm.org/viewvc/llvm-project?rev=320443&view=rev
Log:
[lldb] Switch to add_llvm_install_targets

This adds the install-*-stripped targets to LLDB, which are required for
the install-distribution-stripped option. We also need to create some
install-*-stripped targets manually, which are modeled after their
corresponding install-* targets.

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

Modified:
lldb/trunk/cmake/modules/AddLLDB.cmake

Modified: lldb/trunk/cmake/modules/AddLLDB.cmake
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/cmake/modules/AddLLDB.cmake?rev=320443&r1=320442&r2=320443&view=diff
==
--- lldb/trunk/cmake/modules/AddLLDB.cmake (original)
+++ lldb/trunk/cmake/modules/AddLLDB.cmake Mon Dec 11 16:47:07 2017
@@ -66,11 +66,8 @@ function(add_lldb_library name)
   ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX})
   endif()
   if (NOT CMAKE_CONFIGURATION_TYPES)
-add_custom_target(install-${name}
-  DEPENDS ${name}
-  COMMAND "${CMAKE_COMMAND}"
-  -DCMAKE_INSTALL_COMPONENT=${name}
-  -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+add_llvm_install_targets(install-${name}
+ DEPENDS ${name})
   endif()
 endif()
   endif()
@@ -121,6 +118,8 @@ function(add_lldb_executable name)
   if(ARG_GENERATE_INSTALL)
 add_custom_target(install-${name} DEPENDS ${name})
 add_dependencies(install-liblldb ${name})
+add_custom_target(install-${name}-stripped DEPENDS ${name})
+add_dependencies(install-liblldb-stripped ${name})
   endif()
 else()
   set_target_properties(${name} PROPERTIES
@@ -134,11 +133,8 @@ function(add_lldb_executable name)
   COMPONENT ${name}
   RUNTIME DESTINATION bin)
 if (NOT CMAKE_CONFIGURATION_TYPES)
-  add_custom_target(install-${name}
-DEPENDS ${name}
-COMMAND "${CMAKE_COMMAND}"
--DCMAKE_INSTALL_COMPONENT=${name}
--P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+  add_llvm_install_targets(install-${name}
+   DEPENDS ${name})
 endif()
   endif()
 


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

[Lldb-commits] [PATCH] D41099: [lldb] Switch to add_llvm_install_targets

[Shoaib Meenai via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320443: [lldb] Switch to add_llvm_install_targets (authored 
by smeenai).

Repository:
  rL LLVM

https://reviews.llvm.org/D41099

Files:
  lldb/trunk/cmake/modules/AddLLDB.cmake


Index: lldb/trunk/cmake/modules/AddLLDB.cmake
===
--- lldb/trunk/cmake/modules/AddLLDB.cmake
+++ lldb/trunk/cmake/modules/AddLLDB.cmake
@@ -66,11 +66,8 @@
   ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX})
   endif()
   if (NOT CMAKE_CONFIGURATION_TYPES)
-add_custom_target(install-${name}
-  DEPENDS ${name}
-  COMMAND "${CMAKE_COMMAND}"
-  -DCMAKE_INSTALL_COMPONENT=${name}
-  -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+add_llvm_install_targets(install-${name}
+ DEPENDS ${name})
   endif()
 endif()
   endif()
@@ -121,6 +118,8 @@
   if(ARG_GENERATE_INSTALL)
 add_custom_target(install-${name} DEPENDS ${name})
 add_dependencies(install-liblldb ${name})
+add_custom_target(install-${name}-stripped DEPENDS ${name})
+add_dependencies(install-liblldb-stripped ${name})
   endif()
 else()
   set_target_properties(${name} PROPERTIES
@@ -134,11 +133,8 @@
   COMPONENT ${name}
   RUNTIME DESTINATION bin)
 if (NOT CMAKE_CONFIGURATION_TYPES)
-  add_custom_target(install-${name}
-DEPENDS ${name}
-COMMAND "${CMAKE_COMMAND}"
--DCMAKE_INSTALL_COMPONENT=${name}
--P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+  add_llvm_install_targets(install-${name}
+   DEPENDS ${name})
 endif()
   endif()
 


Index: lldb/trunk/cmake/modules/AddLLDB.cmake
===
--- lldb/trunk/cmake/modules/AddLLDB.cmake
+++ lldb/trunk/cmake/modules/AddLLDB.cmake
@@ -66,11 +66,8 @@
   ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX})
   endif()
   if (NOT CMAKE_CONFIGURATION_TYPES)
-add_custom_target(install-${name}
-  DEPENDS ${name}
-  COMMAND "${CMAKE_COMMAND}"
-  -DCMAKE_INSTALL_COMPONENT=${name}
-  -P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+add_llvm_install_targets(install-${name}
+ DEPENDS ${name})
   endif()
 endif()
   endif()
@@ -121,6 +118,8 @@
   if(ARG_GENERATE_INSTALL)
 add_custom_target(install-${name} DEPENDS ${name})
 add_dependencies(install-liblldb ${name})
+add_custom_target(install-${name}-stripped DEPENDS ${name})
+add_dependencies(install-liblldb-stripped ${name})
   endif()
 else()
   set_target_properties(${name} PROPERTIES
@@ -134,11 +133,8 @@
   COMPONENT ${name}
   RUNTIME DESTINATION bin)
 if (NOT CMAKE_CONFIGURATION_TYPES)
-  add_custom_target(install-${name}
-DEPENDS ${name}
-COMMAND "${CMAKE_COMMAND}"
--DCMAKE_INSTALL_COMPONENT=${name}
--P "${CMAKE_BINARY_DIR}/cmake_install.cmake")
+  add_llvm_install_targets(install-${name}
+   DEPENDS ${name})
 endif()
   endif()
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D41101: [testsuite] Remove testing failures vestiges

[Vedant Kumar via Phabricator via lldb-commits]

vsk accepted this revision.
vsk added a comment.
This revision is now accepted and ready to land.

Thanks! The 6.0 compiler is ancient at this point. This lgtm.


https://reviews.llvm.org/D41101



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

[Lldb-commits] [lldb] r320444 - [testsuite] Remove testing failures vestiges.

[Davide Italiano via lldb-commits]

Author: davide
Date: Mon Dec 11 17:14:11 2017
New Revision: 320444

URL: http://llvm.org/viewvc/llvm-project?rev=320444&view=rev
Log:
[testsuite] Remove testing failures vestiges.

Some tests are failing on macOS when building with the in-tree
clang, and this is because they're conditional on the version released.
Apple releases using a different versioning number, but as these are
conditional on clang < 7, they fail for clang ToT (which is 6.0).
As a general solution, we actually need either a mapping between
Apple internal release version and public ones.

That said, I discussed this with Fred , and Apple Clang 6.0 seems
to be old enough that we can remove this altogether (which means I
can delay implementing the general purpose solution for a bit).

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

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py?rev=320444&r1=320443&r2=320444&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
 Mon Dec 11 17:14:11 2017
@@ -47,12 +47,6 @@ class ObjCNewSyntaxTestCase(TestBase):
 substrs=[' resolved, hit count = 1'])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_read_array(self):
@@ -69,12 +63,6 @@ class ObjCNewSyntaxTestCase(TestBase):
 substrs=["foo"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_update_array(self):
@@ -91,12 +79,6 @@ class ObjCNewSyntaxTestCase(TestBase):
 substrs=["bar"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_read_dictionary(self):
@@ -113,12 +95,6 @@ class ObjCNewSyntaxTestCase(TestBase):
 substrs=["value"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_update_dictionary(self):
@@ -135,12 +111,6 @@ class ObjCNewSyntaxTestCase(TestBase):
 substrs=["object"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_array_literal(self):
@@ -155,12 +125,6 @@ class ObjCNewSyntaxTestCase(TestBase):
 "bar"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_dictionary_literal(self):
@@ -174,12 +138,6 @@ class ObjCNewSyntaxTestCase(TestBase):
 "object"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_char_literal(self):
@@ -189,12 +147,6 @@ class ObjCNewSyntaxTestCase(TestBase):
 VARIABLES_DISPLAYED_CORRECTLY, substrs=[str(ord('a'))])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_integer_literals(self):
@@ -226,12 +178,6 @@ class ObjCNewSyntaxTestCase(TestBase):
 substrs=["1"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expec

[Lldb-commits] [PATCH] D41101: [testsuite] Remove testing failures vestiges

[Davide Italiano via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320444: [testsuite] Remove testing failures vestiges. 
(authored by davide).

Changed prior to commit:
  https://reviews.llvm.org/D41101?vs=126478&id=126479#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D41101

Files:
  
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py

Index: lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
===
--- lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/objc/objc-new-syntax/TestObjCNewSyntax.py
@@ -47,12 +47,6 @@
 substrs=[' resolved, hit count = 1'])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_read_array(self):
@@ -69,12 +63,6 @@
 substrs=["foo"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_update_array(self):
@@ -91,12 +79,6 @@
 substrs=["bar"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_read_dictionary(self):
@@ -113,12 +95,6 @@
 substrs=["value"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_update_dictionary(self):
@@ -135,12 +111,6 @@
 substrs=["object"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_array_literal(self):
@@ -155,12 +125,6 @@
 "bar"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_dictionary_literal(self):
@@ -174,12 +138,6 @@
 "object"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_char_literal(self):
@@ -189,12 +147,6 @@
 VARIABLES_DISPLAYED_CORRECTLY, substrs=[str(ord('a'))])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_integer_literals(self):
@@ -226,12 +178,6 @@
 substrs=["1"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_float_literal(self):
@@ -241,12 +187,6 @@
 substrs=["NSNumber", "123.45"])
 
 @skipUnlessDarwin
-@expectedFailureAll(
-oslist=['macosx'],
-compiler='clang',
-compiler_version=[
-'<',
-'7.0.0'])
 @skipIf(macos_version=["<", "10.12"])
 @expectedFailureAll(archs=["i[3-6]86"])
 def test_expressions_in_literals(self):
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] r320447 - Rollback [Testsuite] Rename this file from *m -> *mm.

[Davide Italiano via lldb-commits]

Author: davide
Date: Mon Dec 11 17:21:43 2017
New Revision: 320447

URL: http://llvm.org/viewvc/llvm-project?rev=320447&view=rev
Log:
Rollback [Testsuite] Rename this file from *m -> *mm.

After discussing this with Jim and Jason, I think my commit was
actually sweeping the issue under the carpet rather than fixing it.
I'll take a closer look between tonight and tomorrow.

Added:
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.m
  - copied, changed from r320444, 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.mm
Removed:
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.mm

Copied: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.m 
(from r320444, 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.mm)
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.m?p2=lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.m&p1=lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.mm&r1=320444&r2=320447&rev=320447&view=diff
==
(empty)

Removed: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.mm
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.mm?rev=320446&view=auto
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.mm 
(original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/variadic_methods/main.mm 
(removed)
@@ -1,31 +0,0 @@
-//===-- main.m ---*- 
Objective-C-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-
-#import 
-
-@interface VarClass : NSObject
-- (id) lottaArgs: (id) first, ...;
-@end
-
-@implementation VarClass
-- (id) lottaArgs: (id) first, ...
-{
-  return first;
-}
-@end
-
-int
-main()
-{
-  VarClass *my_var = [[VarClass alloc] init];
-  id something = [my_var lottaArgs: @"111", @"222", nil];
-  NSLog (@"%@ - %@", my_var, something); //% self.expect("expression -O -- 
[my_var lottaArgs:@\"111\", @\"222\", nil]", DATA_TYPES_DISPLAYED_CORRECTLY, 
substrs = ["111"])
-  return 0;
-}
-


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

[Lldb-commits] [lldb] r320450 - [testsuite] Remove even more testing vestiges.

[Davide Italiano via lldb-commits]

Author: davide
Date: Mon Dec 11 18:10:23 2017
New Revision: 320450

URL: http://llvm.org/viewvc/llvm-project?rev=320450&view=rev
Log:
[testsuite] Remove even more testing vestiges.

With this one, the number of unexpected successes for the LLDB
test suite when building with clang ToT goes down to 18.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/expression_command/persist_objc_pointeetype/TestPersistObjCPointeeType.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/persist_objc_pointeetype/TestPersistObjCPointeeType.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/expression_command/persist_objc_pointeetype/TestPersistObjCPointeeType.py?rev=320450&r1=320449&r2=320450&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/persist_objc_pointeetype/TestPersistObjCPointeeType.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/expression_command/persist_objc_pointeetype/TestPersistObjCPointeeType.py
 Mon Dec 11 18:10:23 2017
@@ -22,9 +22,6 @@ class PersistObjCPointeeType(TestBase):
 self.line = line_number('main.m', '// break here')
 
 @skipUnlessDarwin
-@expectedFailureAll(
-bugnumber='http://llvm.org/pr23504',
-oslist=['macosx'], compiler='clang', compiler_version=['<', '7.0.0'])
 @skipIf(archs=["i386", "i686"])
 @skipIf(debug_info="gmodules", archs=['arm64', 'armv7', 'armv7k'])  # 
compile error with gmodules for iOS
 def test_with(self):


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

[Lldb-commits] [lldb] r320452 - [TestCppScope] This test now passes on Darwin.

[Davide Italiano via lldb-commits]

Author: davide
Date: Mon Dec 11 18:32:49 2017
New Revision: 320452

URL: http://llvm.org/viewvc/llvm-project?rev=320452&view=rev
Log:
[TestCppScope] This test now passes on Darwin.

I tested on x86-64 and Jason on embedded architectures.
This cleans up another couple of reported unexpected successes.



Modified:
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/scope/TestCppScope.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/scope/TestCppScope.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/scope/TestCppScope.py?rev=320452&r1=320451&r2=320452&view=diff
==
--- lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/scope/TestCppScope.py 
(original)
+++ lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/scope/TestCppScope.py 
Mon Dec 11 18:32:49 2017
@@ -15,11 +15,6 @@ class TestCppScopes(TestBase):
 def test_all_but_c(self):
 self.do_test(False)
 
-# There's a global symbol in libsystem on Darwin that messes up
-# the lookup of class C.  Breaking that test out from the others
-# since that is a odd failure, and I don't want it to mask the
-# real purpose of this test.
-@expectedFailureDarwin(bugnumber="")
 @expectedFailureAll(oslist=["windows"])
 def test_c(self):
 self.do_test(True)


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

[Lldb-commits] [lldb] r320454 - [TestModulesInlineFunctions] This test now passes.

[Davide Italiano via lldb-commits]

Author: davide
Date: Mon Dec 11 18:52:01 2017
New Revision: 320454

URL: http://llvm.org/viewvc/llvm-project?rev=320454&view=rev
Log:
[TestModulesInlineFunctions] This test now passes.

Remove yet another spurious unexpected success.
Ack'ed by Jim Ingham.

Fixes PR25743.

Modified:

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py

Modified: 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py?rev=320454&r1=320453&r2=320454&view=diff
==
--- 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
 (original)
+++ 
lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py
 Mon Dec 11 18:52:01 2017
@@ -28,7 +28,6 @@ class ModulesInlineFunctionsTestCase(Tes
 
 @skipUnlessDarwin
 @skipIf(macos_version=["<", "10.12"])
-@expectedFailureDarwin("llvm.org/pr25743")
 def test_expr(self):
 self.build()
 exe = os.path.join(os.getcwd(), "a.out")


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

[Lldb-commits] [lldb] r320456 - Avoid module import in a textual header, NFC

[Vedant Kumar via lldb-commits]

Author: vedantk
Date: Mon Dec 11 19:27:13 2017
New Revision: 320456

URL: http://llvm.org/viewvc/llvm-project?rev=320456&view=rev
Log:
Avoid module import in a textual header, NFC

This unbreaks the lldb modules build (-DLLVM_ENABLE_MODULES=On).

Modified:
lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h

Modified: lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h?rev=320456&r1=320455&r2=320456&view=diff
==
--- lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h (original)
+++ lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h Mon Dec 11 
19:27:13 2017
@@ -7,11 +7,8 @@
 //
 
//===--===//
 
-#include "llvm/Support/Compiler.h"
-#include 
-#include 
-
-// Project includes
+// This file is meant to be textually included. Do not #include modular
+// headers here.
 
 // Computes the offset of the given GPR in the user data area.
 #define GPR_OFFSET(regname) (LLVM_EXTENSION offsetof(GPR, regname))


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

Re: [Lldb-commits] [lldb] r320456 - Avoid module import in a textual header, NFC

[Zachary Turner via lldb-commits]

Long term it would be nice if we could get all these register definitions
automatically generated with llvm-tblgen.  That's a big undertaking, though.

On Mon, Dec 11, 2017 at 7:27 PM Vedant Kumar via lldb-commits <
lldb-commits@lists.llvm.org> wrote:

> Author: vedantk
> Date: Mon Dec 11 19:27:13 2017
> New Revision: 320456
>
> URL: http://llvm.org/viewvc/llvm-project?rev=320456&view=rev
> Log:
> Avoid module import in a textual header, NFC
>
> This unbreaks the lldb modules build (-DLLVM_ENABLE_MODULES=On).
>
> Modified:
> lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
>
> Modified: lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
> URL:
> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h?rev=320456&r1=320455&r2=320456&view=diff
>
> ==
> --- lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
> (original)
> +++ lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h Mon
> Dec 11 19:27:13 2017
> @@ -7,11 +7,8 @@
>  //
>
>  
> //===--===//
>
> -#include "llvm/Support/Compiler.h"
> -#include 
> -#include 
> -
> -// Project includes
> +// This file is meant to be textually included. Do not #include modular
> +// headers here.
>
>  // Computes the offset of the given GPR in the user data area.
>  #define GPR_OFFSET(regname) (LLVM_EXTENSION offsetof(GPR, regname))
>
>
> ___
> lldb-commits mailing list
> lldb-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits