[clang] a075d67 - [Sema] fix nondeterminism in ASTContext::getDeducedTemplateSpecializationType

[Wei Wang via cfe-commits]

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);