Skip to content

Commit

Permalink
Verifier: Check fields of MDVariable subclasses
Browse files Browse the repository at this point in the history
Check fields from `MDLocalVariable` and `MDGlobalVariable` and change
the accessors to downcast to the right types.  `getType()` still returns
`Metadata*` since it could be an `MDString`-based reference.

Since local variables require non-null scopes, I also updated `LLParser`
to require a `scope:` field.

A number of testcases had grown bitrot and started failing with this
patch; I committed them separately in r233349.  If I just broke your
out-of-tree testcases, you're probably hitting similar problems (so have
a look there).

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@233389 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
dexonsmith committed Mar 27, 2015
1 parent 2ee6152 commit a9902da
Show file tree
Hide file tree
Showing 16 changed files with 205 additions and 107 deletions.
64 changes: 42 additions & 22 deletions include/llvm/IR/DebugInfoMetadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -1315,12 +1315,15 @@ class MDVariable : public DebugNode {

public:
unsigned getLine() const { return Line; }
Metadata *getScope() const { return getOperand(0); }
MDScope *getScope() const { return cast_or_null<MDScope>(getRawScope()); }
StringRef getName() const { return getStringOperand(1); }
Metadata *getFile() const { return getOperand(2); }
Metadata *getType() const { return getOperand(3); }
MDFile *getFile() const { return cast_or_null<MDFile>(getRawFile()); }
Metadata *getType() const { return getRawType(); }

Metadata *getRawScope() const { return getOperand(0); }
MDString *getRawName() const { return getOperandAs<MDString>(1); }
Metadata *getRawFile() const { return getOperand(2); }
Metadata *getRawType() const { return getOperand(3); }

static bool classof(const Metadata *MD) {
return MD->getMetadataID() == MDLocalVariableKind ||
Expand All @@ -1347,10 +1350,10 @@ class MDGlobalVariable : public MDVariable {
~MDGlobalVariable() {}

static MDGlobalVariable *
getImpl(LLVMContext &Context, Metadata *Scope, StringRef Name,
StringRef LinkageName, Metadata *File, unsigned Line, Metadata *Type,
bool IsLocalToUnit, bool IsDefinition, Metadata *Variable,
Metadata *StaticDataMemberDeclaration, StorageType Storage,
getImpl(LLVMContext &Context, MDScope *Scope, StringRef Name,
StringRef LinkageName, MDFile *File, unsigned Line, Metadata *Type,
bool IsLocalToUnit, bool IsDefinition, ConstantAsMetadata *Variable,
MDDerivedType *StaticDataMemberDeclaration, StorageType Storage,
bool ShouldCreate = true) {
return getImpl(Context, Scope, getCanonicalMDString(Context, Name),
getCanonicalMDString(Context, LinkageName), File, Line, Type,
Expand All @@ -1373,10 +1376,11 @@ class MDGlobalVariable : public MDVariable {

public:
DEFINE_MDNODE_GET(MDGlobalVariable,
(Metadata * Scope, StringRef Name, StringRef LinkageName,
Metadata *File, unsigned Line, Metadata *Type,
bool IsLocalToUnit, bool IsDefinition, Metadata *Variable,
Metadata *StaticDataMemberDeclaration),
(MDScope * Scope, StringRef Name, StringRef LinkageName,
MDFile *File, unsigned Line, Metadata *Type,
bool IsLocalToUnit, bool IsDefinition,
ConstantAsMetadata *Variable,
MDDerivedType *StaticDataMemberDeclaration),
(Scope, Name, LinkageName, File, Line, Type, IsLocalToUnit,
IsDefinition, Variable, StaticDataMemberDeclaration))
DEFINE_MDNODE_GET(MDGlobalVariable,
Expand All @@ -1393,10 +1397,16 @@ class MDGlobalVariable : public MDVariable {
bool isDefinition() const { return IsDefinition; }
StringRef getDisplayName() const { return getStringOperand(4); }
StringRef getLinkageName() const { return getStringOperand(5); }
Metadata *getVariable() const { return getOperand(6); }
Metadata *getStaticDataMemberDeclaration() const { return getOperand(7); }
ConstantAsMetadata *getVariable() const {
return cast_or_null<ConstantAsMetadata>(getRawVariable());
}
MDDerivedType *getStaticDataMemberDeclaration() const {
return cast_or_null<MDDerivedType>(getRawStaticDataMemberDeclaration());
}

MDString *getRawLinkageName() const { return getOperandAs<MDString>(5); }
Metadata *getRawVariable() const { return getOperand(6); }
Metadata *getRawStaticDataMemberDeclaration() const { return getOperand(7); }

static bool classof(const Metadata *MD) {
return MD->getMetadataID() == MDGlobalVariableKind;
Expand All @@ -1423,10 +1433,10 @@ class MDLocalVariable : public MDVariable {
~MDLocalVariable() {}

static MDLocalVariable *getImpl(LLVMContext &Context, unsigned Tag,
Metadata *Scope, StringRef Name,
Metadata *File, unsigned Line, Metadata *Type,
unsigned Arg, unsigned Flags,
Metadata *InlinedAt, StorageType Storage,
MDScope *Scope, StringRef Name, MDFile *File,
unsigned Line, Metadata *Type, unsigned Arg,
unsigned Flags, MDLocation *InlinedAt,
StorageType Storage,
bool ShouldCreate = true) {
return getImpl(Context, Tag, Scope, getCanonicalMDString(Context, Name),
File, Line, Type, Arg, Flags, InlinedAt, Storage,
Expand All @@ -1447,10 +1457,9 @@ class MDLocalVariable : public MDVariable {

public:
DEFINE_MDNODE_GET(MDLocalVariable,
(unsigned Tag, Metadata *Scope, StringRef Name,
Metadata *File, unsigned Line, Metadata *Type,
unsigned Arg, unsigned Flags,
Metadata *InlinedAt = nullptr),
(unsigned Tag, MDLocalScope *Scope, StringRef Name,
MDFile *File, unsigned Line, Metadata *Type, unsigned Arg,
unsigned Flags, MDLocation *InlinedAt = nullptr),
(Tag, Scope, Name, File, Line, Type, Arg, Flags, InlinedAt))
DEFINE_MDNODE_GET(MDLocalVariable,
(unsigned Tag, Metadata *Scope, MDString *Name,
Expand All @@ -1461,9 +1470,20 @@ class MDLocalVariable : public MDVariable {

TempMDLocalVariable clone() const { return cloneImpl(); }

/// \brief Get the local scope for this variable.
///
/// Variables must be defined in a local scope.
MDLocalScope *getScope() const {
return cast<MDLocalScope>(MDVariable::getScope());
}

unsigned getArg() const { return Arg; }
unsigned getFlags() const { return Flags; }
Metadata *getInlinedAt() const { return getOperand(4); }
MDLocation *getInlinedAt() const {
return cast_or_null<MDLocation>(getRawInlinedAt());
}

Metadata *getRawInlinedAt() const { return getOperand(4); }

/// \brief Get an inlined version of this variable.
///
Expand Down
8 changes: 4 additions & 4 deletions lib/AsmParser/LLParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3035,8 +3035,8 @@ struct MDField : public MDFieldImpl<Metadata *> {
struct MDConstant : public MDFieldImpl<ConstantAsMetadata *> {
MDConstant() : ImplTy(nullptr) {}
};
struct MDStringField : public MDFieldImpl<std::string> {
MDStringField() : ImplTy(std::string()) {}
struct MDStringField : public MDFieldImpl<MDString *> {
MDStringField() : ImplTy(nullptr) {}
};
struct MDFieldList : public MDFieldImpl<SmallVector<Metadata *, 4>> {
MDFieldList() : ImplTy(SmallVector<Metadata *, 4>()) {}
Expand Down Expand Up @@ -3250,7 +3250,7 @@ bool LLParser::ParseMDField(LocTy Loc, StringRef Name, MDStringField &Result) {
if (ParseStringConstant(S))
return true;

Result.assign(std::move(S));
Result.assign(S.empty() ? nullptr : MDString::get(Context, S));
return false;
}

Expand Down Expand Up @@ -3675,7 +3675,7 @@ bool LLParser::ParseMDGlobalVariable(MDNode *&Result, bool IsDistinct) {
bool LLParser::ParseMDLocalVariable(MDNode *&Result, bool IsDistinct) {
#define VISIT_MD_FIELDS(OPTIONAL, REQUIRED) \
REQUIRED(tag, DwarfTagField, ); \
OPTIONAL(scope, MDField, ); \
REQUIRED(scope, MDField, ); \
OPTIONAL(name, MDStringField, ); \
OPTIONAL(file, MDField, ); \
OPTIONAL(line, LineField, ); \
Expand Down
18 changes: 9 additions & 9 deletions lib/IR/AsmWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1720,14 +1720,14 @@ static void writeMDGlobalVariable(raw_ostream &Out, const MDGlobalVariable *N,
MDFieldPrinter Printer(Out, TypePrinter, Machine, Context);
Printer.printString("name", N->getName());
Printer.printString("linkageName", N->getLinkageName());
Printer.printMetadata("scope", N->getScope(), /* ShouldSkipNull */ false);
Printer.printMetadata("file", N->getFile());
Printer.printMetadata("scope", N->getRawScope(), /* ShouldSkipNull */ false);
Printer.printMetadata("file", N->getRawFile());
Printer.printInt("line", N->getLine());
Printer.printMetadata("type", N->getType());
Printer.printMetadata("type", N->getRawType());
Printer.printBool("isLocal", N->isLocalToUnit());
Printer.printBool("isDefinition", N->isDefinition());
Printer.printMetadata("variable", N->getVariable());
Printer.printMetadata("declaration", N->getStaticDataMemberDeclaration());
Printer.printMetadata("variable", N->getRawVariable());
Printer.printMetadata("declaration", N->getRawStaticDataMemberDeclaration());
Out << ")";
}

Expand All @@ -1741,12 +1741,12 @@ static void writeMDLocalVariable(raw_ostream &Out, const MDLocalVariable *N,
Printer.printInt("arg", N->getArg(),
/* ShouldSkipZero */
N->getTag() == dwarf::DW_TAG_auto_variable);
Printer.printMetadata("scope", N->getScope(), /* ShouldSkipNull */ false);
Printer.printMetadata("file", N->getFile());
Printer.printMetadata("scope", N->getRawScope(), /* ShouldSkipNull */ false);
Printer.printMetadata("file", N->getRawFile());
Printer.printInt("line", N->getLine());
Printer.printMetadata("type", N->getType());
Printer.printMetadata("type", N->getRawType());
Printer.printDIFlags("flags", N->getFlags());
Printer.printMetadata("inlinedAt", N->getInlinedAt());
Printer.printMetadata("inlinedAt", N->getRawInlinedAt());
Out << ")";
}

Expand Down
20 changes: 11 additions & 9 deletions lib/IR/DIBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -582,9 +582,10 @@ DIGlobalVariable DIBuilder::createGlobalVariable(
MDNode *Decl) {
checkGlobalVariableScope(Context);

auto *N = MDGlobalVariable::get(VMContext, Context, Name, LinkageName, F,
LineNumber, Ty, isLocalToUnit, true,
getConstantOrNull(Val), Decl);
auto *N = MDGlobalVariable::get(
VMContext, cast_or_null<MDScope>(Context.get()), Name, LinkageName, F,
LineNumber, Ty, isLocalToUnit, true, getConstantOrNull(Val),
cast_or_null<MDDerivedType>(Decl));
AllGVs.push_back(N);
return N;
}
Expand All @@ -595,9 +596,10 @@ DIGlobalVariable DIBuilder::createTempGlobalVariableFwdDecl(
MDNode *Decl) {
checkGlobalVariableScope(Context);

return MDGlobalVariable::getTemporary(VMContext, Context, Name, LinkageName,
F, LineNumber, Ty, isLocalToUnit, false,
getConstantOrNull(Val), Decl).release();
return MDGlobalVariable::getTemporary(
VMContext, cast_or_null<MDScope>(Context.get()), Name, LinkageName,
F, LineNumber, Ty, isLocalToUnit, false, getConstantOrNull(Val),
cast_or_null<MDDerivedType>(Decl)).release();
}

DIVariable DIBuilder::createLocalVariable(unsigned Tag, DIDescriptor Scope,
Expand All @@ -613,9 +615,9 @@ DIVariable DIBuilder::createLocalVariable(unsigned Tag, DIDescriptor Scope,
assert((!Context || Context.isScope()) &&
"createLocalVariable should be called with a valid Context");

auto *Node =
MDLocalVariable::get(VMContext, Tag, getNonCompileUnitScope(Scope), Name,
File, LineNo, Ty, ArgNo, Flags);
auto *Node = MDLocalVariable::get(VMContext, Tag,
cast_or_null<MDLocalScope>(Context.get()),
Name, File, LineNo, Ty, ArgNo, Flags);
if (AlwaysPreserve) {
// The optimizer may remove local variable. If there is an interest
// to preserve variable info in such situation then stash it in a
Expand Down
36 changes: 19 additions & 17 deletions lib/IR/LLVMContextImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -699,21 +699,22 @@ template <> struct MDNodeKeyImpl<MDGlobalVariable> {
IsDefinition(IsDefinition), Variable(Variable),
StaticDataMemberDeclaration(StaticDataMemberDeclaration) {}
MDNodeKeyImpl(const MDGlobalVariable *N)
: Scope(N->getScope()), Name(N->getName()),
LinkageName(N->getLinkageName()), File(N->getFile()),
Line(N->getLine()), Type(N->getType()),
: Scope(N->getRawScope()), Name(N->getName()),
LinkageName(N->getLinkageName()), File(N->getRawFile()),
Line(N->getLine()), Type(N->getRawType()),
IsLocalToUnit(N->isLocalToUnit()), IsDefinition(N->isDefinition()),
Variable(N->getVariable()),
StaticDataMemberDeclaration(N->getStaticDataMemberDeclaration()) {}
Variable(N->getRawVariable()),
StaticDataMemberDeclaration(N->getRawStaticDataMemberDeclaration()) {}

bool isKeyOf(const MDGlobalVariable *RHS) const {
return Scope == RHS->getScope() && Name == RHS->getName() &&
LinkageName == RHS->getLinkageName() && File == RHS->getFile() &&
Line == RHS->getLine() && Type == RHS->getType() &&
return Scope == RHS->getRawScope() && Name == RHS->getName() &&
LinkageName == RHS->getLinkageName() && File == RHS->getRawFile() &&
Line == RHS->getLine() && Type == RHS->getRawType() &&
IsLocalToUnit == RHS->isLocalToUnit() &&
IsDefinition == RHS->isDefinition() &&
Variable == RHS->getVariable() &&
StaticDataMemberDeclaration == RHS->getStaticDataMemberDeclaration();
Variable == RHS->getRawVariable() &&
StaticDataMemberDeclaration ==
RHS->getRawStaticDataMemberDeclaration();
}
unsigned getHashValue() const {
return hash_combine(Scope, Name, LinkageName, File, Line, Type,
Expand All @@ -739,16 +740,17 @@ template <> struct MDNodeKeyImpl<MDLocalVariable> {
: Tag(Tag), Scope(Scope), Name(Name), File(File), Line(Line), Type(Type),
Arg(Arg), Flags(Flags), InlinedAt(InlinedAt) {}
MDNodeKeyImpl(const MDLocalVariable *N)
: Tag(N->getTag()), Scope(N->getScope()), Name(N->getName()),
File(N->getFile()), Line(N->getLine()), Type(N->getType()),
Arg(N->getArg()), Flags(N->getFlags()), InlinedAt(N->getInlinedAt()) {}
: Tag(N->getTag()), Scope(N->getRawScope()), Name(N->getName()),
File(N->getRawFile()), Line(N->getLine()), Type(N->getRawType()),
Arg(N->getArg()), Flags(N->getFlags()),
InlinedAt(N->getRawInlinedAt()) {}

bool isKeyOf(const MDLocalVariable *RHS) const {
return Tag == RHS->getTag() && Scope == RHS->getScope() &&
Name == RHS->getName() && File == RHS->getFile() &&
Line == RHS->getLine() && Type == RHS->getType() &&
return Tag == RHS->getTag() && Scope == RHS->getRawScope() &&
Name == RHS->getName() && File == RHS->getRawFile() &&
Line == RHS->getLine() && Type == RHS->getRawType() &&
Arg == RHS->getArg() && Flags == RHS->getFlags() &&
InlinedAt == RHS->getInlinedAt();
InlinedAt == RHS->getRawInlinedAt();
}
unsigned getHashValue() const {
return hash_combine(Tag, Scope, Name, File, Line, Type, Arg, Flags,
Expand Down
38 changes: 38 additions & 0 deletions lib/IR/Verifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,7 @@ class Verifier : public InstVisitor<Verifier>, VerifierSupport {

#define HANDLE_SPECIALIZED_MDNODE_LEAF(CLASS) void visit##CLASS(const CLASS &N);
#include "llvm/IR/Metadata.def"
void visitMDVariable(const MDVariable &N);

// InstVisitor overrides...
using InstVisitor<Verifier>::visit;
Expand Down Expand Up @@ -658,6 +659,15 @@ void Verifier::visitMetadataAsValue(const MetadataAsValue &MDV, Function *F) {
visitValueAsMetadata(*V, F);
}

/// \brief Check if a value can be a reference to a type.
static bool isTypeRef(const Metadata *MD) {
if (!MD)
return true;
if (auto *S = dyn_cast<MDString>(MD))
return !S->getString().empty();
return isa<MDType>(MD);
}

void Verifier::visitMDLocation(const MDLocation &N) {
Assert(N.getRawScope() && isa<MDLocalScope>(N.getRawScope()),
"location requires a valid scope", &N, N.getRawScope());
Expand Down Expand Up @@ -749,14 +759,42 @@ void Verifier::visitMDTemplateValueParameter(
"invalid tag", &N);
}

void Verifier::visitMDVariable(const MDVariable &N) {
if (auto *S = N.getRawScope())
Assert(isa<MDScope>(S), "invalid scope", &N, S);
Assert(isTypeRef(N.getRawType()), "invalid type ref", &N, N.getRawType());
if (auto *F = N.getRawFile())
Assert(isa<MDFile>(F), "invalid file", &N, F);
}

void Verifier::visitMDGlobalVariable(const MDGlobalVariable &N) {
// Checks common to all variables.
visitMDVariable(N);

Assert(N.getTag() == dwarf::DW_TAG_variable, "invalid tag", &N);
if (auto *V = N.getRawVariable()) {
Assert(isa<ConstantAsMetadata>(V) &&
!isa<Function>(cast<ConstantAsMetadata>(V)->getValue()),
"invalid global varaible ref", &N, V);
}
if (auto *Member = N.getRawStaticDataMemberDeclaration()) {
Assert(isa<MDDerivedType>(Member), "invalid static data member declaration",
&N, Member);
}
}

void Verifier::visitMDLocalVariable(const MDLocalVariable &N) {
// Checks common to all variables.
visitMDVariable(N);

Assert(N.getTag() == dwarf::DW_TAG_auto_variable ||
N.getTag() == dwarf::DW_TAG_arg_variable,
"invalid tag", &N);
Assert(N.getRawScope() && isa<MDLocalScope>(N.getRawScope()),
"local variable requires a valid scope", &N, N.getRawScope());
if (auto *IA = N.getRawInlinedAt())
Assert(isa<MDLocation>(IA), "local variable requires a valid scope", &N,
IA);
}

void Verifier::visitMDExpression(const MDExpression &N) {
Expand Down
4 changes: 0 additions & 4 deletions test/Assembler/invalid-mdlocalvariable-missing-name.ll

This file was deleted.

4 changes: 4 additions & 0 deletions test/Assembler/invalid-mdlocalvariable-missing-scope.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s

; CHECK: <stdin>:[[@LINE+1]]:48: error: missing required field 'scope'
!0 = !MDLocalVariable(tag: DW_TAG_auto_variable)
4 changes: 4 additions & 0 deletions test/Assembler/invalid-mdlocalvariable-missing-tag.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
; RUN: not llvm-as < %s -disable-output 2>&1 | FileCheck %s

; CHECK: <stdin>:[[@LINE+1]]:45: error: missing required field 'tag'
!0 = !MDLocalVariable(scope: !MDSubprogram())
Loading

0 comments on commit a9902da

Please sign in to comment.