wip

CMakeLists.txt

@@ -187,18 +187,6 @@ set (CMAKE_REQUIRED_LIBRARIES dbghelp)
 check_cxx_symbol_exists (UnDecorateSymbolName "windows.h;dbghelp.h" HAVE_DBGHELP)
 cmake_pop_check_state ()
 
-check_cxx_source_compiles ("
-int main(void)
-{
-  int a; if (__sync_val_compare_and_swap(&a, 0, 1)) return 1; return 0;
-}
-" HAVE___SYNC_VAL_COMPARE_AND_SWAP)
-
-check_cxx_source_compiles ("
-__declspec(selectany) int a;
-int main(void) { return 0; }
-" HAVE___DECLSPEC)
-
 if (WITH_TLS)
   set (GLOG_THREAD_LOCAL_STORAGE 1)
 endif (WITH_TLS)

src/config.h.cmake.in

@@ -91,9 +91,6 @@
 /* define if you have libunwind */
 #cmakedefine HAVE_LIBUNWIND
 
-/* define if your compiler has __sync_val_compare_and_swap */
-#cmakedefine HAVE___SYNC_VAL_COMPARE_AND_SWAP
-
 /* define if symbolize support is available */
 #cmakedefine HAVE_SYMBOLIZE
 

src/glog/logging.h

@@ -64,12 +64,26 @@
 #  if __has_attribute(used)
 #    define GLOG_USED __attribute__((used))
 #  endif  // __has_attribute(used)
+#  if __has_attribute(noreturn)
+#    define GLOG_NORETURN __attribute__((noreturn))
+#  endif  // __has_attribute(noreturn)
+#  if __has_attribute(noinline)
+#    define GLOG_NOINLINE __attribute__((noinline))
+#  endif  // __has_attribute(noinline)
 #endif    // defined(__has_attribute)
 
 #if !defined(GLOG_USED)
 #  define GLOG_USED
 #endif  // !defined(GLOG_USED)
 
+#if !defined(GLOG_NORETURN)
+#  define GLOG_NORETURN
+#endif  // !defined(GLOG_NORETURN)
+
+#if !defined(GLOG_NOINLINE)
+#  define GLOG_NOINLINE
+#endif  // !defined(GLOG_NOINLINE)
+
 #if defined(GLOG_USE_GLOG_EXPORT)
 #  include "glog/export.h"
 #endif
@@ -631,11 +645,7 @@ GLOG_EXPORT bool IsGoogleLoggingInitialized();
 // Shutdown google's logging library.
 GLOG_EXPORT void ShutdownGoogleLogging();
 
-#if defined(__GNUC__)
-typedef void (*logging_fail_func_t)() __attribute__((noreturn));
-#else
-typedef void (*logging_fail_func_t)();
-#endif
+typedef void (*logging_fail_func_t)() GLOG_NORETURN;
 
 // Install a function which will be called after LOG(FATAL).
 GLOG_EXPORT void InstallFailureFunction(logging_fail_func_t fail_func);
@@ -780,13 +790,8 @@ GLOG_EXPORT void MakeCheckOpValueString(std::ostream* os,
 
 // Build the error message string. Specify no inlining for code size.
 template <typename T1, typename T2>
-std::string* MakeCheckOpString(const T1& v1, const T2& v2, const char* exprtext)
-#if defined(__has_attribute)
-#  if __has_attribute(used)
-    __attribute__((noinline))
-#  endif
-#endif
-    ;
+std::string* MakeCheckOpString(const T1& v1, const T2& v2,
+                               const char* exprtext) GLOG_NOINLINE;
 
 namespace base {
 namespace internal {

src/raw_logging.cc

@@ -1,4 +1,4 @@
-// Copyright (c) 2006, Google Inc.
+// Copyright (c) 2023, Google Inc.
 // All rights reserved.
 //
 // Redistribution and use in source and binary forms, with or without
@@ -31,6 +31,7 @@
 //
 // logging_unittest.cc covers the functionality herein
 
+#include <atomic>
 #include <cerrno>
 #include <cstdarg>
 #include <cstdio>
@@ -120,7 +121,7 @@ inline static bool VADoRawLog(char** buf, size_t* size, const char* format,
 }
 
 static const int kLogBufSize = 3000;
-static bool crashed = false;
+static std::atomic<bool> crashed{false};
 static CrashReason crash_reason;
 static char crash_buf[kLogBufSize + 1] = {0};  // Will end in '\0'
 
@@ -161,7 +162,8 @@ void RawLog__(LogSeverity severity, const char* file, int line,
   // We write just once to avoid races with other invocations of RawLog__.
   safe_write(STDERR_FILENO, buffer, strlen(buffer));
   if (severity == GLOG_FATAL) {
-    if (!sync_val_compare_and_swap(&crashed, false, true)) {
+    bool expected = false;
+    if (!crashed.compare_exchange_strong(expected, true)) {
       crash_reason.filename = file;
       crash_reason.line_number = line;
       memcpy(crash_buf, msg_start, msg_size);  // Don't include prefix

src/signalhandler.cc

@@ -270,7 +270,7 @@ void InvokeDefaultSignalHandler(int signal_number) {
 // dumping stuff while another thread is doing it.  Our policy is to let
 // the first thread dump stuff and let other threads wait.
 // See also comments in FailureSignalHandler().
-static pthread_t* g_entered_thread_id_pointer = nullptr;
+static std::atomic<pthread_t*> g_entered_thread_id_pointer{nullptr};
 
 // Dumps signal and stack frame information, and invokes the default
 // signal handler once our job is done.
@@ -293,10 +293,9 @@ void FailureSignalHandler(int signal_number, siginfo_t* signal_info,
   // ids, but there is no such guarantee.  We need to distinguish if the
   // old value (value returned from __sync_val_compare_and_swap) is
   // different from the original value (in this case nullptr).
-  pthread_t* old_thread_id_pointer =
-      glog_internal_namespace_::sync_val_compare_and_swap(
-          &g_entered_thread_id_pointer, static_cast<pthread_t*>(nullptr),
-          &my_thread_id);
+  pthread_t* expected = nullptr;
+  g_entered_thread_id_pointer.compare_exchange_strong(expected, &my_thread_id);
+  pthread_t* old_thread_id_pointer = expected;
   if (old_thread_id_pointer != nullptr) {
     // We've already entered the signal handler.  What should we do?
     if (pthread_equal(my_thread_id, *g_entered_thread_id_pointer)) {

src/utilities.cc

@@ -1,4 +1,4 @@
-// Copyright (c) 2008, Google Inc.
+// Copyright (c) 2023, Google Inc.
 // All rights reserved.
 //
 // Redistribution and use in source and binary forms, with or without
@@ -31,15 +31,18 @@
 
 #include "utilities.h"
 
+#include <atomic>
 #include <csignal>
 #include <cstdio>
 #include <cstdlib>
+#include <ctime>
 
+#include "base/googleinit.h"
 #include "config.h"
+
 #ifdef HAVE_SYS_TIME_H
 #  include <sys/time.h>
 #endif
-#include <ctime>
 #if defined(HAVE_SYSCALL_H)
 #  include <syscall.h>  // for syscall()
 #elif defined(HAVE_SYS_SYSCALL_H)
@@ -58,8 +61,6 @@
 #  include <android/log.h>
 #endif
 
-#include "base/googleinit.h"
-
 using std::string;
 
 namespace google {
@@ -152,9 +153,7 @@ static void DumpStackTrace(int skip_count, DebugWriter* writerfn, void* arg) {
   }
 }
 
-#  ifdef __GNUC__
-__attribute__((noreturn))
-#  endif
+GLOG_NORETURN
 static void
 DumpStackTraceAndExit() {
   DumpStackTrace(1, DebugWriteToStderr, nullptr);
@@ -347,11 +346,11 @@ void DumpStackTraceToString(string* stacktrace) {
 
 // We use an atomic operation to prevent problems with calling CrashReason
 // from inside the Mutex implementation (potentially through RAW_CHECK).
-static const CrashReason* g_reason = nullptr;
+static std::atomic<const CrashReason*> g_reason{nullptr};
 
 void SetCrashReason(const CrashReason* r) {
-  sync_val_compare_and_swap(&g_reason, reinterpret_cast<const CrashReason*>(0),
-                            r);
+  const CrashReason* expected = nullptr;
+  g_reason.compare_exchange_strong(expected, r);
 }
 
 void InitGoogleLoggingUtilities(const char* argv0) {

src/utilities.h

@@ -180,34 +180,6 @@ const std::string& MyUserName();
 // (Doesn't modify filepath, contrary to basename() in libgen.h.)
 const char* const_basename(const char* filepath);
 
-// Wrapper of __sync_val_compare_and_swap. If the GCC extension isn't
-// defined, we try the CPU specific logics (we only support x86 and
-// x86_64 for now) first, then use a naive implementation, which has a
-// race condition.
-template <typename T>
-inline T sync_val_compare_and_swap(T* ptr, T oldval, T newval) {
-#if defined(HAVE___SYNC_VAL_COMPARE_AND_SWAP)
-  return __sync_val_compare_and_swap(ptr, oldval, newval);
-#elif defined(__GNUC__) && (defined(__i386__) || defined(__x86_64__))
-  T ret;
-  __asm__ __volatile__("lock; cmpxchg %1, (%2);"
-                       : "=a"(ret)
-                       // GCC may produces %sil or %dil for
-                       // constraint "r", but some of apple's gas
-                       // doesn't know the 8 bit registers.
-                       // We use "q" to avoid these registers.
-                       : "q"(newval), "q"(ptr), "a"(oldval)
-                       : "memory", "cc");
-  return ret;
-#else
-  T ret = *ptr;
-  if (ret == oldval) {
-    *ptr = newval;
-  }
-  return ret;
-#endif
-}
-
 void DumpStackTraceToString(std::string* stacktrace);
 
 struct CrashReason {

src/utilities_unittest.cc

@@ -40,13 +40,6 @@ using namespace GFLAGS_NAMESPACE;
 
 using namespace google;
 
-TEST(utilities, sync_val_compare_and_swap) {
-  bool now_entering = false;
-  EXPECT_FALSE(sync_val_compare_and_swap(&now_entering, false, true));
-  EXPECT_TRUE(sync_val_compare_and_swap(&now_entering, false, true));
-  EXPECT_TRUE(sync_val_compare_and_swap(&now_entering, false, true));
-}
-
 TEST(utilities, InitGoogleLoggingDeathTest) {
   ASSERT_DEATH(InitGoogleLogging("foobar"), "");
 }