Author: Wei Wang Date: 2021-11-19T13:22:07-08:00 New Revision: a075d67222832c234296ffd605f19e33023e6060
URL: https://github.com/llvm/llvm-project/commit/a075d67222832c234296ffd605f19e33023e6060 DIFF: https://github.com/llvm/llvm-project/commit/a075d67222832c234296ffd605f19e33023e6060.diff LOG: [Sema] fix nondeterminism in ASTContext::getDeducedTemplateSpecializationType `DeducedTemplateSpecializationTypes` is a `llvm::FoldingSet<DeducedTemplateSpecializationType>` [1], where `FoldingSetNodeID` is based on the values: {`TemplateName`, `QualType`, `IsDeducedAsDependent`}, those values are also used as `DeducedTemplateSpecializationType` constructor arguments. A `FoldingSetNodeID` created by the static `DeducedTemplateSpecializationType::Profile` may not be equal to`FoldingSetNodeID` created by a member `DeducedTemplateSpecializationType::Profile` of an instance created with the same {`TemplateName`, `QualType`, `IsDeducedAsDependent`}, which makes `DeducedTemplateSpecializationTypes` lookups nondeterministic. Specifically, while `IsDeducedAsDependent` value is passes to the constructor, `IsDependent()` method on the created instance may return a different value, because `IsDependent` is not saved as is: ```name=clang/include/clang/AST/Type.h DeducedTemplateSpecializationType(TemplateName Template, QualType DeducedAsType, bool IsDeducedAsDependent) : DeducedType(DeducedTemplateSpecialization, DeducedAsType, toTypeDependence(Template.getDependence()) | // <~ also considers `TemplateName` parameter (IsDeducedAsDependent ? TypeDependence::DependentInstantiation : TypeDependence::None)), ``` For example, if an instance A with key `FoldingSetNodeID {A, B, false}` is inserted. Then a key `FoldingSetNodeID {A, B, true}` is probed: If it happens to correspond to the same bucket in `FoldingSet` as the first key, and `A.Profile()` returns `FoldingSetNodeID {A, B, true}`, then it's a hit. If the bucket for the second key is different from the first key, instance A is not considered at all, and it's a no hit, even if `A.Profile()` returns `FoldingSetNodeID {A, B, true}`. Since `TemplateName`, `QualType` parameter values involve memory pointers, the lookup result depend on allocator, and may differ from run to run. When this is used as part of modules compilation, it may result in "module out of date" errors, if imported modules are built on different machines. This makes `ASTContext::getDeducedTemplateSpecializationType` consider `Template.isDependent()` similar `DeducedTemplateSpecializationType` constructor. Tested on a very big codebase, by running modules compilations from directories with varied path length (seem to affect allocator seed). 1. https://llvm.org/docs/ProgrammersManual.html#llvm-adt-foldingset-h Patch by Wei Wang and Igor Sugak! Reviewed By: bruno Differential Revision: https://reviews.llvm.org/D112481 Added: Modified: clang/include/clang/AST/Type.h clang/lib/AST/ASTContext.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h index fd25ec25d4f2..4c89c297bf34 100644 --- a/clang/include/clang/AST/Type.h +++ b/clang/include/clang/AST/Type.h @@ -5073,8 +5073,10 @@ class DeducedTemplateSpecializationType : public DeducedType, static void Profile(llvm::FoldingSetNodeID &ID, TemplateName Template, QualType Deduced, bool IsDependent) { Template.Profile(ID); - ID.AddPointer(Deduced.getAsOpaquePtr()); - ID.AddBoolean(IsDependent); + QualType CanonicalType = + Deduced.isNull() ? Deduced : Deduced.getCanonicalType(); + ID.AddPointer(CanonicalType.getAsOpaquePtr()); + ID.AddBoolean(IsDependent || Template.isDependent()); } static bool classof(const Type *T) { diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index f0b931bdc905..294cc20f76c5 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -5676,6 +5676,9 @@ QualType ASTContext::getDeducedTemplateSpecializationType( auto *DTST = new (*this, TypeAlignment) DeducedTemplateSpecializationType(Template, DeducedType, IsDependent); + llvm::FoldingSetNodeID TempID; + DTST->Profile(TempID); + assert(ID == TempID && "ID does not match"); Types.push_back(DTST); DeducedTemplateSpecializationTypes.InsertNode(DTST, InsertPos); return QualType(DTST, 0); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
