Skip to content

[lldb] Convert file address to load address when reading memory for DW_OP_piece #116411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 19, 2024

Conversation

kuilpd
Copy link
Contributor

@kuilpd kuilpd commented Nov 15, 2024

When parsing an optimized value and reading a piece from a file address, LLDB tries to read the data from memory using that address.
This patch converts file address to load address before reading the memory.

Fixes #111313
Fixes #97484

@llvmbot
Copy link
Member

llvmbot commented Nov 15, 2024

@llvm/pr-subscribers-lldb

Author: Ilia Kuklin (kuilpd)

Changes

When parsing an optimized value and reading a piece from a file address, LLDB tries to read the data from memory using that address.
This patch converts file address to load address before reading the memory.

Fixes #111313 #97484


Full diff: https://github.com/llvm/llvm-project/pull/116411.diff

2 Files Affected:

  • (modified) lldb/source/Expression/DWARFExpression.cpp (+16-3)
  • (added) lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-O3.c (+23)
diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index f92f25ed342a9c..a7126b25c1cc38 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -1853,12 +1853,25 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
           const Value::ValueType curr_piece_source_value_type =
               curr_piece_source_value.GetValueType();
           Scalar &scalar = curr_piece_source_value.GetScalar();
-          const lldb::addr_t addr = scalar.ULongLong(LLDB_INVALID_ADDRESS);
+          lldb::addr_t addr = scalar.ULongLong(LLDB_INVALID_ADDRESS);
           switch (curr_piece_source_value_type) {
           case Value::ValueType::Invalid:
             return llvm::createStringError("invalid value type");
-          case Value::ValueType::LoadAddress:
-          case Value::ValueType::FileAddress: {
+          case Value::ValueType::FileAddress:
+            if (target) {
+              curr_piece_source_value.ConvertToLoadAddress(module_sp.get(),
+                                                           target);
+              addr = scalar.ULongLong(LLDB_INVALID_ADDRESS);
+            } else {
+              return llvm::createStringError(
+                  "unable to convert file address 0x%" PRIx64
+                  " to load address "
+                  "for DW_OP_piece(%" PRIu64 "): "
+                  "no target available",
+                  addr, piece_byte_size);
+            }
+            [[fallthrough]];
+          case Value::ValueType::LoadAddress: {
             if (target) {
               if (curr_piece.ResizeData(piece_byte_size) == piece_byte_size) {
                 if (target->ReadMemory(addr, curr_piece.GetBuffer().GetBytes(),
diff --git a/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-O3.c b/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-O3.c
new file mode 100644
index 00000000000000..d31250426dc112
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-O3.c
@@ -0,0 +1,23 @@
+// Check that optimized with -O3 values that have a file address can be read
+// DWARF info:
+// 0x00000023:   DW_TAG_variable
+//                 DW_AT_name      ("array")
+//                 DW_AT_type      (0x00000032 "char[5]")
+//                 DW_AT_location  (DW_OP_piece 0x2, DW_OP_addrx 0x0, DW_OP_piece 0x1)
+
+// RUN: %clang_host -O3 -gdwarf %s -o %t
+// RUN: %lldb %t \
+// RUN:   -o "b 22" \
+// RUN:   -o "r" \
+// RUN:   -o "p/x array[2]" \
+// RUN:   -b | FileCheck %s
+//
+// CHECK: (lldb) p/x array[2]
+// CHECK: (char) 0x03
+
+static char array[5] = {0, 1, 2, 3, 4};
+
+int main(void) {
+  array[2]++;
+  return 0;
+}
\ No newline at end of file

@llvmbot
Copy link
Member

llvmbot commented Nov 15, 2024

@llvm/pr-subscribers-debuginfo

Author: Ilia Kuklin (kuilpd)

Changes

When parsing an optimized value and reading a piece from a file address, LLDB tries to read the data from memory using that address.
This patch converts file address to load address before reading the memory.

Fixes #111313 #97484


Full diff: https://github.com/llvm/llvm-project/pull/116411.diff

2 Files Affected:

  • (modified) lldb/source/Expression/DWARFExpression.cpp (+16-3)
  • (added) lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-O3.c (+23)
diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index f92f25ed342a9c..a7126b25c1cc38 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -1853,12 +1853,25 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
           const Value::ValueType curr_piece_source_value_type =
               curr_piece_source_value.GetValueType();
           Scalar &scalar = curr_piece_source_value.GetScalar();
-          const lldb::addr_t addr = scalar.ULongLong(LLDB_INVALID_ADDRESS);
+          lldb::addr_t addr = scalar.ULongLong(LLDB_INVALID_ADDRESS);
           switch (curr_piece_source_value_type) {
           case Value::ValueType::Invalid:
             return llvm::createStringError("invalid value type");
-          case Value::ValueType::LoadAddress:
-          case Value::ValueType::FileAddress: {
+          case Value::ValueType::FileAddress:
+            if (target) {
+              curr_piece_source_value.ConvertToLoadAddress(module_sp.get(),
+                                                           target);
+              addr = scalar.ULongLong(LLDB_INVALID_ADDRESS);
+            } else {
+              return llvm::createStringError(
+                  "unable to convert file address 0x%" PRIx64
+                  " to load address "
+                  "for DW_OP_piece(%" PRIu64 "): "
+                  "no target available",
+                  addr, piece_byte_size);
+            }
+            [[fallthrough]];
+          case Value::ValueType::LoadAddress: {
             if (target) {
               if (curr_piece.ResizeData(piece_byte_size) == piece_byte_size) {
                 if (target->ReadMemory(addr, curr_piece.GetBuffer().GetBytes(),
diff --git a/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-O3.c b/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-O3.c
new file mode 100644
index 00000000000000..d31250426dc112
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-O3.c
@@ -0,0 +1,23 @@
+// Check that optimized with -O3 values that have a file address can be read
+// DWARF info:
+// 0x00000023:   DW_TAG_variable
+//                 DW_AT_name      ("array")
+//                 DW_AT_type      (0x00000032 "char[5]")
+//                 DW_AT_location  (DW_OP_piece 0x2, DW_OP_addrx 0x0, DW_OP_piece 0x1)
+
+// RUN: %clang_host -O3 -gdwarf %s -o %t
+// RUN: %lldb %t \
+// RUN:   -o "b 22" \
+// RUN:   -o "r" \
+// RUN:   -o "p/x array[2]" \
+// RUN:   -b | FileCheck %s
+//
+// CHECK: (lldb) p/x array[2]
+// CHECK: (char) 0x03
+
+static char array[5] = {0, 1, 2, 3, 4};
+
+int main(void) {
+  array[2]++;
+  return 0;
+}
\ No newline at end of file

Copy link
Collaborator

@adrian-prantl adrian-prantl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks plausible. I wish we had just one place where we read from an address so this logic isn't duplicated all over the interpreter.

@kuilpd kuilpd merged commit 27dcae5 into llvm:main Nov 19, 2024
10 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Nov 19, 2024

LLVM Buildbot has detected a new failure on builder lldb-aarch64-ubuntu running on linaro-lldb-aarch64-ubuntu while building lldb at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/8419

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-shell :: Subprocess/fork-follow-parent-wp.test (1561 of 2051)
UNSUPPORTED: lldb-shell :: SymbolFile/DWARF/TestDedupWarnings.test (1562 of 2051)
PASS: lldb-shell :: SymbolFile/Breakpad/unwind-via-stack-win.test (1563 of 2051)
UNSUPPORTED: lldb-shell :: SymbolFile/DWARF/clang-ast-from-dwarf-objc-property.m (1564 of 2051)
PASS: lldb-shell :: SymbolFile/DWARF/anon_class_w_and_wo_export_symbols.ll (1565 of 2051)
PASS: lldb-shell :: SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp (1566 of 2051)
PASS: lldb-shell :: SymbolFile/DWARF/DW_AT_range-DW_FORM_sec_offset.s (1567 of 2051)
PASS: lldb-shell :: SymbolFile/DWARF/clang-gmodules-type-lookup.c (1568 of 2051)
PASS: lldb-shell :: SymbolFile/DWARF/debug-types-mangled-name.ll (1569 of 2051)
UNSUPPORTED: lldb-shell :: SymbolFile/DWARF/deterministic-build.cpp (1570 of 2051)
FAIL: lldb-shell :: SymbolFile/DWARF/DW_OP_piece-O3.c (1571 of 2051)
******************** TEST 'lldb-shell :: SymbolFile/DWARF/DW_OP_piece-O3.c' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 8: /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang --target=specify-a-target-or-use-a-_host-substitution --target=aarch64-unknown-linux-gnu -pthread -fmodules-cache-path=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-shell -O3 -gdwarf /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-O3.c -o /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/test/Shell/SymbolFile/DWARF/Output/DW_OP_piece-O3.c.tmp
+ /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/clang --target=specify-a-target-or-use-a-_host-substitution --target=aarch64-unknown-linux-gnu -pthread -fmodules-cache-path=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-shell -O3 -gdwarf /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-O3.c -o /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/test/Shell/SymbolFile/DWARF/Output/DW_OP_piece-O3.c.tmp
clang: warning: argument unused during compilation: '-fmodules-cache-path=/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/lldb-test-build.noindex/module-cache-clang/lldb-shell' [-Wunused-command-line-argument]
RUN: at line 9: /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/lldb --no-lldbinit -S /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/test/Shell/lit-lldb-init-quiet /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/test/Shell/SymbolFile/DWARF/Output/DW_OP_piece-O3.c.tmp    -o "b 22"    -o "r"    -o "p/x array[2]"    -b | /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/FileCheck /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-O3.c
+ /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/lldb --no-lldbinit -S /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/test/Shell/lit-lldb-init-quiet /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/tools/lldb/test/Shell/SymbolFile/DWARF/Output/DW_OP_piece-O3.c.tmp -o 'b 22' -o r -o 'p/x array[2]' -b
+ /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/build/bin/FileCheck /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-O3.c
/home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-O3.c:16:11: error: CHECK: expected string not found in input
// CHECK: (char) 0x03
          ^
<stdin>:19:20: note: scanning from here
(lldb) p/x array[2]
                   ^
<stdin>:20:1: note: possible intended match here
(char) 0x02
^

Input file: <stdin>
Check file: /home/tcwg-buildbot/worker/lldb-aarch64-ubuntu/llvm-project/lldb/test/Shell/SymbolFile/DWARF/DW_OP_piece-O3.c

-dump-input=help explains the following input dump.

Input was:
<<<<<<
            .
            .
            .
           14:  20 int main(void) { 
           15:  21 array[2]++; 
           16: -> 22 return 0; 
           17:  ^ 
           18:  23 } 
           19: (lldb) p/x array[2] 
check:16'0                        X error: no match found

kuilpd added a commit that referenced this pull request Nov 19, 2024
kuilpd added a commit that referenced this pull request Nov 21, 2024
…ory for DW_OP_piece" (#117168)

This commit fixes the test so that the breakpoint is triggered correctly
after `array` usage on AArch64.

Reapplies #116411 with a fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants