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

[StephenTozer]

Summary

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.

[markus]

Can't say much besides that it makes sense to fix the problem at the source (and not appending to the list) instead of simply weakening the assert.

llvm/lib/IR/Operator.cpp

if (auto ConstOffset = dyn_cast<ConstantInt>(V)) {
       }
       CollectConstantOffset(ConstOffset->getValue(),
                             DL.getTypeAllocSize(GTI.getIndexedType()));
       continue;
     }
 
     if (STy || ScalableType)
       return false;
+    APInt IndexedSize =
+        APInt(BitWidth, DL.getTypeAllocSize(GTI.getIndexedType()));
     // 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)});
-    APInt IndexedSize =
-        APInt(BitWidth, DL.getTypeAllocSize(GTI.getIndexedType()));
       VariableOffsets[V] += IndexedSize;
     }
+  }
   return true;
 }
 } // namespace llvm

llvm/test/DebugInfo/salvage-gep.ll

 ; 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@Z"(%struct.S* %ptr, i64 %offset) !dbg !8 {
+define void @"?foo@@YAXPEAUS@@_J@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
 }
 
 declare void @llvm.dbg.value(metadata, metadata, metadata)
 
 !llvm.dbg.cu = !{!0}
 !llvm.module.flags = !{!3, !4, !5, !6}
 !llvm.ident = !{!7}

 !19 = !{!20, !21, !22}
 !20 = !DILocalVariable(name: "offset", arg: 2, scope: !8, file: !9, line: 7, type: !18)
 !21 = !DILocalVariable(name: "ptr", arg: 1, scope: !8, file: !9, line: 7, type: !12)
 !22 = !DILocalVariable(name: "offset_ptr", scope: !8, file: !9, line: 8, type: !23)
 !23 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !16, size: 64)
 !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)