labath created this revision.
labath added reviewers: jingham, zturner.

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


https://reviews.llvm.org/D44306

Files:
  include/lldb/Interpreter/Args.h
  include/lldb/Target/Target.h
  source/Commands/CommandObjectBreakpoint.cpp
  source/Commands/CommandObjectDisassemble.cpp
  source/Commands/CommandObjectMemory.cpp
  source/Commands/CommandObjectSource.cpp
  source/Commands/CommandObjectTarget.cpp
  source/Commands/CommandObjectThread.cpp
  source/Interpreter/Args.cpp
  source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
  source/Target/Target.cpp

Index: source/Target/Target.cpp
===================================================================
--- source/Target/Target.cpp
+++ source/Target/Target.cpp
@@ -2326,6 +2326,110 @@
   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;
Index: source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===================================================================
--- source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -682,8 +682,9 @@
           if (!arg_str)
             continue;
           Status error;
-          lldb::addr_t arg_addr = Args::StringToAddress(
-              &exe_ctx, arg_str, LLDB_INVALID_ADDRESS, &error);
+          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);
Index: source/Interpreter/Args.cpp
===================================================================
--- source/Interpreter/Args.cpp
+++ source/Interpreter/Args.cpp
@@ -14,9 +14,9 @@
 // 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"
@@ -401,121 +401,6 @@
   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";
Index: source/Commands/CommandObjectThread.cpp
===================================================================
--- source/Commands/CommandObjectThread.cpp
+++ source/Commands/CommandObjectThread.cpp
@@ -1030,8 +1030,9 @@
 
       switch (short_option) {
       case 'a': {
-        lldb::addr_t tmp_addr = Args::StringToAddress(
-            execution_context, option_arg, LLDB_INVALID_ADDRESS, &error);
+        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;
@@ -1737,8 +1738,9 @@
           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;
Index: source/Commands/CommandObjectTarget.cpp
===================================================================
--- source/Commands/CommandObjectTarget.cpp
+++ source/Commands/CommandObjectTarget.cpp
@@ -2861,8 +2861,9 @@
       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);
@@ -2892,7 +2893,7 @@
       : 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;
@@ -3227,8 +3228,8 @@
       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());
@@ -3543,8 +3544,8 @@
       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':
Index: source/Commands/CommandObjectSource.cpp
===================================================================
--- source/Commands/CommandObjectSource.cpp
+++ source/Commands/CommandObjectSource.cpp
@@ -91,8 +91,8 @@
         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));
@@ -709,8 +709,8 @@
         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));
Index: source/Commands/CommandObjectMemory.cpp
===================================================================
--- source/Commands/CommandObjectMemory.cpp
+++ source/Commands/CommandObjectMemory.cpp
@@ -590,8 +590,8 @@
     }
 
     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.");
@@ -601,8 +601,9 @@
     }
 
     if (argc == 2) {
-      lldb::addr_t end_addr = Args::StringToAddress(
-          &m_exe_ctx, command[1].ref, LLDB_INVALID_ADDRESS, nullptr);
+      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());
@@ -1036,13 +1037,13 @@
     }
 
     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");
@@ -1345,8 +1346,8 @@
     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");
@@ -1642,8 +1643,8 @@
     }
 
     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");
@@ -1711,8 +1712,8 @@
       } 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(),
Index: source/Commands/CommandObjectDisassemble.cpp
===================================================================
--- source/Commands/CommandObjectDisassemble.cpp
+++ source/Commands/CommandObjectDisassemble.cpp
@@ -101,14 +101,14 @@
     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;
@@ -168,8 +168,9 @@
     break;
 
   case 'a': {
-    symbol_containing_addr = Args::StringToAddress(
-        execution_context, option_arg, LLDB_INVALID_ADDRESS, &error);
+    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;
     }
@@ -246,7 +247,7 @@
           "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;
Index: source/Commands/CommandObjectBreakpoint.cpp
===================================================================
--- source/Commands/CommandObjectBreakpoint.cpp
+++ source/Commands/CommandObjectBreakpoint.cpp
@@ -377,8 +377,10 @@
 
       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':
@@ -518,11 +520,12 @@
       }
 
       case 'R': {
-        lldb::addr_t tmp_offset_addr;
-        tmp_offset_addr =
-            Args::StringToAddress(execution_context, option_arg, 0, &error);
-        if (error.Success())
-          m_offset_addr = tmp_offset_addr;
+        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':
Index: include/lldb/Target/Target.h
===================================================================
--- include/lldb/Target/Target.h
+++ include/lldb/Target/Target.h
@@ -1100,6 +1100,11 @@
       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);
Index: include/lldb/Interpreter/Args.h
===================================================================
--- include/lldb/Interpreter/Args.h
+++ include/lldb/Interpreter/Args.h
@@ -343,10 +343,6 @@
     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);
 
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to