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

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: David Peixotto (dmpots)

<details>
<summary>Changes</summary>

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.

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


1 Files Affected:

- (modified) lldb/unittests/Core/PluginManagerTest.cpp (+17-3) 


``````````diff
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",

``````````

</details>


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