[llvm] e5d844b - [Bitcode] Ensure DIArgList in bitcode has no null or forward metadata refs
Stephen Tozer via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 22 04:05:12 PDT 2021
Author: Stephen Tozer
Date: 2021-04-22T12:03:33+01:00
New Revision: e5d844b5874488599dc79e788a2dd6efa02940fb
URL: https://github.com/llvm/llvm-project/commit/e5d844b5874488599dc79e788a2dd6efa02940fb
DIFF: https://github.com/llvm/llvm-project/commit/e5d844b5874488599dc79e788a2dd6efa02940fb.diff
LOG: [Bitcode] Ensure DIArgList in bitcode has no null or forward metadata refs
This patch fixes an issue in which ConstantAsMetadata arguments to a
DIArglist, as well as the Constant values referenced by that metadata,
would not be always be emitted correctly into bitcode. This patch fixes
this issue firstly by searching for ConstantAsMetadata in DIArgLists
(previously we would only search for them when directly wrapped in
MetadataAsValue), and secondly by enumerating all of a DIArgList's
arguments directly prior to enumerating the DIArgList itself.
This patch also adds a number of asserts, and no longer treats the
arguments to a DIArgList as optional fields when reading/writing to
bitcode.
Differential Revision: https://reviews.llvm.org/D100572
Added:
Modified:
llvm/lib/Bitcode/Reader/MetadataLoader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
llvm/lib/Bitcode/Writer/ValueEnumerator.h
llvm/test/DebugInfo/Generic/debug_value_list.ll
Removed:
################################################################################
diff --git a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
index 0cf547c57d9d..8493eb7a28b2 100644
--- a/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
+++ b/llvm/lib/Bitcode/Reader/MetadataLoader.cpp
@@ -2078,8 +2078,15 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
case bitc::METADATA_ARG_LIST: {
SmallVector<ValueAsMetadata *, 4> Elts;
Elts.reserve(Record.size());
- for (uint64_t Elt : Record)
- Elts.push_back(dyn_cast_or_null<ValueAsMetadata>(getMDOrNull(Elt)));
+ for (uint64_t Elt : Record) {
+ Metadata *MD = getMD(Elt);
+ if (isa<MDNode>(MD) && cast<MDNode>(MD)->isTemporary())
+ return error(
+ "Invalid record: DIArgList should not contain forward refs");
+ if (!isa<ValueAsMetadata>(MD))
+ return error("Invalid record");
+ Elts.push_back(cast<ValueAsMetadata>(MD));
+ }
MetadataList.assignValue(DIArgList::get(Context, Elts), NextMetadataNo);
NextMetadataNo++;
diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
index e36ce87554f2..3a7edd4479b3 100644
--- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -1876,7 +1876,7 @@ void ModuleBitcodeWriter::writeDIArgList(const DIArgList *N,
unsigned Abbrev) {
Record.reserve(N->getArgs().size());
for (ValueAsMetadata *MD : N->getArgs())
- Record.push_back(VE.getMetadataOrNullID(MD));
+ Record.push_back(VE.getMetadataID(MD));
Stream.EmitRecord(bitc::METADATA_ARG_LIST, Record, Abbrev);
Record.clear();
diff --git a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
index 8fe4b8c3e736..80c230da6a4b 100644
--- a/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
+++ b/llvm/lib/Bitcode/Writer/ValueEnumerator.cpp
@@ -78,16 +78,6 @@ struct OrderMap {
} // end anonymous namespace
-/// Look for a value that might be wrapped as metadata, e.g. a value in a
-/// metadata operand. Returns nullptr for a non-wrapped input value if
-/// OnlyWrapped is true, or it returns the input value as-is if false.
-static const Value *skipMetadataWrapper(const Value *V, bool OnlyWrapped) {
- if (const auto *MAV = dyn_cast<MetadataAsValue>(V))
- if (const auto *VAM = dyn_cast<ValueAsMetadata>(MAV->getMetadata()))
- return VAM->getValue();
- return OnlyWrapped ? nullptr : V;
-}
-
static void orderValue(const Value *V, OrderMap &OM) {
if (OM.lookup(V).first)
return;
@@ -139,16 +129,25 @@ static OrderMap orderModule(const Module &M) {
// these before global values, as these will be read before setting the
// global values' initializers. The latter matters for constants which have
// uses towards other constants that are used as initializers.
+ auto orderConstantValue = [&OM](const Value *V) {
+ if ((isa<Constant>(V) && !isa<GlobalValue>(V)) || isa<InlineAsm>(V))
+ orderValue(V, OM);
+ };
for (const Function &F : M) {
if (F.isDeclaration())
continue;
for (const BasicBlock &BB : F)
for (const Instruction &I : BB)
for (const Value *V : I.operands()) {
- if (const Value *Op = skipMetadataWrapper(V, true)) {
- if ((isa<Constant>(*Op) && !isa<GlobalValue>(*Op)) ||
- isa<InlineAsm>(*Op))
- orderValue(Op, OM);
+ if (const auto *MAV = dyn_cast<MetadataAsValue>(V)) {
+ if (const auto *VAM =
+ dyn_cast<ValueAsMetadata>(MAV->getMetadata())) {
+ orderConstantValue(VAM->getValue());
+ } else if (const auto *AL =
+ dyn_cast<DIArgList>(MAV->getMetadata())) {
+ for (const auto *VAM : AL->getArgs())
+ orderConstantValue(VAM->getValue());
+ }
}
}
}
@@ -448,10 +447,17 @@ ValueEnumerator::ValueEnumerator(const Module &M,
continue;
}
- // Local metadata is enumerated during function-incorporation.
- if (isa<LocalAsMetadata>(MD->getMetadata()) ||
- isa<DIArgList>(MD->getMetadata()))
+ // Local metadata is enumerated during function-incorporation, but
+ // any ConstantAsMetadata arguments in a DIArgList should be examined
+ // now.
+ if (isa<LocalAsMetadata>(MD->getMetadata()))
+ continue;
+ if (auto *AL = dyn_cast<DIArgList>(MD->getMetadata())) {
+ for (auto *VAM : AL->getArgs())
+ if (isa<ConstantAsMetadata>(VAM))
+ EnumerateMetadata(&F, VAM);
continue;
+ }
EnumerateMetadata(&F, MD->getMetadata());
}
@@ -620,6 +626,11 @@ void ValueEnumerator::EnumerateFunctionLocalMetadata(
EnumerateFunctionLocalMetadata(getMetadataFunctionID(&F), Local);
}
+void ValueEnumerator::EnumerateFunctionLocalListMetadata(
+ const Function &F, const DIArgList *ArgList) {
+ EnumerateFunctionLocalListMetadata(getMetadataFunctionID(&F), ArgList);
+}
+
void ValueEnumerator::dropFunctionFromMetadata(
MetadataMapType::value_type &FirstMD) {
SmallVector<const MDNode *, 64> Worklist;
@@ -730,7 +741,7 @@ const MDNode *ValueEnumerator::enumerateMetadataImpl(unsigned F, const Metadata
return nullptr;
}
-/// EnumerateFunctionLocalMetadataa - Incorporate function-local metadata
+/// EnumerateFunctionLocalMetadata - Incorporate function-local metadata
/// information reachable from the metadata.
void ValueEnumerator::EnumerateFunctionLocalMetadata(
unsigned F, const LocalAsMetadata *Local) {
@@ -750,6 +761,39 @@ void ValueEnumerator::EnumerateFunctionLocalMetadata(
EnumerateValue(Local->getValue());
}
+/// EnumerateFunctionLocalListMetadata - Incorporate function-local metadata
+/// information reachable from the metadata.
+void ValueEnumerator::EnumerateFunctionLocalListMetadata(
+ unsigned F, const DIArgList *ArgList) {
+ assert(F && "Expected a function");
+
+ // Check to see if it's already in!
+ MDIndex &Index = MetadataMap[ArgList];
+ if (Index.ID) {
+ assert(Index.F == F && "Expected the same function");
+ return;
+ }
+
+ for (ValueAsMetadata *VAM : ArgList->getArgs()) {
+ if (isa<LocalAsMetadata>(VAM)) {
+ assert(MetadataMap.count(VAM) &&
+ "LocalAsMetadata should be enumerated before DIArgList");
+ assert(MetadataMap[VAM].F == F &&
+ "Expected LocalAsMetadata in the same function");
+ } else {
+ assert(isa<ConstantAsMetadata>(VAM) &&
+ "Expected LocalAsMetadata or ConstantAsMetadata");
+ assert(ValueMap.count(VAM->getValue()) &&
+ "Constant should be enumerated beforeDIArgList");
+ EnumerateMetadata(F, VAM);
+ }
+ }
+
+ MDs.push_back(ArgList);
+ Index.F = F;
+ Index.ID = MDs.size();
+}
+
static unsigned getMetadataTypeOrder(const Metadata *MD) {
// Strings are emitted in bulk and must come first.
if (isa<MDString>(MD))
@@ -1072,7 +1116,7 @@ void ValueEnumerator::incorporateFunction(const Function &F) {
// DIArgList entries must come after function-local metadata, as it is not
// possible to forward-reference them.
for (const DIArgList *ArgList : ArgListMDVector)
- EnumerateMetadata(&F, ArgList);
+ EnumerateFunctionLocalListMetadata(F, ArgList);
}
void ValueEnumerator::purgeFunction() {
diff --git a/llvm/lib/Bitcode/Writer/ValueEnumerator.h b/llvm/lib/Bitcode/Writer/ValueEnumerator.h
index 3c3bd0d9fdc7..6c3f6d4ff61e 100644
--- a/llvm/lib/Bitcode/Writer/ValueEnumerator.h
+++ b/llvm/lib/Bitcode/Writer/ValueEnumerator.h
@@ -27,6 +27,7 @@ namespace llvm {
class BasicBlock;
class Comdat;
+class DIArgList;
class Function;
class Instruction;
class LocalAsMetadata;
@@ -286,6 +287,9 @@ class ValueEnumerator {
void EnumerateFunctionLocalMetadata(const Function &F,
const LocalAsMetadata *Local);
void EnumerateFunctionLocalMetadata(unsigned F, const LocalAsMetadata *Local);
+ void EnumerateFunctionLocalListMetadata(const Function &F,
+ const DIArgList *ArgList);
+ void EnumerateFunctionLocalListMetadata(unsigned F, const DIArgList *Arglist);
void EnumerateNamedMDNode(const NamedMDNode *NMD);
void EnumerateValue(const Value *V);
void EnumerateType(Type *T);
diff --git a/llvm/test/DebugInfo/Generic/debug_value_list.ll b/llvm/test/DebugInfo/Generic/debug_value_list.ll
index 177ce29b0c96..2a186549a6ad 100644
--- a/llvm/test/DebugInfo/Generic/debug_value_list.ll
+++ b/llvm/test/DebugInfo/Generic/debug_value_list.ll
@@ -8,17 +8,17 @@ target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16
target triple = "x86_64-pc-windows-msvc19.16.27034"
; CHECK-COUNT-3: llvm.dbg.value(
-; CHECK-SAME: metadata !DIArgList(i32 %a, i32 %b)
+; CHECK-SAME: metadata !DIArgList(i32 %a, i32 %b, i32 5)
; CHECK-SAME: metadata !16,
-; CHECK-SAME: metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus)
+; CHECK-SAME: metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_LLVM_arg, 2, DW_OP_plus)
define dso_local i32 @"?foo@@YAHHH at Z"(i32 %a, i32 %b) local_unnamed_addr !dbg !8 {
entry:
call void @llvm.dbg.value(metadata !DIArgList(i32 %b), metadata !14, metadata !DIExpression(DW_OP_LLVM_arg, 0)), !dbg !17
call void @llvm.dbg.value(metadata !DIArgList(i32 %a), metadata !15, metadata !DIExpression(DW_OP_LLVM_arg, 0)), !dbg !17
call void @llvm.dbg.value(
- metadata !DIArgList(i32 %a, i32 %b),
+ metadata !DIArgList(i32 %a, i32 %b, i32 5),
metadata !16,
- metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus)), !dbg !17
+ metadata !DIExpression(DW_OP_LLVM_arg, 0, DW_OP_LLVM_arg, 1, DW_OP_plus, DW_OP_LLVM_arg, 2, DW_OP_plus)), !dbg !17
%mul = mul nsw i32 %b, %a, !dbg !18
ret i32 %mul, !dbg !18
}
More information about the llvm-commits
mailing list