[Lldb-commits] [PATCH] D41245: Reduce x86 register context boilerplate.

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320966: Reduce x86 register context boilerplate. (authored 
by labath, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D41245

Files:
  lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
  lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
  lldb/trunk/source/Plugins/Process/Utility/RegisterContext_x86.h
  lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h
  lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h

Index: lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
===
--- lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
+++ lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h
@@ -16,25 +16,25 @@
 // Computes the offset of the given FPR in the extended data area.
 #define FPR_OFFSET(regname)\
   (LLVM_EXTENSION offsetof(UserArea, fpr) +\
-   LLVM_EXTENSION offsetof(FPR, xstate) +  \
+   LLVM_EXTENSION offsetof(FPR, fxsave) +  \
LLVM_EXTENSION offsetof(FXSAVE, regname))
 
 // Computes the offset of the YMM register assembled from register halves.
 // Based on DNBArchImplX86_64.cpp from debugserver
 #define YMM_OFFSET(reg_index)  \
   (LLVM_EXTENSION offsetof(UserArea, fpr) +\
-   LLVM_EXTENSION offsetof(FPR, xstate) +  \
+   LLVM_EXTENSION offsetof(FPR, xsave) +   \
LLVM_EXTENSION offsetof(XSAVE, ymmh[0]) + (32 * reg_index))
 
-#define BNDR_OFFSET(reg_index) \
-(LLVM_EXTENSION offsetof(UserArea, fpr) + \
- LLVM_EXTENSION offsetof(FPR, xstate) + \
- LLVM_EXTENSION offsetof(XSAVE, mpxr[reg_index]))
-
-#define BNDC_OFFSET(reg_index) \
-(LLVM_EXTENSION offsetof(UserArea, fpr) + \
- LLVM_EXTENSION offsetof(FPR, xstate) + \
- LLVM_EXTENSION offsetof(XSAVE, mpxc[reg_index]))
+#define BNDR_OFFSET(reg_index) \
+  (LLVM_EXTENSION offsetof(UserArea, fpr) +\
+   LLVM_EXTENSION offsetof(FPR, xsave) +   \
+   LLVM_EXTENSION offsetof(XSAVE, mpxr[reg_index]))
+
+#define BNDC_OFFSET(reg_index) \
+  (LLVM_EXTENSION offsetof(UserArea, fpr) +\
+   LLVM_EXTENSION offsetof(FPR, xsave) +   \
+   LLVM_EXTENSION offsetof(XSAVE, mpxc[reg_index]))
 
 #ifdef DECLARE_REGISTER_INFOS_X86_64_STRUCT
 
Index: lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h
===
--- lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h
+++ lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h
@@ -27,19 +27,19 @@
 // Based on DNBArchImplI386.cpp from debugserver
 #define YMM_OFFSET(reg_index)  \
   (LLVM_EXTENSION offsetof(UserArea, i387) +   \
-   LLVM_EXTENSION offsetof(FPR, xstate) +  \
+   LLVM_EXTENSION offsetof(FPR, fxsave) +  \
LLVM_EXTENSION offsetof(FXSAVE, xmm[7]) + sizeof(XMMReg) +  \
(32 * reg_index))
 
-#define BNDR_OFFSET(reg_index) \
-(LLVM_EXTENSION offsetof(UserArea, i387) + \
- LLVM_EXTENSION offsetof(FPR, xstate) + \
- LLVM_EXTENSION offsetof(XSAVE, mpxr[reg_index]))
-
-#define BNDC_OFFSET(reg_index) \
-(LLVM_EXTENSION offsetof(UserArea, i387) + \
- LLVM_EXTENSION offsetof(FPR, xstate) + \
- LLVM_EXTENSION offsetof(XSAVE, mpxc[reg_index]))
+#define BNDR_OFFSET(reg_index) \
+  (LLVM_EXTENSION offsetof(UserArea, i387) +   \
+   LLVM_EXTENSION offsetof(FPR, xsave) +   \
+   LLVM_EXTENSION offsetof(XSAVE, mpxr[reg_index]))
+
+#define BNDC_OFFSET(reg_index) \
+  (LLVM_EXTENSION offsetof(UserArea, i387) +   \
+   LLVM_EXTENSION offsetof(FPR, xsave) +   \
+   LLVM_EXTENSION offsetof(XSAVE, mpxc[reg_index]))
 
 // Number of bytes needed to represent a FPR.
 #if !defined(FPR_SIZE)
Index: lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
===
--- lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
+++ 

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

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:3496
+
+  auto Decompressor = llvm::object::Decompressor::create(
+  section->GetName().GetStringRef(),

tzik wrote:
> This adds new dependency to LLVM Object component.
> Could you add it into LINK_COMPONENTS section of CMakeLists.txt in this 
> directory?
Done in r320967. Thanks for pointing this out.


Repository:
  rL LLVM

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] D41069: NPL: Clean up handling of inferior exit

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320961: NPL: Clean up handling of inferior exit (authored by 
labath, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D41069

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

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
@@ -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: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -825,6 +825,7 @@
 
   // We are ready to exit the debug monitor.
   m_exit_now = true;
+  m_mainloop.RequestTermination();
 }
 
 void GDBRemoteCommunicationServerLLGS::HandleInferiorState_Stopped(
Index: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
===
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
+++ lldb/trunk/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 

[Lldb-commits] [lldb] r320967 - Add LLVMObject dependency to our ObjectFileELF plugin

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Dec 18 02:51:03 2017
New Revision: 320967

URL: http://llvm.org/viewvc/llvm-project?rev=320967=rev
Log:
Add LLVMObject dependency to our ObjectFileELF plugin

Modified:
lldb/trunk/source/Plugins/ObjectFile/ELF/CMakeLists.txt

Modified: lldb/trunk/source/Plugins/ObjectFile/ELF/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/ELF/CMakeLists.txt?rev=320967=320966=320967=diff
==
--- lldb/trunk/source/Plugins/ObjectFile/ELF/CMakeLists.txt (original)
+++ lldb/trunk/source/Plugins/ObjectFile/ELF/CMakeLists.txt Mon Dec 18 02:51:03 
2017
@@ -9,5 +9,6 @@ add_lldb_library(lldbPluginObjectFileELF
 lldbTarget
   LINK_COMPONENTS
 BinaryFormat
+Object
 Support
   )


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

[Lldb-commits] [lldb] r320966 - Reduce x86 register context boilerplate.

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Dec 18 02:50:59 2017
New Revision: 320966

URL: http://llvm.org/viewvc/llvm-project?rev=320966=rev
Log:
Reduce x86 register context boilerplate.

Summary:
The x86 FPR struct was defined as a struct containing a union between
two members: XSAVE and FXSAVE. This patch makes FPR a union directly to
remove one layer of indirection when trying to access the members.

The initial layout of these two structs is identical, which is
recognised by the fact that XSAVE has FXSAVE as its first member, so we
also considered removing one more layer and leave FPR identical to XSAVE
struct, but stopped short of doing that, as the FPR may be used to store
different layouts in the future (e.g., ones generated by the FSAVE
instruction).

Reviewers: clayborg, krytarowski

Subscribers: emaste, lldb-commits

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

Modified:

lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContextPOSIX_x86.cpp
lldb/trunk/source/Plugins/Process/Utility/RegisterContext_x86.h
lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_i386.h
lldb/trunk/source/Plugins/Process/Utility/RegisterInfos_x86_64.h

Modified: 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp?rev=320966=320965=320966=diff
==
--- 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/Linux/NativeRegisterContextLinux_x86_64.cpp 
Mon Dec 18 02:50:59 2017
@@ -332,11 +332,11 @@ NativeRegisterContextLinux_x86_64::Nativ
   // Initialize m_iovec to point to the buffer and buffer size
   // using the conventions of Berkeley style UIO structures, as required
   // by PTRACE extensions.
-  m_iovec.iov_base = _fpr.xstate.xsave;
-  m_iovec.iov_len = sizeof(m_fpr.xstate.xsave);
+  m_iovec.iov_base = _fpr;
+  m_iovec.iov_len = sizeof(m_fpr);
 
   // Clear out the FPR state.
-  ::memset(_fpr, 0, sizeof(FPR));
+  ::memset(_fpr, 0, sizeof(m_fpr));
 
   // Store byte offset of fctrl (i.e. first register of FPR)
   const RegisterInfo *reg_info_fctrl = GetRegisterInfoByName("fctrl");
@@ -439,17 +439,14 @@ NativeRegisterContextLinux_x86_64::ReadR
 
 if (byte_order != lldb::eByteOrderInvalid) {
   if (reg >= m_reg_info.first_st && reg <= m_reg_info.last_st)
-reg_value.SetBytes(
-m_fpr.xstate.fxsave.stmm[reg - m_reg_info.first_st].bytes,
-reg_info->byte_size, byte_order);
+reg_value.SetBytes(m_fpr.fxsave.stmm[reg - m_reg_info.first_st].bytes,
+   reg_info->byte_size, byte_order);
   if (reg >= m_reg_info.first_mm && reg <= m_reg_info.last_mm)
-reg_value.SetBytes(
-m_fpr.xstate.fxsave.stmm[reg - m_reg_info.first_mm].bytes,
-reg_info->byte_size, byte_order);
+reg_value.SetBytes(m_fpr.fxsave.stmm[reg - m_reg_info.first_mm].bytes,
+   reg_info->byte_size, byte_order);
   if (reg >= m_reg_info.first_xmm && reg <= m_reg_info.last_xmm)
-reg_value.SetBytes(
-m_fpr.xstate.fxsave.xmm[reg - m_reg_info.first_xmm].bytes,
-reg_info->byte_size, byte_order);
+reg_value.SetBytes(m_fpr.fxsave.xmm[reg - m_reg_info.first_xmm].bytes,
+   reg_info->byte_size, byte_order);
   if (reg >= m_reg_info.first_ymm && reg <= m_reg_info.last_ymm) {
 // Concatenate ymm using the register halves in xmm.bytes and 
ymmh.bytes
 if (CopyXSTATEtoYMM(reg, byte_order))
@@ -490,7 +487,7 @@ NativeRegisterContextLinux_x86_64::ReadR
 return error;
   }
 
-  // Get pointer to m_fpr.xstate.fxsave variable and set the data from it.
+  // Get pointer to m_fpr.fxsave variable and set the data from it.
 
   // Byte offsets of all registers are calculated wrt 'UserArea' structure.
   // However, ReadFPR() reads fpu registers {using ptrace(PTRACE_GETFPREGS,..)}
@@ -530,7 +527,7 @@ NativeRegisterContextLinux_x86_64::ReadR
 
 void NativeRegisterContextLinux_x86_64::UpdateXSTATEforWrite(
 uint32_t reg_index) {
-  XSAVE_HDR::XFeature _bv = m_fpr.xstate.xsave.header.xstate_bv;
+  XSAVE_HDR::XFeature _bv = m_fpr.xsave.header.xstate_bv;
   if (IsFPR(reg_index)) {
 // IsFPR considers both %st and %xmm registers as floating point, but these
 // map to two features. Set both flags, just in case.
@@ -562,19 +559,16 @@ Status NativeRegisterContextLinux_x86_64
   if (IsFPR(reg_index) || IsAVX(reg_index) || IsMPX(reg_index)) {
 if (reg_info->encoding == lldb::eEncodingVector) {
   if (reg_index >= m_reg_info.first_st && reg_index <= m_reg_info.last_st)
-::memcpy(
-m_fpr.xstate.fxsave.stmm[reg_index - m_reg_info.first_st].bytes,
-

[Lldb-commits] [lldb] r320961 - NPL: Clean up handling of inferior exit

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Dec 18 01:44:29 2017
New Revision: 320961

URL: http://llvm.org/viewvc/llvm-project?rev=320961=rev
Log:
NPL: Clean up handling of inferior exit

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

Reviewers: eugene

Subscribers: lldb-commits

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

Modified:
lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp

Modified: lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp?rev=320961=320960=320961=diff
==
--- lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp (original)
+++ lldb/trunk/source/Plugins/Process/Linux/NativeProcessLinux.cpp Mon Dec 18 
01:44:29 2017
@@ -412,46 +412,20 @@ void NativeProcessLinux::MonitorCallback
 
   // 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 @@ void NativeProcessLinux::MonitorSIGTRAP(
   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 want to implement "break on thread 

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

[Pavel Labath via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL320984: llgs: Propagate the environment when launching the 
inferior from command line (authored by labath, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D41070?vs=126367=127349#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D41070

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

Index: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
@@ -43,32 +43,7 @@
   MainLoop ,
   const NativeProcessProtocol::Factory _factory);
 
-  //--
-  /// 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 );
 
   //--
   /// Launch a process with the current launch settings.
Index: lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
===
--- lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
@@ -204,21 +204,8 @@
 });
 }
 
-Status
-GDBRemoteCommunicationServerLLGS::SetLaunchArguments(const char *const args[],
- int argc) {
-  if ((argc < 1) || !args || !args[0] || !args[0][0])
-return Status("%s: no process command line specified to launch",
-  __FUNCTION__);
-
-  m_process_launch_info.SetArguments(const_cast(args), true);
-  return Status();
-}
-
-Status
-GDBRemoteCommunicationServerLLGS::SetLaunchFlags(unsigned int launch_flags) {
-  m_process_launch_info.GetFlags().Set(launch_flags);
-  return Status();
+void GDBRemoteCommunicationServerLLGS::SetLaunchInfo(const ProcessLaunchInfo ) {
+  m_process_launch_info = info;
 }
 
 Status GDBRemoteCommunicationServerLLGS::LaunchProcess() {
Index: lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
===
--- lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
+++ lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
@@ -177,27 +177,28 @@
 
 void handle_launch(GDBRemoteCommunicationServerLLGS _server, int argc,
const char *const argv[]) {
-  Status error;
-  error = gdb_server.SetLaunchArguments(argv, argc);
-  if (error.Fail()) {
-fprintf(stderr, "error: failed to set launch args for '%s': %s\n", argv[0],
-error.AsCString());
+  ProcessLaunchInfo info;
+  info.GetFlags().Set(eLaunchFlagStopAtEntry | eLaunchFlagDebug |
+  eLaunchFlagDisableASLR);
+  info.SetArguments(const_cast(argv), true);
+
+  llvm::SmallString<64> cwd;
+  if (std::error_code ec = llvm::sys::fs::current_path(cwd)) {
+llvm::errs() << "Error getting current directory: " << ec.message() << "\n";
 exit(1);
   }
+  info.SetWorkingDirectory(FileSpec(cwd, true));
 
-  unsigned int launch_flags = eLaunchFlagStopAtEntry | eLaunchFlagDebug;
+  StringList env;
+  Host::GetEnvironment(env);
+  info.GetEnvironmentEntries() = Args(env);
 
-  error = gdb_server.SetLaunchFlags(launch_flags);
-  if (error.Fail()) {
-fprintf(stderr, "error: failed to set launch flags for '%s': %s\n", argv[0],
-   

[Lldb-commits] [lldb] r320985 - Fix regression in jModulesInfo packet handling

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Dec 18 06:31:44 2017
New Revision: 320985

URL: http://llvm.org/viewvc/llvm-project?rev=320985=rev
Log:
Fix regression in jModulesInfo packet handling

The recent UUID cleanups exposed a bug in the parsing code for the
jModulesInfo response, which was passing wrong value for the second
argument to UUID::SetFromStringRef (it passed the length of the string,
whereas the correct value should be the number of decoded bytes we
expect to receive).

This was not picked up by tests, because they test with 16-byte uuids,
for which the function happens to do the right thing even if the length
does not match (if the length does not match, the function does not
update m_num_uuid_bytes member, but that member is already 16 to begin
with).

I fix that and add a test with 20-byte uuid to catch if this regresses.
I have also added more safeguards into the parsing code to fail if we
cannot parse the entire uuid field we recieve. While testing the latter
part, I noticed that the "negative" jModulesInfo tests were succeeding
because we were sending malformed json (and not because the json
contents was invalid), so I make those tests a bit more robuts as well.

Modified:

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp?rev=320985=320984=320985=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
(original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp 
Mon Dec 18 06:31:44 2017
@@ -3442,7 +3442,9 @@ ParseModuleSpec(StructuredData::Dictiona
 
   if (!dict->GetValueForKeyAsString("uuid", string))
 return llvm::None;
-  result.GetUUID().SetFromStringRef(string, string.size());
+  if (result.GetUUID().SetFromStringRef(string, string.size() / 2) !=
+  string.size())
+return llvm::None;
 
   if (!dict->GetValueForKeyAsInteger("file_offset", integer))
 return llvm::None;

Modified: 
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp?rev=320985=320984=320985=diff
==
--- 
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp 
(original)
+++ 
lldb/trunk/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp 
Mon Dec 18 06:31:44 2017
@@ -197,27 +197,53 @@ TEST_F(GDBRemoteCommunicationClientTest,
   EXPECT_EQ(1234u, result.getValue()[0].GetObjectSize());
 }
 
+TEST_F(GDBRemoteCommunicationClientTest, GetModulesInfo_UUID20) {
+  llvm::Triple triple("i386-pc-linux");
+
+  FileSpec file_spec("/foo/bar.so", false, FileSpec::ePathSyntaxPosix);
+  std::future> async_result =
+  std::async(std::launch::async,
+ [&] { return client.GetModulesInfo(file_spec, triple); });
+  HandlePacket(
+  server,
+  "jModulesInfo:["
+  R"({"file":"/foo/bar.so","triple":"i386-pc-linux"}])",
+  
R"([{"uuid":"404142434445464748494a4b4c4d4e4f50515253","triple":"i386-pc-linux",)"
+  R"("file_path":"/foo/bar.so","file_offset":0,"file_size":1234}]])");
+
+  auto result = async_result.get();
+  ASSERT_TRUE(result.hasValue());
+  ASSERT_EQ(1u, result->size());
+  EXPECT_EQ("/foo/bar.so", result.getValue()[0].GetFileSpec().GetPath());
+  EXPECT_EQ(triple, result.getValue()[0].GetArchitecture().GetTriple());
+  EXPECT_EQ(UUID("@ABCDEFGHIJKLMNOPQRS", 20), result.getValue()[0].GetUUID());
+  EXPECT_EQ(0u, result.getValue()[0].GetObjectOffset());
+  EXPECT_EQ(1234u, result.getValue()[0].GetObjectSize());
+}
+
 TEST_F(GDBRemoteCommunicationClientTest, GetModulesInfoInvalidResponse) {
   llvm::Triple triple("i386-pc-linux");
   FileSpec file_spec("/foo/bar.so", false, FileSpec::ePathSyntaxPosix);
 
   const char *invalid_responses[] = {
-  "OK", "E47", "[]",
   // no UUID
   R"([{"triple":"i386-pc-linux",)"
-  R"("file_path":"/foo/bar.so","file_offset":0,"file_size":1234}])",
+  R"("file_path":"/foo/bar.so","file_offset":0,"file_size":1234}]])",
+  // invalid UUID
+  R"([{"uuid":"XX","triple":"i386-pc-linux",)"
+  R"("file_path":"/foo/bar.so","file_offset":0,"file_size":1234}]])",
   // no triple
   R"([{"uuid":"404142434445464748494a4b4c4d4e4f",)"
-  R"("file_path":"/foo/bar.so","file_offset":0,"file_size":1234}])",
+  R"("file_path":"/foo/bar.so","file_offset":0,"file_size":1234}]])",
   // no file_path
   
R"([{"uuid":"404142434445464748494a4b4c4d4e4f","triple":"i386-pc-linux",)"
-  

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

[Pavel Labath via lldb-commits]

Author: labath
Date: Mon Dec 18 06:31:39 2017
New Revision: 320984

URL: http://llvm.org/viewvc/llvm-project?rev=320984=rev
Log:
llgs: Propagate the environment when launching the inferior from command line

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

Reviewers: clayborg, eugene

Subscribers: lldb-commits, mgorny

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

Added:
lldb/trunk/unittests/tools/lldb-server/inferior/environment_check.cpp
lldb/trunk/unittests/tools/lldb-server/tests/LLGSTest.cpp
Modified:

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

lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
lldb/trunk/unittests/tools/lldb-server/CMakeLists.txt
lldb/trunk/unittests/tools/lldb-server/tests/CMakeLists.txt
lldb/trunk/unittests/tools/lldb-server/tests/TestClient.cpp

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp?rev=320984=320983=320984=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
 (original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp
 Mon Dec 18 06:31:39 2017
@@ -204,21 +204,8 @@ void GDBRemoteCommunicationServerLLGS::R
 });
 }
 
-Status
-GDBRemoteCommunicationServerLLGS::SetLaunchArguments(const char *const args[],
- int argc) {
-  if ((argc < 1) || !args || !args[0] || !args[0][0])
-return Status("%s: no process command line specified to launch",
-  __FUNCTION__);
-
-  m_process_launch_info.SetArguments(const_cast(args), true);
-  return Status();
-}
-
-Status
-GDBRemoteCommunicationServerLLGS::SetLaunchFlags(unsigned int launch_flags) {
-  m_process_launch_info.GetFlags().Set(launch_flags);
-  return Status();
+void GDBRemoteCommunicationServerLLGS::SetLaunchInfo(const ProcessLaunchInfo 
) {
+  m_process_launch_info = info;
 }
 
 Status GDBRemoteCommunicationServerLLGS::LaunchProcess() {

Modified: 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h?rev=320984=320983=320984=diff
==
--- 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h 
(original)
+++ 
lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.h 
Mon Dec 18 06:31:39 2017
@@ -43,32 +43,7 @@ public:
   MainLoop ,
   const NativeProcessProtocol::Factory _factory);
 
-  //--
-  /// 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 );
 
   //--
   /// Launch a process with the current launch settings.

Modified: lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/lldb-server/lldb-gdbserver.cpp?rev=320984=320983=320984=diff

[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)

[Pavel Labath via Phabricator via lldb-commits]

labath created this revision.
labath added reviewers: jasonmolenda, clayborg.
Herald added a subscriber: JDevlieghere.

Make sure we propagate environment when starting debugserver with a pre-loaded
inferior. AFAIK, RNBRunLoopLaunchInferior is only called in pre-loaded inferior
scenario, so we can just pick up the debugserver environment instead of trying
to construct an envp from the (empty) context.

This makes debugserver pass an test added for an equivalent lldb-server fix.


https://reviews.llvm.org/D41352

Files:
  tools/debugserver/source/debugserver.cpp
  unittests/tools/lldb-server/tests/LLGSTest.cpp


Index: unittests/tools/lldb-server/tests/LLGSTest.cpp
===
--- unittests/tools/lldb-server/tests/LLGSTest.cpp
+++ unittests/tools/lldb-server/tests/LLGSTest.cpp
@@ -16,11 +16,6 @@
 using namespace llvm;
 
 TEST_F(TestBase, LaunchModePreservesEnvironment) {
-  if (TestClient::IsDebugServer()) {
-GTEST_LOG_(WARNING) << "Test fails with debugserver: llvm.org/pr35671";
-return;
-  }
-
   putenv(const_cast("LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE"));
 
   auto ClientOr = TestClient::launch(getLogFileName(),
Index: tools/debugserver/source/debugserver.cpp
===
--- tools/debugserver/source/debugserver.cpp
+++ tools/debugserver/source/debugserver.cpp
@@ -190,15 +190,6 @@
   for (i = 0; i < inferior_argc; i++)
 inferior_argv[i] = ctx.ArgumentAtIndex(i);
 
-  // Pass the environment array the same way:
-
-  size_t inferior_envc = ctx.EnvironmentCount();
-  // Initialize inferior_argv with inferior_argc + 1 NULLs
-  std::vector inferior_envp(inferior_envc + 1, NULL);
-
-  for (i = 0; i < inferior_envc; i++)
-inferior_envp[i] = ctx.EnvironmentAtIndex(i);
-
   // Our launch type hasn't been set to anything concrete, so we need to
   // figure our how we are going to launch automatically.
 
@@ -241,7 +232,7 @@
: ctx.GetWorkingDirectory());
   const char *process_event = ctx.GetProcessEvent();
   nub_process_t pid = DNBProcessLaunch(
-  resolved_path, _argv[0], _envp[0], cwd, stdin_path,
+  resolved_path, _argv[0], const_cast(*_NSGetEnviron()), cwd, stdin_path,
   stdout_path, stderr_path, no_stdio, launch_flavor, g_disable_aslr,
   process_event, launch_err_str, sizeof(launch_err_str));
 


Index: unittests/tools/lldb-server/tests/LLGSTest.cpp
===
--- unittests/tools/lldb-server/tests/LLGSTest.cpp
+++ unittests/tools/lldb-server/tests/LLGSTest.cpp
@@ -16,11 +16,6 @@
 using namespace llvm;
 
 TEST_F(TestBase, LaunchModePreservesEnvironment) {
-  if (TestClient::IsDebugServer()) {
-GTEST_LOG_(WARNING) << "Test fails with debugserver: llvm.org/pr35671";
-return;
-  }
-
   putenv(const_cast("LLDB_TEST_MAGIC_VARIABLE=LLDB_TEST_MAGIC_VALUE"));
 
   auto ClientOr = TestClient::launch(getLogFileName(),
Index: tools/debugserver/source/debugserver.cpp
===
--- tools/debugserver/source/debugserver.cpp
+++ tools/debugserver/source/debugserver.cpp
@@ -190,15 +190,6 @@
   for (i = 0; i < inferior_argc; i++)
 inferior_argv[i] = ctx.ArgumentAtIndex(i);
 
-  // Pass the environment array the same way:
-
-  size_t inferior_envc = ctx.EnvironmentCount();
-  // Initialize inferior_argv with inferior_argc + 1 NULLs
-  std::vector inferior_envp(inferior_envc + 1, NULL);
-
-  for (i = 0; i < inferior_envc; i++)
-inferior_envp[i] = ctx.EnvironmentAtIndex(i);
-
   // Our launch type hasn't been set to anything concrete, so we need to
   // figure our how we are going to launch automatically.
 
@@ -241,7 +232,7 @@
: ctx.GetWorkingDirectory());
   const char *process_event = ctx.GetProcessEvent();
   nub_process_t pid = DNBProcessLaunch(
-  resolved_path, _argv[0], _envp[0], cwd, stdin_path,
+  resolved_path, _argv[0], const_cast(*_NSGetEnviron()), cwd, stdin_path,
   stdout_path, stderr_path, no_stdio, launch_flavor, g_disable_aslr,
   process_event, launch_err_str, sizeof(launch_err_str));
 
___
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
   // try really hard not to use a regex match.
-  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();
-  }
-  if (is_regex)
-FindTypesByRegex(name_str, max_matches, types);
+  if (name_str.find_first_of("[]?*.-+\\") != std::string::npos)
+FindTypesByRegex(RegularExpression(name_str), max_matches, types);

clayborg wrote:
> I would rather not sniff the string passed in to see if this could be a regex 
> and add a new FindTypesByRegex() call to lldb_private::SymbolFile that anyone 
> can use. Do we have PDB tests that are using this functionality?
The error can be reproduced with:

image lookup -t char*


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] D41359: Add Utility/Environment class for handling... environments

[Zachary Turner via Phabricator via lldb-commits]

zturner added inline comments.



Comment at: include/lldb/Utility/Environment.h:70-72
+  std::pair insert(llvm::StringRef KeyEqValue) {
+return insert(KeyEqValue.split('='));
+  }

Why'd you decide to go with inserting an entire pre-formatted key value pair in 
the form `{key}={value}`, instead of having the function be `insert(StringRef 
Key, StringRef Value)`?




Comment at: source/Host/macosx/Host.mm:763
 
-proc_env.AppendArgument(llvm::StringRef(cstr));
+   proc_env.insert(cstr);
   }

Indentation is messed up here.


https://reviews.llvm.org/D41359



___
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 inline comments.



Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:393
   // try really hard not to use a regex match.
-  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();
-  }
-  if (is_regex)
-FindTypesByRegex(name_str, max_matches, types);
+  if (name_str.find_first_of("[]?*.-+\\") != std::string::npos)
+FindTypesByRegex(RegularExpression(name_str), max_matches, types);

I would rather not sniff the string passed in to see if this could be a regex 
and add a new FindTypesByRegex() call to lldb_private::SymbolFile that anyone 
can use. Do we have PDB tests that are using this functionality?


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] D41352: debugserver: Propagate environment in launch-mode (pr35671)

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In https://reviews.llvm.org/D41352#958549, @clayborg wrote:

> We already have an option for this named "--forward-env". It might be better 
> to fix this by adding the "--forward-env" argument to the debugserver launch 
> during testing? When we launch through LLDB, it forwards all environment 
> variables manually. Maybe we add a "--forward-env" option to lldb-server too? 
> Not sure what the right thing to do here is. iOS, tvOS and watchOS won't be 
> affected since we never launch directly and all env vars are manually set by 
> LLDB. Jason, any comments on this one?


Ah, I haven't noticed that. I can definitely add the `--forward-env` to the 
test if you that's the behavior you want. However, I think that the default 
behavior of inheriting the environment for the "launch" mode makes more sense.


https://reviews.llvm.org/D41352



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

[Lldb-commits] [lldb] r321016 - Fix more inconsistent line endings. NFC.

[Dimitry Andric via lldb-commits]

Author: dim
Date: Mon Dec 18 11:46:56 2017
New Revision: 321016

URL: http://llvm.org/viewvc/llvm-project?rev=321016=rev
Log:
Fix more inconsistent line endings. NFC.

Modified:
lldb/trunk/www/build.html
lldb/trunk/www/test.html

Modified: lldb/trunk/www/build.html
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/www/build.html?rev=321016=321015=321016=diff
==
--- lldb/trunk/www/build.html (original)
+++ lldb/trunk/www/build.html Mon Dec 18 11:46:56 2017
@@ -123,20 +123,20 @@
   
 
 Sample command line:
-cmake -G Ninja -DLLDB_TEST_DEBUG_TEST_CRASHES=1 
-DPYTHON_HOME=C:\Python35 
-DLLDB_TEST_COMPILER=d:\src\llvmbuild\ninja_release\bin\clang.exe 
..\..\llvm
-Working with both Ninja and MSVC
-
-  Compiling with ninja is both faster and simpler 
than compiling with MSVC, but chances are you still want
-  to debug LLDB with MSVC (at least until we can debug LLDB on 
Windows with LLDB!).  One solution to this is to run 
-  cmake twice and generate the output into two 
different folders.  One for compiling (the ninja
-  folder), and one for editing / browsing / debugging (the MSVC 
folder).
-
-
-  To do this, simply run `cmake -G Ninja 
arguments` from one folder, and 
-  `cmake -G "Visual Studio 14 2015" 
arguments` in another folder.  Then you can open the .sln file
-  in Visual Studio, set lldb as the startup project, 
and use F5 to run it.  You need only edit the project
-  settings to set the executable and the working directory to 
point to binaries inside of the ninja tree.
-
+cmake -G Ninja -DLLDB_TEST_DEBUG_TEST_CRASHES=1 
-DPYTHON_HOME=C:\Python35 
-DLLDB_TEST_COMPILER=d:\src\llvmbuild\ninja_release\bin\clang.exe 
..\..\llvm
+Working with both Ninja and MSVC
+
+  Compiling with ninja is both faster and simpler 
than compiling with MSVC, but chances are you still want
+  to debug LLDB with MSVC (at least until we can debug LLDB on 
Windows with LLDB!).  One solution to this is to run 
+  cmake twice and generate the output into two 
different folders.  One for compiling (the ninja
+  folder), and one for editing / browsing / debugging (the MSVC 
folder).
+
+
+  To do this, simply run `cmake -G Ninja 
arguments` from one folder, and 
+  `cmake -G "Visual Studio 14 2015" 
arguments` in another folder.  Then you can open the .sln file
+  in Visual Studio, set lldb as the startup project, 
and use F5 to run it.  You need only edit the project
+  settings to set the executable and the working directory to 
point to binaries inside of the ninja tree.
+
   
 
 

Modified: lldb/trunk/www/test.html
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/www/test.html?rev=321016=321015=321016=diff
==
--- lldb/trunk/www/test.html (original)
+++ lldb/trunk/www/test.html Mon Dec 18 11:46:56 2017
@@ -27,13 +27,13 @@
   both the LLDB command line interface and the scripting API.
 
   
-  Running tests
+  Running tests
   
 Running the full test suite
 
-  Windows Note: In the examples that follow, any 
invocations of python
-  should be replaced with python_d, the debug 
interpreter, when running the test
-  suite against a debug version of LLDB.
+  Windows Note: In the examples that follow, any 
invocations of python
+  should be replaced with python_d, the debug 
interpreter, when running the test
+  suite against a debug version of LLDB.
 
 
   The easiest way to run the LLDB test suite is to use the 
check-lldb build
@@ -134,93 +134,93 @@
   Debugging test failures
   
 Non-Windows platforms
-
-  On non-Windows platforms, you can use the -d option 
to dotest.py which will cause the script to wait
+
+  On non-Windows platforms, you can use the -d option 
to dotest.py which will cause the script to wait
   for a while until a debugger is attached.
 
-Windows
+Windows
 
-  On Windows, it is strongly recommended to use https://github.com/Microsoft/PTVS/releases;>Python Tools for Visual 
Studio
-  for debugging test failures.  It can seamlessly step between 
native and managed code, which is very helpful when you need to step
-  through the test itself, and then into the LLDB code that backs 
the operations the test is 

[Lldb-commits] [PATCH] D41359: Add Utility/Environment class for handling... environments

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.



Comment at: include/lldb/Target/Platform.h:636
 
-  virtual size_t GetEnvironment(StringList );
+  virtual Environment GetEnvironment();
 

clayborg wrote:
> Do we want to return by value? Not a const reference?
This object can come from the Host::GetEnvironment, which constructs it 
on-demand. Returning a reference would mean we need to cache a value in the 
host layer, but that's tricky as the environment can change during runtime, and 
you want to make sure you always return an up-to-date value.

I don't think this is an issue, as all the returns will be moves, so the 
situation will be the same as before this patch (where we also created a copy 
of the environment in the by-ref argument). 



Comment at: include/lldb/Target/Target.h:118-119
 
-  size_t GetEnvironmentAsArgs(Args ) const;
-
-  void SetEnvironmentFromArgs(const Args );
+  Environment GetEnvironment() const;
+  void SetEnvironment(Environment env);
 

clayborg wrote:
> set/get using reference instead of by value?
There is no persistent object object to back the reference for the getter (it's 
constructed from an OptionValueDictionary). Using a value for the setter is 
actually better, as that allows me to move-assign the environment object in the 
LaunchInfo. If the user calls this with a moved object then we get no copies.



Comment at: source/API/SBLaunchInfo.cpp:34
+private:
+  Environment::Envp m_envp;
+};

clayborg wrote:
> Seems like a lot of work to get indexed environment variable access. Should 
> we not make the Environment class use a std::vector instead of a map? Not 
> sure if environment variable order is ever important? Is so then the 
> StringMap isn't what we want to use. I know we have it in our public API so 
> we can't change it now and thus why we need this work around. Just thinking 
> out loud
I was thinking about the importance of variable order as well. I can certainly 
construct an application that will behave differently depending on the order of 
environment variables it is given. However, something like that seems unlikely 
to happen for real.

I actually started working on an order-preserving implementation, but then 
dropped it when I saw that we are already shoving the variables through a 
std::map in the OptionValueDictionary in target.env-vars setting. If we manage 
to drop that (OptionValueEnvironment?), then I think it would make sense to 
swap out this implementation for an order-preserving one. I would still keep 
the present map-like interface, as we do key-based lookup in quite a lot of 
places.


https://reviews.llvm.org/D41359



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

[Lldb-commits] [PATCH] D41359: Add Utility/Environment class for handling... environments

[Greg Clayton via Phabricator via lldb-commits]

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

ok as long as we don't want to return const reference when returning 
Environment values in getters and setters.




Comment at: include/lldb/Target/Platform.h:636
 
-  virtual size_t GetEnvironment(StringList );
+  virtual Environment GetEnvironment();
 

Do we want to return by value? Not a const reference?



Comment at: include/lldb/Target/Target.h:118-119
 
-  size_t GetEnvironmentAsArgs(Args ) const;
-
-  void SetEnvironmentFromArgs(const Args );
+  Environment GetEnvironment() const;
+  void SetEnvironment(Environment env);
 

set/get using reference instead of by value?



Comment at: source/API/SBLaunchInfo.cpp:34
+private:
+  Environment::Envp m_envp;
+};

Seems like a lot of work to get indexed environment variable access. Should we 
not make the Environment class use a std::vector instead of a map? Not sure if 
environment variable order is ever important? Is so then the StringMap isn't 
what we want to use. I know we have it in our public API so we can't change it 
now and thus why we need this work around. Just thinking out loud


https://reviews.llvm.org/D41359



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

[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

We already have an option for this named "--forward-env". It might be better to 
fix this by adding the "--forward-env" argument to the debugserver launch 
during testing? When we launch through LLDB, it forwards all environment 
variables manually. Maybe we add a "--forward-env" option to lldb-server too? 
Not sure what the right thing to do here is. iOS, tvOS and watchOS won't be 
affected since we never launch directly and all env vars are manually set by 
LLDB. Jason, any comments on this one?


https://reviews.llvm.org/D41352



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

[Lldb-commits] [PATCH] D41359: Add Utility/Environment class for handling... environments

[Pavel Labath via Phabricator via lldb-commits]

labath marked an inline comment as done.
labath added inline comments.



Comment at: include/lldb/Utility/Environment.h:70-72
+  std::pair insert(llvm::StringRef KeyEqValue) {
+return insert(KeyEqValue.split('='));
+  }

zturner wrote:
> Why'd you decide to go with inserting an entire pre-formatted key value pair 
> in the form `{key}={value}`, instead of having the function be 
> `insert(StringRef Key, StringRef Value)`?
> 
The `insert(key, value)` form is still available (and used where appropriate). 
This is a convenience function because in a lot of the cases we only have the 
"KEY=VALUE" form handy (constructing from `char**envp`, reading the env over 
gdb-remote, ...), and I wanted to avoid having each user having to split this  
manually. I wouldn't be opposed to calling this something other than `insert` 
if you think it's confusing, but I think the functionality per-se is quite 
useful.


https://reviews.llvm.org/D41359



___
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]

I agree that's better long term, but this code was already there, and the
patch makes things strictly better than before (literally just fixes a
crash in existing code).  So I think we can agree that this should be
fixed, but as it's a bigger change I don't think it should hold up fixing a
crash.

On Mon, Dec 18, 2017 at 9:44 AM Greg Clayton via Phabricator <
revi...@reviews.llvm.org> wrote:

> clayborg added inline comments.
>
>
> 
> Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:393
>// try really hard not to use a regex match.
> -  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();
> -  }
> -  if (is_regex)
> -FindTypesByRegex(name_str, max_matches, types);
> +  if (name_str.find_first_of("[]?*.-+\\") != std::string::npos)
> +FindTypesByRegex(RegularExpression(name_str), max_matches, types);
> 
> I would rather not sniff the string passed in to see if this could be a
> regex and add a new FindTypesByRegex() call to lldb_private::SymbolFile
> that anyone can use. Do we have PDB tests that are using this functionality?
>
>
> 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]

ok, good starting point then. We can submit another patch to add the find types 
by regex functionality.

Greg

> On Dec 18, 2017, at 9:54 AM, Zachary Turner  wrote:
> 
> I agree that's better long term, but this code was already there, and the 
> patch makes things strictly better than before (literally just fixes a crash 
> in existing code).  So I think we can agree that this should be fixed, but as 
> it's a bigger change I don't think it should hold up fixing a crash.
> 
> On Mon, Dec 18, 2017 at 9:44 AM Greg Clayton via Phabricator 
> > wrote:
> clayborg added inline comments.
> 
> 
> 
> Comment at: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp:393
>// try really hard not to use a regex match.
> -  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();
> -  }
> -  if (is_regex)
> -FindTypesByRegex(name_str, max_matches, types);
> +  if (name_str.find_first_of("[]?*.-+\\") != std::string::npos)
> +FindTypesByRegex(RegularExpression(name_str), max_matches, types);
> 
> I would rather not sniff the string passed in to see if this could be a regex 
> and add a new FindTypesByRegex() call to lldb_private::SymbolFile that anyone 
> can use. Do we have PDB tests that are using this functionality?
> 
> 
> 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] D41352: debugserver: Propagate environment in launch-mode (pr35671)

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

I am ok either way, but I do want to make sure Jason gets input before we do 
anything. He might be out for a few weeks over the break, so there might be a 
delay.


https://reviews.llvm.org/D41352



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

[Lldb-commits] [PATCH] D41352: debugserver: Propagate environment in launch-mode (pr35671)

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda added a comment.

I guess I don't have an opinion on this one.  The correct way to pass 
environment variables to the inferior is through 
SBLaunchInfo::SetEnvironmentEntries or in cmd line lldb, process launch -v 
ENV=val.  A test that assumes an environment variable set in lldb will end up 
in the inferior process isn't going to work when the process is launched on a 
remote target, is it?

Whether llgs or debugserver should be forwarding their environment variables on 
by default - it seems fragile to me.  But maybe there's a use case that needs 
this behavior to be supported?  I guess it's valid to test that it actually 
behaves that way, at least for processes launched on the local system.

(I apologize for not keeping up with lldb-commits the past week while this was 
work was being done -- I'm in the middle of a task where I've had to 
concentrate on that & pause reading emails etc for a bit. ;)


https://reviews.llvm.org/D41352



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

[Lldb-commits] [lldb] r321051 - Tweak to the debugserver entitlements setup in the xcode project

[Jason Molenda via lldb-commits]

Author: jmolenda
Date: Mon Dec 18 17:41:47 2017
New Revision: 321051

URL: http://llvm.org/viewvc/llvm-project?rev=321051=rev
Log:
Tweak to the debugserver entitlements setup in the xcode project
file.  For macos builds specifically, use the macosx entitlements
files; for all other builds, use the ios etc entitlements.

Modified:
lldb/trunk/tools/debugserver/debugserver.xcodeproj/project.pbxproj

Modified: lldb/trunk/tools/debugserver/debugserver.xcodeproj/project.pbxproj
URL: 
http://llvm.org/viewvc/llvm-project/lldb/trunk/tools/debugserver/debugserver.xcodeproj/project.pbxproj?rev=321051=321050=321051=diff
==
--- lldb/trunk/tools/debugserver/debugserver.xcodeproj/project.pbxproj 
(original)
+++ lldb/trunk/tools/debugserver/debugserver.xcodeproj/project.pbxproj Mon Dec 
18 17:41:47 2017
@@ -820,7 +820,7 @@
buildSettings = {
CLANG_CXX_LANGUAGE_STANDARD = "c++0x";
CLANG_CXX_LIBRARY = "libc++";
-   "CODE_SIGN_ENTITLEMENTS[sdk=iphoneos*]" = 
"source/debugserver-entitlements.plist";
+   "CODE_SIGN_ENTITLEMENTS[sdk=*]" = 
"source/debugserver-entitlements.plist";
"CODE_SIGN_ENTITLEMENTS[sdk=macosx*]" = 
"source/debugserver-macosx-entitlements.plist";
"CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "-";
"CODE_SIGN_IDENTITY[sdk=macosx*]" = "-";
@@ -920,8 +920,7 @@
buildSettings = {
CLANG_CXX_LANGUAGE_STANDARD = "c++0x";
CLANG_CXX_LIBRARY = "libc++";
-   "CODE_SIGN_ENTITLEMENTS[sdk=*]" = 
"source/debugserver-macosx-entitlements.plist";
-   "CODE_SIGN_ENTITLEMENTS[sdk=iphoneos*]" = 
"source/debugserver-entitlements.plist";
+   "CODE_SIGN_ENTITLEMENTS[sdk=*]" = 
"source/debugserver-entitlements.plist";
"CODE_SIGN_ENTITLEMENTS[sdk=macosx*]" = 
"source/debugserver-macosx-entitlements.plist";
"CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "";
"CODE_SIGN_IDENTITY[sdk=macosx*]" = "";
@@ -1020,8 +1019,7 @@
buildSettings = {
CLANG_CXX_LANGUAGE_STANDARD = "c++0x";
CLANG_CXX_LIBRARY = "libc++";
-   "CODE_SIGN_ENTITLEMENTS[sdk=*]" = 
"source/debugserver-macosx-entitlements.plist";
-   "CODE_SIGN_ENTITLEMENTS[sdk=iphoneos*]" = 
"source/debugserver-entitlements.plist";
+   "CODE_SIGN_ENTITLEMENTS[sdk=*]" = 
"source/debugserver-entitlements.plist";
"CODE_SIGN_ENTITLEMENTS[sdk=macosx*]" = 
"source/debugserver-macosx-entitlements.plist";
"CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "";
"CODE_SIGN_IDENTITY[sdk=macosx*]" = "";
@@ -1127,8 +1125,7 @@
CLANG_WARN_INT_CONVERSION = YES;
CLANG_WARN_UNREACHABLE_CODE = YES;
CLANG_WARN__DUPLICATE_METHOD_MATCH = YES;
-   "CODE_SIGN_ENTITLEMENTS[sdk=*]" = 
"source/debugserver-macosx-entitlements.plist";
-   "CODE_SIGN_ENTITLEMENTS[sdk=iphoneos*]" = 
"source/debugserver-entitlements.plist";
+   "CODE_SIGN_ENTITLEMENTS[sdk=*]" = 
"source/debugserver-entitlements.plist";
"CODE_SIGN_ENTITLEMENTS[sdk=macosx*]" = 
"source/debugserver-macosx-entitlements.plist";
CODE_SIGN_IDENTITY = "";
"CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "";
@@ -1209,8 +1206,7 @@
buildSettings = {
CLANG_CXX_LANGUAGE_STANDARD = "c++0x";
CLANG_CXX_LIBRARY = "libc++";
-   "CODE_SIGN_ENTITLEMENTS[sdk=*]" = 
"source/debugserver-macosx-entitlements.plist";
-   "CODE_SIGN_ENTITLEMENTS[sdk=iphoneos*]" = 
"source/debugserver-entitlements.plist";
+   "CODE_SIGN_ENTITLEMENTS[sdk=*]" = 
"source/debugserver-entitlements.plist";
"CODE_SIGN_ENTITLEMENTS[sdk=macosx*]" = 
"source/debugserver-macosx-entitlements.plist";
"CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "";
"CODE_SIGN_IDENTITY[sdk=macosx*]" = "";
@@ -1279,8 +1275,7 @@
buildSettings = {
CLANG_CXX_LANGUAGE_STANDARD = "c++0x";
CLANG_CXX_LIBRARY = "libc++";
-

[Lldb-commits] [PATCH] D41359: Add Utility/Environment class for handling... environments

[Pavel Labath via Phabricator via lldb-commits]

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

re-clang-format the patch


https://reviews.llvm.org/D41359

Files:
  include/lldb/API/SBLaunchInfo.h
  include/lldb/Host/Host.h
  include/lldb/Interpreter/Args.h
  include/lldb/Target/Platform.h
  include/lldb/Target/ProcessInfo.h
  include/lldb/Target/Target.h
  include/lldb/Utility/Environment.h
  packages/Python/lldbsuite/test/python_api/sblaunchinfo/TestSBLaunchInfo.py
  source/API/SBLaunchInfo.cpp
  source/API/SBPlatform.cpp
  source/API/SBProcess.cpp
  source/API/SBTarget.cpp
  source/Commands/CommandObjectProcess.cpp
  source/Host/freebsd/Host.cpp
  source/Host/linux/Host.cpp
  source/Host/macosx/Host.mm
  source/Host/netbsd/Host.cpp
  source/Host/openbsd/Host.cpp
  source/Host/posix/ProcessLauncherPosixFork.cpp
  source/Host/windows/Host.cpp
  source/Interpreter/Args.cpp
  source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
  source/Plugins/Platform/MacOSX/PlatformiOSSimulatorCoreSimulatorSupport.mm
  source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
  source/Plugins/Platform/POSIX/PlatformPOSIX.h
  source/Plugins/Platform/Windows/PlatformWindows.cpp
  source/Plugins/Platform/Windows/PlatformWindows.h
  source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
  source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerCommon.cpp
  source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
  source/Plugins/StructuredData/DarwinLog/StructuredDataDarwinLog.cpp
  source/Target/Platform.cpp
  source/Target/Process.cpp
  source/Target/ProcessInfo.cpp
  source/Target/Target.cpp
  source/Utility/CMakeLists.txt
  source/Utility/Environment.cpp
  tools/lldb-server/lldb-gdbserver.cpp
  unittests/Host/HostTest.cpp
  unittests/Interpreter/TestArgs.cpp
  unittests/Utility/CMakeLists.txt
  unittests/Utility/EnvironmentTest.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,10 +89,7 @@
   ProcessLaunchInfo Info;
   Info.SetArchitecture(arch_spec);
   Info.SetArguments(args, true);
-
-  StringList Env;
-  Host::GetEnvironment(Env);
-  Info.GetEnvironmentEntries() = Args(Env);
+  Info.GetEnvironment() = Host::GetEnvironment();
 
   status = Host::LaunchProcess(Info);
   if (status.Fail())
@@ -112,14 +109,9 @@
 }
 
 Error 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) {
-  return make_error(
-  formatv("Failed to set environment variable: {0}", env[i]).str(),
-  inconvertibleErrorCode());
-}
+  if (SendEnvironment(Host::GetEnvironment()) != 0) {
+return make_error("Failed to set launch environment",
+   inconvertibleErrorCode());
   }
   std::stringstream command;
   command << "A";
Index: unittests/Utility/EnvironmentTest.cpp
===
--- /dev/null
+++ unittests/Utility/EnvironmentTest.cpp
@@ -0,0 +1,49 @@
+//===-- EnvironmentTest.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 "lldb/Utility/Environment.h"
+
+using namespace lldb_private;
+
+TEST(EnvironmentTest, EnvpConstruction) {
+  const char **Envp1 = nullptr;
+  EXPECT_EQ(0u, Environment(Envp1).size());
+
+  const char *Envp2[] = {"FOO=BAR", nullptr};
+  EXPECT_EQ("BAR", Environment(Envp2).lookup("FOO"));
+
+  const char *Envp3[] = {"FOO=BAR", "FOO=BAZ", nullptr};
+  EXPECT_EQ("BAR", Environment(Envp3).lookup("FOO"));
+
+  const char *Envp4[] = {"FOO=", "BAR", nullptr};
+  Environment Env4(Envp4);
+  ASSERT_EQ(2u, Env4.size());
+  EXPECT_EQ("", Environment(Envp4).find("FOO")->second);
+  EXPECT_EQ("", Environment(Envp4).find("BAR")->second);
+
+  const char *Envp5[] = {"FOO=BAR=BAZ"};
+  EXPECT_EQ("BAR=BAZ", Environment(Envp5).lookup("FOO"));
+}
+
+TEST(EnvironmentTest, EnvpConversion) {
+  std::string FOO_EQ_BAR("FOO=BAR");
+  std::string BAR_EQ_BAZ("BAR=BAZ");
+
+  Environment Env;
+  Env.insert(FOO_EQ_BAR);
+  Env.insert(BAR_EQ_BAZ);
+  Environment::Envp Envp = Env.getEnvp();
+  const char *const *Envp_ = Envp;
+
+  EXPECT_TRUE(FOO_EQ_BAR == Envp_[0] || FOO_EQ_BAR == Envp_[1]);
+  EXPECT_TRUE(BAR_EQ_BAZ == Envp_[0] || BAR_EQ_BAZ == Envp_[1]);
+  EXPECT_EQ(nullptr,