vendor/lldb/lldb-trunk-r338150

1 files changed, 152 insertions, 182 deletions

diff --git a/source/Target/Target.cpp b/source/Target/Target.cpp
index fdc10cf48275..3f70741713fb 100644
--- a/source/Target/Target.cpp
+++ b/source/Target/Target.cpp
@@ -87,13 +87,14 @@ Target::Target(Debugger &debugger, const ArchSpec &target_arch,
       Broadcaster(debugger.GetBroadcasterManager(),
                   Target::GetStaticBroadcasterClass().AsCString()),
       ExecutionContextScope(), m_debugger(debugger), m_platform_sp(platform_sp),
-      m_mutex(), m_arch(target_arch),
-      m_images(this), m_section_load_history(), m_breakpoint_list(false),
-      m_internal_breakpoint_list(true), m_watchpoint_list(), m_process_sp(),
-      m_search_filter_sp(), m_image_search_paths(ImageSearchPathsChanged, this),
-      m_ast_importer_sp(), m_source_manager_ap(), m_stop_hooks(),
-      m_stop_hook_next_id(0), m_valid(true), m_suppress_stop_hooks(false),
-      m_is_dummy_target(is_dummy_target)
+      m_mutex(), m_arch(target_arch), m_images(this), m_section_load_history(),
+      m_breakpoint_list(false), m_internal_breakpoint_list(true),
+      m_watchpoint_list(), m_process_sp(), m_search_filter_sp(),
+      m_image_search_paths(ImageSearchPathsChanged, this), m_ast_importer_sp(),
+      m_source_manager_ap(), m_stop_hooks(), m_stop_hook_next_id(0),
+      m_valid(true), m_suppress_stop_hooks(false),
+      m_is_dummy_target(is_dummy_target),
+      m_stats_storage(static_cast<int>(StatisticKind::StatisticMax))
 
 {
   SetEventName(eBroadcastBitBreakpointChanged, "breakpoint-changed");
@@ -327,11 +328,7 @@ BreakpointSP Target::CreateBreakpoint(const FileSpecList *containingModules,
                                       bool hardware,
                                       LazyBool move_to_nearest_code) {
   FileSpec remapped_file;
-  ConstString remapped_path;
-  if (GetSourcePathMap().ReverseRemapPath(ConstString(file.GetPath().c_str()),
-                                          remapped_path))
-    remapped_file.SetFile(remapped_path.AsCString(), true);
-  else
+  if (!GetSourcePathMap().ReverseRemapPath(file, remapped_file))
     remapped_file = file;
 
   if (check_inlines == eLazyBoolCalculate) {
@@ -381,8 +378,8 @@ BreakpointSP Target::CreateBreakpoint(lldb::addr_t addr, bool internal,
   // Check for any reason we want to move this breakpoint to other address.
   addr = GetBreakableLoadAddress(addr);
 
-  // Attempt to resolve our load address if possible, though it is ok if
-  // it doesn't resolve to section/offset.
+  // Attempt to resolve our load address if possible, though it is ok if it
+  // doesn't resolve to section/offset.
 
   // Try and resolve as a load address if possible
   GetSectionLoadList().ResolveLoadAddress(addr, so_addr);
@@ -536,8 +533,7 @@ SearchFilterSP Target::GetSearchFilterForModuleAndCUList(
   SearchFilterSP filter_sp;
   if (containingModules == nullptr) {
     // We could make a special "CU List only SearchFilter".  Better yet was if
-    // these could be composable,
-    // but that will take a little reworking.
+    // these could be composable, but that will take a little reworking.
 
     filter_sp.reset(new SearchFilterByModuleListAndCU(
         shared_from_this(), FileSpecList(), *containingSourceFiles));
@@ -743,8 +739,8 @@ static bool CheckIfWatchpointsExhausted(Target *target, Status &error) {
   return true;
 }
 
-// See also Watchpoint::SetWatchpointType(uint32_t type) and
-// the OptionGroupWatchpoint::WatchType enum type.
+// See also Watchpoint::SetWatchpointType(uint32_t type) and the
+// OptionGroupWatchpoint::WatchType enum type.
 WatchpointSP Target::CreateWatchpoint(lldb::addr_t addr, size_t size,
                                       const CompilerType *type, uint32_t kind,
                                       Status &error) {
@@ -775,8 +771,8 @@ WatchpointSP Target::CreateWatchpoint(lldb::addr_t addr, size_t size,
   if (!CheckIfWatchpointsExhausted(this, error))
     return wp_sp;
 
-  // Currently we only support one watchpoint per address, with total number
-  // of watchpoints limited by the hardware which the inferior is running on.
+  // Currently we only support one watchpoint per address, with total number of
+  // watchpoints limited by the hardware which the inferior is running on.
 
   // Grab the list mutex while doing operations.
   const bool notify = false; // Don't notify about all the state changes we do
@@ -813,9 +809,8 @@ WatchpointSP Target::CreateWatchpoint(lldb::addr_t addr, size_t size,
                 wp_sp->GetID());
 
   if (error.Fail()) {
-    // Enabling the watchpoint on the device side failed.
-    // Remove the said watchpoint from the list maintained by the target
-    // instance.
+    // Enabling the watchpoint on the device side failed. Remove the said
+    // watchpoint from the list maintained by the target instance.
     m_watchpoint_list.Remove(wp_sp->GetID(), true);
     // See if we could provide more helpful error message.
     if (!OptionGroupWatchpoint::IsWatchSizeSupported(size))
@@ -1027,8 +1022,7 @@ Status Target::SerializeBreakpointsToFile(const FileSpec &file,
         Breakpoint *bp = GetBreakpointByID(bp_id).get();
         StructuredData::ObjectSP bkpt_save_sp = bp->SerializeToStructuredData();
         // If the user explicitly asked to serialize a breakpoint, and we
-        // can't, then
-        // raise an error:
+        // can't, then raise an error:
         if (!bkpt_save_sp) {
           error.SetErrorStringWithFormat("Unable to serialize breakpoint %d",
                                          bp_id);
@@ -1141,8 +1135,8 @@ bool Target::RemoveAllWatchpoints(bool end_to_end) {
   return true; // Success!
 }
 
-// Assumption: Caller holds the list mutex lock for m_watchpoint_list for end to
-// end operations.
+// Assumption: Caller holds the list mutex lock for m_watchpoint_list for end
+// to end operations.
 bool Target::DisableAllWatchpoints(bool end_to_end) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
   if (log)
@@ -1171,8 +1165,8 @@ bool Target::DisableAllWatchpoints(bool end_to_end) {
   return true; // Success!
 }
 
-// Assumption: Caller holds the list mutex lock for m_watchpoint_list for end to
-// end operations.
+// Assumption: Caller holds the list mutex lock for m_watchpoint_list for end
+// to end operations.
 bool Target::EnableAllWatchpoints(bool end_to_end) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
   if (log)
@@ -1235,8 +1229,8 @@ bool Target::ClearAllWatchpointHistoricValues() {
   return true; // Success!
 }
 
-// Assumption: Caller holds the list mutex lock for m_watchpoint_list
-// during these operations.
+// Assumption: Caller holds the list mutex lock for m_watchpoint_list during
+// these operations.
 bool Target::IgnoreAllWatchpoints(uint32_t ignore_count) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_WATCHPOINTS));
   if (log)
@@ -1459,7 +1453,8 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec) {
 
   if (compatible_local_arch || missing_local_arch) {
     // If we haven't got a valid arch spec, or the architectures are compatible
-    // update the architecture, unless the one we already have is more specified
+    // update the architecture, unless the one we already have is more
+    // specified
     if (replace_local_arch)
       m_arch = other;
     LLDB_LOG(log, "set architecture to {0} ({1})",
@@ -1502,8 +1497,8 @@ bool Target::SetArchitecture(const ArchSpec &arch_spec) {
 bool Target::MergeArchitecture(const ArchSpec &arch_spec) {
   if (arch_spec.IsValid()) {
     if (m_arch.GetSpec().IsCompatibleMatch(arch_spec)) {
-      // The current target arch is compatible with "arch_spec", see if we
-      // can improve our current architecture using bits from "arch_spec"
+      // The current target arch is compatible with "arch_spec", see if we can
+      // improve our current architecture using bits from "arch_spec"
 
       // Merge bits from arch_spec into "merged_arch" and set our architecture
       ArchSpec merged_arch(m_arch.GetSpec());
@@ -1662,8 +1657,8 @@ size_t Target::ReadMemory(const Address &addr, bool prefer_file_cache,
                           lldb::addr_t *load_addr_ptr) {
   error.Clear();
 
-  // if we end up reading this from process memory, we will fill this
-  // with the actual load address
+  // if we end up reading this from process memory, we will fill this with the
+  // actual load address
   if (load_addr_ptr)
     *load_addr_ptr = LLDB_INVALID_ADDRESS;
 
@@ -1675,16 +1670,16 @@ size_t Target::ReadMemory(const Address &addr, bool prefer_file_cache,
   if (!addr.IsSectionOffset()) {
     SectionLoadList &section_load_list = GetSectionLoadList();
     if (section_load_list.IsEmpty()) {
-      // No sections are loaded, so we must assume we are not running
-      // yet and anything we are given is a file address.
+      // No sections are loaded, so we must assume we are not running yet and
+      // anything we are given is a file address.
       file_addr = addr.GetOffset(); // "addr" doesn't have a section, so its
                                     // offset is the file address
       m_images.ResolveFileAddress(file_addr, resolved_addr);
     } else {
-      // We have at least one section loaded. This can be because
-      // we have manually loaded some sections with "target modules load ..."
-      // or because we have have a live process that has sections loaded
-      // through the dynamic loader
+      // We have at least one section loaded. This can be because we have
+      // manually loaded some sections with "target modules load ..." or
+      // because we have have a live process that has sections loaded through
+      // the dynamic loader
       load_addr = addr.GetOffset(); // "addr" doesn't have a section, so its
                                     // offset is the load address
       section_load_list.ResolveLoadAddress(load_addr, resolved_addr);
@@ -1731,19 +1726,19 @@ size_t Target::ReadMemory(const Address &addr, bool prefer_file_cache,
           *load_addr_ptr = load_addr;
         return bytes_read;
       }
-      // If the address is not section offset we have an address that
-      // doesn't resolve to any address in any currently loaded shared
-      // libraries and we failed to read memory so there isn't anything
-      // more we can do. If it is section offset, we might be able to
-      // read cached memory from the object file.
+      // If the address is not section offset we have an address that doesn't
+      // resolve to any address in any currently loaded shared libraries and we
+      // failed to read memory so there isn't anything more we can do. If it is
+      // section offset, we might be able to read cached memory from the object
+      // file.
       if (!resolved_addr.IsSectionOffset())
         return 0;
     }
   }
 
   if (!prefer_file_cache && resolved_addr.IsSectionOffset()) {
-    // If we didn't already try and read from the object file cache, then
-    // try it after failing to read from the process.
+    // If we didn't already try and read from the object file cache, then try
+    // it after failing to read from the process.
     return ReadMemoryFromFileCache(resolved_addr, dst, dst_len, error);
   }
   return 0;
@@ -1760,8 +1755,8 @@ size_t Target::ReadCStringFromMemory(const Address &addr, std::string &out_str,
     if (length == 0)
       break;
     out_str.append(buf, length);
-    // If we got "length - 1" bytes, we didn't get the whole C string, we
-    // need to read some more characters
+    // If we got "length - 1" bytes, we didn't get the whole C string, we need
+    // to read some more characters
     if (length == sizeof(buf) - 1)
       curr_addr += length;
     else
@@ -1782,9 +1777,9 @@ size_t Target::ReadCStringFromMemory(const Address &addr, char *dst,
     addr_t curr_addr = addr.GetLoadAddress(this);
     Address address(addr);
 
-    // We could call m_process_sp->GetMemoryCacheLineSize() but I don't
-    // think this really needs to be tied to the memory cache subsystem's
-    // cache line size, so leave this as a fixed constant.
+    // We could call m_process_sp->GetMemoryCacheLineSize() but I don't think
+    // this really needs to be tied to the memory cache subsystem's cache line
+    // size, so leave this as a fixed constant.
     const size_t cache_line_size = 512;
 
     size_t bytes_left = dst_max_len - 1;
@@ -1875,18 +1870,18 @@ bool Target::ReadPointerFromMemory(const Address &addr, bool prefer_file_cache,
     if (pointer_vm_addr != LLDB_INVALID_ADDRESS) {
       SectionLoadList &section_load_list = GetSectionLoadList();
       if (section_load_list.IsEmpty()) {
-        // No sections are loaded, so we must assume we are not running
-        // yet and anything we are given is a file address.
+        // No sections are loaded, so we must assume we are not running yet and
+        // anything we are given is a file address.
         m_images.ResolveFileAddress(pointer_vm_addr, pointer_addr);
       } else {
-        // We have at least one section loaded. This can be because
-        // we have manually loaded some sections with "target modules load ..."
-        // or because we have have a live process that has sections loaded
-        // through the dynamic loader
+        // We have at least one section loaded. This can be because we have
+        // manually loaded some sections with "target modules load ..." or
+        // because we have have a live process that has sections loaded through
+        // the dynamic loader
         section_load_list.ResolveLoadAddress(pointer_vm_addr, pointer_addr);
       }
-      // We weren't able to resolve the pointer value, so just return
-      // an address with no section
+      // We weren't able to resolve the pointer value, so just return an
+      // address with no section
       if (!pointer_addr.IsValid())
         pointer_addr.SetOffset(pointer_vm_addr);
       return true;
@@ -1902,9 +1897,8 @@ ModuleSP Target::GetSharedModule(const ModuleSpec &module_spec,
   Status error;
 
   // First see if we already have this module in our module list.  If we do,
-  // then we're done, we don't need
-  // to consult the shared modules list.  But only do this if we are passed a
-  // UUID.
+  // then we're done, we don't need to consult the shared modules list.  But
+  // only do this if we are passed a UUID.
 
   if (module_spec.GetUUID().IsValid())
     module_sp = m_images.FindFirstModule(module_spec);
@@ -1959,8 +1953,8 @@ ModuleSP Target::GetSharedModule(const ModuleSpec &module_spec,
     }
 
     // We found a module that wasn't in our target list.  Let's make sure that
-    // there wasn't an equivalent
-    // module in the list already, and if there was, let's remove it.
+    // there wasn't an equivalent module in the list already, and if there was,
+    // let's remove it.
     if (module_sp) {
       ObjectFile *objfile = module_sp->GetObjectFile();
       if (objfile) {
@@ -1994,18 +1988,14 @@ ModuleSP Target::GetSharedModule(const ModuleSpec &module_spec,
           return ModuleSP();
         }
         // GetSharedModule is not guaranteed to find the old shared module, for
-        // instance
-        // in the common case where you pass in the UUID, it is only going to
-        // find the one
-        // module matching the UUID.  In fact, it has no good way to know what
-        // the "old module"
-        // relevant to this target is, since there might be many copies of a
-        // module with this file spec
-        // in various running debug sessions, but only one of them will belong
-        // to this target.
-        // So let's remove the UUID from the module list, and look in the
-        // target's module list.
-        // Only do this if there is SOMETHING else in the module spec...
+        // instance in the common case where you pass in the UUID, it is only
+        // going to find the one module matching the UUID.  In fact, it has no
+        // good way to know what the "old module" relevant to this target is,
+        // since there might be many copies of a module with this file spec in
+        // various running debug sessions, but only one of them will belong to
+        // this target. So let's remove the UUID from the module list, and look
+        // in the target's module list. Only do this if there is SOMETHING else
+        // in the module spec...
         if (!old_module_sp) {
           if (module_spec.GetUUID().IsValid() &&
               !module_spec.GetFileSpec().GetFilename().IsEmpty() &&
@@ -2285,8 +2275,8 @@ ExpressionResults Target::EvaluateExpression(
   if (expr.empty())
     return execution_results;
 
-  // We shouldn't run stop hooks in expressions.
-  // Be sure to reset this if you return anywhere within this function.
+  // We shouldn't run stop hooks in expressions. Be sure to reset this if you
+  // return anywhere within this function.
   bool old_suppress_value = m_suppress_stop_hooks;
   m_suppress_stop_hooks = true;
 
@@ -2300,8 +2290,8 @@ ExpressionResults Target::EvaluateExpression(
     CalculateExecutionContext(exe_ctx);
   }
 
-  // Make sure we aren't just trying to see the value of a persistent
-  // variable (something like "$0")
+  // Make sure we aren't just trying to see the value of a persistent variable
+  // (something like "$0")
   lldb::ExpressionVariableSP persistent_var_sp;
   // Only check for persistent variables the expression starts with a '$'
   if (expr[0] == '$')
@@ -2368,16 +2358,16 @@ lldb::addr_t Target::GetCallableLoadAddress(lldb::addr_t load_addr,
   case llvm::Triple::mips64:
   case llvm::Triple::mips64el:
     switch (addr_class) {
-    case eAddressClassData:
-    case eAddressClassDebug:
+    case AddressClass::eData:
+    case AddressClass::eDebug:
       return LLDB_INVALID_ADDRESS;
 
-    case eAddressClassUnknown:
-    case eAddressClassInvalid:
-    case eAddressClassCode:
-    case eAddressClassCodeAlternateISA:
-    case eAddressClassRuntime:
-      if ((code_addr & 2ull) || (addr_class == eAddressClassCodeAlternateISA))
+    case AddressClass::eUnknown:
+    case AddressClass::eInvalid:
+    case AddressClass::eCode:
+    case AddressClass::eCodeAlternateISA:
+    case AddressClass::eRuntime:
+      if ((code_addr & 2ull) || (addr_class == AddressClass::eCodeAlternateISA))
         code_addr |= 1ull;
       break;
     }
@@ -2386,25 +2376,24 @@ lldb::addr_t Target::GetCallableLoadAddress(lldb::addr_t load_addr,
   case llvm::Triple::arm:
   case llvm::Triple::thumb:
     switch (addr_class) {
-    case eAddressClassData:
-    case eAddressClassDebug:
+    case AddressClass::eData:
+    case AddressClass::eDebug:
       return LLDB_INVALID_ADDRESS;
 
-    case eAddressClassUnknown:
-    case eAddressClassInvalid:
-    case eAddressClassCode:
-    case eAddressClassCodeAlternateISA:
-    case eAddressClassRuntime:
+    case AddressClass::eUnknown:
+    case AddressClass::eInvalid:
+    case AddressClass::eCode:
+    case AddressClass::eCodeAlternateISA:
+    case AddressClass::eRuntime:
       // Check if bit zero it no set?
       if ((code_addr & 1ull) == 0) {
         // Bit zero isn't set, check if the address is a multiple of 2?
         if (code_addr & 2ull) {
           // The address is a multiple of 2 so it must be thumb, set bit zero
           code_addr |= 1ull;
-        } else if (addr_class == eAddressClassCodeAlternateISA) {
+        } else if (addr_class == AddressClass::eCodeAlternateISA) {
           // We checked the address and the address claims to be the alternate
-          // ISA
-          // which means thumb, so set bit zero.
+          // ISA which means thumb, so set bit zero.
           code_addr |= 1ull;
         }
       }
@@ -2429,15 +2418,15 @@ lldb::addr_t Target::GetOpcodeLoadAddress(lldb::addr_t load_addr,
   case llvm::Triple::arm:
   case llvm::Triple::thumb:
     switch (addr_class) {
-    case eAddressClassData:
-    case eAddressClassDebug:
+    case AddressClass::eData:
+    case AddressClass::eDebug:
       return LLDB_INVALID_ADDRESS;
 
-    case eAddressClassInvalid:
-    case eAddressClassUnknown:
-    case eAddressClassCode:
-    case eAddressClassCodeAlternateISA:
-    case eAddressClassRuntime:
+    case AddressClass::eInvalid:
+    case AddressClass::eUnknown:
+    case AddressClass::eCode:
+    case AddressClass::eCodeAlternateISA:
+    case AddressClass::eRuntime:
       opcode_addr &= ~(1ull);
       break;
     }
@@ -2470,8 +2459,8 @@ lldb::addr_t Target::GetBreakableLoadAddress(lldb::addr_t addr) {
     SectionLoadList &section_load_list = GetSectionLoadList();
 
     if (section_load_list.IsEmpty())
-      // No sections are loaded, so we must assume we are not running yet
-      // and need to operate only on file address.
+      // No sections are loaded, so we must assume we are not running yet and
+      // need to operate only on file address.
       m_images.ResolveFileAddress(addr, resolved_addr);
     else
       section_load_list.ResolveLoadAddress(addr, resolved_addr);
@@ -2546,30 +2535,29 @@ lldb::addr_t Target::GetBreakableLoadAddress(lldb::addr_t addr) {
         } else if (i == 2) {
           // Here we may get one 4-byte instruction or two 2-byte instructions.
           if (num_insns == 2) {
-            // Looks like there are two 2-byte instructions above our breakpoint
-            // target address.
-            // Now the upper 2-byte instruction is either a valid 2-byte
-            // instruction or could be a part of it's upper 4-byte instruction.
-            // In both cases we don't care because in this case lower 2-byte
-            // instruction is definitely a valid instruction
-            // and whatever i=1 iteration has found out is true.
+            // Looks like there are two 2-byte instructions above our
+            // breakpoint target address. Now the upper 2-byte instruction is
+            // either a valid 2-byte instruction or could be a part of it's
+            // upper 4-byte instruction. In both cases we don't care because in
+            // this case lower 2-byte instruction is definitely a valid
+            // instruction and whatever i=1 iteration has found out is true.
             inst_to_choose = 1;
             break;
           } else if (insn_size == 4) {
             // This instruction claims its a valid 4-byte instruction. But it
-            // could be a part of it's upper 4-byte instruction.
-            // Lets try scanning upper 2 bytes to verify this.
+            // could be a part of it's upper 4-byte instruction. Lets try
+            // scanning upper 2 bytes to verify this.
             instruction_list.Append(prev_insn);
             inst_to_choose = 2;
           }
         } else if (i == 3) {
           if (insn_size == 4)
             // FIXME: We reached here that means instruction at [target - 4] has
-            // already claimed to be a 4-byte instruction,
-            // and now instruction at [target - 6] is also claiming that it's a
-            // 4-byte instruction. This can not be true.
-            // In this case we can not decide the valid previous instruction so
-            // we let lldb set the breakpoint at the address given by user.
+            // already claimed to be a 4-byte instruction, and now instruction
+            // at [target - 6] is also claiming that it's a 4-byte instruction.
+            // This can not be true. In this case we can not decide the valid
+            // previous instruction so we let lldb set the breakpoint at the
+            // address given by user.
             inst_to_choose = 0;
           else
             // This is straight-forward
@@ -2685,8 +2673,8 @@ void Target::RunStopHooks() {
     return;
 
   // <rdar://problem/12027563> make sure we check that we are not stopped
-  // because of us running a user expression
-  // since in that case we do not want to run the stop-hooks
+  // because of us running a user expression since in that case we do not want
+  // to run the stop-hooks
   if (m_process_sp->GetModIDRef().IsLastResumeForUserExpression())
     return;
 
@@ -2975,18 +2963,16 @@ Status Target::Launch(ProcessLaunchInfo &launch_info, Stream *stream) {
   launch_info.GetFlags().Set(eLaunchFlagDebug);
 
   // Get the value of synchronous execution here.  If you wait till after you
-  // have started to
-  // run, then you could have hit a breakpoint, whose command might switch the
-  // value, and
-  // then you'll pick up that incorrect value.
+  // have started to run, then you could have hit a breakpoint, whose command
+  // might switch the value, and then you'll pick up that incorrect value.
   Debugger &debugger = GetDebugger();
   const bool synchronous_execution =
       debugger.GetCommandInterpreter().GetSynchronous();
 
   PlatformSP platform_sp(GetPlatform());
 
-  // Finalize the file actions, and if none were given, default to opening
-  // up a pseudo terminal
+  // Finalize the file actions, and if none were given, default to opening up a
+  // pseudo terminal
   const bool default_to_use_pty = platform_sp ? platform_sp->IsHost() : false;
   if (log)
     log->Printf("Target::%s have platform=%s, platform_sp->IsHost()=%s, "
@@ -3025,10 +3011,10 @@ Status Target::Launch(ProcessLaunchInfo &launch_info, Stream *stream) {
         GetPlatform()->DebugProcess(launch_info, debugger, this, error);
 
     // Cleanup the old process since someone might still have a strong
-    // reference to this process and we would like to allow it to cleanup
-    // as much as it can without the object being destroyed. We try to
-    // lock the shared pointer and if that works, then someone else still
-    // has a strong reference to the process.
+    // reference to this process and we would like to allow it to cleanup as
+    // much as it can without the object being destroyed. We try to lock the
+    // shared pointer and if that works, then someone else still has a strong
+    // reference to the process.
 
     ProcessSP old_process_sp(process_wp.lock());
     if (old_process_sp)
@@ -3152,8 +3138,8 @@ Status Target::Attach(ProcessAttachInfo &attach_info, Stream *stream) {
 
   const ModuleSP old_exec_module_sp = GetExecutableModule();
 
-  // If no process info was specified, then use the target executable
-  // name as the process to attach to by default
+  // If no process info was specified, then use the target executable name as
+  // the process to attach to by default
   if (!attach_info.ProcessInfoSpecified()) {
     if (old_exec_module_sp)
       attach_info.GetExecutableFile().GetFilename() =
@@ -3580,9 +3566,8 @@ public:
                                      bool will_modify,
                                      uint32_t idx) const override {
     // When getting the value for a key from the target options, we will always
-    // try and grab the setting from the current target if there is one. Else we
-    // just
-    // use the one from this instance.
+    // try and grab the setting from the current target if there is one. Else
+    // we just use the one from this instance.
     if (idx == ePropertyEnvVars)
       GetHostEnvironmentIfNeeded();
 
@@ -3611,36 +3596,19 @@ protected:
                 nullptr, idx, g_properties[idx].default_uint_value != 0)) {
           PlatformSP platform_sp(m_target->GetPlatform());
           if (platform_sp) {
-            StringList env;
-            if (platform_sp->GetEnvironment(env)) {
-              OptionValueDictionary *env_dict =
-                  GetPropertyAtIndexAsOptionValueDictionary(nullptr,
-                                                            ePropertyEnvVars);
-              if (env_dict) {
-                const bool can_replace = false;
-                const size_t envc = env.GetSize();
-                for (size_t idx = 0; idx < envc; idx++) {
-                  const char *env_entry = env.GetStringAtIndex(idx);
-                  if (env_entry) {
-                    const char *equal_pos = ::strchr(env_entry, '=');
-                    ConstString key;
-                    // It is ok to have environment variables with no values
-                    const char *value = nullptr;
-                    if (equal_pos) {
-                      key.SetCStringWithLength(env_entry,
-                                               equal_pos - env_entry);
-                      if (equal_pos[1])
-                        value = equal_pos + 1;
-                    } else {
-                      key.SetCString(env_entry);
-                    }
-                    // Don't allow existing keys to be replaced with ones we get
-                    // from the platform environment
-                    env_dict->SetValueForKey(
-                        key, OptionValueSP(new OptionValueString(value)),
-                        can_replace);
-                  }
-                }
+            Environment env = platform_sp->GetEnvironment();
+            OptionValueDictionary *env_dict =
+                GetPropertyAtIndexAsOptionValueDictionary(nullptr,
+                                                          ePropertyEnvVars);
+            if (env_dict) {
+              const bool can_replace = false;
+              for (const auto &KV : env) {
+                // Don't allow existing keys to be replaced with ones we get
+                // from the platform environment
+                env_dict->SetValueForKey(
+                    ConstString(KV.first()),
+                    OptionValueSP(new OptionValueString(KV.second.c_str())),
+                    can_replace);
               }
             }
           }
@@ -3690,8 +3658,8 @@ TargetProperties::TargetProperties(Target *target)
     m_collection_sp.reset(
         new TargetOptionValueProperties(target, Target::GetGlobalProperties()));
 
-    // Set callbacks to update launch_info whenever "settins set" updated any of
-    // these properties
+    // Set callbacks to update launch_info whenever "settins set" updated any
+    // of these properties
     m_collection_sp->SetValueChangedCallback(
         ePropertyArg0, TargetProperties::Arg0ValueChangedCallback, this);
     m_collection_sp->SetValueChangedCallback(
@@ -3906,15 +3874,19 @@ void TargetProperties::SetRunArguments(const Args &args) {
   m_launch_info.GetArguments() = args;
 }
 
-size_t TargetProperties::GetEnvironmentAsArgs(Args &env) const {
+Environment TargetProperties::GetEnvironment() const {
+  // TODO: Get rid of the Args intermediate step
+  Args env;
   const uint32_t idx = ePropertyEnvVars;
-  return m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, idx, env);
+  m_collection_sp->GetPropertyAtIndexAsArgs(nullptr, idx, env);
+  return Environment(env);
 }
 
-void TargetProperties::SetEnvironmentFromArgs(const Args &env) {
+void TargetProperties::SetEnvironment(Environment env) {
+  // TODO: Get rid of the Args intermediate step
   const uint32_t idx = ePropertyEnvVars;
-  m_collection_sp->SetPropertyAtIndexFromArgs(nullptr, idx, env);
-  m_launch_info.GetEnvironmentEntries() = env;
+  m_collection_sp->SetPropertyAtIndexFromArgs(nullptr, idx, Args(env));
+  m_launch_info.GetEnvironment() = std::move(env);
 }
 
 bool TargetProperties::GetSkipPrologue() const {
@@ -4152,7 +4124,7 @@ void TargetProperties::SetProcessLaunchInfo(
   m_launch_info = launch_info;
   SetArg0(launch_info.GetArg0());
   SetRunArguments(launch_info.GetArguments());
-  SetEnvironmentFromArgs(launch_info.GetEnvironmentEntries());
+  SetEnvironment(launch_info.GetEnvironment());
   const FileAction *input_file_action =
       launch_info.GetFileActionForFD(STDIN_FILENO);
   if (input_file_action) {
@@ -4193,9 +4165,7 @@ void TargetProperties::EnvVarsValueChangedCallback(void *target_property_ptr,
                                                    OptionValue *) {
   TargetProperties *this_ =
       reinterpret_cast<TargetProperties *>(target_property_ptr);
-  Args args;
-  if (this_->GetEnvironmentAsArgs(args))
-    this_->m_launch_info.GetEnvironmentEntries() = args;
+  this_->m_launch_info.GetEnvironment() = this_->GetEnvironment();
 }
 
 void TargetProperties::InputPathValueChangedCallback(void *target_property_ptr,