From 0f3bb466b868b523cf1dc9b2aaaed65c77b28862 Mon Sep 17 00:00:00 2001 From: Derek Mauro <761129+derekmauro@users.noreply.github.com> Date: Wed, 21 Oct 2020 08:32:42 -0400 Subject: Cherry-picks for LTS 2020_09_23 Patch Release 2 * Fixes preprocessor condition for symbols __tsan_mutex_read_lock and __tsan_mutex_try_lock * Fixes race in AddressIsReadable file descriptors using stronger memory ordering * Fixes CMake dependency issues and adds `-Wl,--no-undefined` to avoid these issues in the future. --- absl/debugging/internal/address_is_readable.cc | 9 ++--- absl/flags/CMakeLists.txt | 1 + absl/status/CMakeLists.txt | 1 + absl/synchronization/mutex.cc | 5 +-- ci/linux_gcc-latest_libstdcxx_cmake.sh | 47 ++++++++++++++------------ ci/linux_gcc_alpine_cmake.sh | 46 +++++++++++++------------ ci/macos_xcode_cmake.sh | 30 ++++++++++------ 7 files changed, 80 insertions(+), 59 deletions(-) diff --git a/absl/debugging/internal/address_is_readable.cc b/absl/debugging/internal/address_is_readable.cc index 65376063..329c285f 100644 --- a/absl/debugging/internal/address_is_readable.cc +++ b/absl/debugging/internal/address_is_readable.cc @@ -68,6 +68,7 @@ static void Unpack(uint64_t x, int *pid, int *read_fd, int *write_fd) { // unimplemented. // This is a namespace-scoped variable for correct zero-initialization. static std::atomic pid_and_fds; // initially 0, an invalid pid. + bool AddressIsReadable(const void *addr) { absl::base_internal::ErrnoSaver errno_saver; // We test whether a byte is readable by using write(). Normally, this would @@ -86,7 +87,7 @@ bool AddressIsReadable(const void *addr) { int pid; int read_fd; int write_fd; - uint64_t local_pid_and_fds = pid_and_fds.load(std::memory_order_relaxed); + uint64_t local_pid_and_fds = pid_and_fds.load(std::memory_order_acquire); Unpack(local_pid_and_fds, &pid, &read_fd, &write_fd); while (current_pid != pid) { int p[2]; @@ -98,13 +99,13 @@ bool AddressIsReadable(const void *addr) { fcntl(p[1], F_SETFD, FD_CLOEXEC); uint64_t new_pid_and_fds = Pack(current_pid, p[0], p[1]); if (pid_and_fds.compare_exchange_strong( - local_pid_and_fds, new_pid_and_fds, std::memory_order_relaxed, + local_pid_and_fds, new_pid_and_fds, std::memory_order_release, std::memory_order_relaxed)) { local_pid_and_fds = new_pid_and_fds; // fds exposed to other threads } else { // fds not exposed to other threads; we can close them. close(p[0]); close(p[1]); - local_pid_and_fds = pid_and_fds.load(std::memory_order_relaxed); + local_pid_and_fds = pid_and_fds.load(std::memory_order_acquire); } Unpack(local_pid_and_fds, &pid, &read_fd, &write_fd); } @@ -124,7 +125,7 @@ bool AddressIsReadable(const void *addr) { // If pid_and_fds contains the problematic file descriptors we just used, // this call will forget them, and the loop will try again. pid_and_fds.compare_exchange_strong(local_pid_and_fds, 0, - std::memory_order_relaxed, + std::memory_order_release, std::memory_order_relaxed); } } while (errno == EBADF); diff --git a/absl/flags/CMakeLists.txt b/absl/flags/CMakeLists.txt index 28bd5a85..88551914 100644 --- a/absl/flags/CMakeLists.txt +++ b/absl/flags/CMakeLists.txt @@ -183,6 +183,7 @@ absl_cc_library( DEPS absl::base absl::config + absl::flags_commandlineflag absl::flags_commandlineflag_internal absl::flags_config absl::flags_marshalling diff --git a/absl/status/CMakeLists.txt b/absl/status/CMakeLists.txt index 66728551..f0d798a3 100644 --- a/absl/status/CMakeLists.txt +++ b/absl/status/CMakeLists.txt @@ -64,6 +64,7 @@ absl_cc_library( COPTS ${ABSL_DEFAULT_COPTS} DEPS + absl::status absl::core_headers absl::raw_logging_internal absl::type_traits diff --git a/absl/synchronization/mutex.cc b/absl/synchronization/mutex.cc index 9b7f088a..ad13567a 100644 --- a/absl/synchronization/mutex.cc +++ b/absl/synchronization/mutex.cc @@ -50,6 +50,7 @@ #include "absl/base/internal/spinlock.h" #include "absl/base/internal/sysinfo.h" #include "absl/base/internal/thread_identity.h" +#include "absl/base/internal/tsan_mutex_interface.h" #include "absl/base/port.h" #include "absl/debugging/stacktrace.h" #include "absl/debugging/symbolize.h" @@ -705,7 +706,7 @@ static constexpr bool kDebugMode = false; static constexpr bool kDebugMode = true; #endif -#ifdef ABSL_HAVE_THREAD_SANITIZER +#ifdef ABSL_INTERNAL_HAVE_TSAN_INTERFACE static unsigned TsanFlags(Mutex::MuHow how) { return how == kShared ? __tsan_mutex_read_lock : 0; } @@ -1767,7 +1768,7 @@ static inline bool EvalConditionAnnotated(const Condition *cond, Mutex *mu, // All memory accesses are ignored inside of mutex operations + for unlock // operation tsan considers that we've already released the mutex. bool res = false; -#ifdef ABSL_HAVE_THREAD_SANITIZER +#ifdef ABSL_INTERNAL_HAVE_TSAN_INTERFACE const int flags = read_lock ? __tsan_mutex_read_lock : 0; const int tryflags = flags | (trylock ? __tsan_mutex_try_lock : 0); #endif diff --git a/ci/linux_gcc-latest_libstdcxx_cmake.sh b/ci/linux_gcc-latest_libstdcxx_cmake.sh index 1ba02b26..26415e23 100755 --- a/ci/linux_gcc-latest_libstdcxx_cmake.sh +++ b/ci/linux_gcc-latest_libstdcxx_cmake.sh @@ -34,31 +34,36 @@ if [[ -z ${ABSL_CMAKE_BUILD_TYPES:-} ]]; then ABSL_CMAKE_BUILD_TYPES="Debug Release" fi +if [[ -z ${ABSL_CMAKE_BUILD_SHARED:-} ]]; then + ABSL_CMAKE_BUILD_SHARED="OFF ON" +fi + source "${ABSEIL_ROOT}/ci/linux_docker_containers.sh" readonly DOCKER_CONTAINER=${LINUX_GCC_LATEST_CONTAINER} for std in ${ABSL_CMAKE_CXX_STANDARDS}; do for compilation_mode in ${ABSL_CMAKE_BUILD_TYPES}; do - echo "--------------------------------------------------------------------" - echo "Testing with CMAKE_BUILD_TYPE=${compilation_mode} and -std=c++${std}" - - time docker run \ - --volume="${ABSEIL_ROOT}:/abseil-cpp:ro" \ - --workdir=/abseil-cpp \ - --tmpfs=/buildfs:exec \ - --cap-add=SYS_PTRACE \ - --rm \ - -e CFLAGS="-Werror" \ - -e CXXFLAGS="-Werror" \ - ${DOCKER_CONTAINER} \ - /bin/bash -c " - cd /buildfs && \ - cmake /abseil-cpp \ - -DABSL_USE_GOOGLETEST_HEAD=ON \ - -DABSL_RUN_TESTS=ON \ - -DCMAKE_BUILD_TYPE=${compilation_mode} \ - -DCMAKE_CXX_STANDARD=${std} && \ - make -j$(nproc) && \ - ctest -j$(nproc) --output-on-failure" + for build_shared in ${ABSL_CMAKE_BUILD_SHARED}; do + time docker run \ + --volume="${ABSEIL_ROOT}:/abseil-cpp:ro" \ + --workdir=/abseil-cpp \ + --tmpfs=/buildfs:exec \ + --cap-add=SYS_PTRACE \ + --rm \ + -e CFLAGS="-Werror" \ + -e CXXFLAGS="-Werror" \ + ${DOCKER_CONTAINER} \ + /bin/bash -c " + cd /buildfs && \ + cmake /abseil-cpp \ + -DABSL_USE_GOOGLETEST_HEAD=ON \ + -DABSL_RUN_TESTS=ON \ + -DBUILD_SHARED_LIBS=${build_shared} \ + -DCMAKE_BUILD_TYPE=${compilation_mode} \ + -DCMAKE_CXX_STANDARD=${std} \ + -DCMAKE_MODULE_LINKER_FLAGS=\"-Wl,--no-undefined\" && \ + make -j$(nproc) && \ + ctest -j$(nproc) --output-on-failure" + done done done diff --git a/ci/linux_gcc_alpine_cmake.sh b/ci/linux_gcc_alpine_cmake.sh index f57ab12b..b3b8e7a7 100755 --- a/ci/linux_gcc_alpine_cmake.sh +++ b/ci/linux_gcc_alpine_cmake.sh @@ -34,31 +34,35 @@ if [[ -z ${ABSL_CMAKE_BUILD_TYPES:-} ]]; then ABSL_CMAKE_BUILD_TYPES="Debug Release" fi +if [[ -z ${ABSL_CMAKE_BUILD_SHARED:-} ]]; then + ABSL_CMAKE_BUILD_SHARED="OFF ON" +fi + source "${ABSEIL_ROOT}/ci/linux_docker_containers.sh" readonly DOCKER_CONTAINER=${LINUX_ALPINE_CONTAINER} for std in ${ABSL_CMAKE_CXX_STANDARDS}; do for compilation_mode in ${ABSL_CMAKE_BUILD_TYPES}; do - echo "--------------------------------------------------------------------" - echo "Testing with CMAKE_BUILD_TYPE=${compilation_mode} and -std=c++${std}" - - time docker run \ - --volume="${ABSEIL_ROOT}:/abseil-cpp:ro" \ - --workdir=/abseil-cpp \ - --tmpfs=/buildfs:exec \ - --cap-add=SYS_PTRACE \ - --rm \ - -e CFLAGS="-Werror" \ - -e CXXFLAGS="-Werror" \ - "${DOCKER_CONTAINER}" \ - /bin/sh -c " - cd /buildfs && \ - cmake /abseil-cpp \ - -DABSL_USE_GOOGLETEST_HEAD=ON \ - -DABSL_RUN_TESTS=ON \ - -DCMAKE_BUILD_TYPE=${compilation_mode} \ - -DCMAKE_CXX_STANDARD=${std} && \ - make -j$(nproc) && \ - ctest -j$(nproc) --output-on-failure" + for build_shared in ${ABSL_CMAKE_BUILD_SHARED}; do + time docker run \ + --volume="${ABSEIL_ROOT}:/abseil-cpp:ro" \ + --workdir=/abseil-cpp \ + --tmpfs=/buildfs:exec \ + --cap-add=SYS_PTRACE \ + --rm \ + -e CFLAGS="-Werror" \ + -e CXXFLAGS="-Werror" \ + "${DOCKER_CONTAINER}" \ + /bin/sh -c " + cd /buildfs && \ + cmake /abseil-cpp \ + -DABSL_USE_GOOGLETEST_HEAD=ON \ + -DABSL_RUN_TESTS=ON \ + -DCMAKE_BUILD_TYPE=${compilation_mode} \ + -DCMAKE_CXX_STANDARD=${std} \ + -DCMAKE_MODULE_LINKER_FLAGS=\"-Wl,--no-undefined\" && \ + make -j$(nproc) && \ + ctest -j$(nproc) --output-on-failure" + done done done diff --git a/ci/macos_xcode_cmake.sh b/ci/macos_xcode_cmake.sh index cf78e207..d90e273a 100755 --- a/ci/macos_xcode_cmake.sh +++ b/ci/macos_xcode_cmake.sh @@ -28,17 +28,25 @@ if [[ -z ${ABSL_CMAKE_BUILD_TYPES:-} ]]; then ABSL_CMAKE_BUILD_TYPES="Debug" fi +if [[ -z ${ABSL_CMAKE_BUILD_SHARED:-} ]]; then + ABSL_CMAKE_BUILD_SHARED="OFF ON" +fi + for compilation_mode in ${ABSL_CMAKE_BUILD_TYPES}; do - BUILD_DIR=$(mktemp -d ${compilation_mode}.XXXXXXXX) - cd ${BUILD_DIR} + for build_shared in ${ABSL_CMAKE_BUILD_SHARED}; do + BUILD_DIR=$(mktemp -d ${compilation_mode}.XXXXXXXX) + cd ${BUILD_DIR} - # TODO(absl-team): Enable -Werror once all warnings are fixed. - time cmake ${ABSEIL_ROOT} \ - -GXcode \ - -DCMAKE_BUILD_TYPE=${compilation_mode} \ - -DCMAKE_CXX_STANDARD=11 \ - -DABSL_USE_GOOGLETEST_HEAD=ON \ - -DABSL_RUN_TESTS=ON - time cmake --build . - time ctest -C ${compilation_mode} --output-on-failure + # TODO(absl-team): Enable -Werror once all warnings are fixed. + time cmake ${ABSEIL_ROOT} \ + -GXcode \ + -DBUILD_SHARED_LIBS=${build_shared} \ + -DCMAKE_BUILD_TYPE=${compilation_mode} \ + -DCMAKE_CXX_STANDARD=11 \ + -DCMAKE_MODULE_LINKER_FLAGS="-Wl,--no-undefined" \ + -DABSL_USE_GOOGLETEST_HEAD=ON \ + -DABSL_RUN_TESTS=ON + time cmake --build . + time ctest -C ${compilation_mode} --output-on-failure + done done -- cgit v1.2.3