[Lldb-commits] [lldb] e6ec76c - [LLDB] Apply FixCodeAddress to all forms of address arguments

David Spickett via lldb-commits lldb-commits at lists.llvm.org
Mon Feb 13 02:15:59 PST 2023


Author: David Spickett
Date: 2023-02-13T10:15:52Z
New Revision: e6ec76c647aaa335de48b8534d3a044346d9656f

URL: https://github.com/llvm/llvm-project/commit/e6ec76c647aaa335de48b8534d3a044346d9656f
DIFF: https://github.com/llvm/llvm-project/commit/e6ec76c647aaa335de48b8534d3a044346d9656f.diff

LOG: [LLDB] Apply FixCodeAddress to all forms of address arguments

This is a follow up to https://reviews.llvm.org/D141629
and applies the change it made to all paths through ToAddress
(now DoToAddress).

I have included the test from my previous attempt
https://reviews.llvm.org/D136938.

The initial change only applied fixing to addresses that
would parse as integers, so my test case failed. Since
ToAddress has multiple exit points, I've wrapped it into
a new method DoToAddress.

Now you can call ToAddress, it will call DoToAddress and
no matter what path you take, the address will be fixed.

For the memory tagging commands we actually want the full
address (to work out mismatches). So I added ToRawAddress
for that.

I have tested this on a QEMU AArch64 Linux system with
Memory Tagging, Pointer Authentication and Top Byte Ignore
enabled. By running the new test and all other tests in
API/linux/aarch64.

Some commands have had calls to the ABI plugin removed
as ToAddress now does this for them.

The "memory region" command still needs to use the ABI plugin
to detect the end of memory when there are non-address bits.

Reviewed By: jasonmolenda

Differential Revision: https://reviews.llvm.org/D142715

Added: 
    lldb/test/API/linux/aarch64/non_address_bit_code_break/Makefile
    lldb/test/API/linux/aarch64/non_address_bit_code_break/TestAArch64LinuxNonAddressBitCodeBreak.py
    lldb/test/API/linux/aarch64/non_address_bit_code_break/main.c

Modified: 
    lldb/include/lldb/Interpreter/OptionArgParser.h
    lldb/source/Commands/CommandObjectMemory.cpp
    lldb/source/Commands/CommandObjectMemoryTag.cpp
    lldb/source/Interpreter/OptionArgParser.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Interpreter/OptionArgParser.h b/lldb/include/lldb/Interpreter/OptionArgParser.h
index ce901b0483b41..66b16112fce96 100644
--- a/lldb/include/lldb/Interpreter/OptionArgParser.h
+++ b/lldb/include/lldb/Interpreter/OptionArgParser.h
@@ -11,12 +11,21 @@
 
 #include "lldb/lldb-private-types.h"
 
+#include <optional>
+
 namespace lldb_private {
 
 struct OptionArgParser {
+  /// Try to parse an address. If it succeeds return the address with the
+  /// non-address bits removed.
   static lldb::addr_t ToAddress(const ExecutionContext *exe_ctx,
                                 llvm::StringRef s, lldb::addr_t fail_value,
-                                Status *error);
+                                Status *error_ptr);
+
+  /// As for ToAddress but do not remove non-address bits from the result.
+  static lldb::addr_t ToRawAddress(const ExecutionContext *exe_ctx,
+                                   llvm::StringRef s, lldb::addr_t fail_value,
+                                   Status *error_ptr);
 
   static bool ToBoolean(llvm::StringRef s, bool fail_value, bool *success_ptr);
 
@@ -35,6 +44,11 @@ struct OptionArgParser {
                          size_t *byte_size_ptr); // If non-NULL, then a
                                                  // byte size can precede
                                                  // the format character
+
+private:
+  static std::optional<lldb::addr_t>
+  DoToAddress(const ExecutionContext *exe_ctx, llvm::StringRef s,
+              Status *error);
 };
 
 } // namespace lldb_private

diff  --git a/lldb/source/Commands/CommandObjectMemory.cpp b/lldb/source/Commands/CommandObjectMemory.cpp
index 6606f4564dc85..70dd6ea947bec 100644
--- a/lldb/source/Commands/CommandObjectMemory.cpp
+++ b/lldb/source/Commands/CommandObjectMemory.cpp
@@ -594,18 +594,9 @@ class CommandObjectMemoryRead : public CommandObjectParsed {
       return false;
     }
 
-    ABISP abi;
-    if (Process *proc = m_exe_ctx.GetProcessPtr())
-      abi = proc->GetABI();
-
-    if (abi)
-      addr = abi->FixDataAddress(addr);
-
     if (argc == 2) {
       lldb::addr_t end_addr = OptionArgParser::ToAddress(
           &m_exe_ctx, command[1].ref(), LLDB_INVALID_ADDRESS, nullptr);
-      if (end_addr != LLDB_INVALID_ADDRESS && abi)
-        end_addr = abi->FixDataAddress(end_addr);
 
       if (end_addr == LLDB_INVALID_ADDRESS) {
         result.AppendError("invalid end address expression.");
@@ -1045,12 +1036,6 @@ class CommandObjectMemoryFind : public CommandObjectParsed {
       return false;
     }
 
-    ABISP abi = m_exe_ctx.GetProcessPtr()->GetABI();
-    if (abi) {
-      low_addr = abi->FixDataAddress(low_addr);
-      high_addr = abi->FixDataAddress(high_addr);
-    }
-
     if (high_addr <= low_addr) {
       result.AppendError(
           "starting address must be smaller than ending address");
@@ -1783,7 +1768,6 @@ class CommandObjectMemoryRegion : public CommandObjectParsed {
       }
 
       auto load_addr_str = command[0].ref();
-      // Non-address bits in this will be handled later by GetMemoryRegion
       load_addr = OptionArgParser::ToAddress(&m_exe_ctx, load_addr_str,
                                              LLDB_INVALID_ADDRESS, &error);
       if (error.Fail() || load_addr == LLDB_INVALID_ADDRESS) {

diff  --git a/lldb/source/Commands/CommandObjectMemoryTag.cpp b/lldb/source/Commands/CommandObjectMemoryTag.cpp
index fd0fd2919c95a..b436a185cd145 100644
--- a/lldb/source/Commands/CommandObjectMemoryTag.cpp
+++ b/lldb/source/Commands/CommandObjectMemoryTag.cpp
@@ -51,7 +51,7 @@ class CommandObjectMemoryTagRead : public CommandObjectParsed {
     }
 
     Status error;
-    addr_t start_addr = OptionArgParser::ToAddress(
+    addr_t start_addr = OptionArgParser::ToRawAddress(
         &m_exe_ctx, command[0].ref(), LLDB_INVALID_ADDRESS, &error);
     if (start_addr == LLDB_INVALID_ADDRESS) {
       result.AppendErrorWithFormatv("Invalid address expression, {0}",
@@ -63,8 +63,8 @@ class CommandObjectMemoryTagRead : public CommandObjectParsed {
     addr_t end_addr = start_addr + 1;
 
     if (command.GetArgumentCount() > 1) {
-      end_addr = OptionArgParser::ToAddress(&m_exe_ctx, command[1].ref(),
-                                            LLDB_INVALID_ADDRESS, &error);
+      end_addr = OptionArgParser::ToRawAddress(&m_exe_ctx, command[1].ref(),
+                                               LLDB_INVALID_ADDRESS, &error);
       if (end_addr == LLDB_INVALID_ADDRESS) {
         result.AppendErrorWithFormatv("Invalid end address expression, {0}",
                                       error.AsCString());
@@ -155,8 +155,8 @@ class CommandObjectMemoryTagWrite : public CommandObjectParsed {
 
       switch (short_option) {
       case 'e':
-        m_end_addr = OptionArgParser::ToAddress(execution_context, option_value,
-                                                LLDB_INVALID_ADDRESS, &status);
+        m_end_addr = OptionArgParser::ToRawAddress(
+            execution_context, option_value, LLDB_INVALID_ADDRESS, &status);
         break;
       default:
         llvm_unreachable("Unimplemented option");
@@ -203,7 +203,7 @@ class CommandObjectMemoryTagWrite : public CommandObjectParsed {
     }
 
     Status error;
-    addr_t start_addr = OptionArgParser::ToAddress(
+    addr_t start_addr = OptionArgParser::ToRawAddress(
         &m_exe_ctx, command[0].ref(), LLDB_INVALID_ADDRESS, &error);
     if (start_addr == LLDB_INVALID_ADDRESS) {
       result.AppendErrorWithFormatv("Invalid address expression, {0}",

diff  --git a/lldb/source/Interpreter/OptionArgParser.cpp b/lldb/source/Interpreter/OptionArgParser.cpp
index 63ca0f9d3d4d9..ba2d3416e1838 100644
--- a/lldb/source/Interpreter/OptionArgParser.cpp
+++ b/lldb/source/Interpreter/OptionArgParser.cpp
@@ -140,16 +140,40 @@ lldb::ScriptLanguage OptionArgParser::ToScriptLanguage(
   return fail_value;
 }
 
+lldb::addr_t OptionArgParser::ToRawAddress(const ExecutionContext *exe_ctx,
+                                           llvm::StringRef s,
+                                           lldb::addr_t fail_value,
+                                           Status *error_ptr) {
+  std::optional<lldb::addr_t> maybe_addr = DoToAddress(exe_ctx, s, error_ptr);
+  return maybe_addr ? *maybe_addr : fail_value;
+}
+
 lldb::addr_t OptionArgParser::ToAddress(const ExecutionContext *exe_ctx,
                                         llvm::StringRef s,
                                         lldb::addr_t fail_value,
                                         Status *error_ptr) {
+  std::optional<lldb::addr_t> maybe_addr = DoToAddress(exe_ctx, s, error_ptr);
+  if (!maybe_addr)
+    return fail_value;
+
+  lldb::addr_t addr = *maybe_addr;
+
+  if (Process *process = exe_ctx->GetProcessPtr())
+    if (ABISP abi_sp = process->GetABI())
+      addr = abi_sp->FixCodeAddress(addr);
+
+  return addr;
+}
+
+std::optional<lldb::addr_t>
+OptionArgParser::DoToAddress(const ExecutionContext *exe_ctx, llvm::StringRef s,
+                             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;
+    return {};
   }
 
   llvm::StringRef sref = s;
@@ -158,10 +182,7 @@ lldb::addr_t OptionArgParser::ToAddress(const ExecutionContext *exe_ctx,
   if (!s.getAsInteger(0, addr)) {
     if (error_ptr)
       error_ptr->Clear();
-    Process *process = exe_ctx->GetProcessPtr();
-    if (process)
-      if (ABISP abi_sp = process->GetABI())
-        addr = abi_sp->FixCodeAddress(addr);
+
     return addr;
   }
 
@@ -177,7 +198,7 @@ lldb::addr_t OptionArgParser::ToAddress(const ExecutionContext *exe_ctx,
     if (error_ptr)
       error_ptr->SetErrorStringWithFormat("invalid address expression \"%s\"",
                                           s.str().c_str());
-    return fail_value;
+    return {};
   }
 
   lldb::ValueObjectSP valobj_sp;
@@ -197,7 +218,7 @@ lldb::addr_t OptionArgParser::ToAddress(const ExecutionContext *exe_ctx,
           valobj_sp->GetDynamicValueType(), true);
     // Get the address to watch.
     if (valobj_sp)
-      addr = valobj_sp->GetValueAsUnsigned(fail_value, &success);
+      addr = valobj_sp->GetValueAsUnsigned(0, &success);
     if (success) {
       if (error_ptr)
         error_ptr->Clear();
@@ -249,5 +270,5 @@ lldb::addr_t OptionArgParser::ToAddress(const ExecutionContext *exe_ctx,
       error_ptr->SetErrorStringWithFormat("invalid address expression \"%s\"",
                                           s.str().c_str());
   }
-  return fail_value;
+  return {};
 }

diff  --git a/lldb/test/API/linux/aarch64/non_address_bit_code_break/Makefile b/lldb/test/API/linux/aarch64/non_address_bit_code_break/Makefile
new file mode 100644
index 0000000000000..90555c6c9f607
--- /dev/null
+++ b/lldb/test/API/linux/aarch64/non_address_bit_code_break/Makefile
@@ -0,0 +1,5 @@
+C_SOURCES := main.c
+
+CFLAGS_EXTRAS := -march=armv8.3-a
+
+include Makefile.rules

diff  --git a/lldb/test/API/linux/aarch64/non_address_bit_code_break/TestAArch64LinuxNonAddressBitCodeBreak.py b/lldb/test/API/linux/aarch64/non_address_bit_code_break/TestAArch64LinuxNonAddressBitCodeBreak.py
new file mode 100644
index 0000000000000..071b16d72fd1a
--- /dev/null
+++ b/lldb/test/API/linux/aarch64/non_address_bit_code_break/TestAArch64LinuxNonAddressBitCodeBreak.py
@@ -0,0 +1,57 @@
+"""
+Test that "breakpoint set -a" uses the ABI plugin to remove non-address bits
+before attempting to set a breakpoint.
+"""
+
+import lldb
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class AArch64LinuxNonAddressBitCodeBreak(TestBase):
+
+    NO_DEBUG_INFO_TESTCASE = True
+
+    def do_tagged_break(self, hardware):
+        if not self.isAArch64PAuth():
+            self.skipTest('Target must support pointer authentication.')
+
+        self.build()
+        self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
+
+        lldbutil.run_break_set_by_file_and_line(self, "main.c",
+            line_number('main.c', '// Set break point at this line.'),
+            num_expected_locations=1)
+
+        self.runCmd("run", RUN_SUCCEEDED)
+
+        if self.process().GetState() == lldb.eStateExited:
+            self.fail("Test program failed to run.")
+
+        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+            substrs=['stopped',
+                     'stop reason = breakpoint'])
+
+        cmd = "breakpoint set -a fnptr"
+        # LLDB only has the option to force hardware break, not software.
+        # It prefers sofware break when it can and this will be one of those cases.
+        if hardware:
+            cmd += " --hardware"
+        self.expect(cmd)
+
+        self.runCmd("continue")
+        self.assertEqual(self.process().GetState(), lldb.eStateStopped)
+        self.expect("thread list", STOPPED_DUE_TO_BREAKPOINT,
+            substrs=['stopped', '`foo at main.c', 'stop reason = breakpoint'])
+
+    # AArch64 Linux always enables the top byte ignore feature
+    @skipUnlessArch("aarch64")
+    @skipUnlessPlatform(["linux"])
+    def test_software_break(self):
+        self.do_tagged_break(False)
+
+    @skipUnlessArch("aarch64")
+    @skipUnlessPlatform(["linux"])
+    def test_hardware_break(self):
+        self.do_tagged_break(True)

diff  --git a/lldb/test/API/linux/aarch64/non_address_bit_code_break/main.c b/lldb/test/API/linux/aarch64/non_address_bit_code_break/main.c
new file mode 100644
index 0000000000000..fd32712f5fa77
--- /dev/null
+++ b/lldb/test/API/linux/aarch64/non_address_bit_code_break/main.c
@@ -0,0 +1,18 @@
+#include <stdint.h>
+
+void foo(void) {}
+typedef void (*FooPtr)(void);
+
+int main() {
+  FooPtr fnptr = foo;
+  // Set top byte.
+  fnptr = (FooPtr)((uintptr_t)fnptr | (uintptr_t)0xff << 56);
+  // Then apply a PAuth signature to it.
+  __asm__ __volatile__("pacdza %0" : "=r"(fnptr) : "r"(fnptr));
+  // fnptr is now:
+  // <8 bit top byte tag><pointer signature><virtual address>
+
+  foo(); // Set break point at this line.
+
+  return 0;
+}


        


More information about the lldb-commits mailing list