Skip to content

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