From b4d8e06e953788d22c0587067bf6703abe691c82 Mon Sep 17 00:00:00 2001 From: Fabian Ruffy <5960321+fruffy@users.noreply.github.com> Date: Mon, 13 May 2024 22:47:34 +0200 Subject: [PATCH] Link against the Boehm-Demers-Weiser Garbage Collector using FetchContent. (#3930) * Make the Boehm GC a Git submodule. * Review comments. --- CMakeLists.txt | 39 +++++----- backends/p4tools/cmake/common.cmake | 3 - .../common/control_plane/CMakeLists.txt | 2 +- cmake/BDWGC.cmake | 74 +++++++++++++++++++ cmake/bdwgc.patch | 13 ++++ frontends/CMakeLists.txt | 10 ++- ir/CMakeLists.txt | 3 +- ir/visitor.cpp | 2 +- lib/CMakeLists.txt | 10 ++- lib/bitvec.h | 2 +- lib/gc.cpp | 18 ++--- lib/indent.cpp | 2 +- lib/log.cpp | 2 +- tools/ci-build.sh | 1 - tools/install_fedora_deps.sh | 1 - tools/install_mac_deps.sh | 2 +- 16 files changed, 138 insertions(+), 46 deletions(-) create mode 100644 cmake/BDWGC.cmake create mode 100644 cmake/bdwgc.patch diff --git a/CMakeLists.txt b/CMakeLists.txt index 01451f6c97..54454c91ae 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -44,8 +44,7 @@ OPTION (ENABLE_P4TEST "Build the P4Test backend (required for the full test suit OPTION (ENABLE_TEST_TOOLS "Build the P4Tools development platform" OFF) OPTION (ENABLE_P4C_GRAPHS "Build the p4c-graphs backend" ON) OPTION (ENABLE_PROTOBUF_STATIC "Link against Protobuf statically" ON) -OPTION (ENABLE_GC "Use libgc" ON) -OPTION (ENABLE_MULTITHREAD "Use multithreading" OFF) +OPTION (ENABLE_GC "Compile with the Boehm-Demers-Weiser garbage collector." ON) OPTION (ENABLE_WERROR "Treat warnings as errors" OFF) OPTION (ENABLE_SANITIZERS "Enable sanitizers" OFF) OPTION (STATIC_BUILD_WITH_DYNAMIC_GLIBC "Build a (mostly) statically linked release binary. \ @@ -103,11 +102,18 @@ else() WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}) set (P4C_VERSION "${P4C_SEM_VERSION_STRING} (SHA: ${P4C_GIT_SHA} BUILD: ${CMAKE_BUILD_TYPE})") endif() + # P4 General Utilities include(P4CUtils) # CMake Utilities to fetch dependencies. include(FetchContent) set(CMAKE_POLICY_DEFAULT_CMP0077 NEW) +# More utilities. +include(CheckIncludeFile) +include(CheckIncludeFileCXX) +include(CheckFunctionExists) +include(FindPythonModule) +include(CPack) # TODO: Remove this deprecated include eventually. include(UnifiedBuild) @@ -198,6 +204,10 @@ p4c_obtain_abseil() include(Protobuf) p4c_obtain_protobuf() +# We require -pthread to make std::call_once work, even if we're not using threads... +set(THREADS_PREFER_PTHREAD_FLAG ON) +find_package(Threads REQUIRED) + # The boost graph headers are optional and only required by the graphs back end. find_package (Boost QUIET COMPONENTS graph) if (Boost_FOUND) @@ -207,20 +217,18 @@ else () endif () find_package (Boost REQUIRED COMPONENTS iostreams) +# Compile with the Boehm garbage collector (https://github.com/ivmai/bdwgc), if requested. +# One can disable the GC, e.g., to run under Valgrind, by editing config.h. if (ENABLE_GC) - find_package (LibGc 7.4.2 REQUIRED) - set (HAVE_LIBGC 1) + include(BDWGC) + p4c_obtain_bdwgc() endif () if (ENABLE_MULTITHREAD) add_definitions(-DMULTITHREAD) endif() -# we require -pthread to make std::call_once work, even if we're not using threads... -set(THREADS_PREFER_PTHREAD_FLAG ON) -find_package(Threads REQUIRED) list (APPEND P4C_LIB_DEPS ${CMAKE_THREAD_LIBS_INIT}) include_directories(SYSTEM ${Boost_INCLUDE_DIRS}) include_directories(SYSTEM ${PROTOBUF_INCLUDE_DIRS}) -include_directories(SYSTEM ${LIBGC_INCLUDE_DIR}) set (HAVE_LIBBOOST_IOSTREAMS 1) list (APPEND P4C_LIB_DEPS ${Boost_LIBRARIES}) if (ENABLE_GC) @@ -229,19 +237,18 @@ endif () set (P4C_ABSL_LIBRARIES absl::flat_hash_map absl::flat_hash_set) list (APPEND P4C_LIB_DEPS ${P4C_ABSL_LIBRARIES}) -# other required libraries +# Other required libraries. p4c_add_library (rt clock_gettime HAVE_CLOCK_GETTIME) -# check includes -include (CheckIncludeFile) -include (CheckIncludeFileCXX) +# Check includes. check_include_file(execinfo.h HAVE_EXECINFO_H) check_include_file(ucontext.h HAVE_UCONTEXT_H) check_include_file(backtrace-supported.h HAVE_LIBBACKTRACE) check_include_file_cxx(mm_malloc.h HAVE_MM_MALLOC_H) check_include_file_cxx(cxxabi.h HAVE_CXXABI_H) if (HAVE_LIBBACKTRACE) -set (P4C_LIB_DEPS "${P4C_LIB_DEPS};-lbacktrace") + message(STATUS "Linking libbacktrace.") + set (P4C_LIB_DEPS "${P4C_LIB_DEPS};-lbacktrace") endif () # check functions @@ -252,16 +259,13 @@ if (ENABLE_GC) set (CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES} ${LIBGC_LIBRARIES}) endif () -include (CheckFunctionExists) check_function_exists (memchr HAVE_MEMCHR) check_function_exists (pipe2 HAVE_PIPE2) -check_function_exists (GC_print_stats HAVE_GC_PRINT_STATS) # restore CMAKE_REQUIRED_LIBRARIES set (CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES_PRECHECK}) # python modules -include (FindPythonModule) find_python_module (difflib REQUIRED) find_python_module (shutil REQUIRED) find_python_module (tempfile REQUIRED) @@ -421,7 +425,7 @@ FetchContent_Declare( ) FetchContent_MakeAvailable(p4runtime) set(FETCHCONTENT_QUIET ${FETCHCONTENT_QUIET_PREV}) -message("Done with setting up P4Runtime for P4C.") +message(STATUS "Done with setting up P4Runtime for P4C.") # The standard P4Runtime protocol buffers message definitions live in the PI # repo, which is included in this repo as a git module. set(P4RUNTIME_STD_DIR ${p4runtime_SOURCE_DIR}/proto CACHE INTERNAL @@ -626,6 +630,5 @@ SET(CPACK_SOURCE_PACKAGE_FILE_NAME "p4c-${P4C_SEM_VERSION_STRING}") SET(CPACK_SOURCE_IGNORE_FILES "${PROJECT_SOURCE_DIR}/${CMAKE_PROJECT_NAME}-*;${PROJECT_SOURCE_DIR}/${CMAKE_PROJECT_NAME}_*;/build/;/.git/;/config.log;/CMakeFiles/;CMakeCache.txt$;.tar.gz$;/_CPack_Packages;/Makefile$;~$;/build-deb;/clean-deb;/filter-empty-entries;/make-symbols;/make-ppa;/make-deb;/debian.conf;/make-rpm;/rpm.conf;${CPACK_SOURCE_IGNORE_FILES}") -INCLUDE(CPack) ADD_CUSTOM_TARGET(dist COMMAND ${CMAKE_MAKE_PROGRAM} clean package_source) diff --git a/backends/p4tools/cmake/common.cmake b/backends/p4tools/cmake/common.cmake index 30ba789f78..0cde58e858 100644 --- a/backends/p4tools/cmake/common.cmake +++ b/backends/p4tools/cmake/common.cmake @@ -7,9 +7,6 @@ if(CCACHE_PROGRAM) set_property(GLOBAL PROPERTY RULE_LAUNCH_COMPILE "${CCACHE_PROGRAM}") endif() -# Use libgc. -find_package(LibGc 7.2.0 REQUIRED) - # Helper for defining a p4tools executable target. function(add_p4tools_executable target source) add_executable(${target} ${source} ${ARGN}) diff --git a/backends/p4tools/common/control_plane/CMakeLists.txt b/backends/p4tools/common/control_plane/CMakeLists.txt index f606f3a10d..22920a8da0 100644 --- a/backends/p4tools/common/control_plane/CMakeLists.txt +++ b/backends/p4tools/common/control_plane/CMakeLists.txt @@ -5,4 +5,4 @@ set( ) add_library(p4tools-control-plane STATIC ${P4C_TOOLS_CONTROL_PLANE_SOURCES}) -target_link_libraries(p4tools-control-plane PRIVATE controlplane) +target_link_libraries(p4tools-control-plane PRIVATE controlplane PRIVATE p4ctoolkit) diff --git a/cmake/BDWGC.cmake b/cmake/BDWGC.cmake new file mode 100644 index 0000000000..8acf29159c --- /dev/null +++ b/cmake/BDWGC.cmake @@ -0,0 +1,74 @@ +macro(p4c_obtain_bdwgc) + option( + P4C_USE_PREINSTALLED_BDWGC + "Look for a preinstalled version of BDWGC in the system instead of installing a prebuilt binary using FetchContent." + OFF + ) + set(LIBGC_LIBRARIES gc gccpp cord) + set(HAVE_LIBGC 1) + + if(P4C_USE_PREINSTALLED_BDWGC) + 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) + + # 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.") + # 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 + GIT_REPOSITORY https://github.com/ivmai/bdwgc.git + GIT_TAG e340b2e869e02718de9c9d7fa440ef4b35785388 # 8.2.6 + GIT_PROGRESS TRUE + PATCH_COMMAND + git apply ${P4C_SOURCE_DIR}/cmake/bdwgc.patch || git apply + ${P4C_SOURCE_DIR}/cmake/bdwgc.patch -R --check && echo + "Patch does not apply because the patch was already applied." + ) + fetchcontent_makeavailable_but_exclude_install(bdwgc) + + # Bdwgc source code may trigger warnings which we need to ignore. + 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. + # 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}) + set(CMAKE_REQUIRED_LIBRARIES_PREV ${CMAKE_REQUIRED_LIBRARIES}) + set(CMAKE_REQUIRED_INCLUDES ${gc_SOURCE_DIR}/include) + set(CMAKE_REQUIRED_LIBRARIES ${LIBGC_LIBRARIES}) + check_symbol_exists(GC_print_stats private/gc_priv.h HAVE_GC_PRINT_STATS) + # Reset all temporary variable modifications. + set(FETCHCONTENT_QUIET ${FETCHCONTENT_QUIET_PREV}) + set(CMAKE_REQUIRED_INCLUDES ${CMAKE_REQUIRED_INCLUDES_PREV}) + set(CMAKE_REQUIRED_LIBRARIES ${CMAKE_REQUIRED_LIBRARIES_PREV}) + set(BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS_PREV} CACHE BOOL "") + + message(STATUS "Done with setting up BDWGC for P4C.") + endif() +endmacro(p4c_obtain_bdwgc) diff --git a/cmake/bdwgc.patch b/cmake/bdwgc.patch new file mode 100644 index 0000000000..3ce6a22238 --- /dev/null +++ b/cmake/bdwgc.patch @@ -0,0 +1,13 @@ +diff --git a/gc_cpp.cc b/gc_cpp.cc +index 341805a6..fb6b0a01 100644 +--- a/gc_cpp.cc ++++ b/gc_cpp.cc +@@ -38,7 +38,7 @@ built-in "new" and "delete". + #endif + #include "gc_cpp.h" + +-#if !(defined(_MSC_VER) || defined(__DMC__)) || defined(GC_NO_INLINE_STD_NEW) ++#if !defined(SKIP_CPP_DEFINITIONS) && (!(defined(_MSC_VER) || defined(__DMC__)) || defined(GC_NO_INLINE_STD_NEW)) + + #if defined(GC_NEW_ABORTS_ON_OOM) || defined(_LIBCPP_NO_EXCEPTIONS) + # define GC_ALLOCATOR_THROW_OR_ABORT() GC_abort_on_oom() diff --git a/frontends/CMakeLists.txt b/frontends/CMakeLists.txt index 7e4c4a4869..892eed24d4 100644 --- a/frontends/CMakeLists.txt +++ b/frontends/CMakeLists.txt @@ -239,6 +239,10 @@ add_parser(v1) add_parser(p4) add_library (frontend-parser-gen OBJECT ${p4PARSER_GEN_SRCS} ${v1PARSER_GEN_SRCS}) +target_link_libraries (frontend-parser-gen + # These libraries are exposed by a header. + PUBLIC p4ctoolkit +) add_dependencies (frontend-parser-gen mkp4dirs mkv1dirs ir-generated) set (FRONTEND_SOURCES @@ -259,7 +263,9 @@ if (FLEX_INCLUDE_DIRS) endif () add_library (frontend STATIC ${FRONTEND_SOURCES}) target_link_libraries (frontend - PRIVATE frontend-parser-gen PRIVATE absl::strings + # These libraries are exposed by a header. + PUBLIC frontend-parser-gen PUBLIC absl::flat_hash_set - PUBLIC absl::flat_hash_map) + PUBLIC absl::flat_hash_map +) diff --git a/ir/CMakeLists.txt b/ir/CMakeLists.txt index 16569df200..8ab76e9b81 100644 --- a/ir/CMakeLists.txt +++ b/ir/CMakeLists.txt @@ -67,8 +67,7 @@ set (BASE_IR_DEF_FILES set (IR_DEF_FILES ${IR_DEF_FILES} ${BASE_IR_DEF_FILES} PARENT_SCOPE) add_library (ir STATIC ${IR_SRCS}) -target_link_libraries(ir - PRIVATE absl::flat_hash_map) +target_link_libraries(ir PRIVATE absl::flat_hash_map ${LIBGC_LIBRARIES}) add_dependencies(ir genIR) diff --git a/ir/visitor.cpp b/ir/visitor.cpp index 33f264903d..f2b2299f87 100644 --- a/ir/visitor.cpp +++ b/ir/visitor.cpp @@ -24,7 +24,7 @@ limitations under the License. #include "lib/hash.h" #if HAVE_LIBGC -#include +#include #endif #include "dbprint.h" diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index 2625d46df4..bb8572eb3d 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -set (LIBP4CTOOLKIT_SRCS +set(LIBP4CTOOLKIT_SRCS alloc_trace.cpp backtrace_exception.cpp bitrange.cpp @@ -41,7 +41,7 @@ set (LIBP4CTOOLKIT_SRCS timer.cpp ) -set (LIBP4CTOOLKIT_HDRS +set(LIBP4CTOOLKIT_HDRS algorithm.h alloc_trace.h backtrace_exception.h @@ -95,4 +95,8 @@ add_library(p4ctoolkit STATIC ${LIBP4CTOOLKIT_SRCS}) # Disable libcall (realloc, malloc) optimizations which may cause infinite loops. set_target_properties(p4ctoolkit PROPERTIES COMPILE_FLAGS -fno-builtin) -target_link_libraries(p4ctoolkit absl::bits) +target_link_libraries(p4ctoolkit + # These libraries are exposed by a header. + PUBLIC absl::bits + PUBLIC ${LIBGC_LIBRARIES} +) diff --git a/lib/bitvec.h b/lib/bitvec.h index 352528a81e..d2a3c56d01 100644 --- a/lib/bitvec.h +++ b/lib/bitvec.h @@ -30,7 +30,7 @@ limitations under the License. #include "config.h" #if HAVE_LIBGC -#include +#include #define IF_HAVE_LIBGC(X) X #else #define IF_HAVE_LIBGC(X) diff --git a/lib/gc.cpp b/lib/gc.cpp index ac249eb7e4..c6d8bd65a5 100644 --- a/lib/gc.cpp +++ b/lib/gc.cpp @@ -13,6 +13,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ +#include "lib/gc.h" // Some systems (e.g. those with GNU libc) declare posix_memalign with an // exception specifier. Compilers (clang including) might have special handling @@ -28,8 +29,9 @@ limitations under the License. #endif #if HAVE_LIBGC -#include -#include +#include +#include +#include #endif /* HAVE_LIBGC */ #include #if HAVE_EXECINFO_H @@ -42,7 +44,6 @@ limitations under the License. #include "backtrace_exception.h" #include "cstring.h" -#include "gc.h" #include "log.h" #include "n4.h" @@ -50,6 +51,7 @@ limitations under the License. // -DENABLE_GC=OFF in CMake. #if HAVE_LIBGC static bool done_init, started_init; + // emergency pool to allow a few extra allocations after a bad_alloc is thrown so we // can generate reasonable errors, a stack trace, etc static char emergency_pool[16 * 1024]; @@ -152,15 +154,14 @@ alloc_trace_cb_t set_alloc_trace(void (*fn)(void *, void **, size_t), void *arg) return old; } -// clang-format off -void operator delete(void* p) noexcept { +void operator delete(void *p) noexcept { if (p >= emergency_pool && p < emergency_pool + sizeof(emergency_pool)) { return; } gc::operator delete(p); } -void operator delete(void* p, std::size_t /*size*/) noexcept { +void operator delete(void *p, std::size_t /*size*/) noexcept { if (p >= emergency_pool && p < emergency_pool + sizeof(emergency_pool)) { return; } @@ -169,14 +170,11 @@ void operator delete(void* p, std::size_t /*size*/) noexcept { // FIXME: We can get rid of GC_base here with suitable new libgc // See https://github.com/ivmai/bdwgc/issues/589 for more info -void operator delete(void *p, std::align_val_t) noexcept { - gc::operator delete(GC_base(p)); -} +void operator delete(void *p, std::align_val_t) noexcept { gc::operator delete(GC_base(p)); } void operator delete(void *p, std::size_t, std::align_val_t) noexcept { gc::operator delete(GC_base(p)); } -// clang-format on void *operator new[](std::size_t size) { return ::operator new(size); } void *operator new[](std::size_t size, std::align_val_t alignment) { diff --git a/lib/indent.cpp b/lib/indent.cpp index 05eca2435a..7365477084 100644 --- a/lib/indent.cpp +++ b/lib/indent.cpp @@ -16,7 +16,7 @@ limitations under the License. #include "config.h" #if HAVE_LIBGC -#include +#include #define NOGC_ARGS (NoGC, 0, 0) #else #define NOGC_ARGS diff --git a/lib/log.cpp b/lib/log.cpp index 2b31ffec74..571fce9b7f 100644 --- a/lib/log.cpp +++ b/lib/log.cpp @@ -16,7 +16,7 @@ limitations under the License. #include "config.h" #if HAVE_LIBGC -#include +#include #define NOGC_ARGS (NoGC, 0, 0) #else #define NOGC_ARGS diff --git a/tools/ci-build.sh b/tools/ci-build.sh index ebd2d95b96..6a2bd78dc8 100755 --- a/tools/ci-build.sh +++ b/tools/ci-build.sh @@ -75,7 +75,6 @@ P4C_DEPS="bison \ libboost-graph-dev \ libboost-iostreams-dev \ libfl-dev \ - libgc-dev \ pkg-config \ python3 \ python3-pip \ diff --git a/tools/install_fedora_deps.sh b/tools/install_fedora_deps.sh index d388cc0624..8d9ce30f21 100755 --- a/tools/install_fedora_deps.sh +++ b/tools/install_fedora_deps.sh @@ -20,7 +20,6 @@ sudo dnf install -y -q \ elfutils-libelf-devel \ flex \ g++ \ - gc-devel \ git \ gmp-devel \ grpc-devel \ diff --git a/tools/install_mac_deps.sh b/tools/install_mac_deps.sh index 795e5fbebb..32707cff98 100755 --- a/tools/install_mac_deps.sh +++ b/tools/install_mac_deps.sh @@ -39,7 +39,7 @@ brew update BOOST_LIB="boost@1.85" REQUIRED_PACKAGES=( - autoconf automake bdw-gc ccache cmake libtool + autoconf automake ccache cmake libtool openssl pkg-config coreutils bison grep ninja ${BOOST_LIB} )