From: Bjorn Pettersson Date: Thu, 14 Jun 2018 11:23:42 +0000 (+0000) Subject: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue X-Git-Tag: android-x86-8.1-r1~2137 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=45a02a4f8474b4b8c5cc106b5cecb06cf6e1b3c6;p=android-x86%2Fexternal-llvm.git [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue Summary: Do not convert a DbgDeclare to DbgValue if the store instruction only refer to a fragment of the variable described by the DbgDeclare. Problem was seen when for example having an alloca for an array or struct, and there were stores to individual elements. In the past we inserted a DbgValue intrinsics for each store, just as if the store wrote the whole variable. When handling store instructions we insert a DbgValue that indicates that the variable is "undefined", as we do not know which part of the variable that is updated by the store. When ConvertDebugDeclareToDebugValue is used with a load/phi instruction we assert that the referenced value is large enough to cover the whole variable. Afaict this should be true for all scenarios where those methods are used on trunk. If the assert blows in the future I guess we could simply skip to insert a dbg.value instruction. In the future I think we should examine which part of the variable that is accessed, and add a DbgValue instrinsic with an appropriate DW_OP_LLVM_fragment expression. Reviewers: dblaikie, aprantl, rnk Reviewed By: aprantl Subscribers: JDevlieghere, llvm-commits Tags: #debug-info Differential Revision: https://reviews.llvm.org/D48024 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@334704 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/IR/IntrinsicInst.h b/include/llvm/IR/IntrinsicInst.h index 2592d43819d..6650afcca7f 100644 --- a/include/llvm/IR/IntrinsicInst.h +++ b/include/llvm/IR/IntrinsicInst.h @@ -93,6 +93,10 @@ namespace llvm { return cast(getArgOperand(2))->getMetadata(); } + /// Get the size (in bits) of the variable, or fragment of the variable that + /// is described. + Optional getFragmentSizeInBits() const; + /// \name Casting methods /// @{ static bool classof(const IntrinsicInst *I) { diff --git a/lib/IR/IntrinsicInst.cpp b/lib/IR/IntrinsicInst.cpp index 67bd5b69bb0..9ffcae1f8f8 100644 --- a/lib/IR/IntrinsicInst.cpp +++ b/lib/IR/IntrinsicInst.cpp @@ -24,6 +24,7 @@ #include "llvm/IR/IntrinsicInst.h" #include "llvm/ADT/StringSwitch.h" #include "llvm/IR/Constants.h" +#include "llvm/IR/DebugInfoMetadata.h" #include "llvm/IR/GlobalVariable.h" #include "llvm/IR/Metadata.h" #include "llvm/IR/Module.h" @@ -48,6 +49,12 @@ Value *DbgInfoIntrinsic::getVariableLocation(bool AllowNullOp) const { return nullptr; } +Optional DbgInfoIntrinsic::getFragmentSizeInBits() const { + if (auto Fragment = getExpression()->getFragmentInfo()) + return Fragment->SizeInBits; + return getVariable()->getSizeInBits(); +} + int llvm::Intrinsic::lookupLLVMIntrinsicByName(ArrayRef NameTable, StringRef Name) { assert(Name.startswith("llvm.")); diff --git a/lib/Transforms/Utils/Local.cpp b/lib/Transforms/Utils/Local.cpp index 406b52607f0..5a3a89bed2b 100644 --- a/lib/Transforms/Utils/Local.cpp +++ b/lib/Transforms/Utils/Local.cpp @@ -1228,6 +1228,23 @@ static bool PhiHasDebugValue(DILocalVariable *DIVar, return false; } +/// Check if the store size of \p ValTy is large enough to cover the variable +/// (or fragment of the variable) described by \p DII. +/// +/// This is primarily intended as a helper for the different +/// ConvertDebugDeclareToDebugValue functions. The dbg.declare/dbg.addr that is +/// converted describes an alloca'd variable, so we need to use the +/// store size of the value when doing the comparison. E.g. an i1 value will be +/// identified as covering an n-bit fragment, if the store size of i1 is at +/// least n bits. +static bool valueCoversEntireFragment(Type *ValTy, DbgInfoIntrinsic *DII) { + const DataLayout &DL = DII->getModule()->getDataLayout(); + uint64_t ValueSize = DL.getTypeStoreSizeInBits(ValTy); + if (auto FragmentSize = DII->getFragmentSizeInBits()) + return ValueSize >= *FragmentSize; + return false; +} + /// Inserts a llvm.dbg.value intrinsic before a store to an alloca'd value /// that has an associated llvm.dbg.declare or llvm.dbg.addr intrinsic. void llvm::ConvertDebugDeclareToDebugValue(DbgInfoIntrinsic *DII, @@ -1238,6 +1255,21 @@ void llvm::ConvertDebugDeclareToDebugValue(DbgInfoIntrinsic *DII, auto *DIExpr = DII->getExpression(); Value *DV = SI->getOperand(0); + if (!valueCoversEntireFragment(SI->getValueOperand()->getType(), DII)) { + // FIXME: If storing to a part of the variable described by the dbg.declare, + // then we want to insert a dbg.value for the corresponding fragment. + LLVM_DEBUG(dbgs() << "Failed to convert dbg.declare to dbg.value: " + << *DII << '\n'); + // For now, when there is a store to parts of the variable (but we do not + // know which part) we insert an dbg.value instrinsic to indicate that we + // know nothing about the variable's content. + DV = UndefValue::get(DV->getType()); + if (!LdStHasDebugValue(DIVar, DIExpr, SI)) + Builder.insertDbgValueIntrinsic(DV, DIVar, DIExpr, DII->getDebugLoc(), + SI); + return; + } + // If an argument is zero extended then use argument directly. The ZExt // may be zapped by an optimization pass in future. Argument *ExtendedArg = nullptr; @@ -1281,6 +1313,9 @@ void llvm::ConvertDebugDeclareToDebugValue(DbgInfoIntrinsic *DII, if (LdStHasDebugValue(DIVar, DIExpr, LI)) return; + assert(valueCoversEntireFragment(LI->getType(), DII) && + "Load is not loading the full variable fragment."); + // We are now tracking the loaded value instead of the address. In the // future if multi-location support is added to the IR, it might be // preferable to keep tracking both the loaded value and the original @@ -1301,6 +1336,9 @@ void llvm::ConvertDebugDeclareToDebugValue(DbgInfoIntrinsic *DII, if (PhiHasDebugValue(DIVar, DIExpr, APN)) return; + assert(valueCoversEntireFragment(APN->getType(), DII) && + "PHI node is not describing the full variable."); + BasicBlock *BB = APN->getParent(); auto InsertionPt = BB->getFirstInsertionPt(); diff --git a/test/Transforms/InstCombine/debuginfo.ll b/test/Transforms/InstCombine/debuginfo.ll index 4340c254165..05466590a01 100644 --- a/test/Transforms/InstCombine/debuginfo.ll +++ b/test/Transforms/InstCombine/debuginfo.ll @@ -71,9 +71,11 @@ entry: ; NOLOWER-NOT: alloca ; NOLOWER-NOT: store ; NOLOWER-NOT: call void @llvm.dbg.declare -; NOLOWER: call void @llvm.dbg.value(metadata i64 %o.coerce0, {{.*}}) +; Here we want to find: call void @llvm.dbg.value(metadata i64 %o.coerce0, metadata [[VARIABLE_O]], metadata !DIExpression(DW_OP_LLVM_fragment, 0, 64)) +; NOLOWER: call void @llvm.dbg.value(metadata i64 undef, {{.*}}) ; NOLOWER-NOT: store -; NOLOWER: call void @llvm.dbg.value(metadata i64 %o.coerce1, {{.*}}) +; Here we want to find: call void @llvm.dbg.value(metadata i64 %o.coerce1, metadata [[VARIABLE_O]], metadata !DIExpression(DW_OP_LLVM_fragment, 64, 64)) +; NOLOWER: call void @llvm.dbg.value(metadata i64 undef, {{.*}}) ; NOLOWER-NOT: store ; NOLOWER: call void @tworegs_callee(i64 %o.coerce0, i64 %o.coerce1) diff --git a/test/Transforms/Mem2Reg/debug-alloca-phi-2.ll b/test/Transforms/Mem2Reg/debug-alloca-phi-2.ll new file mode 100644 index 00000000000..82144cb228e --- /dev/null +++ b/test/Transforms/Mem2Reg/debug-alloca-phi-2.ll @@ -0,0 +1,46 @@ +; RUN: opt < %s -mem2reg -S | FileCheck %s +source_filename = "bugpoint-output.bc" +target datalayout = "e-m:o-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-apple-macosx10.12.0" + +define void @scan() #0 !dbg !12 { +entry: + %entry1 = alloca i1, align 8 + call void @llvm.dbg.declare(metadata i1* %entry1, metadata !18, metadata !19), !dbg !20 + store i1 0, i1* %entry1, align 8, !dbg !20 + br label %for.cond, !dbg !20 + +for.cond: +; CHECK: %[[PHI:.*]] = phi i1 [ false, %entry ], [ %0, %for.cond ] + %entryN = load i1, i1* %entry1, align 8, !dbg !20 +; CHECK: call void @llvm.dbg.value(metadata i1 %[[PHI]], +; CHECK-SAME: metadata !DIExpression()) + %0 = add i1 %entryN, 1 +; CHECK: %0 = add i1 %[[PHI]], true +; CHECK: call void @llvm.dbg.value(metadata i1 %0, +; CHECK-SAME: metadata !DIExpression()) + store i1 %0, i1* %entry1, align 8, !dbg !20 + br label %for.cond, !dbg !20 +} + +declare void @llvm.dbg.declare(metadata, metadata, metadata) #1 + +attributes #0 = { nounwind ssp uwtable } +attributes #1 = { nounwind readnone } + +!llvm.dbg.cu = !{!0} +!llvm.module.flags = !{!10, !11} + +!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "adrian", isOptimized: true, runtimeVersion: 0, emissionKind: FullDebug) +!1 = !DIFile(filename: "", directory: "/") +!2 = !{} +!4 = !DIBasicType(name: "char", size: 8, encoding: DW_ATE_signed_char) +!10 = !{i32 2, !"Debug Info Version", i32 3} +!11 = !{i32 1, !"PIC Level", i32 2} +!12 = distinct !DISubprogram(name: "scan", scope: !1, file: !1, line: 4, type: !13, isLocal: false, isDefinition: true, scopeLine: 5, flags: DIFlagPrototyped, isOptimized: true, unit: !0, retainedNodes: !15) +!13 = !DISubroutineType(types: !14) +!14 = !{null, !4, !4} +!15 = !{!18} +!18 = !DILocalVariable(name: "entry", scope: !12, file: !1, line: 6, type: !4) +!19 = !DIExpression() +!20 = !DILocation(line: 6, scope: !12)