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, ®ex_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, ®ex_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