[DebugInfo] Use DW_OP_deref_size for DW_OP_LLVM_extract_bits

[john-brawn-arm]

Using DW_OP_deref can result in the debugger reading past the end of an object into inaccessible memory, causing an error. Instead use DW_OP_deref_size to make sure we don't read any bytes beyond what we need to.

[llvmbot]

@llvm/pr-subscribers-debuginfo

Author: John Brawn (john-brawn-arm)

Changes

Using DW_OP_deref can result in the debugger reading past the end of an object into inaccessible memory, causing an error. Instead use DW_OP_deref_size to make sure we don't read any bytes beyond what we need to.

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp (+7-3)
• (modified) llvm/test/DebugInfo/X86/DW_OP_LLVM_extract_bits.ll (+28-2)

diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
index cc96d3c481f70..9d6e1bb367bc8 100644
--- a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
+++ b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp
@@ -552,9 +552,13 @@ bool DwarfExpression::addExpression(
       unsigned SizeInBits = Op->getArg(1);
       unsigned BitOffset = Op->getArg(0);
 
-      // If we have a memory location then dereference to get the value
-      if (isMemoryLocation())
-        emitOp(dwarf::DW_OP_deref);
+      // If we have a memory location then dereference to get the value, though
+      // we have to make sure we don't dereference any bytes past the end of the
+      // object.
+      if (isMemoryLocation()) {
+        emitOp(dwarf::DW_OP_deref_size);
+        emitUnsigned(alignTo(BitOffset + SizeInBits, 8) / 8);
+      }
 
       // Extract the bits by a shift left (to shift out the bits after what we
       // want to extract) followed by shift right (to shift the bits to position
diff --git a/llvm/test/DebugInfo/X86/DW_OP_LLVM_extract_bits.ll b/llvm/test/DebugInfo/X86/DW_OP_LLVM_extract_bits.ll
index 18fdfa579b9f1..8342d42f6a292 100644
--- a/llvm/test/DebugInfo/X86/DW_OP_LLVM_extract_bits.ll
+++ b/llvm/test/DebugInfo/X86/DW_OP_LLVM_extract_bits.ll
@@ -8,10 +8,10 @@
 ; CHECK-LABEL: DW_TAG_subprogram
 ; CHECK: DW_AT_name ("test1")
 ; CHECK: DW_TAG_variable
-; CHECK: DW_AT_location (DW_OP_fbreg -1, DW_OP_deref, DW_OP_constu 0x3d, DW_OP_shl, DW_OP_constu 0x3d, DW_OP_shr, DW_OP_stack_value)
+; CHECK: DW_AT_location (DW_OP_fbreg -1, DW_OP_deref_size 0x1, DW_OP_constu 0x3d, DW_OP_shl, DW_OP_constu 0x3d, DW_OP_shr, DW_OP_stack_value)
 ; CHECK: DW_AT_name ("x")
 ; CHECK: DW_TAG_variable
-; CHECK: DW_AT_location (DW_OP_fbreg -1, DW_OP_deref, DW_OP_constu 0x39, DW_OP_shl, DW_OP_constu 0x3c, DW_OP_shra, DW_OP_stack_value)
+; CHECK: DW_AT_location (DW_OP_fbreg -1, DW_OP_deref_size 0x1, DW_OP_constu 0x39, DW_OP_shl, DW_OP_constu 0x3c, DW_OP_shra, DW_OP_stack_value)
 ; CHECK: DW_AT_name ("y")
 
 define i32 @test1() !dbg !13 {
@@ -56,6 +56,27 @@ entry:
   ret i64 %0, !dbg !26
 }
 
+; CHECK-LABEL: DW_TAG_subprogram
+; CHECK: DW_AT_name ("test4")
+; CHECK: DW_TAG_variable
+; CHECK: DW_AT_location (DW_OP_fbreg -4, DW_OP_deref_size 0x4, DW_OP_constu 0x20, DW_OP_shl, DW_OP_constu 0x3f, DW_OP_shr, DW_OP_stack_value)
+; CHECK: DW_AT_name ("x")
+; CHECK: DW_TAG_variable
+; CHECK: DW_AT_location (DW_OP_fbreg -4, DW_OP_deref_size 0x4, DW_OP_constu 0x20, DW_OP_shl, DW_OP_constu 0x21, DW_OP_shra, DW_OP_stack_value)
+; CHECK: DW_AT_name ("y")
+; CHECK: DW_TAG_variable
+; CHECK: DW_AT_location (DW_OP_fbreg -4, DW_OP_plus_uconst 0x3, DW_OP_deref_size 0x1, DW_OP_constu 0x38, DW_OP_shl, DW_OP_constu 0x39, DW_OP_shr, DW_OP_stack_value)
+; CHECK: DW_AT_name ("z")
+
+define i32 @test4() !dbg !28 {
+entry:
+  %0 = alloca i32, align 4
+  tail call void @llvm.dbg.declare(metadata ptr %0, metadata !29, metadata !DIExpression(DW_OP_LLVM_extract_bits_zext, 31, 1)), !dbg !30
+  tail call void @llvm.dbg.declare(metadata ptr %0, metadata !31, metadata !DIExpression(DW_OP_LLVM_extract_bits_sext, 1, 31)), !dbg !30
+  tail call void @llvm.dbg.declare(metadata ptr %0, metadata !32, metadata !DIExpression(DW_OP_plus_uconst, 3, DW_OP_LLVM_extract_bits_zext, 1, 7)), !dbg !30
+  ret i32 0, !dbg !30
+}
+
 declare void @llvm.dbg.declare(metadata, metadata, metadata)
 declare void @llvm.dbg.value(metadata, metadata, metadata)
 
@@ -90,3 +111,8 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
 !25 = !DILocalVariable(name: "x", scope: !24, file: !3, type: !9)
 !26 = !DILocation(line: 0, scope: !24)
 !27 = !DILocalVariable(name: "y", scope: !24, file: !3, type: !19)
+!28 = distinct !DISubprogram(name: "test4", linkageName: "test4", scope: !3, file: !3, type: !14, spFlags: DISPFlagDefinition, unit: !2)
+!29 = !DILocalVariable(name: "x", scope: !28, file: !3, type: !9)
+!30 = !DILocation(line: 0, scope: !28)
+!31 = !DILocalVariable(name: "y", scope: !28, file: !3, type: !19)
+!32 = !DILocalVariable(name: "z", scope: !28, file: !3, type: !9)

[john-brawn-arm]

Ping

[SLTozer]

Seems simple enough, LGTM with an optional inline comment.

[SLTozer]

I think ideally we'd only use DW_OP_deref_size if DW_OP_deref will actually dereference too many bytes, but that's a minor point.