[PassInstrumentation] Don't insert extra entries in getPassNameForClassName #150029

danilaml · 2025-07-22T14:20:50Z

Don't modify ClassToPassName map unless ClassName is found. Instead, just return empty StringRef if there is no matching entry. This will prevent possible dangling references in ClassToPassName map in case of ClassName being freed.
See https://github.com/llvm/llvm-project/pull/145059/files#r2219763671 for more context.

Don't modify ClassToPassName map unless ClassName is found. Instead, return nullopt if there is no matching entry. This will prevent possible dangling references in ClassToPassName map in case of ClassName being freed. See https://github.com/llvm/llvm-project/pull/145059/files#r2219763671 for more context.

llvmbot · 2025-07-22T14:21:31Z

@llvm/pr-subscribers-lto

@llvm/pr-subscribers-llvm-ir

Author: Danila Malyutin (danilaml)

Changes

Don't modify ClassToPassName map unless ClassName is found. Instead, return nullopt if there is no matching entry. This will prevent possible dangling references in ClassToPassName map in case of ClassName being freed.
See https://github.com/llvm/llvm-project/pull/145059/files#r2219763671 for more context.

Full diff: https://github.com/llvm/llvm-project/pull/150029.diff

7 Files Affected:

(modified) llvm/include/llvm/IR/PassInstrumentation.h (+4-2)
(modified) llvm/include/llvm/Passes/CodeGenPassBuilder.h (+2-1)
(modified) llvm/lib/IR/PassInstrumentation.cpp (+6-2)
(modified) llvm/lib/LTO/LTOBackend.cpp (+1-1)
(modified) llvm/lib/Passes/StandardInstrumentations.cpp (+9-8)
(modified) llvm/tools/llc/NewPMDriver.cpp (+1-1)
(modified) llvm/tools/opt/NewPMDriver.cpp (+1-1)

diff --git a/llvm/include/llvm/IR/PassInstrumentation.h b/llvm/include/llvm/IR/PassInstrumentation.h
index 031571599f9ad..628103c7fe759 100644
--- a/llvm/include/llvm/IR/PassInstrumentation.h
+++ b/llvm/include/llvm/IR/PassInstrumentation.h
@@ -165,7 +165,8 @@ class PassInstrumentationCallbacks {
   /// Add a class name to pass name mapping for use by pass instrumentation.
   LLVM_ABI void addClassToPassName(StringRef ClassName, StringRef PassName);
   /// Get the pass name for a given pass class name.
-  LLVM_ABI StringRef getPassNameForClassName(StringRef ClassName);
+  LLVM_ABI std::optional<StringRef>
+  getPassNameForClassName(StringRef ClassName);
 
 private:
   friend class PassInstrumentation;
@@ -339,7 +340,8 @@ class PassInstrumentation {
   /// Get the pass name for a given pass class name.
   StringRef getPassNameForClassName(StringRef ClassName) const {
     if (Callbacks)
-      return Callbacks->getPassNameForClassName(ClassName);
+      return Callbacks->getPassNameForClassName(ClassName).value_or(
+          StringRef());
     return {};
   }
 };
diff --git a/llvm/include/llvm/Passes/CodeGenPassBuilder.h b/llvm/include/llvm/Passes/CodeGenPassBuilder.h
index b0360f1903c0e..a392f457c2b3b 100644
--- a/llvm/include/llvm/Passes/CodeGenPassBuilder.h
+++ b/llvm/include/llvm/Passes/CodeGenPassBuilder.h
@@ -681,7 +681,8 @@ void CodeGenPassBuilder<Derived, TargetMachineT>::setStartStopPasses(
       }
 
       auto PassName = PIC->getPassNameForClassName(ClassName);
-      if (Info.StartPass == PassName && ++Count == Info.StartInstanceNum)
+      if (PassName && Info.StartPass == *PassName &&
+          ++Count == Info.StartInstanceNum)
         Started = !Info.StartAfter;
 
       return Started;
diff --git a/llvm/lib/IR/PassInstrumentation.cpp b/llvm/lib/IR/PassInstrumentation.cpp
index 94ad124a6c770..ee0ed8807ed12 100644
--- a/llvm/lib/IR/PassInstrumentation.cpp
+++ b/llvm/lib/IR/PassInstrumentation.cpp
@@ -23,17 +23,21 @@ template struct LLVM_EXPORT_TEMPLATE Any::TypeId<const Loop *>;
 
 void PassInstrumentationCallbacks::addClassToPassName(StringRef ClassName,
                                                       StringRef PassName) {
+  assert(!PassName.empty() && "PassName can't be empty!");
   ClassToPassName.try_emplace(ClassName, PassName.str());
 }
 
-StringRef
+std::optional<StringRef>
 PassInstrumentationCallbacks::getPassNameForClassName(StringRef ClassName) {
   if (!ClassToPassNameCallbacks.empty()) {
     for (auto &Fn : ClassToPassNameCallbacks)
       Fn();
     ClassToPassNameCallbacks.clear();
   }
-  return ClassToPassName[ClassName];
+  auto PassNameIter = ClassToPassName.find(ClassName);
+  if (PassNameIter != ClassToPassName.end())
+    return PassNameIter->second;
+  return std::nullopt;
 }
 
 AnalysisKey PassInstrumentationAnalysis::Key;
diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index 5e8cd12fe040b..4d4370394dece 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -346,7 +346,7 @@ static void runNewPMPasses(const Config &Conf, Module &Mod, TargetMachine *TM,
     raw_string_ostream OS(PipelineStr);
     MPM.printPipeline(OS, [&PIC](StringRef ClassName) {
       auto PassName = PIC.getPassNameForClassName(ClassName);
-      return PassName.empty() ? ClassName : PassName;
+      return PassName ? ClassName : *PassName;
     });
     outs() << "pipeline-passes: " << PipelineStr << '\n';
   }
diff --git a/llvm/lib/Passes/StandardInstrumentations.cpp b/llvm/lib/Passes/StandardInstrumentations.cpp
index f165e85baf611..0155ea44bb8e9 100644
--- a/llvm/lib/Passes/StandardInstrumentations.cpp
+++ b/llvm/lib/Passes/StandardInstrumentations.cpp
@@ -454,12 +454,12 @@ template <typename T>
 void ChangeReporter<T>::registerRequiredCallbacks(
     PassInstrumentationCallbacks &PIC) {
   PIC.registerBeforeNonSkippedPassCallback([&PIC, this](StringRef P, Any IR) {
-    saveIRBeforePass(IR, P, PIC.getPassNameForClassName(P));
+    saveIRBeforePass(IR, P, PIC.getPassNameForClassName(P).value_or(""));
   });
 
   PIC.registerAfterPassCallback(
       [&PIC, this](StringRef P, Any IR, const PreservedAnalyses &) {
-        handleIRAfterPass(IR, P, PIC.getPassNameForClassName(P));
+        handleIRAfterPass(IR, P, PIC.getPassNameForClassName(P).value_or(""));
       });
   PIC.registerAfterPassInvalidatedCallback(
       [this](StringRef P, const PreservedAnalyses &) {
@@ -970,7 +970,7 @@ bool PrintIRInstrumentation::shouldPrintBeforePass(StringRef PassID) {
   if (shouldPrintBeforeAll())
     return true;
 
-  StringRef PassName = PIC->getPassNameForClassName(PassID);
+  StringRef PassName = PIC->getPassNameForClassName(PassID).value_or("");
   return is_contained(printBeforePasses(), PassName);
 }
 
@@ -978,7 +978,7 @@ bool PrintIRInstrumentation::shouldPrintAfterPass(StringRef PassID) {
   if (shouldPrintAfterAll())
     return true;
 
-  StringRef PassName = PIC->getPassNameForClassName(PassID);
+  StringRef PassName = PIC->getPassNameForClassName(PassID).value_or("");
   return is_contained(printAfterPasses(), PassName);
 }
 
@@ -1080,10 +1080,10 @@ void OptPassGateInstrumentation::registerCallbacks(
 
   PIC.registerShouldRunOptionalPassCallback(
       [this, &PIC](StringRef ClassName, Any IR) {
-        StringRef PassName = PIC.getPassNameForClassName(ClassName);
-        if (PassName.empty())
+        auto PassName = PIC.getPassNameForClassName(ClassName);
+        if (!PassName)
           return this->shouldRun(ClassName, IR);
-        return this->shouldRun(PassName, IR);
+        return this->shouldRun(*PassName, IR);
       });
 }
 
@@ -2501,7 +2501,8 @@ void PrintCrashIRInstrumentation::registerCallbacks(
         raw_string_ostream OS(SavedIR);
         OS << formatv("*** Dump of {0}IR Before Last Pass {1}",
                       llvm::forcePrintModuleIR() ? "Module " : "", PassID);
-        if (!isInteresting(IR, PassID, PIC.getPassNameForClassName(PassID))) {
+        if (!isInteresting(IR, PassID,
+                           PIC.getPassNameForClassName(PassID).value_or(""))) {
           OS << " Filtered Out ***\n";
           return;
         }
diff --git a/llvm/tools/llc/NewPMDriver.cpp b/llvm/tools/llc/NewPMDriver.cpp
index fa82689ecf9ae..559aff3533a64 100644
--- a/llvm/tools/llc/NewPMDriver.cpp
+++ b/llvm/tools/llc/NewPMDriver.cpp
@@ -164,7 +164,7 @@ int llvm::compileModuleWithNewPM(
     raw_string_ostream OS(PipelineStr);
     MPM.printPipeline(OS, [&PIC](StringRef ClassName) {
       auto PassName = PIC.getPassNameForClassName(ClassName);
-      return PassName.empty() ? ClassName : PassName;
+      return PassName ? ClassName : *PassName;
     });
     outs() << PipelineStr << '\n';
     return 0;
diff --git a/llvm/tools/opt/NewPMDriver.cpp b/llvm/tools/opt/NewPMDriver.cpp
index 7d168a6ceb17c..dda7204718252 100644
--- a/llvm/tools/opt/NewPMDriver.cpp
+++ b/llvm/tools/opt/NewPMDriver.cpp
@@ -534,7 +534,7 @@ bool llvm::runPassPipeline(
     raw_string_ostream SOS(Pipeline);
     MPM.printPipeline(SOS, [&PIC](StringRef ClassName) {
       auto PassName = PIC.getPassNameForClassName(ClassName);
-      return PassName.empty() ? ClassName : PassName;
+      return PassName ? ClassName : *PassName;
     });
     outs() << Pipeline;
     outs() << "\n";

llvm/include/llvm/IR/PassInstrumentation.h

llvm/lib/LTO/LTOBackend.cpp

llvm/tools/llc/NewPMDriver.cpp

llvm/tools/opt/NewPMDriver.cpp

snehasish

lgtm

llvm/include/llvm/IR/PassInstrumentation.h

nikic

LGTM

nikic · 2025-07-22T15:50:09Z

llvm/lib/IR/PassInstrumentation.cpp

+  auto PassNameIter = ClassToPassName.find(ClassName);
+  if (PassNameIter != ClassToPassName.end())
+    return PassNameIter->second;
+  return {};


Suggested change

return {};

return "";

is a bit clearer, I think.

Isn't StringRef() different from StringRef("")? They'd compare the same, but go through different constructors (no need to check and store the string literal). Not that it'd matter, but at least in other places {} is used so I'd go for consistency.

llvm-ci · 2025-07-22T17:20:13Z

LLVM Buildbot has detected a new failure on builder llvm-clang-x86_64-expensive-checks-ubuntu running on as-builder-4 while building llvm at step 7 "test-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/187/builds/8385

Here is the relevant piece of the build log for the reference

Step 7 (test-check-all) failure: Test just built components: check-all completed (failure)
******************** TEST 'LLVM :: TableGen/RuntimeLibcallEmitter.td' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
/home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/build/bin/llvm-tblgen -gen-runtime-libcalls -I /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/../../include /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/RuntimeLibcallEmitter.td | /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/build/bin/FileCheck /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/RuntimeLibcallEmitter.td # RUN: at line 1
+ /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/build/bin/llvm-tblgen -gen-runtime-libcalls -I /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/../../include /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/RuntimeLibcallEmitter.td
+ /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/build/bin/FileCheck /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/RuntimeLibcallEmitter.td
/home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/RuntimeLibcallEmitter.td:98:16: error: CHECK-NEXT: expected string not found in input
// CHECK-NEXT: sqrtl_f80 = 7, // sqrtl
               ^
<stdin>:32:23: note: scanning from here
 calloc = 6, // calloc
                      ^
<stdin>:34:2: note: possible intended match here
 sqrtl_f80 = 8, // sqrtl
 ^

Input file: <stdin>
Check file: /home/buildbot/worker/as-builder-4/ramdisk/expensive-checks/llvm-project/llvm/test/TableGen/RuntimeLibcallEmitter.td

-dump-input=help explains the following input dump.

Input was:
<<<<<<
           .
           .
           .
          27:  ___memcpy = 1, // ___memcpy 
          28:  ___memset = 2, // ___memset 
          29:  __ashlsi3 = 3, // __ashlsi3 
          30:  __lshrdi3 = 4, // __lshrdi3 
          31:  bzero = 5, // bzero 
          32:  calloc = 6, // calloc 
next:98'0                           X error: no match found
          33:  sqrtl_f128 = 7, // sqrtl 
next:98'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~
          34:  sqrtl_f80 = 8, // sqrtl 
next:98'0     ~~~~~~~~~~~~~~~~~~~~~~~~~
next:98'1      ?                        possible intended match
          35:  NumLibcallImpls = 9 
next:98'0     ~~~~~~~~~~~~~~~~~~~~~
          36: }; 
next:98'0     ~~~
          37: } // End namespace RTLIB 
next:98'0     ~~~~~~~~~~~~~~~~~~~~~~~~~
          38: } // End namespace llvm 
next:98'0     ~~~~~~~~~~~~~~~~~~~~~~~~
          39: #endif 
next:98'0     ~~~~~~~
...

…ssName (llvm#150029) Don't modify ClassToPassName map unless ClassName is found. Instead, just return empty StringRef if there is no matching entry. This will prevent possible dangling references in ClassToPassName map in case of ClassName being freed. See https://github.com/llvm/llvm-project/pull/145059/files#r2219763671 for more context.

danilaml requested review from mtrofin, nikic and snehasish July 22, 2025 14:20

danilaml added the llvm Umbrella label for LLVM issues label Jul 22, 2025

llvmbot added LTO Link time optimization (regular/full LTO or ThinLTO) llvm:ir labels Jul 22, 2025

nikic reviewed Jul 22, 2025

View reviewed changes

llvm/include/llvm/IR/PassInstrumentation.h Outdated Show resolved Hide resolved

danilaml mentioned this pull request Jul 22, 2025

[OptBisect][IR] Adding a new OptPassGate for disabling passes via name #145059

Merged

nikic reviewed Jul 22, 2025

View reviewed changes

llvm/lib/LTO/LTOBackend.cpp Outdated Show resolved Hide resolved

llvm/tools/llc/NewPMDriver.cpp Outdated Show resolved Hide resolved

llvm/tools/opt/NewPMDriver.cpp Outdated Show resolved Hide resolved

Use empty StringRef instead of nullopt optional

16f12ce

snehasish approved these changes Jul 22, 2025

View reviewed changes

llvm/include/llvm/IR/PassInstrumentation.h Outdated Show resolved Hide resolved

nikic approved these changes Jul 22, 2025

View reviewed changes

danilaml changed the title ~~[PassInstrumentation] Make getPassNameForClassName return optional~~ [PassInstrumentation] Don't insert extra entries in getPassNameForClassName Jul 22, 2025

danilaml merged commit b3e720b into llvm:main Jul 22, 2025
9 checks passed

danilaml deleted the fix-passname-for-classname branch July 22, 2025 16:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[PassInstrumentation] Don't insert extra entries in getPassNameForClassName #150029

[PassInstrumentation] Don't insert extra entries in getPassNameForClassName #150029

Uh oh!

danilaml commented Jul 22, 2025 •

edited

Loading

Uh oh!

llvmbot commented Jul 22, 2025 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snehasish left a comment

Uh oh!

Uh oh!

nikic left a comment

Uh oh!

nikic Jul 22, 2025

Uh oh!

danilaml Jul 22, 2025

Uh oh!

Uh oh!

llvm-ci commented Jul 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[PassInstrumentation] Don't insert extra entries in getPassNameForClassName #150029

[PassInstrumentation] Don't insert extra entries in getPassNameForClassName #150029

Uh oh!

Conversation

danilaml commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

snehasish left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

nikic Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

danilaml Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

llvm-ci commented Jul 22, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

danilaml commented Jul 22, 2025 •

edited

Loading

llvmbot commented Jul 22, 2025 •

edited

Loading