[llvm] [SampleProfile] Fix bug where intentionally constructed function with empty name asserts. (PR #71479)

[William Junda Huang via llvm-commits]

https://github.com/huangjd updated https://github.com/llvm/llvm-project/pull/71479

>From 453e690bfedc454fcd040c5d851f45cd25f882e8 Mon Sep 17 00:00:00 2001
From: William Huang <williamjhuang at google.com>
Date: Tue, 7 Nov 2023 02:33:46 +0000
Subject: [PATCH 1/2] [SampleProfile] Fix bug where intentionally constructed
 function with empty name asserts.

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 (which should be
investigated separately) that remaps a non-empty name to 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
---
 llvm/lib/Transforms/IPO/SampleProfile.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

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;
         }

>From 7ea2ed8dc65d76b4a2b460de002702d449a079aa Mon Sep 17 00:00:00 2001
From: William Huang <williamjhuang at google.com>
Date: Wed, 8 Nov 2023 02:28:06 +0000
Subject: [PATCH 2/2] Added test case Added safeguard to remapper

---
 llvm/lib/ProfileData/SampleProfReader.cpp             | 7 +++++--
 llvm/test/Transforms/SampleProfile/remap-unmatched.ll | 8 ++++++++
 2 files changed, 13 insertions(+), 2 deletions(-)
 create mode 100644 llvm/test/Transforms/SampleProfile/remap-unmatched.ll

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/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" }