[Lldb-commits] [lldb] Add register lookup as another fallback computation for address-expressions (PR #85492)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Mar 18 15:18:57 PDT 2024
https://github.com/jimingham updated https://github.com/llvm/llvm-project/pull/85492
>From 59320299f5aa3f9e03695e762c9fec237362c460 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Fri, 15 Mar 2024 17:56:20 -0700
Subject: [PATCH 1/2] Add register lookup as another fallback computation for
address-expressions
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.
---
lldb/source/Interpreter/OptionArgParser.cpp | 49 ++++++++++++++++---
.../commands/target/modules/lookup/Makefile | 4 ++
.../lookup/TestImageLookupPCExpression.py | 27 ++++++++++
.../API/commands/target/modules/lookup/main.c | 15 ++++++
4 files changed, 88 insertions(+), 7 deletions(-)
create mode 100644 lldb/test/API/commands/target/modules/lookup/Makefile
create mode 100644 lldb/test/API/commands/target/modules/lookup/TestImageLookupPCExpression.py
create mode 100644 lldb/test/API/commands/target/modules/lookup/main.c
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;
+}
>From 59fd0e9b4f598b56d79be9e75eeed0043c514f7b Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Mon, 18 Mar 2024 15:18:34 -0700
Subject: [PATCH 2/2] Address review comments.
---
lldb/source/Interpreter/OptionArgParser.cpp | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Interpreter/OptionArgParser.cpp b/lldb/source/Interpreter/OptionArgParser.cpp
index dd914138fe0e6b..9a8275128ede90 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -237,6 +237,16 @@ OptionArgParser::DoToAddress(const ExecutionContext *exe_ctx, llvm::StringRef s,
// 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.
+ // We use a regex to parse these forms, the regex handles:
+ // $reg_name
+ // $reg_name+offset
+ // symbol_name+offset
+ //
+ // The important matching elements in the regex below are:
+ // 1: The reg name if there's no +offset
+ // 3: The symbol/reg name if there is an offset
+ // 4: +/-
+ // 5: The offset value.
static RegularExpression g_symbol_plus_offset_regex(
"^(\\$[^ +-]+)|(([^ +-]+)([-\\+])[[:space:]]*(0x[0-9A-Fa-f]+|[0-9]+)[[:space:]]*)$");
@@ -251,15 +261,16 @@ OptionArgParser::DoToAddress(const ExecutionContext *exe_ctx, llvm::StringRef s,
llvm::StringRef sign = matches[4];
llvm::StringRef str_offset = matches[5];
+
+ // Some languages don't have a natural type for register values, but it
+ // is still useful to look them up here:
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:
+ llvm::StringRef reg_name = name;
+ if (frame && reg_name.consume_front("$")) {
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);
+ const RegisterInfo *reg_info = reg_ctx_sp->GetRegisterInfoByName(reg_name);
if (reg_info) {
RegisterValue reg_val;
bool success = reg_ctx_sp->ReadRegister(reg_info, reg_val);
More information about the lldb-commits
mailing list