Review comments.

CMakeLists.txt

@@ -109,11 +109,11 @@ include(P4CUtils)
 include(FetchContent)
 set(CMAKE_POLICY_DEFAULT_CMP0077 NEW)
 # More utilities.
-include (CheckIncludeFile)
-include (CheckIncludeFileCXX)
-include (CheckFunctionExists)
-include (FindPythonModule)
-INCLUDE(CPack)
+include(CheckIncludeFile)
+include(CheckIncludeFileCXX)
+include(CheckFunctionExists)
+include(FindPythonModule)
+include(CPack)
 # TODO: Remove this deprecated include eventually.
 include(UnifiedBuild)
 

cmake/BDWGC.cmake

@@ -1,6 +1,4 @@
 macro(p4c_obtain_bdwgc)
-  include(CheckSymbolExists)
-
   option(
     P4C_USE_PREINSTALLED_BDWGC
     "Look for a preinstalled version of BDWGC in the system instead of installing a prebuilt binary using FetchContent."
@@ -10,27 +8,31 @@ macro(p4c_obtain_bdwgc)
   set(HAVE_LIBGC 1)
 
   if(P4C_USE_PREINSTALLED_BDWGC)
-    OPTION (ENABLE_MULTITHREAD "Use multithreading" OFF)
+    option(ENABLE_MULTITHREAD "Use multithreading" OFF)
     find_package(LibGc 7.2.0 REQUIRED)
     check_function_exists (GC_print_stats HAVE_GC_PRINT_STATS)
   else()
     # Print out download state while setting up BDWGC.
     set(FETCHCONTENT_QUIET_PREV ${FETCHCONTENT_QUIET})
     set(BUILD_SHARED_LIBS_PREV ${BUILD_SHARED_LIBS})
     set(FETCHCONTENT_QUIET OFF)
-    set(BUILD_SHARED_LIBS OFF CACHE BOOL "")
 
+    # BDWGC options.
+    set(BUILD_SHARED_LIBS OFF CACHE BOOL "")
     # Turn on C++ support.
     set(enable_cplusplus ON CACHE BOOL "C++ support")
     set(install_headers OFF CACHE BOOL "Install header and pkg-config metadata files.")
     set(enable_docs ${ENABLE_DOCS} CACHE BOOL "Build and install documentation.")
     # We may have to compile very large programs which allocate a lot on the heap.
     set(enable_large_config ON CACHE BOOL "Optimize for large heap or root set.")
-    set(without_libatomic_ops OFF CACHE BOOL "Use atomic_ops.h in libatomic_ops/src")
     # Redirect all malloc calls in the program.
     set(enable_redirect_malloc OFF CACHE BOOL "Redirect malloc and friends to GC routines.")
     # Try to enable thread-local-storage for better performance.
     set(enable_thread_local_alloc ON CACHE BOOL "Turn on thread-local allocation optimization")
+    # Other BDWGC options to avoid crashes.
+    set(without_libatomic_ops OFF CACHE BOOL "Use atomic_ops.h in libatomic_ops/src")
+    set(enable_disclaim ON CACHE BOOL "Support alternative finalization interface")
+    set(enable_handle_fork ON CACHE BOOL "Attempt to ensure a usable collector after fork()")
 
     fetchcontent_declare(
       bdwgc
@@ -45,17 +47,15 @@ macro(p4c_obtain_bdwgc)
     fetchcontent_makeavailable_but_exclude_install(bdwgc)
 
     # Bdwgc source code may trigger warnings which we need to ignore.
-    set_target_properties(gc PROPERTIES COMPILE_FLAGS "-Wno-error -w")
-    set_target_properties(gccpp PROPERTIES COMPILE_FLAGS "-Wno-error -w")
-    set_target_properties(cord PROPERTIES COMPILE_FLAGS "-Wno-error -w")
+    target_compile_options(gc PRIVATE "-Wno-error" "-w")
+    target_compile_options(gccpp PRIVATE "-Wno-error" "-w")
+    target_compile_options(cord PRIVATE "-Wno-error" "-w")
 
     # Add some extra compile definitions which allow us to use our customizations.
-    target_compile_definitions(gc PUBLIC
-      HANDLE_FORK ENABLE_DISCLAIM GC_USE_DLOPEN_WRAP SKIP_CPP_DEFINITIONS
-    )
-    target_compile_definitions(gccpp PUBLIC
-      HANDLE_FORK ENABLE_DISCLAIM GC_USE_DLOPEN_WRAP SKIP_CPP_DEFINITIONS
-    )
+    # SKIP_CPP_DEFINITIONS to enable static linking without producing duplicate symbol errors.
+    # GC_USE_DLOPEN_WRAP needs to be enabled to handle threads correctly. This option is usually active with "enable_redirect_malloc" but we currently supply our own malloc overrides.
+    target_compile_definitions(gc PUBLIC GC_USE_DLOPEN_WRAP SKIP_CPP_DEFINITIONS)
+    target_compile_definitions(gccpp PUBLIC GC_USE_DLOPEN_WRAP SKIP_CPP_DEFINITIONS)
 
     # Set up temporary variable modifications for check_symbol_exists.
     set(CMAKE_REQUIRED_INCLUDES_PREV ${CMAKE_REQUIRED_INCLUDES})