[compiler-rt] Replace allow_user_segv_handler=0 with kHandleSignalAlways

[vitalybuka]

Summary

allow_user_segv_handler had confusing name did not allow to control behavior for
signals separately.

[vitalybuka]

Updated allow_user_segv.cc test

[vitalybuka]

Fixed asan_win.cc check

[eugenis]

lib/sanitizer_common/sanitizer_flag_parser.h

68 ↗

(On Diff #99653)

maybe "exclusive"? The meaning of "always" is not very clear in this context.

[vitalybuka]

Always -> Exclusive

[vitalybuka]

Rebase

compiler-rt/trunk/lib/asan/asan_interceptors.cc

 
 DEFINE_REAL_PTHREAD_FUNCTIONS
 #endif  // ASAN_INTERCEPT_PTHREAD_CREATE
 
 #if ASAN_INTERCEPT_SIGNAL_AND_SIGACTION
 
 #if SANITIZER_ANDROID
 INTERCEPTOR(void*, bsd_signal, int signum, void *handler) {
-  if (!IsHandledDeadlySignal(signum) ||
-      common_flags()->allow_user_segv_handler) {
+  if (GetHandleSignalMode(signum) != kHandleSignalExclusive)
     return REAL(bsd_signal)(signum, handler);
-  }
   return 0;
 }
 #endif
 
 INTERCEPTOR(void*, signal, int signum, void *handler) {
-  if (!IsHandledDeadlySignal(signum) ||
-      common_flags()->allow_user_segv_handler) {
+  if (GetHandleSignalMode(signum) != kHandleSignalExclusive)
     return REAL(signal)(signum, handler);
-  }
   return nullptr;
 }
 
 INTERCEPTOR(int, sigaction, int signum, const struct sigaction *act,
                             struct sigaction *oldact) {
-  if (!IsHandledDeadlySignal(signum) ||
-      common_flags()->allow_user_segv_handler) {
+  if (GetHandleSignalMode(signum) != kHandleSignalExclusive)
     return REAL(sigaction)(signum, act, oldact);
-  }
   return 0;
 }
 
 namespace __sanitizer {
 int real_sigaction(int signum, const void *act, void *oldact) {
   return REAL(sigaction)(signum, (const struct sigaction *)act,
                          (struct sigaction *)oldact);
 }

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h

 // Callback will be called if soft_rss_limit_mb is given and the limit is
 // exceeded (exceeded==true) or if rss went down below the limit
 // (exceeded==false).
 // The callback should be registered once at the tool init time.
 void SetSoftRssLimitExceededCallback(void (*Callback)(bool exceeded));
 
 // Functions related to signal handling.
 typedef void (*SignalHandlerType)(int, void *, void *);
-bool IsHandledDeadlySignal(int signum);
+HandleSignalMode GetHandleSignalMode(int signum);
 void InstallDeadlySignalHandlers(SignalHandlerType handler);
 const char *DescribeSignalOrException(int signo);
 // Alternative signal stack (POSIX-only).
 void SetAlternateSignalStack();
 void UnsetAlternateSignalStack();
 
 // We don't want a summary too long.
 const int kMaxSummaryLength = 1024;

compiler-rt/trunk/lib/sanitizer_common/sanitizer_flag_parser.h

 
 template <>
 inline bool FlagHandler<HandleSignalMode>::Parse(const char *value) {
   bool b;
   if (ParseBool(value, &b)) {
     *t_ = b ? kHandleSignalYes : kHandleSignalNo;
     return true;
   }
+  if (internal_strcmp(value, "2") == 0 ||
+      internal_strcmp(value, "exclusive") == 0) {
+    *t_ = kHandleSignalExclusive;
+    return true;
+  }
   Printf("ERROR: Invalid value for signal handler option: '%s'\n", value);
   return false;
 }
 
 template <>
 inline bool FlagHandler<const char *>::Parse(const char *value) {
   *t_ = internal_strdup(value);
   return true;

compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.h

 
 #include "sanitizer_internal_defs.h"
 
 namespace __sanitizer {
 
 enum HandleSignalMode {
   kHandleSignalNo,
   kHandleSignalYes,
+  kHandleSignalExclusive,
 };
 
 struct CommonFlags {
 #define COMMON_FLAG(Type, Name, DefaultValue, Description) Type Name;
 #include "sanitizer_flags.inc"
 #undef COMMON_FLAG
 
   void SetDefaults();

compiler-rt/trunk/lib/sanitizer_common/sanitizer_flags.inc

COMMON_FLAG(bool, print_summary, true,
             "If false, disable printing error summaries in addition to error "
             "reports.")
 COMMON_FLAG(int, print_module_map, 0,
             "OS X only (0 - don't print, 1 - print only once before process "
             "exits, 2 - print after each report).")
 COMMON_FLAG(bool, check_printf, true, "Check printf arguments.")
 #define COMMON_FLAG_HANDLE_SIGNAL_HELP(signal) \
     "Controls custom tool's " #signal " handler (0 - do not registers the " \
-    "handler, 1 - register the handler). "
+    "handler, 1 - register the handler and allow user to set own, " \
+    "2 - registers the handler and block user from changing it). "
 COMMON_FLAG(HandleSignalMode, handle_segv, kHandleSignalYes,
             COMMON_FLAG_HANDLE_SIGNAL_HELP(SIGSEGV))
 COMMON_FLAG(HandleSignalMode, handle_sigbus, kHandleSignalYes,
             COMMON_FLAG_HANDLE_SIGNAL_HELP(SIGBUS))
 COMMON_FLAG(HandleSignalMode, handle_abort, kHandleSignalNo,
             COMMON_FLAG_HANDLE_SIGNAL_HELP(SIGABRT))
 COMMON_FLAG(HandleSignalMode, handle_sigill, kHandleSignalNo,
             COMMON_FLAG_HANDLE_SIGNAL_HELP(SIGILL))
 COMMON_FLAG(HandleSignalMode, handle_sigfpe, kHandleSignalYes,
             COMMON_FLAG_HANDLE_SIGNAL_HELP(SIGFPE))
 #undef COMMON_FLAG_HANDLE_SIGNAL_HELP
-COMMON_FLAG(bool, allow_user_segv_handler, true,
-            "If set, allows user to register a SEGV handler even if the tool "
-            "registers one.")
 COMMON_FLAG(bool, use_sigaltstack, true,
             "If set, uses alternate stack for signal handling.")
 COMMON_FLAG(bool, detect_deadlocks, false,
             "If set, deadlock detection is enabled.")
 COMMON_FLAG(
     uptr, clear_shadow_mmap_threshold, 64 * 1024,
     "Large shadow regions are zero-filled using mmap(NORESERVE) instead of "
     "memset(). This is the threshold size in bytes.")

compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc

AndroidApiLevel AndroidGetApiLevel() {
   if (level) return level;
   level = AndroidDetectApiLevel();
   atomic_store(&android_api_level, level, memory_order_relaxed);
   return level;
 }
 
 #endif
 
-bool IsHandledDeadlySignal(int signum) {
+HandleSignalMode GetHandleSignalMode(int signum) {
   switch (signum) {
     case SIGABRT:
       return common_flags()->handle_abort;
     case SIGILL:
       return common_flags()->handle_sigill;
     case SIGFPE:
       return common_flags()->handle_sigfpe;
     case SIGSEGV:
       return common_flags()->handle_segv;
     case SIGBUS:
       return common_flags()->handle_sigbus;
   }
-  return false;
+  return kHandleSignalNo;
 }
 
 #if !SANITIZER_GO
 void *internal_start_thread(void(*func)(void *arg), void *arg) {
   // Start the thread with signals blocked, otherwise it can steal user signals.
   __sanitizer_sigset_t set, old;
   internal_sigfillset(&set);
 #if SANITIZER_LINUX && !SANITIZER_ANDROID

compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc

 }
 
 void ListOfModules::init() {
   clear();
   MemoryMappingLayout memory_mapping(false);
   memory_mapping.DumpListOfModules(&modules_);
 }
 
-bool IsHandledDeadlySignal(int signum) {
+HandleSignalMode GetHandleSignalMode(int signum) {
   // Handling fatal signals on watchOS and tvOS devices is disallowed.
   if ((SANITIZER_WATCHOS || SANITIZER_TVOS) && !(SANITIZER_IOSSIM))
-    return false;
+    return kHandleSignalNo;
   switch (signum) {
     case SIGABRT:
       return common_flags()->handle_abort;
     case SIGILL:
       return common_flags()->handle_sigill;
     case SIGFPE:
       return common_flags()->handle_sigfpe;
     case SIGSEGV:
       return common_flags()->handle_segv;
     case SIGBUS:
       return common_flags()->handle_sigbus;
   }
-  return false;
+  return kHandleSignalNo;
 }
 
 MacosVersion cached_macos_version = MACOS_VERSION_UNINITIALIZED;
 
 MacosVersion GetMacosVersionInternal() {
   int mib[2] = { CTL_KERN, KERN_OSRELEASE };
   char version[100];
   uptr len = 0, maxlen = sizeof(version) / sizeof(version[0]);

compiler-rt/trunk/lib/sanitizer_common/sanitizer_posix_libcdep.cc

 
 void SleepForMillis(int millis) {
   usleep(millis * 1000);
 }
 
 void Abort() {
 #if !SANITIZER_GO
   // If we are handling SIGABRT, unhandle it first.
-  if (IsHandledDeadlySignal(SIGABRT)) {
+  // TODO(vitalybuka): Check if handler belongs to sanitizer.
+  if (GetHandleSignalMode(SIGABRT) != kHandleSignalNo) {
     struct sigaction sigact;
     internal_memset(&sigact, 0, sizeof(sigact));
     sigact.sa_sigaction = (sa_sigaction_t)SIG_DFL;
     internal_sigaction(SIGABRT, &sigact, nullptr);
   }
 #endif
 
   abort();
void UnsetAlternateSignalStack() {
   altstack.ss_flags = SS_DISABLE;
   altstack.ss_size = kAltStackSize;  // Some sane value required on Darwin.
   CHECK_EQ(0, sigaltstack(&altstack, &oldstack));
   UnmapOrDie(oldstack.ss_sp, oldstack.ss_size);
 }
 
 static void MaybeInstallSigaction(int signum,
                                   SignalHandlerType handler) {
-  if (!IsHandledDeadlySignal(signum))
-    return;
+  if (GetHandleSignalMode(signum) == kHandleSignalNo) return;
+
   struct sigaction sigact;
   internal_memset(&sigact, 0, sizeof(sigact));
   sigact.sa_sigaction = (sa_sigaction_t)handler;
   // Do not block the signal from being received in that signal's handler.
   // Clients are responsible for handling this correctly.
   sigact.sa_flags = SA_SIGINFO | SA_NODEFER;
   if (common_flags()->use_sigaltstack) sigact.sa_flags |= SA_ONSTACK;
   CHECK_EQ(0, internal_sigaction(signum, &sigact, nullptr));

compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc

void UnsetAlternateSignalStack() {
   // FIXME: Decide what to do on Windows.
 }
 
 void InstallDeadlySignalHandlers(SignalHandlerType handler) {
   (void)handler;
   // FIXME: Decide what to do on Windows.
 }
 
-bool IsHandledDeadlySignal(int signum) {
+HandleSignalMode GetHandleSignalMode(int signum) {
   // FIXME: Decide what to do on Windows.
-  return false;
+  return kHandleSignalNo;
 }
 
 // Check based on flags if we should handle this exception.
 bool IsHandledDeadlyException(DWORD exceptionCode) {
   switch (exceptionCode) {
     case EXCEPTION_ACCESS_VIOLATION:
     case EXCEPTION_ARRAY_BOUNDS_EXCEEDED:
     case EXCEPTION_STACK_OVERFLOW:

compiler-rt/trunk/lib/sanitizer_common/tests/sanitizer_flags_test.cc

 
 TEST(SanitizerCommon, HandleSignalMode) {
   TestFlag(kHandleSignalNo, "flag_name=1", kHandleSignalYes);
   TestFlag(kHandleSignalNo, "flag_name=yes", kHandleSignalYes);
   TestFlag(kHandleSignalNo, "flag_name=true", kHandleSignalYes);
   TestFlag(kHandleSignalYes, "flag_name=0", kHandleSignalNo);
   TestFlag(kHandleSignalYes, "flag_name=no", kHandleSignalNo);
   TestFlag(kHandleSignalYes, "flag_name=false", kHandleSignalNo);
+  TestFlag(kHandleSignalNo, "flag_name=2", kHandleSignalExclusive);
+  TestFlag(kHandleSignalYes, "flag_name=exclusive", kHandleSignalExclusive);
 
   EXPECT_DEATH(TestFlag(kHandleSignalNo, "flag_name", kHandleSignalNo),
                "expected '='");
   EXPECT_DEATH(TestFlag(kHandleSignalNo, "flag_name=", kHandleSignalNo),
                "Invalid value for signal handler option: ''");
-  EXPECT_DEATH(TestFlag(kHandleSignalNo, "flag_name=2", kHandleSignalNo),
-               "Invalid value for signal handler option: '2'");
+  EXPECT_DEATH(TestFlag(kHandleSignalNo, "flag_name=3", kHandleSignalNo),
+               "Invalid value for signal handler option: '3'");
   EXPECT_DEATH(TestFlag(kHandleSignalNo, "flag_name=-1", kHandleSignalNo),
                "Invalid value for signal handler option: '-1'");
   EXPECT_DEATH(TestFlag(kHandleSignalNo, "flag_name=on", kHandleSignalNo),
                "Invalid value for signal handler option: 'on'");
 }
 
 TEST(SanitizerCommon, IntFlags) {
   TestFlag(-11, 0, -11);

compiler-rt/trunk/test/asan/TestCases/Posix/allow_user_segv.cc

 // Regression test for
 // https://code.google.com/p/address-sanitizer/issues/detail?id=180
 
-// RUN: %clangxx_asan -O0 %s -o %t && %env_asan_opts=allow_user_segv_handler=true not %run %t 2>&1 | FileCheck %s
-// RUN: %clangxx_asan -O2 %s -o %t && %env_asan_opts=allow_user_segv_handler=true not %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx_asan -O0 %s -o %t && %env_asan_opts=handle_segv=0 not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK0
+// RUN: %clangxx_asan -O2 %s -o %t && %env_asan_opts=handle_segv=0 not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK0
+
+// RUN: %clangxx_asan -O0 %s -o %t && %env_asan_opts=handle_segv=1 not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK1
+// RUN: %clangxx_asan -O2 %s -o %t && %env_asan_opts=handle_segv=1 not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK1
+
+// RUN: %clangxx_asan -O0 %s -o %t && %env_asan_opts=handle_segv=2 not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK2
+// RUN: %clangxx_asan -O2 %s -o %t && %env_asan_opts=handle_segv=2 not %run %t 2>&1 | FileCheck %s --check-prefix=CHECK2
 
 #include <signal.h>
 #include <stdio.h>
 #include <stdlib.h>
 
 struct sigaction original_sigaction_sigbus;
 struct sigaction original_sigaction_sigsegv;
 
 void User_OnSIGSEGV(int signum, siginfo_t *siginfo, void *context) {
   fprintf(stderr, "User sigaction called\n");
   struct sigaction original_sigaction;
   if (signum == SIGBUS)
     original_sigaction = original_sigaction_sigbus;
   else if (signum == SIGSEGV)
     original_sigaction = original_sigaction_sigsegv;
   else {
     printf("Invalid signum");
     exit(1);
   }
-  if (original_sigaction.sa_flags | SA_SIGINFO)
+  if (original_sigaction.sa_flags | SA_SIGINFO) {
+    if (original_sigaction.sa_sigaction)
       original_sigaction.sa_sigaction(signum, siginfo, context);
-  else
+  } else {
+    if (original_sigaction.sa_handler)
       original_sigaction.sa_handler(signum);
   }
+  exit(1);
+}
 
 int DoSEGV() {
   volatile int *x = 0;
   return *x;
 }
 
-int InstallHandler(int signum, struct sigaction *original_sigaction) {
+bool InstallHandler(int signum, struct sigaction *original_sigaction) {
   struct sigaction user_sigaction;
   user_sigaction.sa_sigaction = User_OnSIGSEGV;
   user_sigaction.sa_flags = SA_SIGINFO;
   if (sigaction(signum, &user_sigaction, original_sigaction)) {
     perror("sigaction");
-    return 1;
+    return false;
   }
-  return 0;
+  return true;
 }
 
 int main() {
   // Let's install handlers for both SIGSEGV and SIGBUS, since pre-Yosemite
   // 32-bit Darwin triggers SIGBUS instead.
-  if (InstallHandler(SIGSEGV, &original_sigaction_sigsegv)) return 1;
-  if (InstallHandler(SIGBUS, &original_sigaction_sigbus)) return 1;
+  if (InstallHandler(SIGSEGV, &original_sigaction_sigsegv) &&
+      InstallHandler(SIGBUS, &original_sigaction_sigbus)) {
     fprintf(stderr, "User sigaction installed\n");
+  }
   return DoSEGV();
 }
 
-// CHECK: User sigaction installed
-// CHECK-NEXT: User sigaction called
-// CHECK-NEXT: ASAN:DEADLYSIGNAL
-// CHECK: AddressSanitizer: SEGV on unknown address
+// CHECK0-NOT: ASAN:DEADLYSIGNAL
+// CHECK0-NOT: AddressSanitizer: SEGV on unknown address
+// CHECK0: User sigaction installed
+// CHECK0-NEXT: User sigaction called
+
+// CHECK1: User sigaction installed
+// CHECK1-NEXT: User sigaction called
+// CHECK1-NEXT: ASAN:DEADLYSIGNAL
+// CHECK1: AddressSanitizer: SEGV on unknown address
+
+// CHECK2-NOT: User sigaction called
+// CHECK2: User sigaction installed
+// CHECK2-NEXT: ASAN:DEADLYSIGNAL
+// CHECK2: AddressSanitizer: SEGV on unknown address

compiler-rt/trunk/test/sanitizer_common/TestCases/Linux/signal_segv_handler.cc

-// RUN: %clangxx -O1 %s -o %t && TSAN_OPTIONS="flush_memory_ms=1 memory_limit_mb=1" ASAN_OPTIONS="handle_segv=0 allow_user_segv_handler=1" %run %t 2>&1 | FileCheck %s
+// RUN: %clangxx -O1 %s -o %t && TSAN_OPTIONS="flush_memory_ms=1 memory_limit_mb=1" ASAN_OPTIONS="handle_segv=0" %run %t 2>&1 | FileCheck %s
 
 // JVM uses SEGV to preempt threads. All threads do a load from a known address
 // periodically. When runtime needs to preempt threads, it unmaps the page.
 // Threads start triggering SEGV one by one. The signal handler blocks
 // threads while runtime does its thing. Then runtime maps the page again
 // and resumes the threads.
 // Previously this pattern conflicted with stop-the-world machinery,
 // because it briefly reset SEGV handler to SIG_DFL.