[clang] [clang] Do not clear FP pragma stack when instantiating functions (PR #70646)

Serge Pavlov via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 13 00:13:10 PST 2023


https://github.com/spavloff updated https://github.com/llvm/llvm-project/pull/70646

>From 7882ea746ecf8721356fc6afbd7798b8db4e185d Mon Sep 17 00:00:00 2001
From: Serge Pavlov <sepavloff at gmail.com>
Date: Mon, 30 Oct 2023 14:38:50 +0700
Subject: [PATCH 1/3] [clang] Do not clear FP pragma stack when instantiating
 functions

When instantiation function, a call to Sema::resetFPOption was used to
set the FP options associated with AST node. However this function also
cleared FP pragma stack. It is incorrect, as template instantiation
takes place on AST representation and semantic information like the FP
pragma stack should not influence it. Tt was a reason for miscompilation
in some cases.

To make the Sema interface more convenient, `resetFPOptions` does not
clear FP pragma stack anymore. Instead, it is cleared in
`FpPragmaStackSaveRAII`, which is used in parsing only.

This change must fix https://github.com/llvm/llvm-project/issues/69717
(Problems with float_control pragma stack in Clang 17.x).
---
 clang/include/clang/Sema/Sema.h |  5 +++--
 clang/test/Sema/PR69717.cpp     | 17 +++++++++++++++++
 2 files changed, 20 insertions(+), 2 deletions(-)
 create mode 100644 clang/test/Sema/PR69717.cpp

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1e9752345ffd173..1e8892d1a550453 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -710,9 +710,11 @@ class Sema final {
     return result;
   }
 
+  // Saves the current floating-point pragma stack and clear it in this Sema.
   class FpPragmaStackSaveRAII {
   public:
-    FpPragmaStackSaveRAII(Sema &S) : S(S), SavedStack(S.FpPragmaStack) {}
+    FpPragmaStackSaveRAII(Sema &S)
+        : S(S), SavedStack(std::move(S.FpPragmaStack)) {}
     ~FpPragmaStackSaveRAII() { S.FpPragmaStack = std::move(SavedStack); }
 
   private:
@@ -722,7 +724,6 @@ class Sema final {
 
   void resetFPOptions(FPOptions FPO) {
     CurFPFeatures = FPO;
-    FpPragmaStack.Stack.clear();
     FpPragmaStack.CurrentValue = FPO.getChangesFrom(FPOptions(LangOpts));
   }
 
diff --git a/clang/test/Sema/PR69717.cpp b/clang/test/Sema/PR69717.cpp
new file mode 100644
index 000000000000000..3207092b948ae86
--- /dev/null
+++ b/clang/test/Sema/PR69717.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -verify -fsyntax-only %s
+// expected-no-diagnostics
+
+// Testcase for https://github.com/llvm/llvm-project/issues/69717
+
+#pragma float_control(precise, on, push)
+
+template<typename T>
+constexpr T multi(T x, T y) {
+  return x * y;
+}
+
+int multi_i(int x, int y) {
+  return multi<int>(x, y);
+}
+
+#pragma float_control(pop)

>From 44b096810d88da1d1331be12802c68cdf8e1785b Mon Sep 17 00:00:00 2001
From: Serge Pavlov <sepavloff at gmail.com>
Date: Mon, 13 Nov 2023 15:02:40 +0700
Subject: [PATCH 2/3] Explicitly clear FpPragmaStack

---
 clang/include/clang/Sema/Sema.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1e8892d1a550453..1d61dc7ff0aa26d 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -714,7 +714,9 @@ class Sema final {
   class FpPragmaStackSaveRAII {
   public:
     FpPragmaStackSaveRAII(Sema &S)
-        : S(S), SavedStack(std::move(S.FpPragmaStack)) {}
+      : S(S), SavedStack(std::move(S.FpPragmaStack)) {
+      S.FpPragmaStack.Stack.clear();
+    }
     ~FpPragmaStackSaveRAII() { S.FpPragmaStack = std::move(SavedStack); }
 
   private:

>From 244e967fb48c0b3b75682a6a32c1afd148c8c203 Mon Sep 17 00:00:00 2001
From: Serge Pavlov <sepavloff at gmail.com>
Date: Mon, 13 Nov 2023 15:12:49 +0700
Subject: [PATCH 3/3] Fix clang-format errors

---
 clang/include/clang/Sema/Sema.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 1d61dc7ff0aa26d..d88c20e5170a661 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -714,7 +714,7 @@ class Sema final {
   class FpPragmaStackSaveRAII {
   public:
     FpPragmaStackSaveRAII(Sema &S)
-      : S(S), SavedStack(std::move(S.FpPragmaStack)) {
+        : S(S), SavedStack(std::move(S.FpPragmaStack)) {
       S.FpPragmaStack.Stack.clear();
     }
     ~FpPragmaStackSaveRAII() { S.FpPragmaStack = std::move(SavedStack); }



More information about the cfe-commits mailing list