[PATCH] D112481: [Sema] fix nondeterminism in ASTContext::getDeducedTemplateSpecializationType

[Igor Sugak via Phabricator via cfe-commits]

sugak created this revision.
sugak added a reviewer: rsmith.
Herald added a subscriber: mgrang.
sugak requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

`DeducedTemplateSpecializationTypes` is a  `llvm::FoldingSet<DeducedTemplateSpecializationType>` [1], where  `FoldingSetNodeID` is based on the values: `TemplateName`, `QualType`, `IsDeducedAsDependent`, those values are also used `DeducedTemplateSpecializationType` constructor arguments.

The problem is that `FoldingSetNodeID` created by the static `DeducedTemplateSpecializationType::Profile` may not be equal to`FoldingSetNodeID` created by a member `DeducedTemplateSpecializationType::Profile` of instance created with the same `TemplateName`, `QualType`, `IsDeducedAsDependent`, which makes `DeducedTemplateSpecializationTypes`.

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 we have a cache hit. If the bucket for the second key is different from the first key, it's a no hit, even if `A.Profile()` returns  `FoldingSetNodeID {A, B, true}`. Since  `TemplateName`, `QualType` parameter values involve memory pointers, the query 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.

1. https://llvm.org/docs/ProgrammersManual.html#llvm-adt-foldingset-h


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112481

Files:
  clang/lib/AST/ASTContext.cpp


Index: clang/lib/AST/ASTContext.cpp
===================================================================
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -5491,13 +5491,16 @@
   void *InsertPos = nullptr;
   llvm::FoldingSetNodeID ID;
   DeducedTemplateSpecializationType::Profile(ID, Template, DeducedType,
-                                             IsDependent);
+                                             IsDependent || Template.isDependent());
   if (DeducedTemplateSpecializationType *DTST =
           DeducedTemplateSpecializationTypes.FindNodeOrInsertPos(ID, InsertPos))
     return QualType(DTST, 0);
 
   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);
   if (InsertPos)
     DeducedTemplateSpecializationTypes.InsertNode(DTST, InsertPos);


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D112481.382073.patch
Type: text/x-patch
Size: 979 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20211025/3ac471fb/attachment-0001.bin>