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.
fa5aad · Oct 21, 2020 · 0f3bb46 · 0f3bb46
1 parent bd0de71
commit 0f3bb46
Show file tree

Hide file tree

Showing 7 changed files with 80 additions and 59 deletions.
diff --git 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<uint64_t> 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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