Remove Args::m_argv

[zturner]

Summary

The Args class was trying to maintain two parallel argument vectors. One that owned the memory and contained std::strings, and one that contained const char *'s that could be used similar to how argv is used.

But dealing with pointers to pointers is unsafe and should be avoided wherever possible. Furthermore, there doesn't seem to be much point in maintaining a second copy of the array, as it only serves to complicate the parsing logic since the two have to be kept in sync with each other, and really, the master copy has everything you need, including the null terminated strings.

So I removed m_argv entirely. The Args class will also no longer vend a const char**, and instead you can pass it a std::vector<const char*>& and it will fill it with null terminated argument strings. This can be used exactly like a const char** by writing &args[0], but now you have to explicitly opt into this, and by default you just get a nice safe container that can be used for example in ranged based fors. And the logic is simplified as well since the two arrays don't have to be kept in sync with each other.

All tests pass on Windows.

[zturner]

Ignore the changes to FileSpec, it seems the two CLs I was working on got mixed together.

[labath]

include/lldb/Interpreter/Args.h

150–159

I think doxygen will complain about @return in a function returning void.

BTW, with the NRVO, move semantics and everything, is there any reason to keep using the return-through-reference-argument pattern? It would be a lot more natural if this indeed _was_ a return value.

[amccarth]

include/lldb/Interpreter/Args.h

159

Perhaps this should actually return the vector instead of void, and rely on the return value optimization or, at worst, vector move. It would be easier to use and it would let you initialize variables as you declare them, (which means you could use it in constructor intializers and you could declare the vector as const). It also would eliminate the ambiguity of what happens if the args already contains some values (e.g., the caller might expect it to append rather than replace).

source/API/SBCommandInterpreter.cpp

119

Here's an example where, if GetArgumentVector actually returned the result, you could simplify the delcaration as:

const auto argv = command.GetArgumentVector();

source/Host/common/FileSpec.cpp

251 ↗

(On Diff #72601)

Should name be const? As in:

for (const auto &name : name_list) { ... }

source/Interpreter/Args.cpp

40–41

I think your change eliminated the need for this comment. If not, it at least needs an update.

261

clang-format didn't do a great job wrapping the long lines in this comment. Can you fix it up?

411

This is an example where the ambiguity of whether GetArgumentVector appends or not could mislead the caller into doing the wrong this. This looks plausible (and more efficient):

std::vector<const char *> args;
args.push_back("dummy");
GetArgumentVector(args);

If GetArgumentVector returned the vector, then this ambiguity wouldn't exist.

[zturner]

Updated

[clayborg]

See inlined comments.

source/Interpreter/Args.cpp

264–265

Remove this if, it is no longer needed. Just return m_args.size()

269–272

Convert this over to return StringRef?

286–288

Remove this comment. The whole point off this function is to get an argument vector that is NULL terminated. Feel free to rename the function as desired to indicate this, but that is the whole point of this function. Feel free to document it if needed as well.

327

Is there a conversion operator that converts StringRef to std::string somewhere? How is this working? Iterators being detected?

373–376

Should we change this to an assignment operator? If so, rename "other" to "rhs".

source/Interpreter/CommandInterpreter.cpp

2049–2050

If we do the operator = above, then:

cmd_args = new_args;

2059–2060

If we do the operator = above, then:

cmd_args = new_args;

source/Interpreter/CommandObject.cpp

119

Why was this removed?

source/Interpreter/OptionValueArgs.cpp

33

args = argv;

source/Interpreter/OptionValueArray.cpp

151

args = argv;

source/Target/ProcessInfo.cpp

94

What not have a SetArguments that takes a "const char **" and does this internally? It is no less safe and makes the API more usable

[zturner]

source/Interpreter/Args.cpp

269–272

Yes, that is the next item on my list. But this function is used EVERYWHERE, so that will be a very large CL and should be done orthogonally.

286–288

There are two ways we use these argument vectors. One is in a call to a function which takes an argc and an argv. In that case the null terminator is unnecessary because you're specifying the argc explicitly. Another is in a call to a function which you do not specify an argc, and in tht case it requires it to be null terminated.

The point of this function isn't "return me something that's null terminated", it's "return me something I can use like an argv". That doesn't require null termination. And in fact, if you look at the callsites I fixed up, not a single one depends on the null termination. Everyone just needs to know the size. And for that you can call result.size(). If you add the null terminator, then you have to write result.size()-1, which is unnecessarily confusing. I believe it provides the most logical behavior as it is currently written.

327

Yes, StringRef has an operator std::string().

373–376

I thought about it, but it seems like a minor stylistic issue. I think it could go either way. I'm fine adding it or not.

source/Interpreter/CommandObject.cpp

119

Because it's a little weird to do it at this high of a level. This requires internal knowledge of the workings of Args::ParseOptions(), which itself calls OptionParser::Parse(), It's strange for a function this high up to require such low-level knowledge of the workings of a function. So instead, I moved this behavior into Args::ParseOptions. If you check that function, you will see that it injects this argument into the beginning.

This is nice because now it never modifies the internal state of the Args itself, but only the copy that gets passed to getopt_long_only.

source/Target/ProcessInfo.cpp

94

I would eventually like to get rid of all of these const char*'s. For example, this function's signature could itself be changed to take an ArrayRef<const char *>. Then we can keep raising this conversion higher and higher up. I actually want to make it more awkward to use unsafe types like const char **. So by adding the const char ** overload to Args, it doesn't encourage people to consider the alternative ArrayRef. On the other hand, this is similar to StringRef, in that once you have the ArrayRef<const char *> through all levels, you will almost never need this function, and calls like this will disappear as the ArrayRefs are being passed all the way down.

[clayborg]

Ok, so just add the boolean arg to Args::GetArgumentVector and we are good to go.

source/Interpreter/Args.cpp

270–272

Ok, sounds good, for a later CL then.

286–288

Remove the comment. Even in "main(int argc, const char **argv)" the "argv" is null terminated. So all unix variants expect the null termination. The correct solution is to add a boolean argument:

std::vector<const char *> Args::GetArgumentVector(bool null_terminate = true) const {

Then it is clear.

source/Interpreter/CommandObject.cpp

119

Sounds good. I hate the fact that we have to do this. We might be able to play with the getopt_long globals to work around this and not have to add it, but as long as you took it into account I am fine.

[zturner]

source/Interpreter/Args.cpp

286–288

Even in main though, the null is not counted in the argc, right? So as written it will conform to the semantics of main. argc is the number of non-null pointers, but the null is still there anyway. If you add the boolean argument and pass true, it will give you back something whose argc (i.e. result.size()) is one too high.

[zturner]

source/Interpreter/Args.cpp

286–288

BTW, llvm::SmallString uses this same trick to return a null terminated string without modifying the size. Look at the implementation of SmallString::c_str() in SmallString.h

examples/plugins/commands/fooplugin.cpp

 #include <LLDB/SBDebugger.h>
 
 namespace lldb {
 bool PluginInitialize(lldb::SBDebugger debugger);
 }
 
 class ChildCommand : public lldb::SBCommandPluginInterface {
 public:
-  virtual bool DoExecute(lldb::SBDebugger debugger, char **command,
-                         lldb::SBCommandReturnObject &result) {
+  bool DoExecute(lldb::SBDebugger debugger, const char **command,
+                 lldb::SBCommandReturnObject &result) override {
     if (command) {
       const char *arg = *command;
       while (arg) {
         result.Printf("%s\n", arg);
         arg = *(++command);
       }
       return true;
     }

include/lldb/API/SBCommandInterpreter.h

private:
 
   lldb_private::CommandInterpreter *m_opaque_ptr;
 };
 
 class SBCommandPluginInterface {
 public:
   virtual ~SBCommandPluginInterface() = default;
 
-  virtual bool DoExecute(lldb::SBDebugger /*debugger*/, char ** /*command*/,
+  virtual bool DoExecute(lldb::SBDebugger /*debugger*/,
+                         const char ** /*command*/,
                          lldb::SBCommandReturnObject & /*result*/) {
     return false;
   }
 };
 
 class SBCommand {
 public:
   SBCommand();

include/lldb/Host/OptionParser.h

 class OptionParser {
 public:
   enum OptionArgument { eNoArgument = 0, eRequiredArgument, eOptionalArgument };
 
   static void Prepare(std::unique_lock<std::mutex> &lock);
 
   static void EnableError(bool error);
 
-  static int Parse(int argc, char *const argv[], const char *optstring,
+  static int Parse(int argc, char const *const argv[], const char *optstring,
                    const Option *longopts, int *longindex);
 
   static char *GetOptionArgument();
   static int GetOptionIndex();
   static int GetOptionErrorCause();
   static std::string GetShortOptionString(struct option *long_options);
 };
 }
 
 #endif // liblldb_OptionParser_h_

include/lldb/Interpreter/Args.h

 // C Includes
 // C++ Includes
 #include <list>
 #include <string>
 #include <utility>
 #include <vector>
 
 // Other libraries and framework includes
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
+
 // Project includes
 #include "lldb/Core/Error.h"
 #include "lldb/Host/OptionParser.h"
 #include "lldb/lldb-private-types.h"
 #include "lldb/lldb-types.h"
 
 namespace lldb_private {
 
public:
   ///
   /// The value returned by this function can be used by any function
   /// that takes and vector. The return value is just like \a argv
   /// in the standard C entry point function:
   ///     \code
   ///         int main (int argc, const char **argv);
   ///     \endcode
   ///
   /// @return
-  ///     An array of NULL terminated C string argument pointers that
-  ///     also has a terminating NULL C string pointer
-  //------------------------------------------------------------------
-  char **GetArgumentVector();
-
-  //------------------------------------------------------------------
-  /// Gets the argument vector.
-  ///
-  /// The value returned by this function can be used by any function
-  /// that takes and vector. The return value is just like \a argv
-  /// in the standard C entry point function:
-  ///     \code
-  ///         int main (int argc, const char **argv);
-  ///     \endcode
-  ///
-  /// @return
-  ///     An array of NULL terminate C string argument pointers that
-  ///     also has a terminating NULL C string pointer
+  ///     A vector of NULL terminated C string argument pointers with
+  ///     an equivalent in-memory layout to that of a C-style argv
+  ///     array.  Calling .size() on the return value produces the same
+  ///     value as one would pass for argc, and calling .data() on the
+  ///     return value is guaranteed to be null terminated (even though
+  ///     the terminating null is not part of the vector, and as such
+  ///     is not taken into consideration when calling .size()).
   //------------------------------------------------------------------
-  const char **GetConstArgumentVector() const;
+  std::vector<const char *> GetArgumentVector() const;
 
   //------------------------------------------------------------------
   /// Appends a new argument to the end of the list argument list.
   ///
   /// @param[in] arg_cstr
   ///     The new argument as a NULL terminated C string.
   ///
   /// @param[in] quote_char
   ///     If the argument was originally quoted, put in the quote char here.
   ///
   /// @return
   ///     The NULL terminated C string of the copy of \a arg_cstr.
   //------------------------------------------------------------------
   llvm::StringRef AppendArgument(llvm::StringRef arg_str,
                                  char quote_char = '\0');
 
   void AppendArguments(const Args &rhs);
-
-  void AppendArguments(const char **argv);
-
-  // Delete const char* versions of StringRef functions.  Normally this would
-  // not be necessary, as const char * is implicitly convertible to StringRef.
-  // However, since the use of const char* is so pervasive, and since StringRef
-  // will assert if you try to construct one from nullptr, this allows the
-  // compiler to catch instances of the function being invoked with a
-  // const char *, allowing us to replace them with explicit conversions at each
-  // call-site.  This ensures that no callsites slip through the cracks where
-  // we would be trying to implicitly convert from nullptr, since it will force
-  // us to evaluate and explicitly convert each one.
-  //
-  // Once StringRef use becomes more pervasive, there will be fewer
-  // implicit conversions because we will be using StringRefs across the whole
-  // pipeline, so we won't have to have this "glue" that converts between the
-  // two, and at that point it becomes easy to just make sure you don't pass
-  // nullptr into the function on the odd occasion that you do pass a
-  // const char *.
-  // Call-site fixing methodology:
-  //   1. If you know the string cannot be null (e.g. it's a const char[], or
-  //      it's been checked for null), use llvm::StringRef(ptr).
-  //   2. If you don't know if it can be null (e.g. it's returned from a
-  //      function whose semantics are unclear), use
-  //      llvm::StringRef::withNullAsEmpty(ptr).
-  //   3. If it's .c_str() of a std::string, just pass the std::string directly.
-  //   4. If it's .str().c_str() of a StringRef, just pass the StringRef
-  //      directly.
-  void ReplaceArgumentAtIndex(size_t, const char *, char = '\0') = delete;
-  void AppendArgument(const char *arg_str, char quote_char = '\0') = delete;
-  void InsertArgumentAtIndex(size_t, const char *, char = '\0') = delete;
-  static bool StringToBoolean(const char *, bool, bool *) = delete;
-  static lldb::ScriptLanguage
-  StringToScriptLanguage(const char *, lldb::ScriptLanguage, bool *) = delete;
-  static lldb::Encoding
-  StringToEncoding(const char *,
-                   lldb::Encoding = lldb::eEncodingInvalid) = delete;
-  static uint32_t StringToGenericRegister(const char *) = delete;
-  static bool StringToVersion(const char *, uint32_t &, uint32_t &,
-                              uint32_t &) = delete;
-  const char *Unshift(const char *, char = '\0') = delete;
-  void AddOrReplaceEnvironmentVariable(const char *, const char *) = delete;
-  bool ContainsEnvironmentVariable(const char *,
-                                   size_t * = nullptr) const = delete;
-  static int64_t StringToOptionEnum(const char *, OptionEnumValueElement *,
-                                    int32_t, Error &) = delete;
+  void AppendArguments(llvm::ArrayRef<const char *> args);
 
   //------------------------------------------------------------------
   /// Insert the argument value at index \a idx to \a arg_cstr.
   ///
   /// @param[in] idx
   ///     The index of where to insert the argument.
   ///
   /// @param[in] arg_cstr
public:
   /// Sets the argument vector value, optionally copying all
   /// arguments into an internal buffer.
   ///
   /// Sets the arguments to match those found in \a argv. All argument
   /// strings will be copied into an internal buffers.
   //
   //  FIXME: Handle the quote character somehow.
   //------------------------------------------------------------------
-  void SetArguments(size_t argc, const char **argv);
-
-  void SetArguments(const char **argv);
+  void SetArguments(llvm::ArrayRef<const char *> args);
+  void SetArguments(const Args &other);
 
   //------------------------------------------------------------------
   /// Shifts the first argument C string value of the array off the
   /// argument array.
   ///
   /// The string value will be freed, so a copy of the string should
   /// be made by calling Args::GetArgumentAtIndex (size_t) const
   /// first and copying the returned value before calling
public:
 
   static char StringToChar(llvm::StringRef s, char fail_value,
                            bool *success_ptr);
 
   static int64_t StringToOptionEnum(llvm::StringRef s,
                                     OptionEnumValueElement *enum_values,
                                     int32_t fail_value, Error &error);
 
+  static llvm::ArrayRef<const char *> ArgvToArrayRef(const char **args);
+
   static lldb::ScriptLanguage
   StringToScriptLanguage(llvm::StringRef s, lldb::ScriptLanguage fail_value,
                          bool *success_ptr);
 
   // TODO: Use StringRef
   static Error StringToFormat(const char *s, lldb::Format &format,
                               size_t *byte_size_ptr); // If non-NULL, then a
                                                       // byte size can precede
public:
   //------------------------------------------------------------------
   bool ContainsEnvironmentVariable(llvm::StringRef env_var_name,
                                    size_t *argument_index = nullptr) const;
 
 protected:
   //------------------------------------------------------------------
   // Classes that inherit from Args can see and modify these
   //------------------------------------------------------------------
-  typedef std::list<std::string> arg_sstr_collection;
-  typedef std::vector<const char *> arg_cstr_collection;
+  typedef std::vector<std::string> arg_sstr_collection;
   typedef std::vector<char> arg_quote_char_collection;
   arg_sstr_collection m_args;
-  arg_cstr_collection m_argv; ///< The current argument vector.
   arg_quote_char_collection m_args_quote_char;
 
-  void UpdateArgsAfterOptionParsing();
-
-  void UpdateArgvFromArgs();
+  void UpdateArgsAfterOptionParsing(llvm::ArrayRef<const char *> new_argv);
 
   llvm::StringRef ParseSingleArgument(llvm::StringRef command);
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_Command_h_

source/API/SBCommandInterpreter.cpp

public:
 
   bool IsRemovable() const override { return true; }
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     SBCommandReturnObject sb_return(&result);
     SBCommandInterpreter sb_interpreter(&m_interpreter);
     SBDebugger debugger_sb(m_interpreter.GetDebugger().shared_from_this());
-    bool ret = m_backend->DoExecute(
-        debugger_sb, (char **)command.GetArgumentVector(), sb_return);
+    auto argv = command.GetArgumentVector();
+    bool ret = m_backend->DoExecute(debugger_sb, &argv[0], sb_return);
     sb_return.Release();
     return ret;
   }
   std::shared_ptr<lldb::SBCommandPluginInterface> m_backend;
 };
 
 SBCommandInterpreter::SBCommandInterpreter(CommandInterpreter *interpreter)
     : m_opaque_ptr(interpreter) {

source/API/SBLaunchInfo.cpp

 #include "lldb/Target/ProcessLaunchInfo.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 SBLaunchInfo::SBLaunchInfo(const char **argv)
     : m_opaque_sp(new ProcessLaunchInfo()) {
   m_opaque_sp->GetFlags().Reset(eLaunchFlagDebug | eLaunchFlagDisableASLR);
-  if (argv && argv[0])
-    m_opaque_sp->GetArguments().SetArguments(argv);
+  if (argv) {
+    m_opaque_sp->GetArguments().SetArguments(Args::ArgvToArrayRef(argv));
+  }
 }
 
 SBLaunchInfo::~SBLaunchInfo() {}
 
 lldb_private::ProcessLaunchInfo &SBLaunchInfo::ref() { return *m_opaque_sp; }
 
 const lldb_private::ProcessLaunchInfo &SBLaunchInfo::ref() const {
   return *m_opaque_sp;
uint32_t SBLaunchInfo::GetNumArguments() {
   return m_opaque_sp->GetArguments().GetArgumentCount();
 }
 
 const char *SBLaunchInfo::GetArgumentAtIndex(uint32_t idx) {
   return m_opaque_sp->GetArguments().GetArgumentAtIndex(idx);
 }
 
 void SBLaunchInfo::SetArguments(const char **argv, bool append) {
-  if (append) {
-    if (argv)
-      m_opaque_sp->GetArguments().AppendArguments(argv);
-  } else {
-    if (argv)
-      m_opaque_sp->GetArguments().SetArguments(argv);
-    else
-      m_opaque_sp->GetArguments().Clear();
-  }
+  if (!append)
+
+  if (argv)
+    m_opaque_sp->GetArguments().AppendArguments(Args::ArgvToArrayRef(argv));
 }
 
 uint32_t SBLaunchInfo::GetNumEnvironmentEntries() {
   return m_opaque_sp->GetEnvironmentEntries().GetArgumentCount();
 }
 
 const char *SBLaunchInfo::GetEnvironmentEntryAtIndex(uint32_t idx) {
   return m_opaque_sp->GetEnvironmentEntries().GetArgumentAtIndex(idx);
 }
 
 void SBLaunchInfo::SetEnvironmentEntries(const char **envp, bool append) {
-  if (append) {
-    if (envp)
-      m_opaque_sp->GetEnvironmentEntries().AppendArguments(envp);
-  } else {
-    if (envp)
-      m_opaque_sp->GetEnvironmentEntries().SetArguments(envp);
-    else
-      m_opaque_sp->GetEnvironmentEntries().Clear();
-  }
+  if (!append)
+
+  if (envp)
+    m_opaque_sp->GetEnvironmentEntries().AppendArguments(
+        Args::ArgvToArrayRef(envp));
 }
 
 void SBLaunchInfo::Clear() { m_opaque_sp->Clear(); }
 
 const char *SBLaunchInfo::GetWorkingDirectory() const {
   return m_opaque_sp->GetWorkingDirectory().GetCString();
 }
 

source/API/SBProcess.cpp

if (process_sp->GetState() == eStateConnected) {
       ProcessLaunchInfo launch_info(
           FileSpec{stdin_path, false}, FileSpec{stdout_path, false},
           FileSpec{stderr_path, false}, FileSpec{working_directory, false},
           launch_flags);
       Module *exe_module = process_sp->GetTarget().GetExecutableModulePointer();
       if (exe_module)
         launch_info.SetExecutableFile(exe_module->GetPlatformFileSpec(), true);
       if (argv)
-        launch_info.GetArguments().AppendArguments(argv);
+        launch_info.GetArguments().AppendArguments(Args::ArgvToArrayRef(argv));
       if (envp)
-        launch_info.GetEnvironmentEntries().SetArguments(envp);
+        launch_info.GetEnvironmentEntries().SetArguments(
+            Args::ArgvToArrayRef(envp));
       error.SetError(process_sp->Launch(launch_info));
     } else {
       error.SetErrorString("must be in eStateConnected to call RemoteLaunch");
     }
   } else {
     error.SetErrorString("unable to attach pid");
   }
 

source/API/SBTarget.cpp

ProcessLaunchInfo launch_info(
         FileSpec{stdin_path, false}, FileSpec{stdout_path, false},
         FileSpec{stderr_path, false}, FileSpec{working_directory, false},
         launch_flags);
 
     Module *exe_module = target_sp->GetExecutableModulePointer();
     if (exe_module)
       launch_info.SetExecutableFile(exe_module->GetPlatformFileSpec(), true);
     if (argv)
-      launch_info.GetArguments().AppendArguments(argv);
+      launch_info.GetArguments().AppendArguments(Args::ArgvToArrayRef(argv));
     if (envp)
-      launch_info.GetEnvironmentEntries().SetArguments(envp);
+      launch_info.GetEnvironmentEntries().SetArguments(
+          Args::ArgvToArrayRef(envp));
 
     if (listener.IsValid())
       launch_info.SetListener(listener.GetSP());
 
     error.SetError(target_sp->Launch(launch_info, NULL));
 
     sb_process.SetSP(target_sp->GetProcessSP());
   } else {

source/Commands/CommandObjectBreakpoint.cpp

void CommandObjectMultiwordBreakpoint::VerifyIDs(Args &args, Target *target,
   // strings into TEMP_ARGS.
 
   BreakpointIDList::FindAndReplaceIDRanges(args, target, allow_locations,
                                            result, temp_args);
 
   // NOW, convert the list of breakpoint id strings in TEMP_ARGS into an actual
   // BreakpointIDList:
 
-  valid_ids->InsertStringArray(temp_args.GetConstArgumentVector(),
-                               temp_args.GetArgumentCount(), result);
+  auto argv = temp_args.GetArgumentVector();
+  valid_ids->InsertStringArray(&argv[0], temp_args.GetArgumentCount(), result);
 
   // At this point,  all of the breakpoint ids that the user passed in have been
   // converted to breakpoint IDs
   // and put into valid_ids.
 
   if (result.Succeeded()) {
     // Now that we've converted everything from args into a list of breakpoint
     // ids, go through our tentative list

source/Commands/CommandObjectLog.cpp

bool DoExecute(Args &args, CommandReturnObject &result) override {
     } else {
       std::string channel(args.GetArgumentAtIndex(0));
       args.Shift(); // Shift off the channel
       char log_file[PATH_MAX];
       if (m_options.log_file)
         m_options.log_file.GetPath(log_file, sizeof(log_file));
       else
         log_file[0] = '\0';
+      auto argv = args.GetArgumentVector();
       bool success = m_interpreter.GetDebugger().EnableLog(
-          channel.c_str(), args.GetConstArgumentVector(), log_file,
-          m_options.log_options, result.GetErrorStream());
+          channel.c_str(), &argv[0], log_file, m_options.log_options,
+          result.GetErrorStream());
       if (success)
         result.SetStatus(eReturnStatusSuccessFinishNoResult);
       else
         result.SetStatus(eReturnStatusFailed);
     }
     return result.Succeeded();
   }
 
if (argc == 0) {
       result.AppendErrorWithFormat(
           "%s takes a log channel and one or more log types.\n",
           m_cmd_name.c_str());
     } else {
       Log::Callbacks log_callbacks;
 
       std::string channel(args.GetArgumentAtIndex(0));
       args.Shift(); // Shift off the channel
+      auto argv = args.GetArgumentVector();
+
       if (Log::GetLogChannelCallbacks(ConstString(channel.c_str()),
                                       log_callbacks)) {
-        log_callbacks.disable(args.GetConstArgumentVector(),
-                              &result.GetErrorStream());
+        log_callbacks.disable(&argv[0], &result.GetErrorStream());
         result.SetStatus(eReturnStatusSuccessFinishNoResult);
       } else if (channel == "all") {
         Log::DisableAllLogChannels(&result.GetErrorStream());
       } else {
         LogChannelSP log_channel_sp(LogChannel::FindPlugin(channel.c_str()));
         if (log_channel_sp) {
-          log_channel_sp->Disable(args.GetConstArgumentVector(),
-                                  &result.GetErrorStream());
+          log_channel_sp->Disable(&argv[0], &result.GetErrorStream());
           result.SetStatus(eReturnStatusSuccessFinishNoResult);
         } else
           result.AppendErrorWithFormat("Invalid log channel '%s'.\n",
                                        args.GetArgumentAtIndex(0));
       }
     }
     return result.Succeeded();
   }

source/Host/common/OptionParser.cpp

 #else
   optreset = 1;
   optind = 1;
 #endif
 }
 
 void OptionParser::EnableError(bool error) { opterr = error ? 1 : 0; }
 
-int OptionParser::Parse(int argc, char *const argv[], const char *optstring,
-                        const Option *longopts, int *longindex) {
+int OptionParser::Parse(int argc, char const *const argv[],
+                        const char *optstring, const Option *longopts,
+                        int *longindex) {
   std::vector<option> opts;
   while (longopts->definition != nullptr) {
     option opt;
     opt.flag = longopts->flag;
     opt.val = longopts->val;
     opt.name = longopts->definition->long_option;
     opt.has_arg = longopts->definition->option_has_arg;
     opts.push_back(opt);
     ++longopts;
   }
   opts.push_back(option());
-  return getopt_long_only(argc, argv, optstring, &opts[0], longindex);
+  return getopt_long_only(argc, const_cast<char *const *>(argv), optstring,
+                          &opts[0], longindex);
 }
 
 char *OptionParser::GetOptionArgument() { return optarg; }
 
 int OptionParser::GetOptionIndex() { return optind; }
 
 int OptionParser::GetOptionErrorCause() { return optopt; }
 

source/Interpreter/Args.cpp

 #include "lldb/Interpreter/Args.h"
 #include "lldb/Interpreter/CommandInterpreter.h"
 #include "lldb/Interpreter/CommandReturnObject.h"
 #include "lldb/Interpreter/Options.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StackFrame.h"
 #include "lldb/Target/Target.h"
 
+#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 //----------------------------------------------------------------------
 // Args constructor
 //----------------------------------------------------------------------
-Args::Args(llvm::StringRef command) : m_args(), m_argv(), m_args_quote_char() {
+Args::Args(llvm::StringRef command) : m_args(), m_args_quote_char() {
   SetCommandString(command);
 }
 
-//----------------------------------------------------------------------
-// We have to be very careful on the copy constructor of this class
-// to make sure we copy all of the string values, but we can't copy the
-// rhs.m_argv into m_argv since it will point to the "const char *" c
-// strings in rhs.m_args. We need to copy the string list and update our
-// own m_argv appropriately.
-//----------------------------------------------------------------------
 Args::Args(const Args &rhs)
-    : m_args(rhs.m_args), m_argv(), m_args_quote_char(rhs.m_args_quote_char) {
-  UpdateArgvFromArgs();
-}
+    : m_args(rhs.m_args), m_args_quote_char(rhs.m_args_quote_char) {}
 
-//----------------------------------------------------------------------
-// We have to be very careful on the copy constructor of this class
-// to make sure we copy all of the string values, but we can't copy the
-// rhs.m_argv into m_argv since it will point to the "const char *" c
-// strings in rhs.m_args. We need to copy the string list and update our
-// own m_argv appropriately.
-//----------------------------------------------------------------------
 const Args &Args::operator=(const Args &rhs) {
   // Make sure we aren't assigning to self
   if (this != &rhs) {
     m_args = rhs.m_args;
     m_args_quote_char = rhs.m_args_quote_char;
-    UpdateArgvFromArgs();
   }
   return *this;
 }
 
 //----------------------------------------------------------------------
 // Destructor
 //----------------------------------------------------------------------
 Args::~Args() {}
 
 void Args::Dump(Stream &s, const char *label_name) const {
   if (!label_name)
     return;
 
-  const size_t argc = m_argv.size();
-  for (size_t i = 0; i < argc; ++i) {
+  for (int i = 0; i < m_args.size(); ++i) {
     s.Indent();
-    const char *arg_cstr = m_argv[i];
-    if (arg_cstr)
-      s.Printf("%s[%zi]=\"%s\"\n", label_name, i, arg_cstr);
-    else
-      s.Printf("%s[%zi]=NULL\n", label_name, i);
+    s.Printf("%s[%zi]=\"%s\"\n", label_name, i, m_args[i].c_str());
   }
   s.EOL();
 }
 
 bool Args::GetCommandString(std::string &command) const {
-  command.clear();
-  const size_t argc = GetArgumentCount();
-  for (size_t i = 0; i < argc; ++i) {
-    if (i > 0)
-      command += ' ';
-    command += m_argv[i];
-  }
-  return argc > 0;
+  command = llvm::join(m_args.begin(), m_args.end(), " ");
+
+  return !m_args.empty();
 }
 
 bool Args::GetQuotedCommandString(std::string &command) const {
   command.clear();
   const size_t argc = GetArgumentCount();
   for (size_t i = 0; i < argc; ++i) {
     if (i > 0)
-      command.append(1, ' ');
+      command += ' ';
     char quote_char = GetArgumentQuoteCharAtIndex(i);
     if (quote_char) {
-      command.append(1, quote_char);
-      command.append(m_argv[i]);
-      command.append(1, quote_char);
+      command += quote_char;
+      command += m_args[i];
+      command += quote_char;
     } else
-      command.append(m_argv[i]);
+      command += m_args[i];
   }
   return argc > 0;
 }
 
 // A helper function for argument parsing.
 // Parses the initial part of the first argument using normal double quote
 // rules:
 // backslash escapes the double quote and itself. The parsed string is appended
llvm::StringRef Args::ParseSingleArgument(llvm::StringRef command) {
 
   m_args.push_back(arg);
   m_args_quote_char.push_back(first_quote_char);
   return command;
 }
 
 void Args::SetCommandString(llvm::StringRef command) {
   m_args.clear();
-  m_argv.clear();
   m_args_quote_char.clear();
 
   static const char *k_space_separators = " \t";
   command = command.ltrim(k_space_separators);
   while (!command.empty()) {
     command = ParseSingleArgument(command);
     command = command.ltrim(k_space_separators);
   }
-
-  UpdateArgvFromArgs();
-}
-
-void Args::UpdateArgsAfterOptionParsing() {
-  // Now m_argv might be out of date with m_args, so we need to fix that
-  arg_cstr_collection::const_iterator argv_pos, argv_end = m_argv.end();
-  arg_sstr_collection::iterator args_pos;
-  arg_quote_char_collection::iterator quotes_pos;
-
-  for (argv_pos = m_argv.begin(), args_pos = m_args.begin(),
-      quotes_pos = m_args_quote_char.begin();
-       argv_pos != argv_end && args_pos != m_args.end(); ++argv_pos) {
-    const char *argv_cstr = *argv_pos;
-    if (argv_cstr == nullptr)
-      break;
-
-    while (args_pos != m_args.end()) {
-      const char *args_cstr = args_pos->c_str();
-      if (args_cstr == argv_cstr) {
-        // We found the argument that matches the C string in the
-        // vector, so we can now look for the next one
-        ++args_pos;
-        ++quotes_pos;
-        break;
-      } else {
-        quotes_pos = m_args_quote_char.erase(quotes_pos);
-        args_pos = m_args.erase(args_pos);
-      }
-    }
-  }
-
-  if (args_pos != m_args.end())
-    m_args.erase(args_pos, m_args.end());
 
-  if (quotes_pos != m_args_quote_char.end())
-    m_args_quote_char.erase(quotes_pos, m_args_quote_char.end());
-}
-
-void Args::UpdateArgvFromArgs() {
-  m_argv.clear();
-  arg_sstr_collection::const_iterator pos, end = m_args.end();
-  for (pos = m_args.begin(); pos != end; ++pos)
-    m_argv.push_back(pos->c_str());
-  m_argv.push_back(nullptr);
-  // Make sure we have enough arg quote chars in the array
-  if (m_args_quote_char.size() < m_args.size())
-    m_args_quote_char.resize(m_argv.size());
+void Args::UpdateArgsAfterOptionParsing(llvm::ArrayRef<const char *> new_argv) {
+  // By now many of the arguments will have been removed since they were
+  // consumed. Some options will remain and be potentially in a different order
+  // than they started in due to the behavior of getopt_long_only.  For the
+  // options that remain, map them back to their original location so we can
+  // determine their quote char.  Then move their quote char to the correct new
+  // location.
+  for (int i = 0; i < new_argv.size(); ++i) {
+    auto iter = std::find(m_args.begin(), m_args.end(), new_argv[i]);
+    lldbassert(iter != m_args.end());
+    size_t pos = std::distance(m_args.begin(), iter);
+    std::swap(m_args_quote_char[pos], m_args_quote_char[i]);
+  }
+  m_args_quote_char.resize(new_argv.size());
+
+  // Since new_argv is a collection of *references* to strings from m_args, we
+  // can't simply assign it directly, as saying m_args[x] = new_argv[y] would
+  // invalidate the existing value of m_args[x], which is itself another pointer
+  // somewhere in new_argv.  So make a copy and then swap it in.
+  std::vector<std::string> new_args(new_argv.begin(), new_argv.end());
+  std::swap(m_args, new_args);
 }
 
 size_t Args::GetArgumentCount() const {
-  if (m_argv.empty())
+  if (m_args.empty())
     return 0;
-  return m_argv.size() - 1;
+  return m_args.size();
 }
 
 const char *Args::GetArgumentAtIndex(size_t idx) const {
-  if (idx < m_argv.size())
-    return m_argv[idx];
+  if (idx < m_args.size())
+    return m_args[idx].c_str();
   return nullptr;
 }
 
 char Args::GetArgumentQuoteCharAtIndex(size_t idx) const {
   if (idx < m_args_quote_char.size())
     return m_args_quote_char[idx];
   return '\0';
 }
 
-char **Args::GetArgumentVector() {
-  if (!m_argv.empty())
-    return const_cast<char **>(&m_argv[0]);
-  return nullptr;
-}
-
-const char **Args::GetConstArgumentVector() const {
-  if (!m_argv.empty())
-    return const_cast<const char **>(&m_argv[0]);
-  return nullptr;
+std::vector<const char *> Args::GetArgumentVector() const {
+  std::vector<const char *> result;
+  for (auto &arg : m_args)
+    result.push_back(arg.c_str());
+
+  // This is kind of gross, but it ensures that if someone passes args.data() to
+  // a function which expects an array terminated with a null entry, it will
+  // work.
+  result.push_back(nullptr);
+  result.pop_back();
+  return result;
 }
 
 void Args::Shift() {
-  // Don't pop the last NULL terminator from the argv array
-  if (m_argv.size() > 1) {
-    m_argv.erase(m_argv.begin());
-    m_args.pop_front();
-    if (!m_args_quote_char.empty())
-      m_args_quote_char.erase(m_args_quote_char.begin());
-  }
-}
+  if (m_args.empty())
+    return;
+
+  m_args.erase(m_args.begin());
 
 llvm::StringRef Args::Unshift(llvm::StringRef arg_str, char quote_char) {
-  m_args.push_front(arg_str);
-  m_argv.insert(m_argv.begin(), m_args.front().c_str());
+  m_args.insert(m_args.begin(), arg_str);
   m_args_quote_char.insert(m_args_quote_char.begin(), quote_char);
-  return llvm::StringRef::withNullAsEmpty(GetArgumentAtIndex(0));
+  return m_args[0];
 }
 
 void Args::AppendArguments(const Args &rhs) {
   const size_t rhs_argc = rhs.GetArgumentCount();
   for (size_t i = 0; i < rhs_argc; ++i)
-    AppendArgument(llvm::StringRef(rhs.GetArgumentAtIndex(i)),
+    AppendArgument(rhs.GetArgumentAtIndex(i),
                    rhs.GetArgumentQuoteCharAtIndex(i));
 }
 
-void Args::AppendArguments(const char **argv) {
-  if (argv) {
-    for (uint32_t i = 0; argv[i]; ++i)
-      AppendArgument(llvm::StringRef::withNullAsEmpty(argv[i]));
-  }
+void Args::AppendArguments(llvm::ArrayRef<const char *> args) {
+  for (uint32_t i = 0; args[i]; ++i)
+    AppendArgument(args[i]);
 }
 
 llvm::StringRef Args::AppendArgument(llvm::StringRef arg_str, char quote_char) {
   return InsertArgumentAtIndex(GetArgumentCount(), arg_str, quote_char);
 }
 
 llvm::StringRef Args::InsertArgumentAtIndex(size_t idx, llvm::StringRef arg_str,
                                             char quote_char) {
-  // Since we are using a std::list to hold onto the copied C string and
-  // we don't have direct access to the elements, we have to iterate to
-  // find the value.
-  arg_sstr_collection::iterator pos, end = m_args.end();
-  size_t i = idx;
-  for (pos = m_args.begin(); i > 0 && pos != end; ++pos)
-    --i;
-
-  pos = m_args.insert(pos, arg_str);
+  m_args.insert(m_args.begin() + idx, arg_str);
 
   if (idx >= m_args_quote_char.size()) {
     m_args_quote_char.resize(idx + 1);
     m_args_quote_char[idx] = quote_char;
   } else
     m_args_quote_char.insert(m_args_quote_char.begin() + idx, quote_char);
 
-  UpdateArgvFromArgs();
   return GetArgumentAtIndex(idx);
 }
 
 llvm::StringRef Args::ReplaceArgumentAtIndex(size_t idx,
                                              llvm::StringRef arg_str,
                                              char quote_char) {
-  // Since we are using a std::list to hold onto the copied C string and
-  // we don't have direct access to the elements, we have to iterate to
-  // find the value.
-  arg_sstr_collection::iterator pos, end = m_args.end();
-  size_t i = idx;
-  for (pos = m_args.begin(); i > 0 && pos != end; ++pos)
-    --i;
-
-  if (pos != end) {
-    pos->assign(arg_str);
-    assert(idx < m_argv.size() - 1);
-    m_argv[idx] = pos->c_str();
-    if (idx >= m_args_quote_char.size())
-      m_args_quote_char.resize(idx + 1);
-    m_args_quote_char[idx] = quote_char;
-    return GetArgumentAtIndex(idx);
-  }
-  return llvm::StringRef();
-}
+  if (idx >= m_args.size())
+    return llvm::StringRef();
+
+  m_args[idx] = arg_str;
 
 void Args::DeleteArgumentAtIndex(size_t idx) {
-  // Since we are using a std::list to hold onto the copied C string and
-  // we don't have direct access to the elements, we have to iterate to
-  // find the value.
-  arg_sstr_collection::iterator pos, end = m_args.end();
-  size_t i = idx;
-  for (pos = m_args.begin(); i > 0 && pos != end; ++pos)
-    --i;
-
-  if (pos != end) {
-    m_args.erase(pos);
-    assert(idx < m_argv.size() - 1);
-    m_argv.erase(m_argv.begin() + idx);
-    if (idx < m_args_quote_char.size())
-      m_args_quote_char.erase(m_args_quote_char.begin() + idx);
-  }
-}
+  if (idx >= m_args.size())
+    return;
+
+  m_args.erase(m_args.begin() + idx);
 
-void Args::SetArguments(size_t argc, const char **argv) {
-  // m_argv will be rebuilt in UpdateArgvFromArgs() below, so there is
-  // no need to clear it here.
+void Args::SetArguments(llvm::ArrayRef<const char *> args) {
   m_args.clear();
   m_args_quote_char.clear();
 
-  // First copy each string
-  for (size_t i = 0; i < argc; ++i) {
-    m_args.push_back(argv[i]);
-    if ((argv[i][0] == '\'') || (argv[i][0] == '"') || (argv[i][0] == '`'))
-      m_args_quote_char.push_back(argv[i][0]);
-    else
-      m_args_quote_char.push_back('\0');
-  }
-
-  UpdateArgvFromArgs();
-}
-
-void Args::SetArguments(const char **argv) {
-  // m_argv will be rebuilt in UpdateArgvFromArgs() below, so there is
-  // no need to clear it here.
-  m_args.clear();
-  m_args_quote_char.clear();
-
-  if (argv) {
-    // First copy each string
-    for (size_t i = 0; argv[i]; ++i) {
-      m_args.push_back(argv[i]);
-      if ((argv[i][0] == '\'') || (argv[i][0] == '"') || (argv[i][0] == '`'))
-        m_args_quote_char.push_back(argv[i][0]);
-      else
-        m_args_quote_char.push_back('\0');
-    }
-  }
+  for (const char *arg : args) {
+    m_args.push_back(arg);
+    if ((arg[0] == '\'') || (arg[0] == '"') || (arg[0] == '`'))
+      m_args_quote_char.push_back(arg[0]);
 
-  UpdateArgvFromArgs();
+void Args::SetArguments(const Args &other) {
+  m_args = other.m_args;
+  m_args_quote_char = other.m_args_quote_char;
 }
 
 Error Args::ParseOptions(Options &options, ExecutionContext *execution_context,
                          PlatformSP platform_sp, bool require_validation) {
   StreamString sstr;
   Error error;
   Option *long_options = options.GetLongOptions();
   if (long_options == nullptr) {
     error.SetErrorStringWithFormat("invalid long options");
if (long_options[i].flag == nullptr) {
           break;
         }
       }
     }
   }
   std::unique_lock<std::mutex> lock;
   OptionParser::Prepare(lock);
   int val;
+  auto args = GetArgumentVector();
+  args.insert(args.begin(), "dummy");
   while (1) {
     int long_options_index = -1;
-    val =
-        OptionParser::Parse(GetArgumentCount(), GetArgumentVector(),
-                            sstr.GetData(), long_options, &long_options_index);
+    val = OptionParser::Parse(args.size(), &args[0], sstr.GetData(),
+                              long_options, &long_options_index);
     if (val == -1)
       break;
 
     // Did we get an error?
     if (val == '?') {
       error.SetErrorStringWithFormat("unknown or ambiguous option");
       break;
     }
if (long_options_index >= 0 &&
             execution_context);
     } else {
       error.SetErrorStringWithFormat("invalid option with value '%i'", val);
     }
     if (error.Fail())
       break;
   }
 
-  // Update our ARGV now that get options has consumed all the options
-  m_argv.erase(m_argv.begin(), m_argv.begin() + OptionParser::GetOptionIndex());
-  UpdateArgsAfterOptionParsing();
+  args.erase(args.begin(), args.begin() + OptionParser::GetOptionIndex());
+  UpdateArgsAfterOptionParsing(llvm::makeArrayRef(args));
+
   return error;
 }
 
 void Args::Clear() {
   m_args.clear();
-  m_argv.clear();
   m_args_quote_char.clear();
 }
 
 lldb::addr_t Args::StringToAddress(const ExecutionContext *exe_ctx,
                                    const char *s, lldb::addr_t fail_value,
                                    Error *error_ptr) {
   bool error_set = false;
   if (s && s[0]) {
int64_t Args::StringToOptionEnum(llvm::StringRef s,
   strm.PutCString("invalid enumeration value, valid values are: ");
   for (int i = 0; enum_values[i].string_value != nullptr; i++) {
     strm.Printf("%s\"%s\"", i > 0 ? ", " : "", enum_values[i].string_value);
   }
   error.SetErrorString(strm.GetData());
   return fail_value;
 }
 
+llvm::ArrayRef<const char *> Args::ArgvToArrayRef(const char **args) {
+  int argc = 0;
+  const char **argv2 = args;
+  while (*argv2)
+    ++argc;
+  return llvm::ArrayRef<const char *>(args, argc);
+}
+
 lldb::ScriptLanguage
 Args::StringToScriptLanguage(llvm::StringRef s, lldb::ScriptLanguage fail_value,
                              bool *success_ptr) {
   if (success_ptr)
     *success_ptr = true;
 
   if (s.equals_lower("python"))
     return eScriptLanguagePython;
if (long_options[i].flag == nullptr) {
         break;
       }
     }
   }
 
   std::unique_lock<std::mutex> lock;
   OptionParser::Prepare(lock);
   int val;
+  const auto args = GetArgumentVector();
   while (1) {
     int long_options_index = -1;
-    val =
-        OptionParser::Parse(GetArgumentCount(), GetArgumentVector(),
-                            sstr.GetData(), long_options, &long_options_index);
+    val = OptionParser::Parse(args.size(), &args[0], sstr.GetData(),
+                              long_options, &long_options_index);
 
     if (val == -1)
       break;
 
     if (val == '?') {
       result.AppendError("unknown or ambiguous option");
       result.SetStatus(eReturnStatusFailed);
       break;
void Args::ParseArgsForCompletion(Options &options,
 
   std::unique_lock<std::mutex> lock;
   OptionParser::Prepare(lock);
   OptionParser::EnableError(false);
 
   int val;
   auto opt_defs = options.GetDefinitions();
 
-  // Fooey... OptionParser::Parse permutes the GetArgumentVector to move the
-  // options to the front.
-  // So we have to build another Arg and pass that to OptionParser::Parse so it
-  // doesn't
-  // change the one we have.
-
-  std::vector<const char *> dummy_vec(
-      GetArgumentVector(), GetArgumentVector() + GetArgumentCount() + 1);
-
+  const auto dummy_vec = GetArgumentVector();
   bool failed_once = false;
   uint32_t dash_dash_pos = -1;
 
   while (1) {
     bool missing_argument = false;
     int long_options_index = -1;
 
-    val = OptionParser::Parse(
-        dummy_vec.size() - 1, const_cast<char *const *>(&dummy_vec.front()),
-        sstr.GetData(), long_options, &long_options_index);
+    val = OptionParser::Parse(dummy_vec.size(), &dummy_vec[0], sstr.GetData(),
+                              long_options, &long_options_index);
 
     if (val == -1) {
       // When we're completing a "--" which is the last option on line,
       if (failed_once)
         break;
 
       failed_once = true;
 

source/Interpreter/CommandInterpreter.cpp

if (option_arg_vector_sp.get()) {
     }
 
     for (size_t j = 0; j < cmd_args.GetArgumentCount(); ++j) {
       if (!used[j] && !wants_raw_input)
         new_args.AppendArgument(
             llvm::StringRef(cmd_args.GetArgumentAtIndex(j)));
     }
 
     cmd_args.Clear();
-    cmd_args.SetArguments(new_args.GetArgumentCount(),
-                          new_args.GetConstArgumentVector());
+    cmd_args.SetArguments(new_args);
   } else {
     result.SetStatus(eReturnStatusSuccessFinishNoResult);
     // This alias was not created with any options; nothing further needs to be
     // done, unless it is a command that
     // wants raw input, in which case we need to clear the rest of the data from
     // cmd_args, since its in the raw
     // input string.
     if (wants_raw_input) {
       cmd_args.Clear();
-      cmd_args.SetArguments(new_args.GetArgumentCount(),
-                            new_args.GetConstArgumentVector());
+      cmd_args.SetArguments(new_args);
     }
     return;
   }
 
   result.SetStatus(eReturnStatusSuccessFinishNoResult);
   return;
 }
 

source/Interpreter/CommandObject.cpp

if (options != nullptr) {
     Error error;
 
     auto exe_ctx = GetCommandInterpreter().GetExecutionContext();
     options->NotifyOptionParsingStarting(&exe_ctx);
 
     // ParseOptions calls getopt_long_only, which always skips the zero'th item
     // in the array and starts at position 1,
     // so we need to push a dummy value into position zero.
-    args.Unshift(llvm::StringRef("dummy_string"));
     const bool require_validation = true;
     error = args.ParseOptions(*options, &exe_ctx,
                               GetCommandInterpreter().GetPlatform(true),
                               require_validation);
 
     // The "dummy_string" will have already been removed by ParseOptions,
     // so no need to remove it.
 

 
 bool CommandObjectParsed::Execute(const char *args_string,
                                   CommandReturnObject &result) {
   bool handled = false;
   Args cmd_args(args_string);
   if (HasOverrideCallback()) {
     Args full_args(GetCommandName());
     full_args.AppendArguments(cmd_args);
-    handled =
-        InvokeOverrideCallback(full_args.GetConstArgumentVector(), result);
+
+    auto argv = full_args.GetArgumentVector();
+    handled = InvokeOverrideCallback(&argv[0], result);
   }
   if (!handled) {
     for (size_t i = 0; i < cmd_args.GetArgumentCount(); ++i) {
       const char *tmp_str = cmd_args.GetArgumentAtIndex(i);
       if (tmp_str[0] == '`') // back-quote
         cmd_args.ReplaceArgumentAtIndex(
             i, llvm::StringRef::withNullAsEmpty(
                    m_interpreter.ProcessEmbeddedScriptCommands(tmp_str)));

source/Interpreter/OptionValueArgs.cpp

for (uint32_t i = 0; i < size; ++i) {
     const char *string_value = m_values[i]->GetStringValue();
     if (string_value)
       argv.push_back(string_value);
   }
 
   if (argv.empty())
     args.Clear();
   else
-    args.SetArguments(argv.size(), &argv[0]);
+    args.SetArguments(argv);
   return args.GetArgumentCount();
 }

source/Interpreter/OptionValueArray.cpp

for (uint32_t i = 0; i < size; ++i) {
     const char *string_value = m_values[i]->GetStringValue();
     if (string_value)
       argv.push_back(string_value);
   }
 
   if (argv.empty())
     args.Clear();
   else
-    args.SetArguments(argv.size(), &argv[0]);
+    args.SetArguments(argv);
   return args.GetArgumentCount();
 }
 
 Error OptionValueArray::SetArgs(const Args &args, VarSetOperationType op) {
   Error error;
   const size_t argc = args.GetArgumentCount();
   switch (op) {
   case eVarSetOperationInvalid:

source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

 }
 
 int32_t
 PlatformDarwin::GetResumeCountForLaunchInfo(ProcessLaunchInfo &launch_info) {
   const FileSpec &shell = launch_info.GetShell();
   if (!shell)
     return 1;
 
+  // CLEANUP: This function could be made nicer / safer with StringRef.
   std::string shell_string = shell.GetPath();
   const char *shell_name = strrchr(shell_string.c_str(), '/');
   if (shell_name == NULL)
     shell_name = shell_string.c_str();
   else
     shell_name++;
 
   if (strcmp(shell_name, "sh") == 0) {
     // /bin/sh re-exec's itself as /bin/bash requiring another resume.
     // But it only does this if the COMMAND_MODE environment variable
     // is set to "legacy".
-    const char **envp =
-        launch_info.GetEnvironmentEntries().GetConstArgumentVector();
-    if (envp != NULL) {
-      for (int i = 0; envp[i] != NULL; i++) {
-        if (strcmp(envp[i], "COMMAND_MODE=legacy") == 0)
-          return 2;
-      }
-    }
+    const auto argv = launch_info.GetEnvironmentEntries().GetArgumentVector();
+    for (auto env_entry : argv) {
+      if (strcmp(env_entry, "COMMAND_MODE=legacy") == 0)
     return 1;
   } else if (strcmp(shell_name, "csh") == 0 ||
              strcmp(shell_name, "tcsh") == 0 ||
              strcmp(shell_name, "zsh") == 0) {
     // csh and tcsh always seem to re-exec themselves.
     return 2;
   } else
     return 1;

source/Plugins/Platform/gdb-server/PlatformRemoteGDBServer.cpp

m_gdb_client.SetDetachOnError(
       launch_info.GetFlags().Test(eLaunchFlagDetachOnError));
 
   FileSpec working_dir = launch_info.GetWorkingDirectory();
   if (working_dir) {
     m_gdb_client.SetWorkingDir(working_dir);
   }
 
   // Send the environment and the program + arguments after we connect
-  const char **envp =
-      launch_info.GetEnvironmentEntries().GetConstArgumentVector();
+  const auto argv = launch_info.GetEnvironmentEntries().GetArgumentVector();
 
-  if (envp) {
-    const char *env_entry;
-    for (int i = 0; (env_entry = envp[i]); ++i) {
-      if (m_gdb_client.SendEnvironmentPacket(env_entry) != 0)
-        break;
-    }
-  }
+  for (auto env_entry : argv) {
 
   ArchSpec arch_spec = launch_info.GetArchitecture();
   const char *arch_triple = arch_spec.GetTriple().str().c_str();
 
   m_gdb_client.SendLaunchArchPacket(arch_triple);
   if (log)
     log->Printf(
         "PlatformRemoteGDBServer::%s() set launch architecture triple to '%s'",

source/Target/ProcessInfo.cpp

void ProcessInfo::SetArg0(const char *arg) {
   if (arg && arg[0])
     m_arg0 = arg;
   else
     m_arg0.clear();
 }
 
 void ProcessInfo::SetArguments(char const **argv,
                                bool first_arg_is_executable) {
-  m_arguments.SetArguments(argv);
+  m_arguments.SetArguments(Args::ArgvToArrayRef(argv));
 
   // Is the first argument the executable?
   if (first_arg_is_executable) {
     const char *first_arg = m_arguments.GetArgumentAtIndex(0);
     if (first_arg) {
       // Yes the first argument is an executable, set it as the executable
       // in the launch options. Don't resolve the file path as the path
       // could be a remote platform path

source/Target/ProcessLaunchInfo.cpp

bool ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell(
     Error &error, bool localhost, bool will_debug,
     bool first_arg_is_full_shell_command, int32_t num_resumes) {
   error.Clear();
 
   if (GetFlags().Test(eLaunchFlagLaunchInShell)) {
     if (m_shell) {
       std::string shell_executable = m_shell.GetPath();
 
-      const char **argv = GetArguments().GetConstArgumentVector();
-      if (argv == nullptr || argv[0] == nullptr)
+      const auto argv = GetArguments().GetArgumentVector();
+      if (!argv[0])
         return false;
       Args shell_arguments;
       std::string safe_arg;
       shell_arguments.AppendArgument(shell_executable);
       const llvm::Triple &triple = GetArchitecture().GetTriple();
       if (triple.getOS() == llvm::Triple::Win32 &&
           !triple.isWindowsCygwinEnvironment())
         shell_arguments.AppendArgument(llvm::StringRef("/C"));