llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: None (jimingham) <details> <summary>Changes</summary> The idea behind the address-expression is that it handles all the common expressions that produce addresses. It handles actual valid expressions that return a scalar, and it handles useful cases that the various source languages don't support. At present, the fallback handles: <symbol_name>{+-}<offset> which isn't valid C but is very handy. This patch adds handling of: $<reg_name> and $<reg_name>{+-}<offset> That's kind of pointless in C because the C expression parser handles that expression already. But some languages don't have a straightforward way to represent register values like this (swift) so having this fallback is quite a quality of life improvement. I added a test which tests that I didn't mess up either of these fallbacks, though it doesn't test the actually handling of registers that I added, since the expression parser for C succeeds in that case and returns before this code gets run. I will add a test on the swift fork for that checks that this works the same way for a swift frame after this check. --- Full diff: https://github.com/llvm/llvm-project/pull/85492.diff 4 Files Affected: - (modified) lldb/source/Interpreter/OptionArgParser.cpp (+42-7) - (added) lldb/test/API/commands/target/modules/lookup/Makefile (+4) - (added) lldb/test/API/commands/target/modules/lookup/TestImageLookupPCExpression.py (+27) - (added) lldb/test/API/commands/target/modules/lookup/main.c (+15) ``````````diff diff --git a/lldb/source/Interpreter/OptionArgParser.cpp b/lldb/source/Interpreter/OptionArgParser.cpp index 75ccad87467e95..dd914138fe0e6b 100644 --- a/lldb/source/Interpreter/OptionArgParser.cpp +++ b/lldb/source/Interpreter/OptionArgParser.cpp @@ -9,7 +9,9 @@ #include "lldb/Interpreter/OptionArgParser.h" #include "lldb/DataFormatters/FormatManager.h" #include "lldb/Target/ABI.h" +#include "lldb/Target/RegisterContext.h" #include "lldb/Target/Target.h" +#include "lldb/Utility/RegisterValue.h" #include "lldb/Utility/Status.h" #include "lldb/Utility/StreamString.h" @@ -233,24 +235,57 @@ OptionArgParser::DoToAddress(const ExecutionContext *exe_ctx, llvm::StringRef s, // 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. + // Some languages also don't have a natural representation for register + // values (e.g. swift) so handle simple uses of them here as well. static RegularExpression g_symbol_plus_offset_regex( - "^(.*)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*$"); + "^(\\$[^ +-]+)|(([^ +-]+)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*)$"); llvm::SmallVector<llvm::StringRef, 4> matches; if (g_symbol_plus_offset_regex.Execute(sref, &matches)) { uint64_t offset = 0; - llvm::StringRef name = matches[1]; - llvm::StringRef sign = matches[2]; - llvm::StringRef str_offset = matches[3]; - if (!str_offset.getAsInteger(0, offset)) { + llvm::StringRef name; + if (!matches[1].empty()) + name = matches[1]; + else + name = matches[3]; + + llvm::StringRef sign = matches[4]; + llvm::StringRef str_offset = matches[5]; + std::optional<lldb::addr_t> register_value; + StackFrame *frame = exe_ctx->GetFramePtr(); + if (frame && !name.empty() && name[0] == '$') { + // Some languages don't have a natural type for register values, but it + // is still useful to look them up here: + RegisterContextSP reg_ctx_sp = frame->GetRegisterContext(); + if (reg_ctx_sp) { + llvm::StringRef base_name = name.substr(1); + const RegisterInfo *reg_info = reg_ctx_sp->GetRegisterInfoByName(base_name); + if (reg_info) { + RegisterValue reg_val; + bool success = reg_ctx_sp->ReadRegister(reg_info, reg_val); + if (success && reg_val.GetType() != RegisterValue::eTypeInvalid) { + register_value = reg_val.GetAsUInt64(0, &success); + if (!success) + register_value.reset(); + } + } + } + } + if (!str_offset.empty() && !str_offset.getAsInteger(0, offset)) { Status error; - addr = ToAddress(exe_ctx, name, LLDB_INVALID_ADDRESS, &error); + if (register_value) + addr = register_value.value(); + else + addr = ToAddress(exe_ctx, name, LLDB_INVALID_ADDRESS, &error); if (addr != LLDB_INVALID_ADDRESS) { if (sign[0] == '+') return addr + offset; return addr - offset; } - } + } else if (register_value) + // In the case of register values, someone might just want to get the + // value in a language whose expression parser doesn't support registers. + return register_value.value(); } if (error_ptr) diff --git a/lldb/test/API/commands/target/modules/lookup/Makefile b/lldb/test/API/commands/target/modules/lookup/Makefile new file mode 100644 index 00000000000000..695335e068c0c9 --- /dev/null +++ b/lldb/test/API/commands/target/modules/lookup/Makefile @@ -0,0 +1,4 @@ +C_SOURCES := main.c +CFLAGS_EXTRAS := -std=c99 + +include Makefile.rules diff --git a/lldb/test/API/commands/target/modules/lookup/TestImageLookupPCExpression.py b/lldb/test/API/commands/target/modules/lookup/TestImageLookupPCExpression.py new file mode 100644 index 00000000000000..9872e057cbbfba --- /dev/null +++ b/lldb/test/API/commands/target/modules/lookup/TestImageLookupPCExpression.py @@ -0,0 +1,27 @@ +""" +Make sure that "target modules lookup -va $pc" works +""" + + +import lldb +import lldbsuite.test.lldbutil as lldbutil +from lldbsuite.test.lldbtest import * + + +class TestImageLookupPCInC(TestBase): + def test_sample_rename_this(self): + """There can be many tests in a test case - describe this test here.""" + self.build() + self.main_source_file = lldb.SBFileSpec("main.c") + self.sample_test() + + def sample_test(self): + """Make sure the address expression resolves to the right function""" + + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint( + self, "Set a breakpoint here", self.main_source_file + ) + + self.expect("target modules lookup -va $pc", substrs=["doSomething"]) + self.expect("target modules lookup -va $pc+4", substrs=["doSomething"]) + diff --git a/lldb/test/API/commands/target/modules/lookup/main.c b/lldb/test/API/commands/target/modules/lookup/main.c new file mode 100644 index 00000000000000..afe962f30916c9 --- /dev/null +++ b/lldb/test/API/commands/target/modules/lookup/main.c @@ -0,0 +1,15 @@ +#include <stdio.h> + +void +doSomething() +{ + printf ("Set a breakpoint here.\n"); + printf ("Need a bit more code.\n"); +} + +int +main() +{ + doSomething(); + return 0; +} `````````` </details> https://github.com/llvm/llvm-project/pull/85492 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits