Skip to content

Commit

Permalink
Reapply r257105 "[Verifier] Check that debug values have proper size"
Browse files Browse the repository at this point in the history
I originally reapplied this in 257550, but had to revert again due to bot
breakage. The only change in this version is to allow either the TypeSize
or the TypeAllocSize of the variable to be the one represented in debug info
(hopefully in the future we can figure out how to encode the difference).
Additionally, several bot failures following r257550, were due to
optimizer bugs now fixed in r257787 and r257795.

r257550 commit message was:

```
The follow extra changes were made to test cases:

Manually making the variable be the actual type instead of a pointer
to avoid pointer-size differences in generic code:

    LLVM :: DebugInfo/Generic/2010-03-24-MemberFn.ll
    LLVM :: DebugInfo/Generic/2010-04-06-NestedFnDbgInfo.ll
    LLVM :: DebugInfo/Generic/2010-05-03-DisableFramePtr.ll
    LLVM :: DebugInfo/Generic/varargs.ll

Delete sizing information from debug info for the same reason
(but the presence of the pointer was important to the test case):

    LLVM :: DebugInfo/Generic/restrict.ll
    LLVM :: DebugInfo/Generic/tu-composite.ll
    LLVM :: Linker/type-unique-type-array-a.ll
    LLVM :: Linker/type-unique-simple2.ll

Fixing an incorrect DW_OP_deref

    LLVM :: DebugInfo/Generic/2010-05-03-OriginDIE.ll

Fixing a missing DW_OP_deref

    LLVM :: DebugInfo/Generic/incorrect-variable-debugloc.ll

Additionally, clang should no longer complain during bootstrap should no
longer happen after r257534.

The original commit message was:
``
Summary:
Teach the Verifier to make sure that the storage size given to llvm.dbg.declare
or the value size given to llvm.dbg.value agree with what is declared in
DebugInfo. This is implicitly assumed in a number of passes (e.g. in SROA).
Additionally this catches a number of common mistakes, such as passing a
pointer when a value was intended or vice versa.

One complication comes from stack coloring which modifies the original IR when
it merges allocas in order to make sure that if AA falls back to the IR it gets
the correct result. However, given this new invariant, indiscriminately
replacing one alloca by a different (differently sized one) is no longer valid.
Fix this by just undefing out any use of the alloca in a dbg.declare in this
case.

Additionally, I had to fix a number of test cases. Of particular note:
- I regenerated dbg-changes-codegen-branch-folding.ll from the given source as
  it was affected by the bug fixed in r256077
- two-cus-from-same-file.ll was changed to avoid having a variable-typed debug
  variable as that would depend on the target, even though this test is
  supposed to be generic
- I had to manually declared size/align for reference type. See also the
  discussion for D14275/r253186.
- fpstack-debuginstr-kill.ll required changing `double` to `long double`
- most others were just a question of adding OP_deref
``

```

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@257850 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
Keno committed Jan 15, 2016
1 parent a0bd162 commit 621b821
Show file tree
Hide file tree
Showing 38 changed files with 290 additions and 207 deletions.
16 changes: 14 additions & 2 deletions lib/CodeGen/StackColoring.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
//
//===----------------------------------------------------------------------===//

#include "llvm/CodeGen/Passes.h"
#include "llvm/ADT/BitVector.h"
#include "llvm/ADT/DepthFirstIterator.h"
#include "llvm/ADT/PostOrderIterator.h"
Expand All @@ -40,6 +39,7 @@
#include "llvm/CodeGen/MachineMemOperand.h"
#include "llvm/CodeGen/MachineModuleInfo.h"
#include "llvm/CodeGen/MachineRegisterInfo.h"
#include "llvm/CodeGen/Passes.h"
#include "llvm/CodeGen/PseudoSourceValue.h"
#include "llvm/CodeGen/SlotIndexes.h"
#include "llvm/CodeGen/StackProtector.h"
Expand All @@ -48,6 +48,7 @@
#include "llvm/IR/Dominators.h"
#include "llvm/IR/Function.h"
#include "llvm/IR/Instructions.h"
#include "llvm/IR/IntrinsicInst.h"
#include "llvm/IR/Module.h"
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Debug.h"
Expand Down Expand Up @@ -495,10 +496,21 @@ void StackColoring::remapInstructions(DenseMap<int, int> &SlotRemap) {
// upcoming replacement.
SP->adjustForColoring(From, To);

// The new alloca might not be valid in a llvm.dbg.declare for this
// variable, so undef out the use to make the verifier happy.
AllocaInst *FromAI = const_cast<AllocaInst *>(From);
if (FromAI->isUsedByMetadata())
ValueAsMetadata::handleRAUW(FromAI, UndefValue::get(FromAI->getType()));
for (auto &Use : FromAI->uses()) {
if (BitCastInst *BCI = dyn_cast<BitCastInst>(Use.get()))
if (BCI->isUsedByMetadata())
ValueAsMetadata::handleRAUW(BCI, UndefValue::get(BCI->getType()));
}

// Note that this will not replace uses in MMOs (which we'll update below),
// or anywhere else (which is why we won't delete the original
// instruction).
const_cast<AllocaInst *>(From)->replaceAllUsesWith(Inst);
FromAI->replaceAllUsesWith(Inst);
}

// Remap all instructions to the new stack slots.
Expand Down
77 changes: 63 additions & 14 deletions lib/IR/Verifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -446,8 +446,7 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {
// Module-level debug info verification...
void verifyTypeRefs();
template <class MapTy>
void verifyBitPieceExpression(const DbgInfoIntrinsic &I,
const MapTy &TypeRefs);
void verifyDIExpression(const DbgInfoIntrinsic &I, const MapTy &TypeRefs);
void visitUnresolvedTypeRef(const MDString *S, const MDNode *N);
};
} // End anonymous namespace
Expand Down Expand Up @@ -4129,15 +4128,43 @@ static uint64_t getVariableSize(const DILocalVariable &V, const MapTy &Map) {
}

template <class MapTy>
void Verifier::verifyBitPieceExpression(const DbgInfoIntrinsic &I,
const MapTy &TypeRefs) {
void Verifier::verifyDIExpression(const DbgInfoIntrinsic &I,
const MapTy &TypeRefs) {
DILocalVariable *V;
DIExpression *E;
const Value *Arg;
// For now we check both the TypeSize and the TypeAllocSize because the
// difference is not clear in the IR Metadata
uint64_t ArgumentTypeSizeInBits = 0, ArgumentTypeAllocSizeInBits = 0;
if (auto *DVI = dyn_cast<DbgValueInst>(&I)) {
Arg = DVI->getValue();
if (Arg) {
ArgumentTypeAllocSizeInBits =
M->getDataLayout().getTypeAllocSizeInBits(Arg->getType());
ArgumentTypeSizeInBits =
M->getDataLayout().getTypeSizeInBits(Arg->getType());
}
V = dyn_cast_or_null<DILocalVariable>(DVI->getRawVariable());
E = dyn_cast_or_null<DIExpression>(DVI->getRawExpression());
} else {
auto *DDI = cast<DbgDeclareInst>(&I);
// For declare intrinsics, get the total size of the alloca, to allow
// case where the variable may span more than one element.
Arg = DDI->getAddress();
if (Arg)
Arg = Arg->stripPointerCasts();
const AllocaInst *AI = dyn_cast_or_null<AllocaInst>(Arg);
if (AI) {
// We can only say something about constant size allocations
if (const ConstantInt *CI = dyn_cast<ConstantInt>(AI->getArraySize())) {
ArgumentTypeAllocSizeInBits =
CI->getLimitedValue() *
M->getDataLayout().getTypeAllocSizeInBits(AI->getAllocatedType());
ArgumentTypeSizeInBits =
CI->getLimitedValue() *
M->getDataLayout().getTypeSizeInBits(AI->getAllocatedType());
}
}
V = dyn_cast_or_null<DILocalVariable>(DDI->getRawVariable());
E = dyn_cast_or_null<DIExpression>(DDI->getRawExpression());
}
Expand All @@ -4146,10 +4173,6 @@ void Verifier::verifyBitPieceExpression(const DbgInfoIntrinsic &I,
if (!V || !E || !E->isValid())
return;

// Nothing to do if this isn't a bit piece expression.
if (!E->isBitPiece())
return;

// The frontend helps out GDB by emitting the members of local anonymous
// unions as artificial local variables with shared storage. When SROA splits
// the storage for artificial local variables that are smaller than the entire
Expand All @@ -4165,11 +4188,37 @@ void Verifier::verifyBitPieceExpression(const DbgInfoIntrinsic &I,
if (!VarSize)
return;

unsigned PieceSize = E->getBitPieceSize();
unsigned PieceOffset = E->getBitPieceOffset();
Assert(PieceSize + PieceOffset <= VarSize,
"piece is larger than or outside of variable", &I, V, E);
Assert(PieceSize != VarSize, "piece covers entire variable", &I, V, E);
if (E->isBitPiece()) {
unsigned PieceSize = E->getBitPieceSize();
unsigned PieceOffset = E->getBitPieceOffset();
Assert(PieceSize + PieceOffset <= VarSize,
"piece is larger than or outside of variable", &I, V, E);
Assert(PieceSize != VarSize, "piece covers entire variable", &I, V, E);
return;
}

if (!ArgumentTypeSizeInBits)
return; // We were unable to determine the size of the argument

if (E->getNumElements() == 0) {
// In the case where the expression is empty, verify the size of the
// argument. Doing this in the general case would require looking through
// any dereferences that may be in the expression.
Assert(ArgumentTypeSizeInBits == VarSize ||
ArgumentTypeAllocSizeInBits == VarSize,
"size of passed value (" + Twine(ArgumentTypeSizeInBits) +
", with padding " + Twine(ArgumentTypeAllocSizeInBits) +
") does not match size of declared variable (" + Twine(VarSize) +
")",
&I, Arg, V, V->getType(), E);
} else if (E->getElement(0) == dwarf::DW_OP_deref) {
// Pointers shouldn't have any alignment padding, so no need to check
// the alloc size
Assert(ArgumentTypeSizeInBits == M->getDataLayout().getPointerSizeInBits(),
"the operation of the expression is a deref, but the passed value "
"is not pointer sized",
&I, Arg, V, V->getType(), E);
}
}

void Verifier::visitUnresolvedTypeRef(const MDString *S, const MDNode *N) {
Expand Down Expand Up @@ -4202,7 +4251,7 @@ void Verifier::verifyTypeRefs() {
for (const BasicBlock &BB : F)
for (const Instruction &I : BB)
if (auto *DII = dyn_cast<DbgInfoIntrinsic>(&I))
verifyBitPieceExpression(*DII, TypeRefs);
verifyDIExpression(*DII, TypeRefs);

// Return early if all typerefs were resolved.
if (UnresolvedTypeRefs.empty())
Expand Down
11 changes: 7 additions & 4 deletions test/CodeGen/ARM/2010-08-04-StackVariable.ll
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
; RUN: llc -O0 -mtriple=arm-apple-darwin < %s | grep DW_OP_breg
; Use DW_OP_breg in variable's location expression if the variable is in a stack slot.

target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:32-f32:32:32-f64:32:32-v64:64:64-v128:128:128-a0:0:64"
target triple = "arm-apple-darwin"

%struct.SVal = type { i8*, i32 }

define i32 @_Z3fooi4SVal(i32 %i, %struct.SVal* noalias %location) nounwind ssp !dbg !17 {
Expand Down Expand Up @@ -78,7 +81,7 @@ declare void @llvm.dbg.value(metadata, i64, metadata, metadata) nounwind readnon
!llvm.module.flags = !{!49}

!0 = !DISubprogram(name: "SVal", line: 11, isLocal: false, isDefinition: false, virtualIndex: 6, isOptimized: false, file: !48, scope: !1, type: !14)
!1 = !DICompositeType(tag: DW_TAG_structure_type, name: "SVal", line: 1, size: 128, align: 64, file: !48, elements: !4)
!1 = !DICompositeType(tag: DW_TAG_structure_type, name: "SVal", line: 1, size: 64, align: 64, file: !48, elements: !4)
!2 = !DIFile(filename: "small.cc", directory: "/Users/manav/R8248330")
!3 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus, producer: "4.2.1 (Based on Apple Inc. build 5658) (LLVM build)", isOptimized: false, emissionKind: 1, file: !48, enums: !47, retainedTypes: !47, subprograms: !46, globals: !47, imports: !47)
!4 = !{!5, !7, !0, !9}
Expand All @@ -103,14 +106,14 @@ declare void @llvm.dbg.value(metadata, i64, metadata, metadata) nounwind readnon
!23 = !DILocalVariable(name: "i", line: 16, arg: 1, scope: !17, file: !2, type: !13)
!24 = !DILocation(line: 16, scope: !17)
!25 = !DILocalVariable(name: "location", line: 16, arg: 2, scope: !17, file: !2, type: !26)
!26 = !DIDerivedType(tag: DW_TAG_reference_type, name: "SVal", size: 64, align: 64, file: !48, scope: !2, baseType: !1)
!26 = !DIDerivedType(tag: DW_TAG_reference_type, name: "SVal", size: 32, align: 32, file: !48, scope: !2, baseType: !1)
!27 = !DILocation(line: 17, scope: !28)
!28 = distinct !DILexicalBlock(line: 16, column: 0, file: !2, scope: !17)
!29 = !DILocation(line: 18, scope: !28)
!30 = !DILocation(line: 20, scope: !28)
!31 = !DILocalVariable(name: "this", line: 11, arg: 1, scope: !16, file: !2, type: !32)
!32 = !DIDerivedType(tag: DW_TAG_const_type, size: 64, align: 64, flags: DIFlagArtificial, file: !48, scope: !2, baseType: !33)
!33 = !DIDerivedType(tag: DW_TAG_pointer_type, size: 64, align: 64, file: !48, scope: !2, baseType: !1)
!32 = !DIDerivedType(tag: DW_TAG_const_type, flags: DIFlagArtificial, file: !48, scope: !2, baseType: !33)
!33 = !DIDerivedType(tag: DW_TAG_pointer_type, size: 32, align: 32, file: !48, scope: !2, baseType: !1)
!34 = !DILocation(line: 11, scope: !16)
!35 = !DILocation(line: 11, scope: !36)
!36 = distinct !DILexicalBlock(line: 11, column: 0, file: !48, scope: !37)
Expand Down
2 changes: 1 addition & 1 deletion test/CodeGen/MIR/X86/invalid-metadata-node-type.mir
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
entry:
%x.i = alloca i8, align 1
%y.i = alloca [256 x i8], align 16
%0 = bitcast [256 x i8]* %y.i to i8*
%0 = bitcast i8* %x.i to i8*
br label %for.body

for.body:
Expand Down
7 changes: 5 additions & 2 deletions test/CodeGen/MIR/X86/stack-object-debug-info.mir
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,14 @@
!1 = !DIFile(filename: "t.c", directory: "")
!2 = !{}
!3 = !{i32 1, !"Debug Info Version", i32 3}
!4 = !DILocalVariable(name: "x", scope: !5, file: !1, line: 16, type: !6)
!4 = !DILocalVariable(name: "x", scope: !5, file: !1, line: 16, type: !9)
!5 = distinct !DISubprogram(scope: null, isLocal: false, isDefinition: true, isOptimized: false)
!6 = !DIBasicType(name: "char", size: 8, align: 8, encoding: DW_ATE_signed_char)
!7 = !DIExpression()
!8 = !DILocation(line: 0, scope: !5)
!9 = !DICompositeType(tag: DW_TAG_array_type, baseType: !6, size: 2048, align: 8, elements: !10)
!10 = !{!11}
!11 = !DISubrange(count: 256)
...
---
name: foo
Expand All @@ -50,7 +53,7 @@ frameInfo:
# CHECK-LABEL: foo
# CHECK: stack:
# CHECK: - { id: 0, name: y.i, offset: 0, size: 256, alignment: 16, di-variable: '!4',
# CHECK-NEXT: di-expression: '!7', di-location: '!8' }
# CHECK-NEXT: di-expression: '!10', di-location: '!11' }
stack:
- { id: 0, name: y.i, offset: 0, size: 256, alignment: 16, di-variable: '!4',
di-expression: '!7', di-location: '!8' }
Expand Down
2 changes: 1 addition & 1 deletion test/CodeGen/X86/2012-11-30-regpres-dbg.ll
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ invoke.cont44: ; preds = %if.end
!1 = !{!2}
!2 = distinct !DISubprogram(name: "test", isLocal: false, isDefinition: true, virtualIndex: 6, flags: DIFlagPrototyped, isOptimized: true, scopeLine: 1, file: !6, scope: !5, type: !7)
!3 = !DILocalVariable(name: "callback", line: 214, scope: !2, type: !4)
!4 = !DICompositeType(tag: DW_TAG_structure_type, name: "btCompoundLeafCallback", line: 90, size: 512, align: 64, file: !6)
!4 = !DICompositeType(tag: DW_TAG_structure_type, name: "btCompoundLeafCallback", line: 90, size: 64, align: 64, file: !6)
!5 = !DIFile(filename: "MultiSource/Benchmarks/Bullet/btCompoundCollisionAlgorithm.cpp", directory: "MultiSource/Benchmarks/Bullet")
!6 = !DIFile(filename: "MultiSource/Benchmarks/Bullet/btCompoundCollisionAlgorithm.cpp", directory: "MultiSource/Benchmarks/Bullet")
!7 = !DISubroutineType(types: !9)
Expand Down
4 changes: 2 additions & 2 deletions test/CodeGen/X86/MachineSink-DbgValue.ll
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@ declare void @llvm.dbg.value(metadata, i64, metadata, metadata) nounwind readnon
!5 = !DIBasicType(tag: DW_TAG_base_type, name: "int", size: 32, align: 32, encoding: DW_ATE_signed)
!6 = !DILocalVariable(name: "i", line: 2, arg: 1, scope: !1, file: !2, type: !5)
!7 = !DILocalVariable(name: "c", line: 2, arg: 2, scope: !1, file: !2, type: !8)
!8 = !DIDerivedType(tag: DW_TAG_pointer_type, size: 64, align: 64, scope: !0, baseType: !9)
!8 = !DIDerivedType(tag: DW_TAG_pointer_type, size: 64, align: 64, scope: !0, baseType: !5)
!9 = !DIBasicType(tag: DW_TAG_base_type, name: "char", size: 8, align: 8, encoding: DW_ATE_signed_char)
!10 = !DILocalVariable(name: "a", line: 3, scope: !11, file: !2, type: !9)
!10 = !DILocalVariable(name: "a", line: 3, scope: !11, file: !2, type: !5)
!11 = distinct !DILexicalBlock(line: 2, column: 25, file: !20, scope: !1)
!12 = !DILocation(line: 2, column: 13, scope: !1)
!13 = !DILocation(line: 2, column: 22, scope: !1)
Expand Down
Loading

0 comments on commit 621b821

Please sign in to comment.