[PATCH] D108142: [SamplePGO] Fixing a memory issue when creating profiles on-demand

[Hongtao Yu via Phabricator via llvm-commits]

hoy created this revision.
Herald added subscribers: modimo, wenlei.
hoy requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

There is a on-dmeand creation of function profile during top-down processing in the sample loader when merging uninlined callees.  During the profile creation, a stack string object is used to store a newly-created MD5 name, which is then used by reference as hash key in the profile map. This makes the hash key a dangling reference when later on the stack string object is deallocated.

The issue only happens with md5 profile use and was exposed by context split work for CS profile. I'm making a fix by storing newly created names in the reader.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108142

Files:
  llvm/include/llvm/ProfileData/SampleProf.h
  llvm/include/llvm/ProfileData/SampleProfReader.h


Index: llvm/include/llvm/ProfileData/SampleProfReader.h
===================================================================
--- llvm/include/llvm/ProfileData/SampleProfReader.h
+++ llvm/include/llvm/ProfileData/SampleProfReader.h
@@ -407,7 +407,14 @@
     std::string FGUID;
     StringRef CanonName = FunctionSamples::getCanonicalFnName(F);
     CanonName = getRepInFormat(CanonName, useMD5(), FGUID);
-    return &Profiles[CanonName];
+    auto It = Profiles.find(CanonName);
+    if (It == Profiles.end()) {
+      if (!FGUID.empty())
+        CanonName = *NameBuffer.insert(FGUID).first;
+      return &Profiles[CanonName];
+    } else {
+      return &It->second;
+    }
   }
 
   /// Return the samples collected for function \p F.
@@ -503,6 +510,9 @@
   /// Memory buffer holding the profile file.
   std::unique_ptr<MemoryBuffer> Buffer;
 
+  /// Extra name buffer holding names created on demand.
+  std::set<std::string> NameBuffer;
+
   /// Profile summary information.
   std::unique_ptr<ProfileSummary> Summary;
 
Index: llvm/include/llvm/ProfileData/SampleProf.h
===================================================================
--- llvm/include/llvm/ProfileData/SampleProf.h
+++ llvm/include/llvm/ProfileData/SampleProf.h
@@ -104,10 +104,10 @@
 /// current Format uses MD5 to represent the string.
 static inline StringRef getRepInFormat(StringRef Name, bool UseMD5,
                                        std::string &GUIDBuf) {
-  if (Name.empty())
+  if (Name.empty() || !UseMD5)
     return Name;
   GUIDBuf = std::to_string(Function::getGUID(Name));
-  return UseMD5 ? StringRef(GUIDBuf) : Name;
+  return GUIDBuf;
 }
 
 static inline uint64_t SPVersion() { return 103; }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D108142.366659.patch
Type: text/x-patch
Size: 1695 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210816/429c1bee/attachment.bin>