[PATCH] D103162: [DebugInfo] Limit the number of values that may be referenced by a dbg.value

Wed May 26 05:38:21 PDT 2021

StephenTozer created this revision.
StephenTozer added reviewers: wenlei, aprantl, vsk, probinson.
StephenTozer added a project: debug-info.
Herald added a subscriber: hiraditya.
StephenTozer requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Following the merge of D91722 <https://reviews.llvm.org/D91722>, a case has been found where a large number of excessively large DIArgLists were created as a result of salvaging, effectively freezing the compiler. In order to prevent such extreme cases from affecting performance, this patch arbitrarily constrains DIArgLists to containing 16 arguments - any attempt to salvage them further will result in an undef dbg.value.

Although this patch is straightforward and should help deal with the prior performance issue, there is a further step that can be taken that may also help, which would be to reduce the size of undef dbg.values. Currently when we set a dbg.value undef, we just replace the lost value with an UndefValue of equal type, but we could further reduce the size of these dbg.values by removing the contents of the DIArgList and DIExpression, with the exception of any DIExpression operators that still have meaning (such as DW_OP_LLVM_fragment). I can make this into a separate patch, or fold it into this one - any thoughts?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D103162

Files:
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/test/DebugInfo/limit-arglist-size.ll


Index: llvm/test/DebugInfo/limit-arglist-size.ll
===================================================================

--- /dev/null
+++ llvm/test/DebugInfo/limit-arglist-size.ll
@@ -0,0 +1,63 @@
+; RUN: opt -S -instcombine %s -o - | FileCheck %s
+
+; For performance reasons, we currently limit the number of values that can be
+; referenced by a dbg.value to 16. This test checks that we do not exceed this
+; limit during salvaging.
+
+; CHECK: DIArgList(i32 undef
+; CHECK-NOT: DW_OP_LLVM_arg, 16
+
+target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @_Z3foov() local_unnamed_addr !dbg !9 {
+entry:
+  %call = call i32 @_Z3barv(), !dbg !14
+  %call1 = call i32 @_Z3barv(), !dbg !14
+  %add = add nsw i32 %call, %call1, !dbg !14
+  %call2 = call i32 @_Z3barv(), !dbg !14
+  %call4 = call i32 @_Z3barv(), !dbg !14
+  %call6 = call i32 @_Z3barv(), !dbg !14
+  %call8 = call i32 @_Z3barv(), !dbg !14
+  %call10 = call i32 @_Z3barv(), !dbg !14
+  %call12 = call i32 @_Z3barv(), !dbg !14
+  %call14 = call i32 @_Z3barv(), !dbg !14
+  %call16 = call i32 @_Z3barv(), !dbg !14
+  %call18 = call i32 @_Z3barv(), !dbg !14
+  %call20 = call i32 @_Z3barv(), !dbg !14
+  %call22 = call i32 @_Z3barv(), !dbg !14
+  %call24 = call i32 @_Z3barv(), !dbg !14
+  %call26 = call i32 @_Z3barv(), !dbg !14
+  %call28 = call i32 @_Z3barv(), !dbg !14
+  %call30 = call i32 @_Z3barv(), !dbg !14
+  call void @llvm.dbg.value(metadata !DIArgList(i32 %add, i32 %call30, i32 %call28, i32 %call26, i32 %call24, i32 %call22, i32 %call20, i32 %call18, i32 %call16, i32 %call14, i32 %call12, i32 %call10, i32 %call8, i32 %call6, i32 %call4, i32 %call2), metadata !15, metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 15, DW_OP_plus, DW_OP_LLVM_arg, 14, DW_OP_plus, DW_OP_LLVM_arg, 13, DW_OP_plus, DW_OP_LLVM_arg, 12, DW_OP_plus, DW_OP_LLVM_arg, 11, DW_OP_plus, DW_OP_LLVM_arg, 10, DW_OP_plus, DW_OP_LLVM_arg, 9, DW_OP_plus, DW_OP_LLVM_arg, 8, DW_OP_plus, DW_OP_LLVM_arg, 7, DW_OP_plus, DW_OP_LLVM_arg, 6, DW_OP_plus, DW_OP_LLVM_arg, 5, DW_OP_plus, DW_OP_LLVM_arg, 4, DW_OP_plus, DW_OP_LLVM_arg, 3, DW_OP_plus, DW_OP_LLVM_arg, 2, DW_OP_plus, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_stack_value)), !dbg !16
+  %call32 = call i32 @_Z3barv(), !dbg !17
+  ret i32 %call32, !dbg !17
+}
+
+declare dso_local i32 @_Z3barv() local_unnamed_addr
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5, !6, !7}
+!llvm.ident = !{!8}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, file: !1, producer: "clang version 13.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "limit-arglist-size.cpp", directory: "/")
+!2 = !{}
+!3 = !{i32 7, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"wchar_size", i32 4}
+!6 = !{i32 7, !"uwtable", i32 1}
+!7 = !{i32 7, !"frame-pointer", i32 2}
+!8 = !{!"clang version 13.0.0"}
+!9 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !10, file: !10, line: 3, type: !11, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !2)
+!10 = !DIFile(filename: "./limit-arglist-size.cpp", directory: "/")
+!11 = !DISubroutineType(types: !12)
+!12 = !{!13}
+!13 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!14 = !DILocation(line: 4, scope: !9)
+!15 = !DILocalVariable(name: "v16", scope: !9, file: !10, line: 4, type: !13)
+!16 = !DILocation(line: 0, scope: !9)
+!17 = !DILocation(line: 5, scope: !9)
Index: llvm/lib/Transforms/Utils/Local.cpp
===================================================================
--- llvm/lib/Transforms/Utils/Local.cpp
+++ llvm/lib/Transforms/Utils/Local.cpp
@@ -1724,6 +1724,9 @@
 
 void llvm::salvageDebugInfoForDbgValues(
     Instruction &I, ArrayRef<DbgVariableIntrinsic *> DbgUsers) {
+  // This is an arbitrary chosen limit on the maximum number of values we can
+  // salvage up to in a DIArgList, used for performance reasons.
+  const unsigned MaxDebugArgs = 16;
   bool Salvaged = false;
 
   for (auto *DII : DbgUsers) {
@@ -1748,11 +1751,15 @@
     DII->replaceVariableLocationOp(&I, I.getOperand(0));
     if (AdditionalValues.empty()) {
       DII->setExpression(SalvagedExpr);
-    } else if (isa<DbgValueInst>(DII)) {
+    } else if (isa<DbgValueInst>(DII) &&
+               DII->getNumVariableLocationOps() + AdditionalValues.size() <=
+                   MaxDebugArgs) {
       DII->addVariableLocationOps(AdditionalValues, SalvagedExpr);
     } else {
       // Do not salvage using DIArgList for dbg.addr/dbg.declare, as it is
       // currently only valid for stack value expressions.
+      // Also do not salvage if the resulting DIArgList would contain an
+      // unreasonably large number of values.
       Value *Undef = UndefValue::get(I.getOperand(0)->getType());
       DII->replaceVariableLocationOp(I.getOperand(0), Undef);
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D103162.347930.patch
Type: text/x-patch
Size: 5084 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210526/556f397d/attachment.bin>
