[PATCH] D155380: [clang] Fix delayed template parsing

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff created this revision.
sepavloff added reviewers: rsmith, rjmccall, aaron.ballman, efriedma, mgorny.
Herald added a project: All.
sepavloff requested review of this revision.
Herald added a project: clang.

Commit 98390ccb80569e8fbb20e6c996b4b8cff87fbec6 <https://reviews.llvm.org/rG98390ccb80569e8fbb20e6c996b4b8cff87fbec6> fixed late template
instantiation by clearing FP pragma stack before instantiation. This
solution was based on the assumptions:

- FP pragma stack is not used anymore and it is safe to clear it,
- Default FP options are determined by command line options.

Both the assumptions are wrong. When compilation produces precompiled
header file, state of the stack is serialized and then restored when the
precompiled header is used. Delayed template parsing occurs at the end
of translation unit but before serialization, so clearing FP pragma
stack effects serialized representation. When the precompiled file is
loaded, some conditions can be broken and clang crashed, it was
described in https://github.com/llvm/llvm-project/issues/63704. The
crash was observed only in few cases, on most buildbots it was absent.

The violation of expected conditions was caused by violation of the
second assumption. FPEvalMethod can be modified by target, so it is not
possible to deduce it from LangOptions only. So default FP state read
from precompiled header was different from the state in the initialized
Sema, and this was the crash reason.

Only two targets do such modification of default FP options, these are
i386 and AIX. so the problem was hard to reproduce.

Delayed template parsing should occur with empty pragma stack, so it
must be cleared before the instantiation, but the stack now is saved
and restored after the instantiation is done. Also default FP state is
not deduced from command line options, it is taken from the value stored
in the FP pragma stack.

This change should fix https://github.com/llvm/llvm-project/issues/63704.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D155380

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseTemplate.cpp
  clang/test/PCH/late-parsed-instantiations.cpp


Index: clang/test/PCH/late-parsed-instantiations.cpp
===================================================================
--- clang/test/PCH/late-parsed-instantiations.cpp
+++ clang/test/PCH/late-parsed-instantiations.cpp
@@ -4,7 +4,9 @@
 // RUN: %clang_cc1 -fdelayed-template-parsing -std=c++14 -emit-pch -fpch-instantiate-templates %s -o %t.pch -verify
 // RUN: %clang_cc1 -fdelayed-template-parsing -std=c++14 -include-pch %t.pch %s -verify
 
-// XFAIL: target={{.*}}-aix{{.*}}
+// Run this test for i686 as this is the target that modifies default FP options.
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -fdelayed-template-parsing -std=c++14 -emit-pch -fpch-instantiate-templates %s -o %t.pch -verify
+// RUN: %clang_cc1 -triple i686-pc-linux-gnu -fdelayed-template-parsing -std=c++14 -include-pch %t.pch %s -verify
 
 #ifndef HEADER_INCLUDED
 
Index: clang/lib/Parse/ParseTemplate.cpp
===================================================================
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -1744,6 +1744,7 @@
 
   // Parsing should occur with empty FP pragma stack and FP options used in the
   // point of the template definition.
+  Sema::FpPragmaStackSaveRAII SavedStack(Actions);
   Actions.resetFPOptions(LPT.FPO);
 
   assert(!LPT.Toks.empty() && "Empty body!");
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -710,10 +710,19 @@
     return result;
   }
 
+  class FpPragmaStackSaveRAII {
+  public:
+    FpPragmaStackSaveRAII(Sema &S) : S(S), SavedStack(S.FpPragmaStack) {}
+    ~FpPragmaStackSaveRAII() { S.FpPragmaStack = std::move(SavedStack); }
+  private:
+    Sema &S;
+    PragmaStack<FPOptionsOverride> SavedStack;
+  };
+
   void resetFPOptions(FPOptions FPO) {
     CurFPFeatures = FPO;
     FpPragmaStack.Stack.clear();
-    FpPragmaStack.CurrentValue = FPO.getChangesFrom(FPOptions(LangOpts));
+    FpPragmaStack.CurrentValue = FpPragmaStack.DefaultValue;
   }
 
   // RAII object to push / pop sentinel slots for all MS #pragma stacks.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D155380.540729.patch
Type: text/x-patch
Size: 2139 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230715/50b08855/attachment.bin>