Support Unit Testing debugserver

[beanz]

Summary

This patch refactors the CMake build system's support for building debugserver to allow us to build the majority of debugserver's sources into the debugserverCommon library which can then be reused by unit tests.

The first unit test I've written tests debug server's ability to accept incoming connections from LLDB. The test forks the process, and one side creates a listening socket using debugserver's socket API, the other side creates a transmitting socket using LLDB's TCPSocket class.

I have no clue where to even start getting this connected into the LLDB Xcode project, so for now these tests are CMake-only.

[jingham]

Todd added a gtest target to the lldb.xcodeproj. Presumably you could use that as a template and do the same thing in the tools/debugserver/debugserver.xcodeproj. Then make an aggregate target in the lldb workspace that runs these two targets.

[beanz]

Xcode is pretty magic to me. I don't know how to do much of anything in it other than build. I think the right solution would be to take most of the source files out of the debugserver target and generate a static archive from them, then have debugserver and the debugserverTest target link the static archive.

I don't know how to do that and not screw up the Xcode target settings on the archive target.

[beanz]

Fleshed out the unit test logic to be more meaningful.

[jingham]

It's a little weird to have the unit tests for debugserver in the top-level lldb directory's unittests. debugserver really is a stand-alone tool that shares no code with lldb proper. How hard would it be to put the tests under debugserver?

[beanz]

@jingham I put the unit tests at the top because they depend on LLDB's Host library (at least the current tests do). I'm attempting to write tests which cover the socket communication between LLDB and debugserver by sending data between the two using each side of the API.

[jingham]

debugserver only runs on darwin and we have no intentions of using it elsewhere. lldb-server is the way to do debugserver for new platforms. I don't think you need the generality provided by host to test debugserver, OTOH, if using those classes saves you lots of time over writing macOS specific code, then I'm mostly concerned that it is clear that debugserver isn't the way to do debug servers for lldb, lldb-server is...

[beanz]

Added a note to the unit test CMake file about why the tests are where they are. Generally we isolate debugserver from the rest of LLDB, and this comment explains the breach of isolation.

[labath]

I am afraid I will be away for two weeks as well.. :(

The FileLogCallback thingy seems a bit unfortunate, but I don't see anything too controversial based on a quick scan..

[beanz]

Some cleanup to the test case:

• Auto-select a port, which will make this more reliable
• Open listening sockets before the fork() so that the sockets are ready for connection (this avoids a race)
• Put test logic into reusable functions so that test logic can be reused for IPv6 tests when I add them

[beanz]

Adding Jim as a reviewer.

Jim, with the added comment about debugserver being Darwin-only, are you happy with this patch?

[jingham]

On the small points side: for lldb code we use lower-case, _ separated names for local variables, the point being that it allow you to tell at a glance what's a local and what's an ivar. Looks like you use a mixture of the two styles? exit_status, result, etc. alongside WriteSize, etc... It doesn't matter as much for a test case, but still it's good to be consistent.

More substantial, what happens to a gtest case if the test case program aborts? Remembering that the way the world works, this failure is bound to happen only intermittently on some server you can't access, so it's good to collect as much info as you can on failure. If the tests report a good stack trace on abort, that's probably fine, but otherwise it would be good to figure out a way to back out and leave some trace.

[beanz]

I will fix up the naming conventions. Switching back and forth between LLVM and LLDB conventions has done a number on my brain.

The test cases only abort on the child process side of the fork calls. This results in printing a stack trace, but it also will get captured as a failure because the parent process side checks the exit status of the child.

[beanz]

Fixing variable naming conventions

[labath]

This is fine, as far as i am concerned.

tools/debugserver/source/CMakeLists.txt

117 ↗

(On Diff #94526)

The comment does not make sense anymore.

lldb/trunk/tools/debugserver/source/CMakeLists.txt

 include_directories(${CMAKE_CURRENT_BINARY_DIR}/..)
 include_directories(${LLDB_SOURCE_DIR}/source)
 include_directories(MacOSX/DarwinLog)
 
-if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
-  include_directories(MacOSX)
-  #include_directories(${CMAKE_CURRENT_BINARY_DIR}/MacOSX)
 
-  set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -stdlib=libc++ -Wl,-sectcreate,__TEXT,__info_plist,${CMAKE_CURRENT_SOURCE_DIR}/../resources/lldb-debugserver-Info.plist")
-endif()
 
 check_cxx_compiler_flag("-Wno-gnu-zero-variadic-macro-arguments"
                         CXX_SUPPORTS_NO_GNU_ZERO_VARIADIC_MACRO_ARGUMENTS)
 if (CXX_SUPPORTS_NO_GNU_ZERO_VARIADIC_MACRO_ARGUMENTS)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-gnu-zero-variadic-macro-arguments")
 endif ()
 
 check_cxx_compiler_flag("-Wno-zero-length-array"
                         CXX_SUPPORTS_NO_ZERO_LENGTH_ARRAY)
 if (CXX_SUPPORTS_NO_ZERO_LENGTH_ARRAY)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-zero-length-array")
 endif ()
 
 check_cxx_compiler_flag("-Wno-extended-offsetof"
                         CXX_SUPPORTS_NO_EXTENDED_OFFSETOF)
 if (CXX_SUPPORTS_NO_EXTENDED_OFFSETOF)
   set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-extended-offsetof")
 endif ()
 
-if (NOT CMAKE_SYSTEM_NAME MATCHES "Darwin")
-  add_definitions(
-    -DDEBUGSERVER_VERSION_STR="${LLDB_VERSION}"
-    )
-endif ()
-
-if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
-  find_library(COCOA_LIBRARY Cocoa)
-  add_subdirectory(MacOSX)
-endif()
 
-add_lldb_library(lldbDebugserverCommon
-  debugserver.cpp
+set(generated_mach_interfaces
+  ${CMAKE_CURRENT_BINARY_DIR}/mach_exc.h
+  ${CMAKE_CURRENT_BINARY_DIR}/mach_excServer.c
+  ${CMAKE_CURRENT_BINARY_DIR}/mach_excUser.c
+  )
+add_custom_command(OUTPUT ${generated_mach_interfaces}
+  COMMAND mig ${CMAKE_CURRENT_SOURCE_DIR}/MacOSX/dbgnub-mig.defs
+  DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/MacOSX/dbgnub-mig.defs
+  )
+
+set(DEBUGSERVER_VERS_GENERATED_FILE ${CMAKE_CURRENT_BINARY_DIR}/debugserver_vers.c)
+set_source_files_properties(${DEBUGSERVER_VERS_GENERATED_FILE} PROPERTIES GENERATED 1)
+
+add_custom_command(OUTPUT ${DEBUGSERVER_VERS_GENERATED_FILE}
+  COMMAND ${LLDB_SOURCE_DIR}/scripts/generate-vers.pl
+          ${LLDB_SOURCE_DIR}/lldb.xcodeproj/project.pbxproj debugserver
+          > ${DEBUGSERVER_VERS_GENERATED_FILE}
+  DEPENDS ${LLDB_SOURCE_DIR}/scripts/generate-vers.pl
+          ${LLDB_SOURCE_DIR}/lldb.xcodeproj/project.pbxproj
+  )
+
+set(lldbDebugserverCommonSources
   DNBArch.cpp
   DNBBreakpoint.cpp
   DNB.cpp
   DNBDataRef.cpp
   DNBError.cpp
   DNBLog.cpp
   DNBRegisterInfo.cpp
   DNBThreadResumeActions.cpp
   JSON.cpp
   StdStringExtractor.cpp
   # JSON reader depends on the following LLDB-common files
   ${LLDB_SOURCE_DIR}/source/Host/common/StringConvert.cpp
   # end JSON reader dependencies
   libdebugserver.cpp
   PseudoTerminal.cpp
   PThreadEvent.cpp
   PThreadMutex.cpp
   RNBContext.cpp
   RNBRemote.cpp
   RNBServices.cpp
   RNBSocket.cpp
   SysSignal.cpp
   TTYState.cpp
   
+  MacOSX/CFBundle.cpp
+  MacOSX/CFString.cpp
+  MacOSX/Genealogy.cpp
+  MacOSX/MachException.cpp
+  MacOSX/MachProcess.mm
+  MacOSX/MachTask.mm
+  MacOSX/MachThread.cpp
+  MacOSX/MachThreadList.cpp
+  MacOSX/MachVMMemory.cpp
+  MacOSX/MachVMRegion.cpp
+  MacOSX/OsLogger.cpp
+  ${generated_mach_interfaces}
+  ${DEBUGSERVER_VERS_GENERATED_FILE})
+
+add_library(lldbDebugserverCommon ${lldbDebugserverCommonSources})
+
+target_link_libraries(lldbDebugserverCommon
+                      INTERFACE ${COCOA_LIBRARY}
+                      lldbDebugserverMacOSX_I386
+                      lldbDebugserverMacOSX_X86_64
+                      lldbDebugserverMacOSX_DarwinLog)
+
+set(LLVM_OPTIONAL_SOURCES ${lldbDebugserverCommonSources})
+add_lldb_tool(debugserver INCLUDE_IN_FRAMEWORK
+  debugserver.cpp
+
   LINK_LIBS
-    lldbHost
-    ${COCOA_LIBRARY}
+    lldbDebugserverCommon
   )
 
+set(LLDB_CODESIGN_IDENTITY "lldb_codesign"
+  CACHE STRING "Identity used for code signing. Set to empty string to skip the signing step.")
+if (NOT ("${LLDB_CODESIGN_IDENTITY}" STREQUAL ""))
+  execute_process(
+    COMMAND xcrun -f codesign_allocate
+    OUTPUT_STRIP_TRAILING_WHITESPACE
+    OUTPUT_VARIABLE CODESIGN_ALLOCATE
+    )
+  add_custom_command(TARGET debugserver
+    POST_BUILD
+    COMMAND ${CMAKE_COMMAND} -E env CODESIGN_ALLOCATE=${CODESIGN_ALLOCATE}
+            codesign --force --sign ${LLDB_CODESIGN_IDENTITY}
+            $<TARGET_FILE:debugserver>
+    WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/bin
+  )
+endif()
 
 
 
 

lldb/trunk/tools/debugserver/source/MacOSX/CMakeLists.txt

 #add_subdirectory(arm64)
 #add_subdirectory(arm)
 add_subdirectory(i386)
 #add_subdirectory(ppc)
 add_subdirectory(x86_64)
 add_subdirectory(DarwinLog)
 
 include_directories(..)
-
-set(generated_mach_interfaces
-  ${CMAKE_CURRENT_BINARY_DIR}/mach_exc.h
-  ${CMAKE_CURRENT_BINARY_DIR}/mach_excServer.c
-  ${CMAKE_CURRENT_BINARY_DIR}/mach_excUser.c
-  )
-add_custom_command(OUTPUT ${generated_mach_interfaces}
-  COMMAND mig ${CMAKE_CURRENT_SOURCE_DIR}/dbgnub-mig.defs
-  DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/dbgnub-mig.defs
-  )
-
-set(DEBUGSERVER_VERS_GENERATED_FILE ${CMAKE_CURRENT_BINARY_DIR}/debugserver_vers.c)
-set_source_files_properties(${DEBUGSERVER_VERS_GENERATED_FILE} PROPERTIES GENERATED 1)
-
-add_custom_command(OUTPUT ${DEBUGSERVER_VERS_GENERATED_FILE}
-  COMMAND ${LLDB_SOURCE_DIR}/scripts/generate-vers.pl
-          ${LLDB_SOURCE_DIR}/lldb.xcodeproj/project.pbxproj debugserver
-          > ${DEBUGSERVER_VERS_GENERATED_FILE}
-  DEPENDS ${LLDB_SOURCE_DIR}/scripts/generate-vers.pl
-          ${LLDB_SOURCE_DIR}/lldb.xcodeproj/project.pbxproj
-  )
-
-add_lldb_tool(debugserver INCLUDE_IN_FRAMEWORK
-  CFBundle.cpp
-  CFString.cpp
-  Genealogy.cpp
-  MachException.cpp
-  MachProcess.mm
-  MachTask.mm
-  MachThread.cpp
-  MachThreadList.cpp
-  MachVMMemory.cpp
-  MachVMRegion.cpp
-  OsLogger.cpp
-  ${generated_mach_interfaces}
-  ${DEBUGSERVER_VERS_GENERATED_FILE}
-
-  LINK_LIBS
-    lldbDebugserverCommon
-    lldbDebugserverMacOSX_I386
-    lldbDebugserverMacOSX_X86_64
-    lldbDebugserverMacOSX_DarwinLog
-  )
-
-set_source_files_properties(
-  HasAVX.s
-  # Necessary since compilation will fail with stand-alone assembler
-  PROPERTIES LANGUAGE C COMPILE_FLAGS "-x assembler-with-cpp"
-  )
-
-set(LLDB_CODESIGN_IDENTITY "lldb_codesign"
-  CACHE STRING "Identity used for code signing. Set to empty string to skip the signing step.")
-if (NOT ("${LLDB_CODESIGN_IDENTITY}" STREQUAL ""))
-  execute_process(
-    COMMAND xcrun -f codesign_allocate
-    OUTPUT_STRIP_TRAILING_WHITESPACE
-    OUTPUT_VARIABLE CODESIGN_ALLOCATE
-    )
-  add_custom_command(TARGET debugserver
-    POST_BUILD
-    # Note: --entitlements option removed (see comment above).
-    COMMAND ${CMAKE_COMMAND} -E env CODESIGN_ALLOCATE=${CODESIGN_ALLOCATE}
-            codesign --force --sign ${LLDB_CODESIGN_IDENTITY}
-            $<TARGET_FILE:debugserver>
-    WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/bin
-  )
-endif()

lldb/trunk/unittests/CMakeLists.txt

 add_subdirectory(Process)
 add_subdirectory(ScriptInterpreter)
 add_subdirectory(Signals)
 add_subdirectory(Symbol)
 add_subdirectory(SymbolFile)
 add_subdirectory(Target)
 add_subdirectory(UnwindAssembly)
 add_subdirectory(Utility)
+
+if(LLDB_CAN_USE_DEBUGSERVER)
+  add_subdirectory(debugserver)
+endif()
 No newline at end of file

lldb/trunk/unittests/debugserver/CMakeLists.txt

+# Note: debugserver is a Darwin-only implementation of a remote debugging
+# server. It is not intended to be used on other platforms. The tests are here
+# because using the LLDB Host API is convienent and allows testing of both parts
+# of the debugserver communication path. If you are looking for a non-darwin
+# remote debugging server, please use lldb-server.
+
+include_directories(${LLDB_SOURCE_DIR}/tools/debugserver/source
+                    ${LLDB_SOURCE_DIR}/tools/debugserver/source/MacOSX)
+
+add_lldb_unittest(debugserverTests
+  RNBSocketTest.cpp
+  debugserver_LogCallback.cpp
+
+  LINK_LIBS
+    lldbDebugserverCommon
+    lldbHost
+  LINK_COMPONENTS
+    Support
+  )

lldb/trunk/unittests/debugserver/RNBSocketTest.cpp

+//===-- RNBSocketTest.cpp ---------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "gtest/gtest.h"
+
+#include <arpa/inet.h>
+#include <sys/sysctl.h>
+#include <unistd.h>
+
+#include "RNBDefs.h"
+#include "RNBSocket.h"
+#include "lldb/Host/Socket.h"
+#include "lldb/Host/StringConvert.h"
+#include "lldb/Host/common/TCPSocket.h"
+
+using namespace lldb_private;
+
+std::string hello = "Hello, world!";
+std::string goodbye = "Goodbye!";
+
+static void ServerCallbackv4(const void *baton, in_port_t port) {
+  auto child_pid = fork();
+  if (child_pid == 0) {
+    Socket *client_socket;
+    char addr_buffer[256];
+    sprintf(addr_buffer, "%s:%d", baton, port);
+    Error err = Socket::TcpConnect(addr_buffer, false, client_socket);
+    if (err.Fail())
+      abort();
+    char buffer[32];
+    size_t read_size = 32;
+    err = client_socket->Read((void *)&buffer[0], read_size);
+    if (err.Fail())
+      abort();
+    std::string Recv(&buffer[0], read_size);
+    if (Recv != hello)
+      abort();
+    size_t write_size = goodbye.length();
+    err = client_socket->Write(goodbye.c_str(), write_size);
+    if (err.Fail())
+      abort();
+    if (write_size != goodbye.length())
+      abort();
+    delete client_socket;
+    exit(0);
+  }
+}
+
+void TestSocketListen(const char *addr) {
+  RNBSocket server_socket;
+  auto result =
+      server_socket.Listen(addr, 0, ServerCallbackv4, (const void *)addr);
+  ASSERT_TRUE(result == rnb_success);
+  result = server_socket.Write(hello.c_str(), hello.length());
+  ASSERT_TRUE(result == rnb_success);
+  std::string bye;
+  result = server_socket.Read(bye);
+  ASSERT_TRUE(result == rnb_success);
+  ASSERT_EQ(bye, goodbye);
+
+  int exit_status;
+  wait(&exit_status);
+  ASSERT_EQ(exit_status, 0);
+}
+
+TEST(RNBSocket, LoopBackListenIPv4) { TestSocketListen("127.0.0.1"); }
+
+void TestSocketConnect(const char *addr) {
+  char addr_wrap[256];
+  sprintf(addr_wrap, "%s:0", addr);
+
+  Socket *server_socket;
+  Predicate<uint16_t> port_predicate;
+  port_predicate.SetValue(0, eBroadcastNever);
+  Error err =
+      Socket::TcpListen(addr_wrap, false, server_socket, &port_predicate);
+  ASSERT_FALSE(err.Fail());
+
+  auto port = ((TCPSocket *)server_socket)->GetLocalPortNumber();
+  auto child_pid = fork();
+  if (child_pid != 0) {
+    RNBSocket client_socket;
+    auto result = client_socket.Connect(addr, port);
+    ASSERT_TRUE(result == rnb_success);
+    result = client_socket.Write(hello.c_str(), hello.length());
+    ASSERT_TRUE(result == rnb_success);
+    std::string bye;
+    result = client_socket.Read(bye);
+    ASSERT_TRUE(result == rnb_success);
+    ASSERT_EQ(bye, goodbye);
+  } else {
+    Socket *connected_socket;
+    err = server_socket->Accept(addr_wrap, false, connected_socket);
+    if (err.Fail()) {
+      llvm::errs() << err.AsCString();
+      abort();
+    }
+    char buffer[32];
+    size_t read_size = 32;
+    err = connected_socket->Read((void *)&buffer[0], read_size);
+    if (err.Fail()) {
+      llvm::errs() << err.AsCString();
+      abort();
+    }
+    std::string Recv(&buffer[0], read_size);
+    if (Recv != hello) {
+      llvm::errs() << err.AsCString();
+      abort();
+    }
+    size_t write_size = goodbye.length();
+    err = connected_socket->Write(goodbye.c_str(), write_size);
+    if (err.Fail()) {
+      llvm::errs() << err.AsCString();
+      abort();
+    }
+    if (write_size != goodbye.length()) {
+      llvm::errs() << err.AsCString();
+      abort();
+    }
+    exit(0);
+  }
+  int exit_status;
+  wait(&exit_status);
+  ASSERT_EQ(exit_status, 0);
+}
+
+TEST(RNBSocket, LoopBackConnectIPv4) { TestSocketConnect("127.0.0.1"); }

lldb/trunk/unittests/debugserver/debugserver_LogCallback.cpp

+//===-- debugserver_LogCallback.cpp -----------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+//------------------------------------------------------------------------------
+// this function is defined in debugserver.cpp, but is needed to link the
+// debugserver Common library. It is for logging only, so it is left
+// unimplemented here.
+//------------------------------------------------------------------------------
+
+#include <stdint.h>
+#include <stdarg.h>
+
+void FileLogCallback(void *baton, uint32_t flags, const char *format,
+                     va_list args) {}