[llvm] [Offload] Stop using global ctor/dtors in Offload unit tests (PR #149299)

[via llvm-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-offload

Author: Callum Fare (callumfare)

<details>
<summary>Changes</summary>

Use of a static global to manage the lifetime of liboffload could lead to problems with teardown order. Change the Offload unit test environment to explicitly init and teardown liboffload in `main` instead.

* This requires opting out of using `llvm_gtest_main`, so we can't use `add_unittest`, but manually create the test executables instead
* We can't use GTest's `::testing::AddGlobalTestEnvironment()` to do this as test discovery happens before global environment setup, and we depend on liboffload calls to discover devices for parameterized tests.

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


3 Files Affected:

- (modified) offload/unittests/CMakeLists.txt (+15-4) 
- (modified) offload/unittests/OffloadAPI/CMakeLists.txt (+1-1) 
- (modified) offload/unittests/OffloadAPI/common/Environment.cpp (+22-10) 


``````````diff
diff --git a/offload/unittests/CMakeLists.txt b/offload/unittests/CMakeLists.txt
index 388d15f834b1d..ada2706bb0320 100644
--- a/offload/unittests/CMakeLists.txt
+++ b/offload/unittests/CMakeLists.txt
@@ -85,13 +85,24 @@ function(add_offload_unittest test_dirname)
   set(target_name "${test_dirname}.unittests")
 
   list(TRANSFORM ARGN PREPEND "${CMAKE_CURRENT_SOURCE_DIR}/" OUTPUT_VARIABLE files)
+  list(APPEND files ${CMAKE_CURRENT_SOURCE_DIR}/common/Environment.cpp)
 
-  add_unittest(OffloadUnitTests "${target_name}"
-    ${CMAKE_CURRENT_SOURCE_DIR}/common/Environment.cpp
-    ${files})
+  if( NOT LLVM_BUILD_TESTS )
+    set(EXCLUDE_FROM_ALL ON)
+  endif()
+  add_llvm_executable(${target_name} IGNORE_EXTERNALIZE_DEBUGINFO NO_INSTALL_RPATH ${files})
+  get_subproject_title(subproject_title)
+  set_target_properties(${target_name} PROPERTIES FOLDER "${subproject_title}/Tests/Unit")
+
+  target_link_options(${target_name} PRIVATE "${LLVM_UNITTEST_LINK_FLAGS}")
+
+  set(outdir ${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_CFG_INTDIR})
+  set_output_directory(${target_name} BINARY_DIR ${outdir} LIBRARY_DIR ${outdir})
+  target_link_libraries(${target_name} PRIVATE llvm_gtest ${LLVM_PTHREAD_LIB} ${PLUGINS_TEST_COMMON})
+
+  add_dependencies(OffloadUnitTests ${target_name})
   add_dependencies(${target_name} ${PLUGINS_TEST_COMMON} offload_device_binaries)
   target_compile_definitions(${target_name} PRIVATE DEVICE_CODE_PATH="${OFFLOAD_TEST_DEVICE_CODE_PATH}")
-  target_link_libraries(${target_name} PRIVATE ${PLUGINS_TEST_COMMON})
   target_include_directories(${target_name} PRIVATE ${PLUGINS_TEST_INCLUDE})
 endfunction()
 
diff --git a/offload/unittests/OffloadAPI/CMakeLists.txt b/offload/unittests/OffloadAPI/CMakeLists.txt
index d76338612210d..a393bf5b39bb6 100644
--- a/offload/unittests/OffloadAPI/CMakeLists.txt
+++ b/offload/unittests/OffloadAPI/CMakeLists.txt
@@ -16,7 +16,7 @@ add_offload_unittest("event"
 
 add_offload_unittest("init"
     init/olInit.cpp)
-target_compile_definitions("init.unittests" PRIVATE DISABLE_WRAPPER)
+target_compile_definitions("init.unittests" PRIVATE DISABLE_OL_INIT)
 
 add_offload_unittest("kernel"
     kernel/olLaunchKernel.cpp)
diff --git a/offload/unittests/OffloadAPI/common/Environment.cpp b/offload/unittests/OffloadAPI/common/Environment.cpp
index ef092cd4187d3..d04add55ac54d 100644
--- a/offload/unittests/OffloadAPI/common/Environment.cpp
+++ b/offload/unittests/OffloadAPI/common/Environment.cpp
@@ -11,20 +11,11 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include <OffloadAPI.h>
+#include <OffloadPrint.hpp>
 #include <fstream>
 
 using namespace llvm;
 
-// Wrapper so we don't have to constantly init and shutdown Offload in every
-// test, while having sensible lifetime for the platform environment
-#ifndef DISABLE_WRAPPER
-struct OffloadInitWrapper {
-  OffloadInitWrapper() { olInit(); }
-  ~OffloadInitWrapper() { olShutDown(); }
-};
-static OffloadInitWrapper Wrapper{};
-#endif
-
 static cl::opt<std::string>
     SelectedPlatform("platform", cl::desc("Only test the specified platform"),
                      cl::value_desc("platform"));
@@ -193,3 +184,24 @@ bool TestEnvironment::loadDeviceBinary(
   BinaryOut = std::move(SourceFile.get());
   return true;
 }
+
+int main(int argc, char **argv) {
+#ifndef DISABLE_OL_INIT
+  if (auto Err = olInit()) {
+    errs() << "Failed to initialize liboffload: " << Err->Code << "\n";
+    return -1;
+  }
+#endif
+
+  ::testing::InitGoogleTest(&argc, argv);
+  int Result = RUN_ALL_TESTS();
+
+#ifndef DISABLE_OL_INIT
+  if (auto Err = olShutDown()) {
+    errs() << "Failed to shut down liboffload: " << Err->Code << "\n";
+    return -1;
+  }
+#endif
+
+  return Result;
+}

``````````

</details>


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