|
| 1 | +From 7e40076d7cff79d8412d5b1f40c36b9f4aaa1f76 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Romaric Jodin <rjodin@google.com> |
| 3 | +Date: Fri, 8 May 2026 16:14:06 +0200 |
| 4 | +Subject: [PATCH] [SSAF] Fix MSVC template parsing error in SerializationFormat |
| 5 | + |
| 6 | +This commit fixes a hard compilation error on Windows when building |
| 7 | +translation units that instantiate the `Add` registry template |
| 8 | +(such as `PointerFlowAnalysis.cpp`). |
| 9 | + |
| 10 | +**Root Cause:** |
| 11 | +When compiling on Windows, Clang defaults to MSVC compatibility mode |
| 12 | +(`-fms-compatibility`). Under this mode, Clang's two-phase template |
| 13 | +lookup struggles to resolve function-local static variables |
| 14 | +(`SavedSerialize` and `SavedDeserialize`) captured by a local class |
| 15 | +(`ConcreteCodec`) inside an uninstantiated template. During Phase 1 |
| 16 | +parsing, Clang incorrectly falls back to assuming these variables must |
| 17 | +be members of a dependent base class, rewriting them to |
| 18 | +`this->SavedSerialize`. During Phase 2 instantiation, compilation |
| 19 | +fails because the base class (`Codec`) has no such members. |
| 20 | + |
| 21 | +**The Fix:** |
| 22 | +Hoisted `SavedSerialize` and `SavedDeserialize` out of the constructor |
| 23 | +scope, making them `static inline` members of the `Add` class template. |
| 24 | +This allows Clang's Phase 1 parser to perfectly resolve the symbols |
| 25 | +without relying on broken MSVC fallbacks. |
| 26 | + |
| 27 | +**Why this is safe (Addressing the `dlopen` comment):** |
| 28 | +The original author explicitly commented that they avoided `static inline` |
| 29 | +class members to prevent Linux symbol visibility issues across shared |
| 30 | +library boundaries (`dlopen` with `RTLD_LOCAL`). |
| 31 | + |
| 32 | +That concern is still honored by this fix. The visibility risk only applies |
| 33 | +if the `ConcreteCodec`'s *own* execution state relied directly on static |
| 34 | +members. By moving the static variables to the `Add` factory class, |
| 35 | +`ConcreteCodec` continues to store `SerFn` and `DesFn` as strictly |
| 36 | +**instance members**. The `ConcreteCodec` constructor safely snapshots |
| 37 | +the plugin's local copy of `Add::SavedSerialize` at the moment of |
| 38 | +instantiation. Since the virtual methods executed by the host still |
| 39 | +read exclusively from the isolated instance state, the runtime behavior |
| 40 | +remains completely identical and safe. |
| 41 | +--- |
| 42 | + .../Core/Serialization/SerializationFormat.h | 7 +++++-- |
| 43 | + 1 file changed, 5 insertions(+), 2 deletions(-) |
| 44 | + |
| 45 | +diff --git a/clang/include/clang/ScalableStaticAnalysisFramework/Core/Serialization/SerializationFormat.h b/clang/include/clang/ScalableStaticAnalysisFramework/Core/Serialization/SerializationFormat.h |
| 46 | +index fd261c6d9a72..9ab79dda3fc1 100644 |
| 47 | +--- a/clang/include/clang/ScalableStaticAnalysisFramework/Core/Serialization/SerializationFormat.h |
| 48 | ++++ b/clang/include/clang/ScalableStaticAnalysisFramework/Core/Serialization/SerializationFormat.h |
| 49 | +@@ -133,6 +133,9 @@ protected: |
| 50 | + using TypedSerializerFn = |
| 51 | + llvm::function_ref<SerRet(const AnalysisResultT &, SerArgs...)>; |
| 52 | + |
| 53 | ++ static inline TypedSerializerFn SavedSerialize; |
| 54 | ++ static inline DeserializerFn SavedDeserialize; |
| 55 | ++ |
| 56 | + /// Takes the plugin's typed serializer and the deserializer, and |
| 57 | + /// inserts them into \c llvm::Registry<Codec>. |
| 58 | + Add(TypedSerializerFn TypedSerialize, DeserializerFn Deserialize) { |
| 59 | +@@ -154,8 +157,8 @@ protected: |
| 60 | + /// visibility issues across shared library boundaries on Linux |
| 61 | + /// (where \c dlopen with \c RTLD_LOCAL can give the host and |
| 62 | + /// plugin separate copies of \c static \c inline members). |
| 63 | +- static TypedSerializerFn SavedSerialize = TypedSerialize; |
| 64 | +- static DeserializerFn SavedDeserialize = Deserialize; |
| 65 | ++ SavedSerialize = TypedSerialize; |
| 66 | ++ SavedDeserialize = Deserialize; |
| 67 | + |
| 68 | + /// Concrete subclass of \c Codec for \c AnalysisResultT. |
| 69 | + /// The \c serialize() override performs the downcast from |
| 70 | +-- |
| 71 | +2.54.0.563.g4f69b47b94-goog |
| 72 | + |
0 commit comments