forked from OSchip/llvm-project
Avoid using unspecified ordering in MetadataLoader::MetadataLoaderImpl::parseOneMetadata.
Summary:
MetadataLoader::MetadataLoaderImpl::parseOneMetadata uses
the following construct in a number of places:
```
MetadataList.assignValue(<...>, NextMetadataNo++);
```
There, NextMetadataNo gets incremented, and since the order
of arguments evaluation is not specified, that can happen
before or after other arguments are evaluated.
In a few cases the other arguments indirectly use NextMetadataNo.
For instance, it's
```
MetadataList.assignValue(
GET_OR_DISTINCT(DIModule,
(Context, getMDOrNull(Record[1]),
getMDString(Record[2]), getMDString(Record[3]),
getMDString(Record[4]), getMDString(Record[5]))),
NextMetadataNo++);
```
getMDOrNull calls getMD that uses NextMetadataNo:
```
MetadataList.getMetadataFwdRef(NextMetadataNo);
```
Therefore, the order of evaluation becomes important. That caused
a very subtle LLD crash that only happens if compiled with GCC or
if LLD is built with LTO. In the case if LLD is compiled with Clang
and regular linking mode, everything worked as intended.
This change extracts incrementing of NextMetadataNo outside of
the arguments list to guarantee the correct order of evaluation.
For the record, this has taken 3 days to track to the origin. It all
started with a ThinLTO bot in Chrome not being able to link a target
if debug info is enabled.
Reviewers: pcc, mehdi_amini
Reviewed By: mehdi_amini
Subscribers: aprantl, llvm-commits
Differential Revision: https://reviews.llvm.org/D29204
llvm-svn: 293291
This commit is contained in:
Ivan Krasin
parent
403b093eff
commit
c05c9db364
|
|
@ -922,7 +922,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
// If this isn't a LocalAsMetadata record, we're dropping it. This used
|
||||
// to be legal, but there's no upgrade path.
|
||||
auto dropRecord = [&] {
|
||||
MetadataList.assignValue(MDNode::get(Context, None), NextMetadataNo++);
|
||||
MetadataList.assignValue(MDNode::get(Context, None), NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
};
|
||||
if (Record.size() != 2) {
|
||||
dropRecord();
|
||||
|
|
@ -937,7 +938,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
|
||||
MetadataList.assignValue(
|
||||
LocalAsMetadata::get(ValueList.getValueFwdRef(Record[1], Ty)),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_OLD_NODE: {
|
||||
|
|
@ -962,7 +964,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
} else
|
||||
Elts.push_back(nullptr);
|
||||
}
|
||||
MetadataList.assignValue(MDNode::get(Context, Elts), NextMetadataNo++);
|
||||
MetadataList.assignValue(MDNode::get(Context, Elts), NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_VALUE: {
|
||||
|
|
@ -975,7 +978,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
|
||||
MetadataList.assignValue(
|
||||
ValueAsMetadata::get(ValueList.getValueFwdRef(Record[1], Ty)),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_DISTINCT_NODE:
|
||||
|
|
@ -988,7 +992,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
Elts.push_back(getMDOrNull(ID));
|
||||
MetadataList.assignValue(IsDistinct ? MDNode::getDistinct(Context, Elts)
|
||||
: MDNode::get(Context, Elts),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_LOCATION: {
|
||||
|
|
@ -1002,7 +1007,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
Metadata *InlinedAt = getMDOrNull(Record[4]);
|
||||
MetadataList.assignValue(
|
||||
GET_OR_DISTINCT(DILocation, (Context, Line, Column, Scope, InlinedAt)),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_GENERIC_DEBUG: {
|
||||
|
|
@ -1022,7 +1028,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
DwarfOps.push_back(getMDOrNull(Record[I]));
|
||||
MetadataList.assignValue(
|
||||
GET_OR_DISTINCT(GenericDINode, (Context, Tag, Header, DwarfOps)),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_SUBRANGE: {
|
||||
|
|
@ -1033,7 +1040,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
MetadataList.assignValue(
|
||||
GET_OR_DISTINCT(DISubrange,
|
||||
(Context, Record[1], unrotateSign(Record[2]))),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_ENUMERATOR: {
|
||||
|
|
@ -1044,7 +1052,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
MetadataList.assignValue(
|
||||
GET_OR_DISTINCT(DIEnumerator, (Context, unrotateSign(Record[1]),
|
||||
getMDString(Record[2]))),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_BASIC_TYPE: {
|
||||
|
|
@ -1056,7 +1065,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
GET_OR_DISTINCT(DIBasicType,
|
||||
(Context, Record[1], getMDString(Record[2]), Record[3],
|
||||
Record[4], Record[5])),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_DERIVED_TYPE: {
|
||||
|
|
@ -1072,7 +1082,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
getDITypeRefOrNull(Record[5]),
|
||||
getDITypeRefOrNull(Record[6]), Record[7], Record[8],
|
||||
Record[9], Flags, getDITypeRefOrNull(Record[11]))),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_COMPOSITE_TYPE: {
|
||||
|
|
@ -1137,7 +1148,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
if (!IsNotUsedInTypeRef && Identifier)
|
||||
MetadataList.addTypeRef(*Identifier, *cast<DICompositeType>(CT));
|
||||
|
||||
MetadataList.assignValue(CT, NextMetadataNo++);
|
||||
MetadataList.assignValue(CT, NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_SUBROUTINE_TYPE: {
|
||||
|
|
@ -1154,7 +1166,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
|
||||
MetadataList.assignValue(
|
||||
GET_OR_DISTINCT(DISubroutineType, (Context, Flags, CC, Types)),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
|
||||
|
|
@ -1168,7 +1181,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
(Context, getMDOrNull(Record[1]),
|
||||
getMDString(Record[2]), getMDString(Record[3]),
|
||||
getMDString(Record[4]), getMDString(Record[5]))),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
|
||||
|
|
@ -1184,7 +1198,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
Record.size() == 3 ? DIFile::CSK_None
|
||||
: static_cast<DIFile::ChecksumKind>(Record[3]),
|
||||
Record.size() == 3 ? nullptr : getMDString(Record[4]))),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_COMPILE_UNIT: {
|
||||
|
|
@ -1203,7 +1218,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
Record.size() <= 14 ? 0 : Record[14],
|
||||
Record.size() <= 16 ? true : Record[16]);
|
||||
|
||||
MetadataList.assignValue(CU, NextMetadataNo++);
|
||||
MetadataList.assignValue(CU, NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
|
||||
// Move the Upgrade the list of subprograms.
|
||||
if (Metadata *SPs = getMDOrNullWithoutPlaceholders(Record[11]))
|
||||
|
|
@ -1250,7 +1266,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
getMDOrNull(Record[16 + Offset]), // declaration
|
||||
getMDOrNull(Record[17 + Offset]) // variables
|
||||
));
|
||||
MetadataList.assignValue(SP, NextMetadataNo++);
|
||||
MetadataList.assignValue(SP, NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
|
||||
// Upgrade sp->function mapping to function->sp mapping.
|
||||
if (HasFn) {
|
||||
|
|
@ -1275,7 +1292,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
GET_OR_DISTINCT(DILexicalBlock,
|
||||
(Context, getMDOrNull(Record[1]),
|
||||
getMDOrNull(Record[2]), Record[3], Record[4])),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_LEXICAL_BLOCK_FILE: {
|
||||
|
|
@ -1287,7 +1305,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
GET_OR_DISTINCT(DILexicalBlockFile,
|
||||
(Context, getMDOrNull(Record[1]),
|
||||
getMDOrNull(Record[2]), Record[3])),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_NAMESPACE: {
|
||||
|
|
@ -1301,7 +1320,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
(Context, getMDOrNull(Record[1]),
|
||||
getMDOrNull(Record[2]), getMDString(Record[3]),
|
||||
Record[4], ExportSymbols)),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_MACRO: {
|
||||
|
|
@ -1313,7 +1333,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
GET_OR_DISTINCT(DIMacro,
|
||||
(Context, Record[1], Record[2], getMDString(Record[3]),
|
||||
getMDString(Record[4]))),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_MACRO_FILE: {
|
||||
|
|
@ -1325,7 +1346,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
GET_OR_DISTINCT(DIMacroFile,
|
||||
(Context, Record[1], Record[2], getMDOrNull(Record[3]),
|
||||
getMDOrNull(Record[4]))),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_TEMPLATE_TYPE: {
|
||||
|
|
@ -1336,7 +1358,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
MetadataList.assignValue(GET_OR_DISTINCT(DITemplateTypeParameter,
|
||||
(Context, getMDString(Record[1]),
|
||||
getDITypeRefOrNull(Record[2]))),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_TEMPLATE_VALUE: {
|
||||
|
|
@ -1349,7 +1372,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
(Context, Record[1], getMDString(Record[2]),
|
||||
getDITypeRefOrNull(Record[3]),
|
||||
getMDOrNull(Record[4]))),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_GLOBAL_VAR: {
|
||||
|
|
@ -1367,7 +1391,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
getMDOrNull(Record[4]), Record[5],
|
||||
getDITypeRefOrNull(Record[6]), Record[7], Record[8],
|
||||
getMDOrNull(Record[10]), Record[11])),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
} else if (Version == 0) {
|
||||
// Upgrade old metadata, which stored a global variable reference or a
|
||||
// ConstantInt here.
|
||||
|
|
@ -1399,7 +1424,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
getMDOrNull(Record[10]), AlignInBits));
|
||||
|
||||
auto *DGVE = DIGlobalVariableExpression::getDistinct(Context, DGV, Expr);
|
||||
MetadataList.assignValue(DGVE, NextMetadataNo++);
|
||||
MetadataList.assignValue(DGVE, NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
if (Attach)
|
||||
Attach->addDebugInfo(DGVE);
|
||||
} else
|
||||
|
|
@ -1432,7 +1458,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
getMDOrNull(Record[3 + HasTag]), Record[4 + HasTag],
|
||||
getDITypeRefOrNull(Record[5 + HasTag]),
|
||||
Record[6 + HasTag], Flags, AlignInBits)),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_EXPRESSION: {
|
||||
|
|
@ -1449,7 +1476,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
|
||||
MetadataList.assignValue(
|
||||
GET_OR_DISTINCT(DIExpression, (Context, makeArrayRef(Record).slice(1))),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_GLOBAL_VAR_EXPR: {
|
||||
|
|
@ -1460,7 +1488,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
MetadataList.assignValue(GET_OR_DISTINCT(DIGlobalVariableExpression,
|
||||
(Context, getMDOrNull(Record[1]),
|
||||
getMDOrNull(Record[2]))),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_OBJC_PROPERTY: {
|
||||
|
|
@ -1474,7 +1503,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
getMDOrNull(Record[2]), Record[3],
|
||||
getMDString(Record[4]), getMDString(Record[5]),
|
||||
Record[6], getDITypeRefOrNull(Record[7]))),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_IMPORTED_ENTITY: {
|
||||
|
|
@ -1487,7 +1517,8 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
(Context, Record[1], getMDOrNull(Record[2]),
|
||||
getDITypeRefOrNull(Record[3]), Record[4],
|
||||
getMDString(Record[5]))),
|
||||
NextMetadataNo++);
|
||||
NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_STRING_OLD: {
|
||||
|
|
@ -1497,13 +1528,15 @@ Error MetadataLoader::MetadataLoaderImpl::parseOneMetadata(
|
|||
HasSeenOldLoopTags |= mayBeOldLoopAttachmentTag(String);
|
||||
++NumMDStringLoaded;
|
||||
Metadata *MD = MDString::get(Context, String);
|
||||
MetadataList.assignValue(MD, NextMetadataNo++);
|
||||
MetadataList.assignValue(MD, NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
break;
|
||||
}
|
||||
case bitc::METADATA_STRINGS: {
|
||||
auto CreateNextMDString = [&](StringRef Str) {
|
||||
++NumMDStringLoaded;
|
||||
MetadataList.assignValue(MDString::get(Context, Str), NextMetadataNo++);
|
||||
MetadataList.assignValue(MDString::get(Context, Str), NextMetadataNo);
|
||||
NextMetadataNo++;
|
||||
};
|
||||
if (Error Err = parseMetadataStrings(Record, Blob, CreateNextMDString))
|
||||
return Err;
|
||||
|
|
|
|||
Loading…
Reference in New Issue