[PATCH] D111809: [DebugInfo] Correctly handle arrays with 0-width elements in GEP salvaging

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer created this revision.
StephenTozer added reviewers: markus, jmorse, aprantl, Orlando.
StephenTozer added a project: debug-info.
Herald added subscribers: dexonsmith, hiraditya.
StephenTozer requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Fixes a bug observed here: https://reviews.llvm.org/rGc72705678c47#inline-6620

Currently when salvaging a GEP instruction, we calculate variable offsets (such as `%offset` in `getelementptr inbounds %struct.S, %struct.S* %ptr, i64 %offset`) by appending `{<var>, DW_OP_constu, <type-width>, DW_OP_mul, DW_OP_plus}` to the existing expression, or in otherwords adding `... + (<var> * <type-width>)`. When calculating variable offsets however, we make no special allowances for variable offsets where the type-width is 0; such an offset will always be 0 and hence a no-op that needlessly lengthens the DIExpression and adds a new argument (potentially causing the dbg.value to start using a DIArgList).

This patch fixes this issue by ignoring these always-0 offsets. It also fixes a bug (linked above) where an assertion `assert(Offset.second.isStrictlyPositive())` would trigger in the above case, causing a crash for valid code.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D111809

Files:
  llvm/lib/IR/Operator.cpp
  llvm/test/DebugInfo/salvage-gep.ll


Index: llvm/test/DebugInfo/salvage-gep.ll
===================================================================
--- llvm/test/DebugInfo/salvage-gep.ll
+++ llvm/test/DebugInfo/salvage-gep.ll
@@ -1,23 +1,35 @@
 ; RUN: opt %s -dce -S | FileCheck %s
 
-; Tests the salvaging of GEP instructions, specifically struct indexing and
-; non-constant array indexing.
+; Tests the salvaging of GEP instructions, specifically struct indexing,
+; non-constant array indexing, and non-constant array indexing into an array of
+; a type with width 0.
 
 %struct.S = type { i32, i32 }
+%zero = type [0 x [10 x i32]]
 
+;; The constant and variable offsets should be applied correctly.
 ; CHECK: call void @llvm.dbg.value(metadata !DIArgList(%struct.S* %ptr, i64 %offset),
 ; CHECK-SAME: ![[VAR_OFFSET_PTR:[0-9]+]],
 ; CHECK-SAME: !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_constu, 8, DW_OP_mul, DW_OP_plus, DW_OP_plus_uconst, 4, DW_OP_stack_value))
 
+;; The variable offset should be ignored, as it applies to a type of width 0,
+;; leaving only the constant offset.
+; CHECK: call void @llvm.dbg.value(metadata [0 x [10 x i32]]* %zptr,
+; CHECK-SAME: ![[VAR_ZERO_PTR:[0-9]+]],
+; CHECK-SAME: !DIExpression(DW_OP_plus_uconst, 44, DW_OP_stack_value))
+
 ; CHECK: ![[VAR_OFFSET_PTR]] = !DILocalVariable(name: "offset_ptr"
+; CHECK: ![[VAR_ZERO_PTR]] = !DILocalVariable(name: "zero_ptr"
 
-define void @"?foo@@YAXPEAUS@@_J at Z"(%struct.S* %ptr, i64 %offset) !dbg !8 {
+define void @"?foo@@YAXPEAUS@@_J at Z"(%struct.S* %ptr, %zero* %zptr, i64 %offset) !dbg !8 {
 entry:
   call void @llvm.dbg.value(metadata i64 %offset, metadata !20, metadata !DIExpression()), !dbg !24
   call void @llvm.dbg.value(metadata %struct.S* %ptr, metadata !21, metadata !DIExpression()), !dbg !24
   %arrayidx = getelementptr inbounds %struct.S, %struct.S* %ptr, i64 %offset, !dbg !25
   %b = getelementptr inbounds %struct.S, %struct.S* %arrayidx, i32 0, i32 1, !dbg !25
+  %c = getelementptr inbounds %zero, %zero* %zptr, i64 %offset, i32 1, i32 1, !dbg !25
   call void @llvm.dbg.value(metadata i32* %b, metadata !22, metadata !DIExpression()), !dbg !24
+  call void @llvm.dbg.value(metadata i32* %c, metadata !27, metadata !DIExpression()), !dbg !24
   ret void, !dbg !26
 }
 
@@ -54,3 +66,4 @@
 !24 = !DILocation(line: 0, scope: !8)
 !25 = !DILocation(line: 8, scope: !8)
 !26 = !DILocation(line: 9, scope: !8)
+!27 = !DILocalVariable(name: "zero_ptr", scope: !8, file: !9, line: 8, type: !23)
Index: llvm/lib/IR/Operator.cpp
===================================================================
--- llvm/lib/IR/Operator.cpp
+++ llvm/lib/IR/Operator.cpp
@@ -190,12 +190,14 @@
 
     if (STy || ScalableType)
       return false;
-    // Insert an initial offset of 0 for V iff none exists already, then
-    // increment the offset by IndexedSize.
-    VariableOffsets.insert({V, APInt(BitWidth, 0)});
     APInt IndexedSize =
         APInt(BitWidth, DL.getTypeAllocSize(GTI.getIndexedType()));
-    VariableOffsets[V] += IndexedSize;
+    // Insert an initial offset of 0 for V iff none exists already, then
+    // increment the offset by IndexedSize.
+    if (!IndexedSize.isZero()) {
+      VariableOffsets.insert({V, APInt(BitWidth, 0)});
+      VariableOffsets[V] += IndexedSize;
+    }
   }
   return true;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D111809.379715.patch
Type: text/x-patch
Size: 3350 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211014/8a7fa511/attachment.bin>