[Lldb-commits] [lldb] [lldb] Fix plugin manager test failure on windows (PR #134173)

[David Peixotto via lldb-commits]

https://github.com/dmpots created https://github.com/llvm/llvm-project/pull/134173

This is an attempt to fix a test failure from #133794 when running on windows builds. I suspect we are running into a case where the [ICF](https://learn.microsoft.com/en-us/cpp/build/reference/opt-optimizations?view=msvc-170) optimization kicks in and combines the CreateSystemRuntimePlugin* functions into a single address. This means that we cannot uniquely unregister the plugin based on its create function address.

The fix is have each create function return a different (bogus) value.

>From 99870557937f87277d62d8ec1fb511ad2e7c8aed Mon Sep 17 00:00:00 2001
From: David Peixotto <peix at meta.com>
Date: Wed, 2 Apr 2025 16:06:06 -0700
Subject: [PATCH] [lldb] Fix plugin manager test failure on windows

This is an attempt to fix a test failure from #133794 when running on
windows builds. I suspect we are running into a case where the
[ICF](https://learn.microsoft.com/en-us/cpp/build/reference/opt-optimizations?view=msvc-170)
optimization kicks in and combines the CreateSystemRuntimePlugin*
functions into a single address. This means that we cannot uniquely
unregister the plugin based on its create function address.

The fix is have each create function return a different (bogus)
value.
---
 lldb/unittests/Core/PluginManagerTest.cpp | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/lldb/unittests/Core/PluginManagerTest.cpp b/lldb/unittests/Core/PluginManagerTest.cpp
index ca1003ca9a85a..9b0ce2286d273 100644
--- a/lldb/unittests/Core/PluginManagerTest.cpp
+++ b/lldb/unittests/Core/PluginManagerTest.cpp
@@ -7,11 +7,21 @@ using namespace lldb;
 using namespace lldb_private;
 
 // Mock system runtime plugin create functions.
-SystemRuntime *CreateSystemRuntimePluginA(Process *process) { return nullptr; }
+// Make them all return different values to avoid the ICF optimization
+// from combining them into the same function. The values returned
+// are not valid SystemRuntime pointers, but they are unique and
+// sufficient for testing.
+SystemRuntime *CreateSystemRuntimePluginA(Process *process) {
+  return (SystemRuntime *)0x1;
+}
 
-SystemRuntime *CreateSystemRuntimePluginB(Process *process) { return nullptr; }
+SystemRuntime *CreateSystemRuntimePluginB(Process *process) {
+  return (SystemRuntime *)0x2;
+}
 
-SystemRuntime *CreateSystemRuntimePluginC(Process *process) { return nullptr; }
+SystemRuntime *CreateSystemRuntimePluginC(Process *process) {
+  return (SystemRuntime *)0x3;
+}
 
 // Test class for testing the PluginManager.
 // The PluginManager modifies global state when registering new plugins. This
@@ -24,6 +34,10 @@ class PluginManagerTest : public testing::Test {
 
   // Add mock system runtime plugins for testing.
   void RegisterMockSystemRuntimePlugins() {
+    // Make sure the create functions all have different addresses.
+    ASSERT_NE(CreateSystemRuntimePluginA, CreateSystemRuntimePluginB);
+    ASSERT_NE(CreateSystemRuntimePluginB, CreateSystemRuntimePluginC);
+
     ASSERT_TRUE(PluginManager::RegisterPlugin("a", "test instance A",
                                               CreateSystemRuntimePluginA));
     ASSERT_TRUE(PluginManager::RegisterPlugin("b", "test instance B",