[llvm] 683f2df - [SampleProfile] Fix bug where remapper returns empty string and crashing Sample Profile loader (#71479)

[via llvm-commits]

Author: William Junda Huang
Date: 2023-11-10T21:38:13Z
New Revision: 683f2df6e5efc72b1b61d9863e3eec86ca92824b

URL: https://github.com/llvm/llvm-project/commit/683f2df6e5efc72b1b61d9863e3eec86ca92824b
DIFF: https://github.com/llvm/llvm-project/commit/683f2df6e5efc72b1b61d9863e3eec86ca92824b.diff

LOG: [SampleProfile] Fix bug where remapper returns empty string and crashing Sample Profile loader (#71479)

Normally SampleContext does not allow using an empty StirngRef to
construct an object, this is to prevent bugs reading the profile.
However empty names may be emitted by a function which its name is
intentionally set to empty, or a bug in the remapper that returns an
empty string. Regardless, converting it to FunctionId first will prevent
the assert, and that assert check is unnecessary, which will be
addressed in another patch

Added: 
    llvm/test/Transforms/SampleProfile/remap-unmatched.ll

Modified: 
    llvm/lib/ProfileData/SampleProfReader.cpp
    llvm/lib/Transforms/IPO/SampleProfile.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/lib/ProfileData/SampleProfReader.cpp b/llvm/lib/ProfileData/SampleProfReader.cpp
index a69e9d5849047ab..ed92713c2c627ca 100644
--- a/llvm/lib/ProfileData/SampleProfReader.cpp
+++ b/llvm/lib/ProfileData/SampleProfReader.cpp
@@ -1788,8 +1788,11 @@ void SampleProfileReaderItaniumRemapper::applyRemapping(LLVMContext &Ctx) {
 
 std::optional<StringRef>
 SampleProfileReaderItaniumRemapper::lookUpNameInProfile(StringRef Fname) {
-  if (auto Key = Remappings->lookup(Fname))
-    return NameMap.lookup(Key);
+  if (auto Key = Remappings->lookup(Fname)) {
+    StringRef Result = NameMap.lookup(Key);
+    if (!Result.empty())
+      return Result;
+  }
   return std::nullopt;
 }
 

diff  --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp
index a7773737ef16bd3..063f7b42022ff83 100644
--- a/llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -1630,7 +1630,7 @@ void SampleProfileLoader::promoteMergeNotInlinedContextSamples(
         // separate map so that it does not rehash the original profile.
         if (!OutlineFS)
           OutlineFS = &OutlineFunctionSamples[
-              FunctionSamples::getCanonicalFnName(Callee->getName())];
+              FunctionId(FunctionSamples::getCanonicalFnName(Callee->getName()))];
         OutlineFS->merge(*FS, 1);
         // Set outlined profile to be synthetic to not bias the inliner.
         OutlineFS->SetContextSynthetic();
@@ -2675,12 +2675,12 @@ bool SampleProfileLoader::runOnFunction(Function &F, ModuleAnalysisManager *AM)
     // into base.
     if (!Samples) {
       StringRef CanonName = FunctionSamples::getCanonicalFnName(F);
-      auto It = OutlineFunctionSamples.find(CanonName);
+      auto It = OutlineFunctionSamples.find(FunctionId(CanonName));
       if (It != OutlineFunctionSamples.end()) {
         Samples = &It->second;
       } else if (auto Remapper = Reader->getRemapper()) {
         if (auto RemppedName = Remapper->lookUpNameInProfile(CanonName)) {
-          It = OutlineFunctionSamples.find(*RemppedName);
+          It = OutlineFunctionSamples.find(FunctionId(*RemppedName));
           if (It != OutlineFunctionSamples.end())
             Samples = &It->second;
         }

diff  --git a/llvm/test/Transforms/SampleProfile/remap-unmatched.ll b/llvm/test/Transforms/SampleProfile/remap-unmatched.ll
new file mode 100644
index 000000000000000..b1dc2266e03fab6
--- /dev/null
+++ b/llvm/test/Transforms/SampleProfile/remap-unmatched.ll
@@ -0,0 +1,8 @@
+; This test should not crash.
+; RUN: opt %s -passes=sample-profile -sample-profile-file=%S/Inputs/remap.prof -sample-profile-remapping-file=%S/Inputs/remap.map
+
+define void @foo() #0 {
+  ret void
+}
+
+attributes #0 = { "use-sample-profile" }