From 09023bdaadcfbb8f967b9739c31adfb48b65f43d Mon Sep 17 00:00:00 2001 From: David Stenberg Date: Wed, 13 Nov 2019 10:37:53 +0100 Subject: [PATCH] [DebugInfo] Avoid creating entry values for clobbered registers Summary: Entry values are considered for parameters that have register-described DBG_VALUEs in the entry block (along with other conditions). If a parameter's value has been propagated from the caller to the callee, then the parameter's DBG_VALUE in the entry block may be described using a register defined by some instruction, and entry values should not be emitted for the parameter, which can currently occur. One such case was seen in the attached test case, in which the second parameter, which is described by a redefinition of the first parameter's register, would incorrectly get an entry value using the first parameter's register. This commit intends to solve such cases by keeping track of register defines, and ignoring DBG_VALUEs in the entry block that are described by such registers. In a RelWithDebInfo build of clang-8, the average size of the set was 27, and in a RelWithDebInfo+ASan build it was 30. Reviewers: djtodoro, NikolaPrica, aprantl, vsk Reviewed By: djtodoro, vsk Subscribers: hiraditya, llvm-commits Tags: #debug-info, #llvm Differential Revision: https://reviews.llvm.org/D69889 --- lib/CodeGen/LiveDebugValues.cpp | 35 +++- .../MIR/ARM/dbgcall-site-propagated-value.mir | 185 ++++++++++++++++++ 2 files changed, 216 insertions(+), 4 deletions(-) create mode 100644 test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir diff --git a/lib/CodeGen/LiveDebugValues.cpp b/lib/CodeGen/LiveDebugValues.cpp index 1b2dfe59906..065a9e2b498 100644 --- a/lib/CodeGen/LiveDebugValues.cpp +++ b/lib/CodeGen/LiveDebugValues.cpp @@ -109,6 +109,8 @@ static bool isRegOtherThanSPAndFP(const MachineOperand &Op, namespace { +using DefinedRegsSet = SmallSet; + class LiveDebugValues : public MachineFunctionPass { private: const TargetRegisterInfo *TRI; @@ -479,7 +481,8 @@ private: /// /// Currently, we generate debug entry values only for parameters that are /// unmodified throughout the function and located in a register. - bool isEntryValueCandidate(const MachineInstr &MI) const; + bool isEntryValueCandidate(const MachineInstr &MI, + const DefinedRegsSet &Regs) const; /// If a given instruction is identified as a spill, return the spill location /// and set \p Reg to the spilled register. @@ -1278,7 +1281,8 @@ void LiveDebugValues::flushPendingLocs(VarLocInMBB &PendingInLocs, } } -bool LiveDebugValues::isEntryValueCandidate(const MachineInstr &MI) const { +bool LiveDebugValues::isEntryValueCandidate( + const MachineInstr &MI, const DefinedRegsSet &DefinedRegs) const { if (!MI.isDebugValue()) return false; @@ -1304,6 +1308,13 @@ bool LiveDebugValues::isEntryValueCandidate(const MachineInstr &MI) const { if (!isRegOtherThanSPAndFP(MI.getOperand(0), MI, TRI)) return false; + // If a parameter's value has been propagated from the caller, then the + // parameter's DBG_VALUE may be described using a register defined by some + // instruction in the entry block, in which case we shouldn't create an + // entry value. + if (DefinedRegs.count(MI.getOperand(0).getReg())) + return false; + // TODO: Add support for parameters that are described as fragments. if (MI.getDebugExpression()->isFragment()) return false; @@ -1311,6 +1322,15 @@ bool LiveDebugValues::isEntryValueCandidate(const MachineInstr &MI) const { return true; } +/// Collect all register defines (including aliases) for the given instruction. +static void collectRegDefs(const MachineInstr &MI, DefinedRegsSet &Regs, + const TargetRegisterInfo *TRI) { + for (const MachineOperand &MO : MI.operands()) + if (MO.isReg() && MO.isDef() && MO.getReg()) + for (MCRegAliasIterator AI(MO.getReg(), TRI, true); AI.isValid(); ++AI) + Regs.insert(*AI); +} + /// Calculate the liveness information for the given machine function and /// extend ranges across basic blocks. bool LiveDebugValues::ExtendRanges(MachineFunction &MF) { @@ -1351,13 +1371,20 @@ bool LiveDebugValues::ExtendRanges(MachineFunction &MF) { // representing candidates for production of debug entry values. DebugParamMap DebugEntryVals; + // Set of register defines that are seen when traversing the entry block + // looking for debug entry value candidates. + DefinedRegsSet DefinedRegs; + // Only in the case of entry MBB collect DBG_VALUEs representing // function parameters in order to generate debug entry values for them. + MachineBasicBlock &First_MBB = *(MF.begin()); - for (auto &MI : First_MBB) - if (isEntryValueCandidate(MI) && + for (auto &MI : First_MBB) { + collectRegDefs(MI, DefinedRegs, TRI); + if (isEntryValueCandidate(MI, DefinedRegs) && !DebugEntryVals.count(MI.getDebugVariable())) DebugEntryVals[MI.getDebugVariable()] = &MI; + } // Initialize per-block structures and scan for fragment overlaps. for (auto &MBB : MF) { diff --git a/test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir b/test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir new file mode 100644 index 00000000000..e3bd6bf0f61 --- /dev/null +++ b/test/DebugInfo/MIR/ARM/dbgcall-site-propagated-value.mir @@ -0,0 +1,185 @@ +# RUN: llc -debug-entry-values -run-pass=livedebugvalues -o - %s | FileCheck %s + +# Based on the following C reproducer: +# +# extern void ext(int *); +# extern int interesting(int *); +# extern int *value(void); +# +# static void callee(int *p1, int *p2) { +# if (interesting(p2)) +# for (;;) +# ext(p1); +# ext(p2); +# } +# +# void caller() { +# int *local = value(); +# callee(local, (int *)0xabcd); +# } +# +# Generated using: +# +# clang -Os -fno-inline -Xclang -femit-debug-entry-values -g --target=armeb. + +--- | + target datalayout = "E-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64" + target triple = "armebv4t-unknown-unknown" + + ; Function Attrs: noinline nounwind optsize + define arm_aapcs_vfpcc void @caller() #0 !dbg !20 { + entry: + unreachable + } + + ; Function Attrs: noinline nounwind optsize + define internal arm_aapcs_vfpcc void @callee(i32* %p1) unnamed_addr #0 !dbg !29 { + entry: + unreachable + } + + declare !dbg !4 arm_aapcs_vfpcc i32* @value() + declare !dbg !9 arm_aapcs_vfpcc i32 @interesting(i32*) + declare !dbg !12 arm_aapcs_vfpcc void @ext(i32*) + + ; Function Attrs: nounwind readnone speculatable willreturn + declare void @llvm.dbg.value(metadata, metadata, metadata) #1 + + attributes #0 = { noinline nounwind optsize "frame-pointer"="all" } + attributes #1 = { nounwind readnone speculatable willreturn } + + !llvm.dbg.cu = !{!0} + !llvm.module.flags = !{!15, !16, !17, !18} + !llvm.ident = !{!19} + + !0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 10.0.0", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug, enums: !2, retainedTypes: !3, nameTableKind: None) + !1 = !DIFile(filename: "armeb.c", directory: "/") + !2 = !{} + !3 = !{!4, !7, !9, !12} + !4 = !DISubprogram(name: "value", scope: !1, file: !1, line: 3, type: !5, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2) + !5 = !DISubroutineType(types: !6) + !6 = !{!7} + !7 = !DIDerivedType(tag: DW_TAG_pointer_type, baseType: !8, size: 32) + !8 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed) + !9 = !DISubprogram(name: "interesting", scope: !1, file: !1, line: 2, type: !10, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2) + !10 = !DISubroutineType(types: !11) + !11 = !{!8, !7} + !12 = !DISubprogram(name: "ext", scope: !1, file: !1, line: 1, type: !13, flags: DIFlagPrototyped, spFlags: DISPFlagOptimized, retainedNodes: !2) + !13 = !DISubroutineType(types: !14) + !14 = !{null, !7} + !15 = !{i32 2, !"Dwarf Version", i32 4} + !16 = !{i32 2, !"Debug Info Version", i32 3} + !17 = !{i32 1, !"wchar_size", i32 4} + !18 = !{i32 1, !"min_enum_size", i32 4} + !19 = !{!"clang version 10.0.0"} + !20 = distinct !DISubprogram(name: "caller", scope: !1, file: !1, line: 12, type: !21, scopeLine: 12, flags: DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !23) + !21 = !DISubroutineType(types: !22) + !22 = !{null} + !23 = !{!24} + !24 = !DILocalVariable(name: "local", scope: !20, file: !1, line: 13, type: !7) + !25 = !DILocation(line: 13, scope: !20) + !26 = !DILocation(line: 0, scope: !20) + !27 = !DILocation(line: 14, scope: !20) + !28 = !DILocation(line: 15, scope: !20) + !29 = distinct !DISubprogram(name: "callee", scope: !1, file: !1, line: 5, type: !30, scopeLine: 5, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition | DISPFlagOptimized, unit: !0, retainedNodes: !32) + !30 = !DISubroutineType(types: !31) + !31 = !{null, !7, !7} + !32 = !{!33, !34} + !33 = !DILocalVariable(name: "p1", arg: 1, scope: !29, file: !1, line: 5, type: !7, flags: DIFlagArgumentNotModified) + !34 = !DILocalVariable(name: "p2", arg: 2, scope: !29, file: !1, line: 5, type: !7, flags: DIFlagArgumentNotModified) + !35 = !DILocation(line: 0, scope: !29) + !36 = !DILocation(line: 6, scope: !37) + !37 = distinct !DILexicalBlock(scope: !29, file: !1, line: 6) + !38 = !DILocation(line: 6, scope: !29) + !39 = !DILocation(line: 7, scope: !40) + !40 = distinct !DILexicalBlock(scope: !37, file: !1, line: 7) + !41 = !DILocation(line: 8, scope: !42) + !42 = distinct !DILexicalBlock(scope: !40, file: !1, line: 7) + !43 = !DILocation(line: 7, scope: !42) + !44 = distinct !{!44, !39, !45} + !45 = !DILocation(line: 8, scope: !40) + !46 = !DILocation(line: 9, scope: !29) + !47 = !DILocation(line: 10, scope: !29) + +... +--- +name: caller +alignment: 4 +tracksRegLiveness: true +callSites: + - { bb: 0, offset: 6 } + - { bb: 0, offset: 9, fwdArgRegs: + - { arg: 0, reg: '$r0' } } +body: | + bb.0: + liveins: $lr + + $sp = frame-setup STMDB_UPD $sp, 14, $noreg, $r11, killed $lr + frame-setup CFI_INSTRUCTION def_cfa_offset 8 + frame-setup CFI_INSTRUCTION offset $lr, -4 + frame-setup CFI_INSTRUCTION offset $r11, -8 + $r11 = frame-setup MOVr $sp, 14, $noreg, $noreg + frame-setup CFI_INSTRUCTION def_cfa_register $r11 + BL @value, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit-def $sp, implicit-def $r0, debug-location !25 + DBG_VALUE $r0, $noreg, !24, !DIExpression(), debug-location !26 + $sp = LDMIA_UPD $sp, 14, $noreg, def $r11, def $lr, debug-location !27 + TAILJMPd @callee, implicit $sp, implicit $sp, implicit killed $r0, debug-location !27 + +... +--- +name: callee +tracksRegLiveness: true +body: | + bb.0: + successors: %bb.2(0x30000000), %bb.1(0x50000000) + liveins: $r0, $r4, $r10, $lr + + DBG_VALUE $r0, $noreg, !33, !DIExpression(), debug-location !35 + $sp = frame-setup STMDB_UPD $sp, 14, $noreg, killed $r4, killed $r10, $r11, killed $lr + frame-setup CFI_INSTRUCTION def_cfa_offset 16 + frame-setup CFI_INSTRUCTION offset $lr, -4 + frame-setup CFI_INSTRUCTION offset $r11, -8 + frame-setup CFI_INSTRUCTION offset $r10, -12 + frame-setup CFI_INSTRUCTION offset $r4, -16 + $r11 = frame-setup ADDri $sp, 8, 14, $noreg, $noreg + frame-setup CFI_INSTRUCTION def_cfa $r11, 8 + $r4 = MOVr killed $r0, 14, $noreg, $noreg + DBG_VALUE $r4, $noreg, !33, !DIExpression(), debug-location !35 + $r0 = MOVi 205, 14, $noreg, $noreg + $r0 = ORRri killed $r0, 43776, 14, $noreg, $noreg + ; The MOVI and ORRri produce the second parameter's (!34) value which has + ; been propagated from caller(). + DBG_VALUE $r0, $noreg, !34, !DIExpression(), debug-location !35 + BL @interesting, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $r0, implicit-def $sp, implicit-def $r0, debug-location !36 + CMPri killed renamable $r0, 0, 14, $noreg, implicit-def $cpsr, debug-location !38 + Bcc %bb.2, 0, killed $cpsr, debug-location !38 + + bb.1: + liveins: $r4 + + $r0 = MOVr $r4, 14, $noreg, $noreg, debug-location !41 + BL @ext, csr_aapcs, implicit-def dead $lr, implicit $sp, implicit killed $r0, implicit-def $sp, debug-location !41 + B %bb.1, debug-location !43 + + bb.2: + $r0 = MOVi 205, 14, $noreg, $noreg + $r0 = ORRri killed $r0, 43776, 14, $noreg, $noreg + $sp = LDMIA_UPD $sp, 14, $noreg, def $r4, def $r10, def $r11, def $lr, debug-location !46 + TAILJMPd @ext, implicit $sp, implicit $sp, implicit killed $r0, debug-location !46 + +... + +# In this test case the second parameter's value has been propagated from the +# caller to the callee, so we should not emit any entry value DBG_VALUEs for +# that parameter. The second parameter reuses the first parameter's register +# ($r0), and previously we would incorrectly emit an entry value for the +# parameter using that register. + +# CHECK-DAG: ![[P1:[0-9]+]] = !DILocalVariable(name: "p1", arg: 1 +# CHECK-DAG: ![[P2:[0-9]+]] = !DILocalVariable(name: "p2", arg: 2 + +# CHECK-NOT: DBG_VALUE $r0, $noreg, ![[P2]], !DIExpression(DW_OP_LLVM_entry_value + +# CHECK: DBG_VALUE $r0, $noreg, ![[P1]], !DIExpression(DW_OP_LLVM_entry_value + +# CHECK-NOT: DBG_VALUE $r0, $noreg, ![[P2]], !DIExpression(DW_OP_LLVM_entry_value