Fix some race condition in ConstString around Mangled name handling

[tberghammer]

Summary

Fix some race condition in ConstString around Mangled name handling

[clayborg]

Looks fine. Might want to make "h" const in places noted by inlined comments, but other than that it looks good.

source/Core/ConstString.cpp

39 ↗

(On Diff #38006)

const uint8_t h?

52 ↗

(On Diff #38006)

const uint8_t h?

65 ↗

(On Diff #38006)

const uint8_t h?

70 ↗

(On Diff #38006)

const uint8_t h?

139 ↗

(On Diff #38006)

const uint8_t h?

lldb/trunk/source/Core/ConstString.cpp

GetStringMapEntryFromKeyData (const char *keyData)
         return *reinterpret_cast<StringPoolEntryType*>(ptr);
     }
 
     size_t
     GetConstCStringLength (const char *ccstr) const
     {
         if (ccstr)
         {
-            const StringPoolEntryType&entry = GetStringMapEntryFromKeyData (ccstr);
+            const uint8_t h = hash (llvm::StringRef(ccstr));
+            llvm::sys::SmartScopedReader<false> rlock(m_string_pools[h].m_mutex);
+            const StringPoolEntryType& entry = GetStringMapEntryFromKeyData (ccstr);
             return entry.getKey().size();
         }
         return 0;
     }
 
     StringPoolValueType
     GetMangledCounterpart (const char *ccstr) const
     {
         if (ccstr)
+        {
+            const uint8_t h = hash (llvm::StringRef(ccstr));
+            llvm::sys::SmartScopedReader<false> rlock(m_string_pools[h].m_mutex);
             return GetStringMapEntryFromKeyData (ccstr).getValue();
+        }
         return 0;
     }
 
     bool
     SetMangledCounterparts (const char *key_ccstr, const char *value_ccstr)
     {
         if (key_ccstr && value_ccstr)
         {
+            {
+                const uint8_t h = hash (llvm::StringRef(key_ccstr));
+                llvm::sys::SmartScopedWriter<false> wlock(m_string_pools[h].m_mutex);
                 GetStringMapEntryFromKeyData (key_ccstr).setValue(value_ccstr);
+            }
+            {
+                const uint8_t h = hash (llvm::StringRef(value_ccstr));
+                llvm::sys::SmartScopedWriter<false> wlock(m_string_pools[h].m_mutex);
                 GetStringMapEntryFromKeyData (value_ccstr).setValue(key_ccstr);
+            }
             return true;
         }
         return false;
     }
 
     const char *
     GetConstCString (const char *cstr)
     {
GetConstCStringWithLength (const char *cstr, size_t cstr_len)
         return nullptr;
     }
 
     const char *
     GetConstCStringWithStringRef (const llvm::StringRef &string_ref)
     {
         if (string_ref.data())
         {
-            uint8_t h = hash (string_ref);
+            const uint8_t h = hash (string_ref);
 
             {
                 llvm::sys::SmartScopedReader<false> rlock(m_string_pools[h].m_mutex);
                 auto it = m_string_pools[h].m_string_map.find (string_ref);
                 if (it != m_string_pools[h].m_string_map.end())
                     return it->getKeyData();
             }
 
             llvm::sys::SmartScopedWriter<false> wlock(m_string_pools[h].m_mutex);
             StringPoolEntryType& entry = *m_string_pools[h].m_string_map.insert (std::make_pair (string_ref, nullptr)).first;
             return entry.getKeyData();
         }
         return nullptr;
     }
 
     const char *
     GetConstCStringAndSetMangledCounterPart (const char *demangled_cstr, const char *mangled_ccstr)
     {
         if (demangled_cstr)
         {
+            const char *demangled_ccstr = nullptr;
+
+            {
                 llvm::StringRef string_ref (demangled_cstr);
-            uint8_t h = hash (string_ref);
+                const uint8_t h = hash (string_ref);
                 llvm::sys::SmartScopedWriter<false> wlock(m_string_pools[h].m_mutex);
 
                 // Make string pool entry with the mangled counterpart already set
-            StringPoolEntryType& entry = *m_string_pools[h].m_string_map.insert (std::make_pair (string_ref, mangled_ccstr)).first;
+                StringPoolEntryType& entry = *m_string_pools[h].m_string_map.insert (
+                    std::make_pair (string_ref, mangled_ccstr)).first;
 
                 // Extract the const version of the demangled_cstr
-            const char *demangled_ccstr = entry.getKeyData();
+                demangled_ccstr = entry.getKeyData();
+            }
+
+            {
                 // Now assign the demangled const string as the counterpart of the
                 // mangled const string...
+                const uint8_t h = hash (llvm::StringRef(mangled_ccstr));
+                llvm::sys::SmartScopedWriter<false> wlock(m_string_pools[h].m_mutex);
                 GetStringMapEntryFromKeyData (mangled_ccstr).setValue(demangled_ccstr);
+            }
+
             // Return the constant demangled C string
             return demangled_ccstr;
         }
         return nullptr;
     }
 
     const char *
     GetConstTrimmedCStringWithLength (const char *cstr, size_t cstr_len)
MemorySize() const
             for (const auto& entry : pool.m_string_map)
                 mem_size += sizeof(StringPoolEntryType) + entry.getKey().size();
         }
         return mem_size;
     }
 
 protected:
     uint8_t
-    hash(const llvm::StringRef &s)
+    hash(const llvm::StringRef &s) const
     {
         uint32_t h = llvm::HashString(s);
         return ((h >> 24) ^ (h >> 16) ^ (h >> 8) ^ h) & 0xff;
     }
 
     struct PoolEntry
     {
         mutable llvm::sys::SmartRWMutex<false> m_mutex;