[Lldb-commits] [PATCH] D38938: Logging: provide a way to safely disable logging in a forked process

2017-10-17 Thread Pavel Labath via Phabricator via lldb-commits
labath updated this revision to Diff 119315.
labath added a comment.

Solve the problem using pthread_atfork().

This way the locks are properly taken before forking and released in both child
and parent processes. This behavior is encapsulated within the Log class and is
completely transparent to all fork users (but it requires a tiny bit of
system-specific code).


https://reviews.llvm.org/D38938

Files:
  include/lldb/Utility/Log.h
  include/lldb/Utility/Logging.h
  source/Initialization/SystemInitializerCommon.cpp
  source/Utility/Log.cpp
  source/Utility/Logging.cpp

Index: source/Utility/Logging.cpp
===
--- source/Utility/Logging.cpp
+++ source/Utility/Logging.cpp
@@ -51,7 +51,7 @@
 
 static Log::Channel g_log_channel(g_categories, LIBLLDB_LOG_DEFAULT);
 
-void lldb_private::InitializeLog() {
+void lldb_private::InitializeLldbChannel() {
   Log::Register("lldb", g_log_channel);
 }
 
Index: source/Utility/Log.cpp
===
--- source/Utility/Log.cpp
+++ source/Utility/Log.cpp
@@ -32,6 +32,7 @@
 #include  // for getpid
 #else
 #include 
+#include 
 #endif
 
 using namespace lldb_private;
@@ -181,6 +182,13 @@
   Printf("warning: %s", Content.c_str());
 }
 
+void Log::Initialize() {
+#ifdef LLVM_ON_UNIX
+  pthread_atfork(&Log::LockAllChannels, &Log::UnlockAllChannels, &Log::UnlockAllChannels);
+#endif
+  InitializeLldbChannel();
+}
+
 void Log::Register(llvm::StringRef name, Channel &channel) {
   auto iter = g_channel_map->try_emplace(name, channel);
   assert(iter.second == true);
@@ -321,3 +329,13 @@
   message << payload << "\n";
   WriteMessage(message.str());
 }
+
+void Log::LockAllChannels() {
+  for (auto &c: *g_channel_map)
+c.second.m_mutex.lock();
+}
+
+void Log::UnlockAllChannels() {
+  for (auto &c: *g_channel_map)
+c.second.m_mutex.unlock();
+}
Index: source/Initialization/SystemInitializerCommon.cpp
===
--- source/Initialization/SystemInitializerCommon.cpp
+++ source/Initialization/SystemInitializerCommon.cpp
@@ -70,7 +70,7 @@
 #endif
 
   llvm::EnablePrettyStackTrace();
-  InitializeLog();
+  Log::Initialize();
   HostInfo::Initialize();
   static Timer::Category func_cat(LLVM_PRETTY_FUNCTION);
   Timer scoped_timer(func_cat, LLVM_PRETTY_FUNCTION);
Index: include/lldb/Utility/Logging.h
===
--- include/lldb/Utility/Logging.h
+++ include/lldb/Utility/Logging.h
@@ -62,7 +62,7 @@
 
 Log *GetLogIfAnyCategoriesSet(uint32_t mask);
 
-void InitializeLog();
+void InitializeLldbChannel();
 
 } // namespace lldb_private
 
Index: include/lldb/Utility/Log.h
===
--- include/lldb/Utility/Log.h
+++ include/lldb/Utility/Log.h
@@ -96,6 +96,9 @@
 }
   };
 
+
+  static void Initialize();
+
   //--
   // Static accessors for logging channels
   //--
@@ -193,6 +196,9 @@
   static uint32_t GetFlags(llvm::raw_ostream &stream, const ChannelMap::value_type &entry,
llvm::ArrayRef categories);
 
+  static void LockAllChannels();
+  static void UnlockAllChannels();
+
   Log(const Log &) = delete;
   void operator=(const Log &) = delete;
 };
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D38897: Add specific ppc64le hardware watchpoint handler

2017-10-17 Thread Ana Julia Caetano via Phabricator via lldb-commits
anajuliapc updated this revision to Diff 119336.
anajuliapc marked 6 inline comments as done.
anajuliapc added a comment.

- Add values to header file
- Use std::array to declare m_hwp_regs
- Use llvm's mask
- Remove switch and use assert instead
- Remove unnecessary 'else'
- Change return


https://reviews.llvm.org/D38897

Files:
  source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.cpp
  source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp

Index: source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
===
--- source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -1601,21 +1601,24 @@
   // and we only want to override this behavior if we have explicitly
   // received a qHostInfo telling us otherwise
   if (m_qHostInfo_is_valid != eLazyBoolYes) {
-// On targets like MIPS, watchpoint exceptions are always generated
-// before the instruction is executed. The connected target may not
-// support qHostInfo or qWatchpointSupportInfo packets.
+// On targets like MIPS and ppc64le, watchpoint exceptions are always
+// generated before the instruction is executed. The connected target
+// may not support qHostInfo or qWatchpointSupportInfo packets.
 if (atype == llvm::Triple::mips || atype == llvm::Triple::mipsel ||
-atype == llvm::Triple::mips64 || atype == llvm::Triple::mips64el)
+atype == llvm::Triple::mips64 || atype == llvm::Triple::mips64el ||
+atype == llvm::Triple::ppc64le)
   after = false;
 else
   after = true;
   } else {
-// For MIPS, set m_watchpoints_trigger_after_instruction to eLazyBoolNo
-// if it is not calculated before.
-if (m_watchpoints_trigger_after_instruction == eLazyBoolCalculate &&
+// For MIPS and ppc64le, set m_watchpoints_trigger_after_instruction to
+// eLazyBoolNo if it is not calculated before.
+if ((m_watchpoints_trigger_after_instruction == eLazyBoolCalculate &&
 (atype == llvm::Triple::mips || atype == llvm::Triple::mipsel ||
- atype == llvm::Triple::mips64 || atype == llvm::Triple::mips64el))
+ atype == llvm::Triple::mips64 || atype == llvm::Triple::mips64el)) ||
+atype == llvm::Triple::ppc64le) {
   m_watchpoints_trigger_after_instruction = eLazyBoolNo;
+}
 
 after = (m_watchpoints_trigger_after_instruction != eLazyBoolNo);
   }
Index: source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h
===
--- source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h
+++ source/Plugins/Process/Linux/NativeRegisterContextLinux_ppc64le.h
@@ -1,4 +1,4 @@
-//===-- NativeRegisterContextLinux_ppc64le.h -*- C++ -*-===//
+//===-- NativeRegisterContextLinux_ppc64le.h *- C++ -*-===//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -49,6 +49,28 @@
 
   Status WriteAllRegisterValues(const lldb::DataBufferSP &data_sp) override;
 
+  //--
+  // Hardware watchpoint mangement functions
+  //--
+
+  uint32_t NumSupportedHardwareWatchpoints() override;
+
+  uint32_t SetHardwareWatchpoint(lldb::addr_t addr, size_t size,
+ uint32_t watch_flags) override;
+
+  bool ClearHardwareWatchpoint(uint32_t hw_index) override;
+
+  Status GetWatchpointHitIndex(uint32_t &wp_index,
+   lldb::addr_t trap_addr) override;
+
+  lldb::addr_t GetWatchpointHitAddress(uint32_t wp_index) override;
+
+  lldb::addr_t GetWatchpointAddress(uint32_t wp_index) override;
+
+  uint32_t GetWatchpointSize(uint32_t wp_index);
+
+  bool WatchpointIsEnabled(uint32_t wp_index);
+
 protected:
   Status DoReadGPR(void *buf, size_t buf_size) override;
 
@@ -60,6 +82,29 @@
   GPR m_gpr_ppc64le; // 64-bit general purpose registers.
 
   bool IsGPR(unsigned reg) const;
+
+  Status ReadHardwareDebugInfo();
+
+  Status WriteHardwareDebugRegs();
+
+  // Debug register info for hardware watchpoints management.
+  struct DREG {
+lldb::addr_t address;   // Breakpoint/watchpoint address value.
+lldb::addr_t hit_addr;  // Address at which last watchpoint trigger
+// exception occurred.
+lldb::addr_t real_addr; // Address value that should cause target to stop.
+uint32_t control;   // Breakpoint/watchpoint control value.
+uint32_t refcount;  // Serves as enable/disable and reference counter.
+long slot;  // Saves the value returned from PTRACE_SETHWDEBUG.
+int mode;   // Defines if watchpoint is read/write/access.
+  };
+
+  std::array m_hwp_regs;
+
+  // 16 is jus

[Lldb-commits] [lldb] r316007 - Remove shared_pointer from NativeThreadProtocol

2017-10-17 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Oct 17 08:52:16 2017
New Revision: 316007

URL: http://llvm.org/viewvc/llvm-project?rev=316007&view=rev
Log:
Remove shared_pointer from NativeThreadProtocol

Summary:
The NativeThread class is useless without the containing process (and in
some places it is already assuming the process is always around). This
makes it clear that the NativeProcessProtocol is the object owning the
threads, and makes the destruction order deterministic (first threads,
then process). The NativeProcess is the only thing holding a thread
unique_ptr, and methods that used to hand out thread shared pointers now
return raw pointers or references.

Reviewers: krytarowski, eugene

Subscribers: lldb-commits

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

Modified:
lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
lldb/trunk/include/lldb/Host/common/NativeThreadProtocol.h
lldb/trunk/include/lldb/lldb-private-forward.h
lldb/trunk/source/Host/common/NativeProcessProtocol.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h
lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Modified: lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h?rev=316007&r1=316006&r2=316007&view=diff
==
--- lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h (original)
+++ lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h Tue Oct 17 
08:52:16 2017
@@ -10,6 +10,9 @@
 #ifndef liblldb_NativeProcessProtocol_h_
 #define liblldb_NativeProcessProtocol_h_
 
+#include "NativeBreakpointList.h"
+#include "NativeThreadProtocol.h"
+#include "NativeWatchpointList.h"
 #include "lldb/Host/Host.h"
 #include "lldb/Host/MainLoop.h"
 #include "lldb/Utility/Status.h"
@@ -23,9 +26,6 @@
 #include "llvm/Support/MemoryBuffer.h"
 #include 
 
-#include "NativeBreakpointList.h"
-#include "NativeWatchpointList.h"
-
 namespace lldb_private {
 class MemoryRegionInfo;
 class ResumeActionList;
@@ -166,15 +166,15 @@ public:
   //--
   // Access to threads
   //--
-  NativeThreadProtocolSP GetThreadAtIndex(uint32_t idx);
+  NativeThreadProtocol *GetThreadAtIndex(uint32_t idx);
 
-  NativeThreadProtocolSP GetThreadByID(lldb::tid_t tid);
+  NativeThreadProtocol *GetThreadByID(lldb::tid_t tid);
 
   void SetCurrentThreadID(lldb::tid_t tid) { m_current_thread_id = tid; }
 
   lldb::tid_t GetCurrentThreadID() { return m_current_thread_id; }
 
-  NativeThreadProtocolSP GetCurrentThread() {
+  NativeThreadProtocol *GetCurrentThread() {
 return GetThreadByID(m_current_thread_id);
   }
 
@@ -401,7 +401,7 @@ public:
 protected:
   lldb::pid_t m_pid;
 
-  std::vector m_threads;
+  std::vector> m_threads;
   lldb::tid_t m_current_thread_id = LLDB_INVALID_THREAD_ID;
   mutable std::recursive_mutex m_threads_mutex;
 
@@ -461,7 +461,7 @@ protected:
   // ---
   void NotifyDidExec();
 
-  NativeThreadProtocolSP GetThreadByIDUnlocked(lldb::tid_t tid);
+  NativeThreadProtocol *GetThreadByIDUnlocked(lldb::tid_t tid);
 
   // ---
   // Static helper methods for derived classes.

Modified: lldb/trunk/include/lldb/Host/common/NativeThreadProtocol.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Host/common/NativeThreadProtocol.h?rev=316007&r1=316006&r2=316007&view=diff
==
--- lldb/trunk/include/lldb/Host/common/NativeThreadProtocol.h (original)
+++ lldb/trunk/include/lldb/Host/common/NativeThreadProtocol.h Tue Oct 17 
08:52:16 2017
@@ -20,8 +20,7 @@ namespace lldb_private {
 //--
 // NativeThreadProtocol
 //--
-class NativeThreadProtocol
-: public std::enable_shared_from_this {
+class NativeThreadProtocol {
 public:
   NativeThreadProtocol(NativeProcessProtocol &process, lldb::tid_t tid);
 

Modified: lldb/trunk/include/lldb/lldb-private-forward.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/lldb-private-forward.h?rev=316007&r1=316006&r2=316007&view=diff
==
--- lldb/trunk/include/lldb/lldb-private-f

[Lldb-commits] [PATCH] D35618: Remove shared_pointer from NativeThreadProtocol

2017-10-17 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316007: Remove shared_pointer from NativeThreadProtocol 
(authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D35618?vs=107289&id=119338#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35618

Files:
  lldb/trunk/include/lldb/Host/common/NativeProcessProtocol.h
  lldb/trunk/include/lldb/Host/common/NativeThreadProtocol.h
  lldb/trunk/include/lldb/lldb-private-forward.h
  lldb/trunk/source/Host/common/NativeProcessProtocol.cpp
  lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
  lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.h
  lldb/trunk/source/Plugins/Process/Linux/NativeThreadLinux.h
  lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
  lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
  
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
  
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h

Index: lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
===
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.h
@@ -121,7 +121,7 @@
 
   bool HasThreadNoLock(lldb::tid_t thread_id);
 
-  NativeThreadNetBSDSP AddThread(lldb::tid_t thread_id);
+  NativeThreadNetBSD &AddThread(lldb::tid_t thread_id);
 
   void MonitorCallback(lldb::pid_t pid, int signal);
   void MonitorExited(lldb::pid_t pid, WaitStatus status);
Index: lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
===
--- lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
+++ lldb/trunk/source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp
@@ -109,10 +109,8 @@
   if (status.Fail())
 return status.ToError();
 
-  for (const auto &thread_sp : process_up->m_threads) {
-static_pointer_cast(thread_sp)->SetStoppedBySignal(
-SIGSTOP);
-  }
+  for (const auto &thread : process_up->m_threads)
+static_cast(*thread).SetStoppedBySignal(SIGSTOP);
   process_up->SetState(StateType::eStateStopped);
 
   return std::move(process_up);
@@ -198,9 +196,9 @@
 // Handle SIGSTOP from LLGS (LLDB GDB Server)
 if (info.psi_siginfo.si_code == SI_USER &&
 info.psi_siginfo.si_pid == ::getpid()) {
-  /* Stop Tracking All Threads attached to Process */
-  for (const auto &thread_sp : m_threads) {
-static_pointer_cast(thread_sp)->SetStoppedBySignal(
+  /* Stop Tracking all Threads attached to Process */
+  for (const auto &thread : m_threads) {
+static_cast(*thread).SetStoppedBySignal(
 SIGSTOP, &info.psi_siginfo);
   }
 }
@@ -221,18 +219,15 @@
 
   switch (info.psi_siginfo.si_code) {
   case TRAP_BRKPT:
-for (const auto &thread_sp : m_threads) {
-  static_pointer_cast(thread_sp)
-  ->SetStoppedByBreakpoint();
-  FixupBreakpointPCAsNeeded(
-  *static_pointer_cast(thread_sp));
+for (const auto &thread : m_threads) {
+  static_cast(*thread).SetStoppedByBreakpoint();
+  FixupBreakpointPCAsNeeded(static_cast(*thread));
 }
 SetState(StateType::eStateStopped, true);
 break;
   case TRAP_TRACE:
-for (const auto &thread_sp : m_threads) {
-  static_pointer_cast(thread_sp)->SetStoppedByTrace();
-}
+for (const auto &thread : m_threads)
+  static_cast(*thread).SetStoppedByTrace();
 SetState(StateType::eStateStopped, true);
 break;
   case TRAP_EXEC: {
@@ -245,49 +240,44 @@
 // Let our delegate know we have just exec'd.
 NotifyDidExec();
 
-for (const auto &thread_sp : m_threads) {
-  static_pointer_cast(thread_sp)->SetStoppedByExec();
-}
+for (const auto &thread : m_threads)
+  static_cast(*thread).SetStoppedByExec();
 SetState(StateType::eStateStopped, true);
   } break;
   case TRAP_DBREG: {
 // If a watchpoint was hit, report it
 uint32_t wp_index;
-Status error =
-static_pointer_cast(m_threads[info.psi_lwpid])
-->GetRegisterContext()
-->GetWatchpointHitIndex(wp_index,
-(uintptr_t)info.psi_siginfo.si_addr);
+Status error = static_cast(*m_threads[info.psi_lwpid])
+   .GetRegisterContext()
+   ->GetWatchpointHitIndex(
+   wp_index, (uintptr_t)info.psi_siginfo.si_addr);
 if (error.Fail())
   LLDB_LOG(log,
"received error while checking for watchpoint hits, pid = "
"{0}, LWP = {1}, error = {2}",
GetID(), info.psi_lwpid, error);
 if (wp_index != LLDB_INVALID_INDEX32) {
-  for (const auto &thread_sp : m_threads) {
-static_pointer_cast(thread_sp)
-->SetStoppedByWatchpoint(wp_index);
-  }
+  for (const

[Lldb-commits] [PATCH] D38897: Add specific ppc64le hardware watchpoint handler

2017-10-17 Thread Ana Julia Caetano via Phabricator via lldb-commits
anajuliapc added a comment.

I applied some of the suggestions. I'm still working on the others, since I may 
need to change some logic

Thanks for the comments


https://reviews.llvm.org/D38897



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


[Lldb-commits] [PATCH] D35311: lldb-server tests: Add support for testing debugserver

2017-10-17 Thread Pavel Labath via Phabricator via lldb-commits
labath added a comment.

In https://reviews.llvm.org/D35311#830779, @beanz wrote:

> This all looks fine to me. The one thing I'd like you to consider is that 
> someday (when lldb-server is supported on Darwin) we will want to run these 
> tests against both debugserver and lldb-server. I'm not asking you to make 
> those changes to this patch, but if there are things you would change about 
> how you're doing things to make that easier to support I'd ask that you 
> consider that.
>
> I have one specific comment below that is along those lines. Otherwise, this 
> LGTM.


I did consider this to some depth while making this change. I think that the 
easiest way to make that happen would be to create two cmake targets 
(`add_lldb_unittest(LLDBServerTests...`, 
`add_lldb_unittest(DebugServerTests...`). The main difference between these 
would be the value of the LLDB_SERVER define, but other differences are 
possible as well (for example, if only one of the servers implements some 
feature). It means we will compile some things twice, but in return it will 
integrate nicely with the existing llvm unit testing infrastructure.

It may be possible to use the googletest's parameterized test mechanism for 
this, but I don't know that well enough to tell. In any case, this shouldn't 
interfere with this patch -- the only thing that will change is that the return 
value of isdebugserver() will be determined through some runtime mechanism.




Comment at: unittests/tools/lldb-server/tests/TestClient.cpp:32
+bool TestClient::IsDebugServer() {
+#ifdef __APPLE__
+  return true;

beanz wrote:
> Instead of making this ifdef'd can you just check to see if `LLDB_SERVER` is 
> debugserver or lldb-server?
> 
> One of the items on my todo list is to get rid of all the places where 
> `#ifdef __APPLE__` equates to using debugserver so that we can start 
> evaluating making lldb-server work on Darwin.
I'll change this to detect the server type based on the file name.


https://reviews.llvm.org/D35311



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


[Lldb-commits] [lldb] r316010 - lldb-server tests: Add support for testing debugserver

2017-10-17 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Oct 17 09:28:28 2017
New Revision: 316010

URL: http://llvm.org/viewvc/llvm-project?rev=316010&view=rev
Log:
lldb-server tests: Add support for testing debugserver

Summary:
This adds support for running the lldb-server test suite (currently consisting
of only one test) against the debugserver. Currently, the choice which binary
to test is based on the host system. This will need to be replaced by something
more elaborate if/when lldb-server starts supporting debugging on darwin.

I need to make a couple of tweaks to the test client to work with debugserver:
- debugserver has different command-line arguments - launching code adjusted to
  handle that
- debugserver sends duplicate "medata" fields in the stop reply packet -
  adjusted stop-reply parsing code to handle that
- debugserver replies to the k packet instead of just dropping the connection -
  stopping code adjusted, although we should probably consider aligning the
  behavior of the two stubs in this case

Reviewers: jmajors, beanz

Subscribers: srhines, mgorny, lldb-commits

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

Modified:
lldb/trunk/unittests/tools/CMakeLists.txt
lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt
lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp
lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.h
lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
lldb/trunk/unittests/tools/lldb-server/tests/TestClient.h

Modified: lldb/trunk/unittests/tools/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/CMakeLists.txt?rev=316010&r1=316009&r2=316010&view=diff
==
--- lldb/trunk/unittests/tools/CMakeLists.txt (original)
+++ lldb/trunk/unittests/tools/CMakeLists.txt Tue Oct 17 09:28:28 2017
@@ -1,3 +1,3 @@
-if(CMAKE_SYSTEM_NAME MATCHES "Android|Linux|NetBSD")
+if(CMAKE_SYSTEM_NAME MATCHES "Android|Darwin|Linux|NetBSD")
   add_subdirectory(lldb-server)
 endif()

Modified: lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt?rev=316010&r1=316009&r2=316010&view=diff
==
--- lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt (original)
+++ lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt Tue Oct 17 09:28:28 
2017
@@ -7,7 +7,12 @@ endfunction()
 
 add_lldb_test_executable(thread_inferior inferior/thread_inferior.cpp)
 
-add_definitions(-DLLDB_SERVER="$")
+if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
+  add_definitions(-DLLDB_SERVER="$")
+else()
+  add_definitions(-DLLDB_SERVER="$")
+endif()
+
 
add_definitions(-DTHREAD_INFERIOR="${CMAKE_CURRENT_BINARY_DIR}/thread_inferior")
 add_subdirectory(tests)
 add_dependencies(LLDBServerTests thread_inferior)

Modified: lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp?rev=316010&r1=316009&r2=316010&view=diff
==
--- lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp (original)
+++ lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp Tue Oct 17 
09:28:28 2017
@@ -19,7 +19,7 @@ namespace llgs_tests {
 
 Expected ProcessInfo::Create(StringRef response) {
   ProcessInfo process_info;
-  auto elements_or_error = SplitPairList("ProcessInfo", response);
+  auto elements_or_error = SplitUniquePairList("ProcessInfo", response);
   if (!elements_or_error)
 return elements_or_error.takeError();
 
@@ -140,21 +140,36 @@ const U64Map &StopReply::GetThreadPcs()
 
 Expected StopReply::Create(StringRef response,
   llvm::support::endianness endian) {
+  if (response.size() < 3 || !response.consume_front("T"))
+return make_parsing_error("StopReply: Invalid packet");
+
   StopReply stop_reply;
 
-  auto elements_or_error = SplitPairList("StopReply", response);
-  if (auto split_error = elements_or_error.takeError()) {
-return std::move(split_error);
+  StringRef signal = response.take_front(2);
+  response = response.drop_front(2);
+  if (!llvm::to_integer(signal, stop_reply.m_signal, 16))
+return make_parsing_error("StopReply: stop signal");
+
+  auto elements = SplitPairList(response);
+  for (StringRef field :
+   {"name", "reason", "thread", "threads", "thread-pcs"}) {
+// This will insert an empty field if there is none. In the future, we
+// should probably differentiate between these fields not being present and
+// them being empty, but right now no tests depends on this.
+if (elements.insert({field, {""}}).first->second.size() != 1)
+  return make_parsing_error(
+  "StopReply: got multiple responses for the {0} field", field);
   }
+  stop_repl

[Lldb-commits] [PATCH] D35311: lldb-server tests: Add support for testing debugserver

2017-10-17 Thread Pavel Labath via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL316010: lldb-server tests: Add support for testing 
debugserver (authored by labath).

Changed prior to commit:
  https://reviews.llvm.org/D35311?vs=106247&id=119344#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D35311

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

Index: lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
===
--- lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
+++ lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp
@@ -15,6 +15,7 @@
 #include "lldb/Interpreter/Args.h"
 #include "lldb/Target/ProcessLaunchInfo.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Support/Path.h"
 #include "gtest/gtest.h"
 #include 
 #include 
@@ -28,6 +29,12 @@
 namespace llgs_tests {
 void TestClient::Initialize() { HostInfo::Initialize(); }
 
+bool TestClient::IsDebugServer() {
+  return sys::path::filename(LLDB_SERVER).contains("debugserver");
+}
+
+bool TestClient::IsLldbServer() { return !IsDebugServer(); }
+
 TestClient::TestClient(const std::string &test_name,
const std::string &test_case_name)
 : m_test_name(test_name), m_test_case_name(test_case_name),
@@ -39,13 +46,16 @@
   const ArchSpec &arch_spec = HostInfo::GetArchitecture();
   Args args;
   args.AppendArgument(LLDB_SERVER);
-  args.AppendArgument("gdbserver");
-  args.AppendArgument("--log-channels=gdb-remote packets");
+  if (IsLldbServer()) {
+args.AppendArgument("gdbserver");
+args.AppendArgument("--log-channels=gdb-remote packets");
+  } else {
+args.AppendArgument("--log-flags=0x80");
+  }
   args.AppendArgument("--reverse-connect");
   std::string log_file_name = GenerateLogFileName(arch_spec);
-  if (log_file_name.size()) {
+  if (log_file_name.size())
 args.AppendArgument("--log-file=" + log_file_name);
-  }
 
   Status error;
   TCPSocket listen_socket(true, false);
@@ -83,7 +93,11 @@
 
 bool TestClient::StopDebugger() {
   std::string response;
-  return SendMessage("k", response, PacketResult::ErrorDisconnected);
+  // Debugserver (non-conformingly?) sends a reply to the k packet instead of
+  // simply closing the connection.
+  PacketResult result =
+  IsDebugServer() ? PacketResult::Success : PacketResult::ErrorDisconnected;
+  return SendMessage("k", response, result);
 }
 
 bool TestClient::SetInferior(llvm::ArrayRef inferior_args) {
@@ -192,7 +206,7 @@
   return UINT_MAX;
 }
 
-auto elements_or_error = SplitPairList("GetPcRegisterId", response);
+auto elements_or_error = SplitUniquePairList("GetPcRegisterId", response);
 if (auto split_error = elements_or_error.takeError()) {
   GTEST_LOG_(ERROR) << "GetPcRegisterId: Error splitting response: "
 << response;
Index: lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp
===
--- lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp
+++ lldb/trunk/unittests/tools/lldb-server/tests/MessageObjects.cpp
@@ -19,7 +19,7 @@
 
 Expected ProcessInfo::Create(StringRef response) {
   ProcessInfo process_info;
-  auto elements_or_error = SplitPairList("ProcessInfo", response);
+  auto elements_or_error = SplitUniquePairList("ProcessInfo", response);
   if (!elements_or_error)
 return elements_or_error.takeError();
 
@@ -140,21 +140,36 @@
 
 Expected StopReply::Create(StringRef response,
   llvm::support::endianness endian) {
+  if (response.size() < 3 || !response.consume_front("T"))
+return make_parsing_error("StopReply: Invalid packet");
+
   StopReply stop_reply;
 
-  auto elements_or_error = SplitPairList("StopReply", response);
-  if (auto split_error = elements_or_error.takeError()) {
-return std::move(split_error);
+  StringRef signal = response.take_front(2);
+  response = response.drop_front(2);
+  if (!llvm::to_integer(signal, stop_reply.m_signal, 16))
+return make_parsing_error("StopReply: stop signal");
+
+  auto elements = SplitPairList(response);
+  for (StringRef field :
+   {"name", "reason", "thread", "threads", "thread-pcs"}) {
+// This will insert an empty field if there is none. In the future, we
+// should probably differentiate between these fields not being present and
+// them being empty, but right now no tests depends on this.
+if (elements.insert({field, {""}}).first->second.size() != 1)
+  return make_parsing_error(
+  "StopReply: got multiple responses for the {0} field", field);
   }
+  stop_reply.m_n

[Lldb-commits] [PATCH] D39010: lldb-server tests: Propagate environment variables (pr34192)

2017-10-17 Thread Pavel Labath via Phabricator via lldb-commits
labath created this revision.

Without this, the launching of the test inferior may fail if it depends
on some component of the environment (most likely LD_LIBRARY_PATH). This
makes sure we propagate the environment variable to the inferior
process.


https://reviews.llvm.org/D39010

Files:
  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
@@ -101,6 +101,14 @@
 }
 
 bool TestClient::SetInferior(llvm::ArrayRef inferior_args) {
+  StringList env;
+  Host::GetEnvironment(env);
+  for (size_t i = 0; i < env.GetSize(); ++i) {
+if (SendEnvironmentPacket(env[i].c_str()) != 0) {
+  GTEST_LOG_(ERROR) << "failed to set environment variable `" << env[i] << 
"`";
+  return false;
+}
+  }
   std::stringstream command;
   command << "A";
   for (size_t i = 0; i < inferior_args.size(); i++) {


Index: unittests/tools/lldb-server/tests/TestClient.cpp
===
--- unittests/tools/lldb-server/tests/TestClient.cpp
+++ unittests/tools/lldb-server/tests/TestClient.cpp
@@ -101,6 +101,14 @@
 }
 
 bool TestClient::SetInferior(llvm::ArrayRef inferior_args) {
+  StringList env;
+  Host::GetEnvironment(env);
+  for (size_t i = 0; i < env.GetSize(); ++i) {
+if (SendEnvironmentPacket(env[i].c_str()) != 0) {
+  GTEST_LOG_(ERROR) << "failed to set environment variable `" << env[i] << "`";
+  return false;
+}
+  }
   std::stringstream command;
   command << "A";
   for (size_t i = 0; i < inferior_args.size(); i++) {
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits


[Lldb-commits] [PATCH] D38938: Logging: provide a way to safely disable logging in a forked process

2017-10-17 Thread Greg Clayton via Phabricator via lldb-commits
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.

Much better.


https://reviews.llvm.org/D38938



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


[Lldb-commits] [lldb] r316038 - Silence some "implicit conversion of string literal" warnings

2017-10-17 Thread Pavel Labath via lldb-commits
Author: labath
Date: Tue Oct 17 14:52:29 2017
New Revision: 316038

URL: http://llvm.org/viewvc/llvm-project?rev=316038&view=rev
Log:
Silence some "implicit conversion of string literal" warnings

Modified:
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

Modified: lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp?rev=316038&r1=316037&r2=316038&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp 
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp Tue Oct 
17 14:52:29 2017
@@ -441,7 +441,7 @@ void ClangASTSource::CompleteType(clang:
 
   GetMergerUnchecked().CompleteType(interface_decl);
 } else {
-  lldbassert(!"No mechanism for completing a type!");
+  lldbassert(0 && "No mechanism for completing a type!");
 }
 return;
   }
@@ -1979,7 +1979,7 @@ clang::QualType ClangASTSource::CopyType
 clang::ExternalASTMerger &merger,
 clang::QualType type) {
   if (!merger.HasImporterForOrigin(from_context)) {
-lldbassert(!"Couldn't find the importer for a source context!");
+lldbassert(0 && "Couldn't find the importer for a source context!");
 return QualType();
   }
 
@@ -1992,13 +1992,13 @@ clang::Decl *ClangASTSource::CopyDecl(De
 return m_ast_importer_sp->CopyDecl(m_ast_context, &from_context, src_decl);
   } else if (m_merger_up) {
 if (!m_merger_up->HasImporterForOrigin(from_context)) {
-  lldbassert(!"Couldn't find the importer for a source context!");
+  lldbassert(0 && "Couldn't find the importer for a source context!");
   return nullptr;
 }
 
 return m_merger_up->ImporterForOrigin(from_context).Import(src_decl);
   } else {
-lldbassert("No mechanism for copying a decl!");
+lldbassert(0 && "No mechanism for copying a decl!");
 return nullptr;
   }
 }
@@ -2043,7 +2043,7 @@ CompilerType ClangASTSource::GuardedCopy
 CopyTypeWithMerger(*src_ast->getASTContext(), *m_merger_up,
  ClangUtil::GetQualType(src_type));
   } else {
-lldbassert("No mechanism for copying a type!");
+lldbassert(0 && "No mechanism for copying a type!");
 return CompilerType();
   }
 

Modified: 
lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp?rev=316038&r1=316037&r2=316038&view=diff
==
--- lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp 
(original)
+++ lldb/trunk/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp 
Tue Oct 17 14:52:29 2017
@@ -303,7 +303,7 @@ TypeFromUser ClangExpressionDeclMap::Dep
 clang::QualType::getFromOpaquePtr(parser_type.GetOpaqueQualType()));
 return TypeFromUser(exported_type.getAsOpaquePtr(), &target);
   } else {
-lldbassert(!"No mechanism for deporting a type!");
+lldbassert(0 && "No mechanism for deporting a type!");
 return TypeFromUser();
   }
 }
@@ -1916,7 +1916,7 @@ bool ClangExpressionDeclMap::ResolveUnkn
 scratch_ast_context->GetMergerUnchecked(),
 var_type).getAsOpaquePtr();
   } else {
-lldbassert(!"No mechanism to copy a resolved unknown type!");
+lldbassert(0 && "No mechanism to copy a resolved unknown type!");
 return false;
   }
 


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