[llvm] 95b27b2 - [llvm][ADT] Fix Any with msvc and lto

Sebastian Neubauer via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 19 08:15:44 PST 2022


Author: Sebastian Neubauer
Date: 2022-12-19T17:05:53+01:00
New Revision: 95b27b2a1e0e9fe0781ec76015b4016b9d04423d

URL: https://github.com/llvm/llvm-project/commit/95b27b2a1e0e9fe0781ec76015b4016b9d04423d
DIFF: https://github.com/llvm/llvm-project/commit/95b27b2a1e0e9fe0781ec76015b4016b9d04423d.diff

LOG: [llvm][ADT] Fix Any with msvc and lto

llvm::Any had and has several bugs, so we eventually want to replace it
with std::any.

Unfortunately, we cannot do that right now because of bugs in the msvc
standard library that are only fixed in VS 2022 17.4.

When lto is enabled in msvc, constant symbols end up at the same
address, breaking the TypeId implementation of llvm::Any.
Make the TypeId<T>::Id non-const to fix this.

I was able to find an easy reproducer (tried in godbolt with
x64 msvc v19.32 and `/GL` as compiler flags to enable lto):

```c++

template <typename T> struct TypeId {
  // Remove the const here and below to make it work.
  static const char Id;
};

template <typename T> const char TypeId<T>::Id = 0;

template <typename A, typename B>
bool isSame() {
  return &TypeId<A>::Id == &TypeId<B>::Id;
}

class A {};
class B {};

int main() {
  // This should output "is same 0" because the addresses of A's and B's
  // TypeId::Id should be different.
  printf("is same %d\n", isSame<A, B>());
  return 0;
}
```

Differential Revision: https://reviews.llvm.org/D139974

Added: 
    

Modified: 
    llvm/include/llvm/ADT/Any.h

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/ADT/Any.h b/llvm/include/llvm/ADT/Any.h
index 0e2f587c7cba4..934dfc142d376 100644
--- a/llvm/include/llvm/ADT/Any.h
+++ b/llvm/include/llvm/ADT/Any.h
@@ -31,9 +31,9 @@ class LLVM_EXTERNAL_VISIBILITY Any {
   // identifier for the type `T`. It is explicitly marked with default
   // visibility so that when `-fvisibility=hidden` is used, the loader still
   // merges duplicate definitions across DSO boundaries.
-  template <typename T> struct TypeId {
-    static const char Id;
-  };
+  // We also cannot mark it as `const`, otherwise msvc merges all definitions
+  // when lto is enabled, making any comparison return true.
+  template <typename T> struct TypeId { static char Id; };
 
   struct StorageBase {
     virtual ~StorageBase() = default;
@@ -117,7 +117,7 @@ class LLVM_EXTERNAL_VISIBILITY Any {
   std::unique_ptr<StorageBase> Storage;
 };
 
-template <typename T> const char Any::TypeId<T>::Id = 0;
+template <typename T> char Any::TypeId<T>::Id = 0;
 
 template <typename T> bool any_isa(const Any &Value) {
   if (!Value.Storage)


        


More information about the llvm-commits mailing list