From bab85e2506c4272bbb71eddb7d9b49571b6fa4b0 Mon Sep 17 00:00:00 2001
From: Yedidya Feldblum <yfeldblum@fb.com>
Date: Fri, 16 Oct 2015 12:46:00 -0700
Subject: [PATCH] Avoid the ODR issue with ThreadLocalDetail's kInvalid

Summary: [Folly] Avoid the ODR issue with `ThreadLocalDetail`'s `kInvalid`.

The problem is that it is a `static constexpr` class member, so pull it out of the class.

Reviewed By: bmaurer

Differential Revision: D2549272

fb-gh-sync-id: 28a73e73b9cf9f21bee2bba2125513c02ef56ce2
---
 folly/detail/ThreadLocalDetail.h | 22 +++++++++-------------
 folly/test/ThreadLocalTest.cpp   |  8 --------
 2 files changed, 9 insertions(+), 21 deletions(-)

diff --git a/folly/detail/ThreadLocalDetail.h b/folly/detail/ThreadLocalDetail.h
index 691a5fa5..da3ee073 100644
--- a/folly/detail/ThreadLocalDetail.h
+++ b/folly/detail/ThreadLocalDetail.h
@@ -161,7 +161,7 @@ struct ThreadEntry {
   ThreadEntry* prev;
 };
 
-
+constexpr uint32_t kEntryIDInvalid = std::numeric_limits<uint32_t>::max();
 
 // Held in a singleton to track our global instances.
 // We have one of these per "Tag", by default one for the whole system
@@ -178,20 +178,19 @@ struct StaticMeta {
   // fail). It allows us to keep a constexpr constructor and avoid SIOF.
   class EntryID {
    public:
-    static constexpr uint32_t kInvalid = std::numeric_limits<uint32_t>::max();
     std::atomic<uint32_t> value;
 
-    constexpr EntryID() : value(kInvalid) {
+    constexpr EntryID() : value(kEntryIDInvalid) {
     }
 
     EntryID(EntryID&& other) noexcept : value(other.value.load()) {
-      other.value = kInvalid;
+      other.value = kEntryIDInvalid;
     }
 
     EntryID& operator=(EntryID&& other) {
       assert(this != &other);
       value = other.value.load();
-      other.value = kInvalid;
+      other.value = kEntryIDInvalid;
       return *this;
     }
 
@@ -208,7 +207,7 @@ struct StaticMeta {
 
     uint32_t getOrAllocate() {
       uint32_t id = getOrInvalid();
-      if (id != kInvalid) {
+      if (id != kEntryIDInvalid) {
         return id;
       }
       // The lock inside allocate ensures that a single value is allocated
@@ -356,7 +355,7 @@ struct StaticMeta {
     std::lock_guard<std::mutex> g(meta.lock_);
 
     id = ent->value.load();
-    if (id != EntryID::kInvalid) {
+    if (id != kEntryIDInvalid) {
       return id;
     }
 
@@ -368,7 +367,7 @@ struct StaticMeta {
     }
 
     uint32_t old_id = ent->value.exchange(id);
-    DCHECK_EQ(old_id, EntryID::kInvalid);
+    DCHECK_EQ(old_id, kEntryIDInvalid);
     return id;
   }
 
@@ -379,8 +378,8 @@ struct StaticMeta {
       std::vector<ElementWrapper> elements;
       {
         std::lock_guard<std::mutex> g(meta.lock_);
-        uint32_t id = ent->value.exchange(EntryID::kInvalid);
-        if (id == EntryID::kInvalid) {
+        uint32_t id = ent->value.exchange(kEntryIDInvalid);
+        if (id == kEntryIDInvalid) {
           return;
         }
 
@@ -524,9 +523,6 @@ struct StaticMeta {
   }
 };
 
-template <class Tag>
-constexpr uint32_t StaticMeta<Tag>::EntryID::kInvalid;
-
 #ifdef FOLLY_TLD_USE_FOLLY_TLS
 template <class Tag>
 FOLLY_TLS ThreadEntry StaticMeta<Tag>::threadEntry_ = {nullptr, 0,
diff --git a/folly/test/ThreadLocalTest.cpp b/folly/test/ThreadLocalTest.cpp
index 16d7a35c..82e6eb4f 100644
--- a/folly/test/ThreadLocalTest.cpp
+++ b/folly/test/ThreadLocalTest.cpp
@@ -250,14 +250,6 @@ TEST(ThreadLocal, InterleavedDestructors) {
   EXPECT_EQ(wVersionMax * 10, Widget::totalVal_);
 }
 
-TEST(ThreadLocalPtr, ODRUseEntryIDkInvalid) {
-  // EntryID::kInvalid is odr-used
-  // see http://en.cppreference.com/w/cpp/language/static
-  const uint32_t* pInvalid =
-    &(threadlocal_detail::StaticMeta<void>::EntryID::kInvalid);
-  EXPECT_EQ(std::numeric_limits<uint32_t>::max(), *pInvalid);
-}
-
 class SimpleThreadCachedInt {
 
   class NewTag;
-- 
2.34.1