[PATCH] D65848: [ThinLTO][AutoFDO] Fix memory corruption due to race condition from thin backends

[Wenlei He via Phabricator via llvm-commits]

wenlei created this revision.
wenlei added reviewers: wmi, davidxl, danielcdh.
Herald added subscribers: llvm-commits, dexonsmith, hiraditya, inglorion, mehdi_amini.
Herald added a project: LLVM.

This commit fixed a race condition from multi-threaded thinLTO backends that causes non-deterministic memory corruption for a data structure used only by AutoFDO with compact binary profile.
GUIDToFuncNameMap, a static data member of type DenseMap in FunctionSamples is used as a per-module mapping from function name MD5 to name string when input AutoFDO profile is in compact binary format. However with ThinLTO, we can have parallel backends modifying and accessing the class static map concurrently. The fix is to mark GUIDToFuncNameMap as thread_local.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D65848

Files:
  llvm/include/llvm/ProfileData/SampleProf.h
  llvm/lib/ProfileData/SampleProf.cpp


Index: llvm/lib/ProfileData/SampleProf.cpp
===================================================================
--- llvm/lib/ProfileData/SampleProf.cpp
+++ llvm/lib/ProfileData/SampleProf.cpp
@@ -28,8 +28,8 @@
 namespace llvm {
 namespace sampleprof {
 SampleProfileFormat FunctionSamples::Format;
-DenseMap<uint64_t, StringRef> FunctionSamples::GUIDToFuncNameMap;
-Module *FunctionSamples::CurrentModule;
+thread_local DenseMap<uint64_t, StringRef> FunctionSamples::GUIDToFuncNameMap;
+thread_local Module *FunctionSamples::CurrentModule;
 } // namespace sampleprof
 } // namespace llvm
 
Index: llvm/include/llvm/ProfileData/SampleProf.h
===================================================================
--- llvm/include/llvm/ProfileData/SampleProf.h
+++ llvm/include/llvm/ProfileData/SampleProf.h
@@ -473,9 +473,10 @@
 
   static SampleProfileFormat Format;
   /// GUIDToFuncNameMap saves the mapping from GUID to the symbol name, for
-  /// all the function symbols defined or declared in CurrentModule.
-  static DenseMap<uint64_t, StringRef> GUIDToFuncNameMap;
-  static Module *CurrentModule;
+  /// all the function symbols defined or declared in CurrentModule. These
+  /// need to be thread_local because we can have parallel ThinLTO backends.
+  thread_local static DenseMap<uint64_t, StringRef> GUIDToFuncNameMap;
+  thread_local static Module *CurrentModule;
 
   class GUIDToFuncNameMapper {
   public:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D65848.213798.patch
Type: text/x-patch
Size: 1418 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190807/e68be03e/attachment.bin>