Move Args::StringToAddress to Target::EvaluateAddressExpression

[labath]

Summary

StringToAddress could end up evaluating an expression, which is a somewhat
unexpected behavior of a seemingly simple method. I rename it to
EvaluateAddressExpression and move it to the Target class, next to the standard
EvaluateExpression function.

This functionality is now invoked via
execution_context->GetTargetPtr()->EvaluateAddressExpression(). To avoid null
checking the target pointer everywhere, I've added the eCommandRequiresTarget
flag to the commands which didn't have it, and which did not seem to be useful
without a target (disassemble && target modules list).

[jingham]

I liked the fact that all these Argument input type string converters were all in one place, though arguably Args was not the right place. Among other things it made it easy to see that if you were adding a new command that took in an Address, AddressToString was the canonical way to do it. That's less obvious now that this one is a target function, and you would have to go look at another argument to see what it does. I wonder if this problem isn't better solved by moving all the parsing functions somewhere more appropriate (like CommandInterpreter?) and then it isn't surprising what they might do.

[labath]

Yes, that thought occurred to me almost immediately after I submitted this. I'll try to implement something like that instead.

Since this will be just a bunch of static functions with no state, I think we don't even have to put them into an existing class, but instead create a new file/class for holding just this. I just need to figure out a good name for that.

[labath]

This is a version that moves the StringTo*** functions to a new
"OptionArgParser" class. I'm not terribly proud of the name, but I couldn't
think of anything better -- we already have a OptionParser class, so I wanted
to make it clear that this is for parsing the *arguments* of the options.

I deliberately left out three functions out of this move (StringToVersion,
StringToGenericRegister and StringToVersion), as these are not used by the
Interpreter/Command libraries, but things like Host and ProcessGDBRemote, and
so I think they should have a different home.

Let me know what you think.

[jingham]

Except for the to -> To to keep consistent with all the other lldb function naming this looks fine.

Now that they are all together it's easy to see we haven't been consistent in these functions. We really should make ToFormat return the format & take an error reference, and have ToBoolean take an error so callers don't have to cons it up. But that's orthogonal to this patch.

include/lldb/Interpreter/OptionArgParser.h

18–19 ↗

(On Diff #138418)

Should be ToAddress.

[zturner]

I think having all parsing functions in a single place will just move the
layering problem elsewhere, since a bunch of conversion functions for
objects from various libraries will be mushed together into one place.

[jingham]

As long as these functions are used for parsing options args in individual CommandObject implementations, and don't get dragged lower in the stack, I don't think that's a problem. By the time you're getting to individual commands you are on top of pretty much everything else in lldb, and can expect to rely on whatever. And the value of having all the converters gathered in one place so people writing new commands can easily do the right thing when ingesting arguments more than justifies this gathering of disparate elements IMO.

[labath]

I agree with Jim. I deliberately put here only the types that are used for command parsing, since I presume these are the things that the Command/Interpreter modules will depend on (it's not fully clear to me where to draw the line between these two, so it may end up needing to be moved from one to the other in the future, but I think that future will be pretty far away).

I also thought about the case when something else, which is logically lower than Interpreter/Command needs the string conversion functions -- in this case we could put the conversion function in a different module, but still keep the function in OptionArgParser as a wrapper on top of that. This way you would still have all the argument parsing functions in one place, while the things in the other modules (which by definition will not be parsing option arguments, or else they would be in these modules), can call the lower level function directly.

Ultimately, solving the layering of string conversion functions is not my goal here. I would be fully satisfied with breaking even. My goal is to move this stuff out of the Args class, so it can be moved down to Utility, as it is one of the main causes that everything depends on Interpreter (and then everything else).

[labath]

s/toAddress/ToAddress/

[labath]

So, any objections to this patch?

[labath]

Jim, judging by the comment in https://reviews.llvm.org/D44306#1037587 you were fine with this patch except the naming part. Now that that's fixed, I hope this should be fine. Unless I hear from you, I am going to commit this so I can continue with re-layering the Args class.

[jingham]

Yes, with your updated naming, I am fine with this.

[labath]

Thank you.

PS: I will need someone to update the XCode project after this.

include/lldb/Interpreter/Args.h

static bool SInt64ValueIsValidForByteSize(int64_t sval64,
     if (total_byte_size == 8)
       return true;
 
     const int64_t max = ((int64_t)1 << (uint64_t)(total_byte_size * 8 - 1)) - 1;
     const int64_t min = ~(max);
     return min <= sval64 && sval64 <= max;
   }
 
-  static lldb::addr_t StringToAddress(const ExecutionContext *exe_ctx,
-                                      llvm::StringRef s,
-                                      lldb::addr_t fail_value, Status *error);
-
   static bool StringToBoolean(llvm::StringRef s, bool fail_value,
                               bool *success_ptr);
 
   static char StringToChar(llvm::StringRef s, char fail_value,
                            bool *success_ptr);
 
   static int64_t StringToOptionEnum(llvm::StringRef s,
                                     OptionEnumValueElement *enum_values,

include/lldb/Target/Target.h

void ApplyNameToBreakpoints(BreakpointName &bp_name);
   // If an expression is going to be run, then it should have a frame filled
   // in in th execution context.
   lldb::ExpressionResults EvaluateExpression(
       llvm::StringRef expression, ExecutionContextScope *exe_scope,
       lldb::ValueObjectSP &result_valobj_sp,
       const EvaluateExpressionOptions &options = EvaluateExpressionOptions(),
       std::string *fixed_expression = nullptr);
 
+  lldb::addr_t EvaluateAddressExpression(const ExecutionContext *exe_ctx,
+                                         llvm::StringRef expr,
+                                         lldb::addr_t fail_value,
+                                         Status *error_ptr);
+
   lldb::ExpressionVariableSP GetPersistentVariable(const ConstString &name);
 
   lldb::addr_t GetPersistentSymbol(const ConstString &name);
 
   //------------------------------------------------------------------
   // Target Stop Hooks
   //------------------------------------------------------------------
   class StopHook : public UserID {

source/Commands/CommandObjectBreakpoint.cpp

public:
 
     Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
                           ExecutionContext *execution_context) override {
       Status error;
       const int short_option = g_breakpoint_set_options[option_idx].short_option;
 
       switch (short_option) {
       case 'a': {
-        m_load_addr = Args::StringToAddress(execution_context, option_arg,
-                                            LLDB_INVALID_ADDRESS, &error);
+        if (Target *target = execution_context->GetTargetPtr()) {
+          m_load_addr = target->EvaluateAddressExpression(
+              execution_context, option_arg, LLDB_INVALID_ADDRESS, &error);
+        }
       } break;
 
       case 'A':
         m_all_files = true;
         break;
 
       case 'b':
         m_func_names.push_back(option_arg);
Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
           m_breakpoint_names.push_back(option_arg);
         else
           error.SetErrorStringWithFormat("Invalid breakpoint name: %s",
                                          option_arg.str().c_str());
         break;
       }
 
       case 'R': {
-        lldb::addr_t tmp_offset_addr;
-        tmp_offset_addr =
-            Args::StringToAddress(execution_context, option_arg, 0, &error);
+        if (Target *target = execution_context->GetTargetPtr()) {
+          lldb::addr_t tmp_offset_addr = target->EvaluateAddressExpression(
+              execution_context, option_arg, 0, &error);
           if (error.Success())
             m_offset_addr = tmp_offset_addr;
+        }
       } break;
 
       case 'O':
         m_exception_extra_args.AppendArgument("-O");
         m_exception_extra_args.AppendArgument(option_arg);
         break;
 
       case 'p':

source/Commands/CommandObjectDisassemble.cpp

if (option_arg.getAsInteger(0, num_instructions))
           option_arg.str().c_str());
     break;
 
   case 'b':
     show_bytes = true;
     break;
 
   case 's': {
-    start_addr = Args::StringToAddress(execution_context, option_arg,
-                                       LLDB_INVALID_ADDRESS, &error);
+    start_addr = execution_context->GetTargetRef().EvaluateAddressExpression(
+        execution_context, option_arg, LLDB_INVALID_ADDRESS, &error);
     if (start_addr != LLDB_INVALID_ADDRESS)
       some_location_specified = true;
   } break;
   case 'e': {
-    end_addr = Args::StringToAddress(execution_context, option_arg,
-                                     LLDB_INVALID_ADDRESS, &error);
+    end_addr = execution_context->GetTargetRef().EvaluateAddressExpression(
+        execution_context, option_arg, LLDB_INVALID_ADDRESS, &error);
     if (end_addr != LLDB_INVALID_ADDRESS)
       some_location_specified = true;
   } break;
 
   case 'n':
     func_name.assign(option_arg);
     some_location_specified = true;
     break;
if (execution_context) {
       auto target_sp =
           execution_context ? execution_context->GetTargetSP() : TargetSP();
       auto platform_sp = target_sp ? target_sp->GetPlatform() : PlatformSP();
       arch = Platform::GetAugmentedArchSpec(platform_sp.get(), option_arg);
     }
     break;
 
   case 'a': {
-    symbol_containing_addr = Args::StringToAddress(
+    symbol_containing_addr =
+        execution_context->GetTargetRef().EvaluateAddressExpression(
             execution_context, option_arg, LLDB_INVALID_ADDRESS, &error);
     if (symbol_containing_addr != LLDB_INVALID_ADDRESS) {
       some_location_specified = true;
     }
   } break;
 
   default:
     error.SetErrorStringWithFormat("unrecognized short option '%c'",
                                    short_option);

 
 CommandObjectDisassemble::CommandObjectDisassemble(
     CommandInterpreter &interpreter)
     : CommandObjectParsed(
           interpreter, "disassemble",
           "Disassemble specified instructions in the current target.  "
           "Defaults to the current function for the current thread and "
           "stack frame.",
-          "disassemble [<cmd-options>]"),
+          "disassemble [<cmd-options>]", eCommandRequiresTarget),
       m_options() {}
 
 CommandObjectDisassemble::~CommandObjectDisassemble() = default;
 
 bool CommandObjectDisassemble::DoExecute(Args &command,
                                          CommandReturnObject &result) {
   Target *target = m_interpreter.GetDebugger().GetSelectedTarget().get();
   if (target == nullptr) {

source/Commands/CommandObjectMemory.cpp

#undef ALL_KEYWORDS
 
     if (total_byte_size == 0) {
       total_byte_size = item_count * item_byte_size;
       if (total_byte_size == 0)
         total_byte_size = 32;
     }
 
     if (argc > 0)
-      addr = Args::StringToAddress(&m_exe_ctx, command[0].ref,
-                                   LLDB_INVALID_ADDRESS, &error);
+      addr = m_exe_ctx.GetTargetRef().EvaluateAddressExpression(
+          &m_exe_ctx, command[0].ref, LLDB_INVALID_ADDRESS, &error);
 
     if (addr == LLDB_INVALID_ADDRESS) {
       result.AppendError("invalid start address expression.");
       result.AppendError(error.AsCString());
       result.SetStatus(eReturnStatusFailed);
       return false;
     }
 
     if (argc == 2) {
-      lldb::addr_t end_addr = Args::StringToAddress(
+      lldb::addr_t end_addr =
+          m_exe_ctx.GetTargetRef().EvaluateAddressExpression(
               &m_exe_ctx, command[1].ref, LLDB_INVALID_ADDRESS, nullptr);
       if (end_addr == LLDB_INVALID_ADDRESS) {
         result.AppendError("invalid end address expression.");
         result.AppendError(error.AsCString());
         result.SetStatus(eReturnStatusFailed);
         return false;
       } else if (end_addr <= addr) {
         result.AppendErrorWithFormat(
             "end address (0x%" PRIx64
bool DoExecute(Args &command, CommandReturnObject &result) override {
     const size_t argc = command.GetArgumentCount();
 
     if (argc != 2) {
       result.AppendError("two addresses needed for memory find");
       return false;
     }
 
     Status error;
-    lldb::addr_t low_addr = Args::StringToAddress(&m_exe_ctx, command[0].ref,
-                                                  LLDB_INVALID_ADDRESS, &error);
+    lldb::addr_t low_addr = m_exe_ctx.GetTargetRef().EvaluateAddressExpression(
+        &m_exe_ctx, command[0].ref, LLDB_INVALID_ADDRESS, &error);
     if (low_addr == LLDB_INVALID_ADDRESS || error.Fail()) {
       result.AppendError("invalid low address");
       return false;
     }
-    lldb::addr_t high_addr = Args::StringToAddress(
+    lldb::addr_t high_addr = m_exe_ctx.GetTargetRef().EvaluateAddressExpression(
         &m_exe_ctx, command[1].ref, LLDB_INVALID_ADDRESS, &error);
     if (high_addr == LLDB_INVALID_ADDRESS || error.Fail()) {
       result.AppendError("invalid high address");
       return false;
     }
 
     if (high_addr <= low_addr) {
       result.AppendError(
StreamString buffer(
         Stream::eBinary,
         process->GetTarget().GetArchitecture().GetAddressByteSize(),
         process->GetTarget().GetArchitecture().GetByteOrder());
 
     OptionValueUInt64 &byte_size_value = m_format_options.GetByteSizeValue();
     size_t item_byte_size = byte_size_value.GetCurrentValue();
 
     Status error;
-    lldb::addr_t addr = Args::StringToAddress(&m_exe_ctx, command[0].ref,
-                                              LLDB_INVALID_ADDRESS, &error);
+    lldb::addr_t addr = m_exe_ctx.GetTargetRef().EvaluateAddressExpression(
+        &m_exe_ctx, command[0].ref, LLDB_INVALID_ADDRESS, &error);
 
     if (addr == LLDB_INVALID_ADDRESS) {
       result.AppendError("invalid address expression\n");
       result.AppendError(error.AsCString());
       result.SetStatus(eReturnStatusFailed);
       return false;
     }
 
bool DoExecute(Args &command, CommandReturnObject &result) override {
     if (argc == 0 || argc > 1) {
       result.AppendErrorWithFormat("%s takes an address expression",
                                    m_cmd_name.c_str());
       result.SetStatus(eReturnStatusFailed);
       return false;
     }
 
     Status error;
-    lldb::addr_t addr = Args::StringToAddress(&m_exe_ctx, command[0].ref,
-                                              LLDB_INVALID_ADDRESS, &error);
+    lldb::addr_t addr = m_exe_ctx.GetTargetRef().EvaluateAddressExpression(
+        &m_exe_ctx, command[0].ref, LLDB_INVALID_ADDRESS, &error);
 
     if (addr == LLDB_INVALID_ADDRESS) {
       result.AppendError("invalid address expression");
       result.AppendError(error.AsCString());
       result.SetStatus(eReturnStatusFailed);
       return false;
     }
 
if (process_sp) {
       const size_t argc = command.GetArgumentCount();
       if (argc > 1 || (argc == 0 && load_addr == LLDB_INVALID_ADDRESS)) {
         result.AppendErrorWithFormat("'%s' takes one argument:\nUsage: %s\n",
                                      m_cmd_name.c_str(), m_cmd_syntax.c_str());
         result.SetStatus(eReturnStatusFailed);
       } else {
         auto load_addr_str = command[0].ref;
         if (command.GetArgumentCount() == 1) {
-          load_addr = Args::StringToAddress(&m_exe_ctx, load_addr_str,
-                                            LLDB_INVALID_ADDRESS, &error);
+          load_addr = m_exe_ctx.GetTargetRef().EvaluateAddressExpression(
+              &m_exe_ctx, load_addr_str, LLDB_INVALID_ADDRESS, &error);
           if (error.Fail() || load_addr == LLDB_INVALID_ADDRESS) {
             result.AppendErrorWithFormat(
                 "invalid address argument \"%s\": %s\n", command[0].c_str(),
                 error.AsCString());
             result.SetStatus(eReturnStatusFailed);
           }
         }
 

source/Commands/CommandObjectSource.cpp

Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
         file_name = option_arg;
         break;
 
       case 'n':
         symbol_name = option_arg;
         break;
 
       case 'a': {
-        address = Args::StringToAddress(execution_context, option_arg,
-                                        LLDB_INVALID_ADDRESS, &error);
+        address = execution_context->GetTargetRef().EvaluateAddressExpression(
+            execution_context, option_arg, LLDB_INVALID_ADDRESS, &error);
       } break;
       case 's':
         modules.push_back(std::string(option_arg));
         break;
       default:
         error.SetErrorStringWithFormat("unrecognized short option '%c'",
                                        short_option);
         break;
Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
         file_name = option_arg;
         break;
 
       case 'n':
         symbol_name = option_arg;
         break;
 
       case 'a': {
-        address = Args::StringToAddress(execution_context, option_arg,
-                                        LLDB_INVALID_ADDRESS, &error);
+        address = execution_context->GetTargetRef().EvaluateAddressExpression(
+            execution_context, option_arg, LLDB_INVALID_ADDRESS, &error);
       } break;
       case 's':
         modules.push_back(std::string(option_arg));
         break;
 
       case 'b':
         show_bp_locs = true;
         break;

source/Commands/CommandObjectTarget.cpp

public:
     Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
                           ExecutionContext *execution_context) override {
       Status error;
 
       const int short_option = m_getopt_table[option_idx].val;
       if (short_option == 'g') {
         m_use_global_module_list = true;
       } else if (short_option == 'a') {
-        m_module_addr = Args::StringToAddress(execution_context, option_arg,
-                                              LLDB_INVALID_ADDRESS, &error);
+        m_module_addr =
+            execution_context->GetTargetRef().EvaluateAddressExpression(
+                execution_context, option_arg, LLDB_INVALID_ADDRESS, &error);
       } else {
         unsigned long width = 0;
         option_arg.getAsInteger(0, width);
         m_format_array.push_back(std::make_pair(short_option, width));
       }
       return error;
     }
 
public:
     bool m_use_global_module_list;
     lldb::addr_t m_module_addr;
   };
 
   CommandObjectTargetModulesList(CommandInterpreter &interpreter)
       : CommandObjectParsed(
             interpreter, "target modules list",
             "List current executable and dependent shared library images.",
-            "target modules list [<cmd-options>]"),
+            "target modules list [<cmd-options>]", eCommandRequiresTarget),
         m_options() {}
 
   ~CommandObjectTargetModulesList() override = default;
 
   Options *GetOptions() override { return &m_options; }
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
       Status error;
 
       const int short_option = m_getopt_table[option_idx].val;
 
       switch (short_option) {
       case 'a': {
         m_str = option_arg;
         m_type = eLookupTypeAddress;
-        m_addr = Args::StringToAddress(execution_context, option_arg,
-                                       LLDB_INVALID_ADDRESS, &error);
+        m_addr = execution_context->GetTargetRef().EvaluateAddressExpression(
+            execution_context, option_arg, LLDB_INVALID_ADDRESS, &error);
         if (m_addr == LLDB_INVALID_ADDRESS)
           error.SetErrorStringWithFormat("invalid address string '%s'",
                                          option_arg.str().c_str());
         break;
       }
 
       case 'n':
         m_str = option_arg;
Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
                           ExecutionContext *execution_context) override {
       Status error;
 
       const int short_option = m_getopt_table[option_idx].val;
 
       switch (short_option) {
       case 'a': {
         m_type = eLookupTypeAddress;
-        m_addr = Args::StringToAddress(execution_context, option_arg,
-                                       LLDB_INVALID_ADDRESS, &error);
+        m_addr = execution_context->GetTargetRef().EvaluateAddressExpression(
+            execution_context, option_arg, LLDB_INVALID_ADDRESS, &error);
       } break;
 
       case 'o':
         if (option_arg.getAsInteger(0, m_offset))
           error.SetErrorStringWithFormat("invalid offset string '%s'",
                                          option_arg.str().c_str());
         break;
 

source/Commands/CommandObjectThread.cpp

public:
 
     Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
                           ExecutionContext *execution_context) override {
       Status error;
       const int short_option = m_getopt_table[option_idx].val;
 
       switch (short_option) {
       case 'a': {
-        lldb::addr_t tmp_addr = Args::StringToAddress(
+        lldb::addr_t tmp_addr =
+            execution_context->GetTargetRef().EvaluateAddressExpression(
                 execution_context, option_arg, LLDB_INVALID_ADDRESS, &error);
         if (error.Success())
           m_until_addrs.push_back(tmp_addr);
       } break;
       case 't':
         if (option_arg.getAsInteger(0, m_thread_idx)) {
           m_thread_idx = LLDB_INVALID_INDEX32;
           error.SetErrorStringWithFormat("invalid thread index '%s'",
                                          option_arg.str().c_str());
Status SetOptionValue(uint32_t option_idx, llvm::StringRef option_arg,
         if (option_arg.getAsInteger(0, m_line_num))
           return Status("invalid line number: '%s'.", option_arg.str().c_str());
         break;
       case 'b':
         if (option_arg.getAsInteger(0, m_line_offset))
           return Status("invalid line offset: '%s'.", option_arg.str().c_str());
         break;
       case 'a':
-        m_load_addr = Args::StringToAddress(execution_context, option_arg,
-                                            LLDB_INVALID_ADDRESS, &error);
+        m_load_addr =
+            execution_context->GetTargetRef().EvaluateAddressExpression(
+                execution_context, option_arg, LLDB_INVALID_ADDRESS, &error);
         break;
       case 'r':
         m_force = true;
         break;
       default:
         return Status("invalid short option character '%c'", short_option);
       }
       return error;

source/Interpreter/Args.cpp

 //===-- Args.cpp ------------------------------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 // C Includes
 #include <cstdlib>
 // C++ Includes
 // Other libraries and framework includes
 // Project includes
 #include "lldb/DataFormatters/FormatManager.h"
 #include "lldb/Interpreter/Args.h"
-#include "lldb/Target/Target.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StreamString.h"
+#include "lldb/Utility/StringList.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 
 using namespace lldb;
 using namespace lldb_private;
 

 }
 
 void Args::Clear() {
   m_entries.clear();
   m_argv.clear();
   m_argv.push_back(nullptr);
 }
 
-lldb::addr_t Args::StringToAddress(const ExecutionContext *exe_ctx,
-                                   llvm::StringRef s, lldb::addr_t fail_value,
-                                   Status *error_ptr) {
-  bool error_set = false;
-  if (s.empty()) {
-    if (error_ptr)
-      error_ptr->SetErrorStringWithFormat("invalid address expression \"%s\"",
-                                          s.str().c_str());
-    return fail_value;
-  }
-
-  llvm::StringRef sref = s;
-
-  lldb::addr_t addr = LLDB_INVALID_ADDRESS;
-  if (!s.getAsInteger(0, addr)) {
-    if (error_ptr)
-      error_ptr->Clear();
-    return addr;
-  }
-
-  // Try base 16 with no prefix...
-  if (!s.getAsInteger(16, addr)) {
-    if (error_ptr)
-      error_ptr->Clear();
-    return addr;
-  }
-
-  Target *target = nullptr;
-  if (!exe_ctx || !(target = exe_ctx->GetTargetPtr())) {
-    if (error_ptr)
-      error_ptr->SetErrorStringWithFormat("invalid address expression \"%s\"",
-                                          s.str().c_str());
-    return fail_value;
-  }
-
-  lldb::ValueObjectSP valobj_sp;
-  EvaluateExpressionOptions options;
-  options.SetCoerceToId(false);
-  options.SetUnwindOnError(true);
-  options.SetKeepInMemory(false);
-  options.SetTryAllThreads(true);
-
-  ExpressionResults expr_result =
-      target->EvaluateExpression(s, exe_ctx->GetFramePtr(), valobj_sp, options);
-
-  bool success = false;
-  if (expr_result == eExpressionCompleted) {
-    if (valobj_sp)
-      valobj_sp = valobj_sp->GetQualifiedRepresentationIfAvailable(
-          valobj_sp->GetDynamicValueType(), true);
-    // Get the address to watch.
-    if (valobj_sp)
-      addr = valobj_sp->GetValueAsUnsigned(fail_value, &success);
-    if (success) {
-      if (error_ptr)
-        error_ptr->Clear();
-      return addr;
-    } else {
-      if (error_ptr) {
-        error_set = true;
-        error_ptr->SetErrorStringWithFormat(
-            "address expression \"%s\" resulted in a value whose type "
-            "can't be converted to an address: %s",
-            s.str().c_str(), valobj_sp->GetTypeName().GetCString());
-      }
-    }
-
-  } else {
-    // Since the compiler can't handle things like "main + 12" we should
-    // try to do this for now. The compiler doesn't like adding offsets
-    // to function pointer types.
-    static RegularExpression g_symbol_plus_offset_regex(
-        "^(.*)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*$");
-    RegularExpression::Match regex_match(3);
-    if (g_symbol_plus_offset_regex.Execute(sref, &regex_match)) {
-      uint64_t offset = 0;
-      bool add = true;
-      std::string name;
-      std::string str;
-      if (regex_match.GetMatchAtIndex(s, 1, name)) {
-        if (regex_match.GetMatchAtIndex(s, 2, str)) {
-          add = str[0] == '+';
-
-          if (regex_match.GetMatchAtIndex(s, 3, str)) {
-            if (!llvm::StringRef(str).getAsInteger(0, offset)) {
-              Status error;
-              addr = StringToAddress(exe_ctx, name.c_str(),
-                                     LLDB_INVALID_ADDRESS, &error);
-              if (addr != LLDB_INVALID_ADDRESS) {
-                if (add)
-                  return addr + offset;
-                else
-                  return addr - offset;
-              }
-            }
-          }
-        }
-      }
-    }
-
-    if (error_ptr) {
-      error_set = true;
-      error_ptr->SetErrorStringWithFormat(
-          "address expression \"%s\" evaluation failed", s.str().c_str());
-    }
-  }
-
-  if (error_ptr) {
-    if (!error_set)
-      error_ptr->SetErrorStringWithFormat("invalid address expression \"%s\"",
-                                          s.str().c_str());
-  }
-  return fail_value;
-}
-
 const char *Args::StripSpaces(std::string &s, bool leading, bool trailing,
                               bool return_null_if_empty) {
   static const char *k_white_space = " \t\v";
   if (!s.empty()) {
     if (leading) {
       size_t pos = s.find_first_not_of(k_white_space);
       if (pos == std::string::npos)
         s.clear();

source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp

if (objc_runtime) {
       ObjCLanguageRuntime::TaggedPointerVendor *tagged_ptr_vendor =
           objc_runtime->GetTaggedPointerVendor();
       if (tagged_ptr_vendor) {
         for (size_t i = 0; i < command.GetArgumentCount(); i++) {
           const char *arg_str = command.GetArgumentAtIndex(i);
           if (!arg_str)
             continue;
           Status error;
-          lldb::addr_t arg_addr = Args::StringToAddress(
+          lldb::addr_t arg_addr =
+              exe_ctx.GetTargetRef().EvaluateAddressExpression(
                   &exe_ctx, arg_str, LLDB_INVALID_ADDRESS, &error);
           if (arg_addr == 0 || arg_addr == LLDB_INVALID_ADDRESS || error.Fail())
             continue;
           auto descriptor_sp = tagged_ptr_vendor->GetClassDescriptor(arg_addr);
           if (!descriptor_sp)
             continue;
           uint64_t info_bits = 0;
           uint64_t value_bits = 0;
           uint64_t payload = 0;

source/Target/Target.cpp

execution_results = UserExpression::Evaluate(exe_ctx, options, expr, prefix,
                                                  fixed_expression);
   }
 
   m_suppress_stop_hooks = old_suppress_value;
 
   return execution_results;
 }
 
+lldb::addr_t Target::EvaluateAddressExpression(const ExecutionContext *exe_ctx,
+                                               llvm::StringRef expr,
+                                               lldb::addr_t fail_value,
+                                               Status *error_ptr) {
+  if (expr.empty()) {
+    if (error_ptr)
+      error_ptr->SetErrorStringWithFormatv("invalid address expression \"{0}\"",
+                                           expr);
+    return fail_value;
+  }
+
+  lldb::addr_t addr = LLDB_INVALID_ADDRESS;
+  if (!expr.getAsInteger(0, addr)) {
+    if (error_ptr)
+      error_ptr->Clear();
+    return addr;
+  }
+
+  // Try base 16 with no prefix...
+  if (!expr.getAsInteger(16, addr)) {
+    if (error_ptr)
+      error_ptr->Clear();
+    return addr;
+  }
+
+  Target *target = nullptr;
+  if (!exe_ctx || !(target = exe_ctx->GetTargetPtr())) {
+    if (error_ptr)
+      error_ptr->SetErrorStringWithFormatv("invalid address expression \"{0}\"",
+                                           expr);
+    return fail_value;
+  }
+
+  lldb::ValueObjectSP valobj_sp;
+  EvaluateExpressionOptions options;
+  options.SetCoerceToId(false);
+  options.SetUnwindOnError(true);
+  options.SetKeepInMemory(false);
+  options.SetTryAllThreads(true);
+
+  ExpressionResults expr_result = target->EvaluateExpression(
+      expr, exe_ctx->GetFramePtr(), valobj_sp, options);
+
+  bool success = false;
+  if (expr_result == eExpressionCompleted) {
+    if (valobj_sp)
+      valobj_sp = valobj_sp->GetQualifiedRepresentationIfAvailable(
+          valobj_sp->GetDynamicValueType(), true);
+    // Get the address to watch.
+    if (valobj_sp)
+      addr = valobj_sp->GetValueAsUnsigned(fail_value, &success);
+    if (success) {
+      if (error_ptr)
+        error_ptr->Clear();
+      return addr;
+    }
+    if (error_ptr) {
+      error_ptr->SetErrorStringWithFormatv(
+          "address expression \"%{0}\" resulted in a value whose type "
+          "can't be converted to an address: {1}",
+          expr, valobj_sp->GetTypeName().GetCString());
+    }
+    return fail_value;
+  }
+
+  // Since the compiler can't handle things like "main + 12" we should
+  // try to do this for now. The compiler doesn't like adding offsets
+  // to function pointer types.
+  static RegularExpression g_symbol_plus_offset_regex(
+      "^(.*)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*$");
+  RegularExpression::Match regex_match(3);
+  if (g_symbol_plus_offset_regex.Execute(expr, &regex_match)) {
+    uint64_t offset = 0;
+    bool add = true;
+    std::string name;
+    std::string str;
+    if (regex_match.GetMatchAtIndex(expr, 1, name)) {
+      if (regex_match.GetMatchAtIndex(expr, 2, str)) {
+        add = str[0] == '+';
+
+        if (regex_match.GetMatchAtIndex(expr, 3, str)) {
+          if (!llvm::StringRef(str).getAsInteger(0, offset)) {
+            Status error;
+            addr = EvaluateAddressExpression(exe_ctx, name,
+                                             LLDB_INVALID_ADDRESS, &error);
+            if (addr != LLDB_INVALID_ADDRESS) {
+              if (add)
+                return addr + offset;
+              else
+                return addr - offset;
+            }
+          }
+        }
+      }
+    }
+  }
+
+  if (error_ptr) {
+    error_ptr->SetErrorStringWithFormatv(
+        "address expression \"{0}\" evaluation failed", expr);
+  }
+  return fail_value;
+}
+
 lldb::ExpressionVariableSP
 Target::GetPersistentVariable(const ConstString &name) {
   lldb::ExpressionVariableSP variable_sp;
   m_scratch_type_system_map.ForEach(
       [name, &variable_sp](TypeSystem *type_system) -> bool {
         if (PersistentExpressionState *persistent_state =
                 type_system->GetPersistentExpressionState()) {
           variable_sp = persistent_state->GetVariable(name);