[asan] Move exception code to sanitizer_common.

[etienneb]

This code will be used by an other sanitizer?

lib/asan/asan_win.cc

239 ↗

(On Diff #86831)

You should move the comment too.

[rnk]

lgtm

[mpividori]

In D29458#664838, @etienneb wrote:

This code will be used by an other sanitizer?

Hi @etienneb,
We want to have general support for Windows, not only on asan. Because of that all the refactory in:

• https://reviews.llvm.org/D29148
• https://reviews.llvm.org/D29154
• https://reviews.llvm.org/D29155
• https://reviews.llvm.org/D29158

lib/asan/asan_win.cc

239 ↗

(On Diff #86831)

You are right.

[kcc]

lib/sanitizer_common/sanitizer_common.h

384 ↗

(On Diff #86831)

no #if please.
The only #if we accept (except for some special files) is a single #if around the entire file.

[etienneb]

lgtm

[rnk]

lib/sanitizer_common/sanitizer_common.h

384 ↗

(On Diff #86831)

sanitizer_common.h has a lot of #ifs, so I thought this was acceptable. If we just remove the if here, a caller will get a link error. Would you prefer that?

[mpividori]

lib/sanitizer_common/sanitizer_common.h

384 ↗

(On Diff #86831)

@kcc if you prefer I can add an empty implementation for Posix.

[kcc]

The amount if #if statements is extremely annoying, but we clearly don't want any more.

I am not sure why you need this function declaration in lib/sanitizer_common/sanitizer_common.h
given that it's definition and its use is in *win* files.

[mpividori]

In D29458#665040, @kcc wrote:

The amount if #if statements is extremely annoying, but we clearly don't want any more.

I am not sure why you need this function declaration in lib/sanitizer_common/sanitizer_common.h
given that it's definition and its use is in *win* files.

Because we don't have a specific header for Windows interface in sanitizer_common. If you prefer, I can create: sanitizer_windows.h and add it there.

[kcc]

yes.
maybe, sanitizer_win.h

[mpividori]

@kcc I added a new header. Let me know if you agree.
Thanks!

[kcc]

LGTM

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

 #include "asan_interceptors.h"
 #include "asan_internal.h"
 #include "asan_report.h"
 #include "asan_stack.h"
 #include "asan_thread.h"
 #include "asan_mapping.h"
 #include "sanitizer_common/sanitizer_libc.h"
 #include "sanitizer_common/sanitizer_mutex.h"
+#include "sanitizer_common/sanitizer_win.h"
 #include "sanitizer_common/sanitizer_win_defs.h"
 
 using namespace __asan;  // NOLINT
 
 extern "C" {
 SANITIZER_INTERFACE_ATTRIBUTE
 int __asan_should_detect_stack_use_after_return() {
   __asan_init();
#if SANITIZER_WINDOWS64
   // On Win64, we map memory on demand with access violation handler.
   // Install our exception handler.
   CHECK(AddVectoredExceptionHandler(TRUE, &ShadowExceptionHandler));
 #endif
 }
 
 static LPTOP_LEVEL_EXCEPTION_FILTER default_seh_handler;
 
-// Check based on flags if we should report this exception.
-static bool ShouldReportDeadlyException(unsigned code) {
-  switch (code) {
-    case EXCEPTION_ACCESS_VIOLATION:
-    case EXCEPTION_ARRAY_BOUNDS_EXCEEDED:
-    case EXCEPTION_STACK_OVERFLOW:
-    case EXCEPTION_DATATYPE_MISALIGNMENT:
-    case EXCEPTION_IN_PAGE_ERROR:
-      return common_flags()->handle_segv;
-    case EXCEPTION_ILLEGAL_INSTRUCTION:
-    case EXCEPTION_PRIV_INSTRUCTION:
-    case EXCEPTION_BREAKPOINT:
-      return common_flags()->handle_sigill;
-    case EXCEPTION_FLT_DENORMAL_OPERAND:
-    case EXCEPTION_FLT_DIVIDE_BY_ZERO:
-    case EXCEPTION_FLT_INEXACT_RESULT:
-    case EXCEPTION_FLT_INVALID_OPERATION:
-    case EXCEPTION_FLT_OVERFLOW:
-    case EXCEPTION_FLT_STACK_CHECK:
-    case EXCEPTION_FLT_UNDERFLOW:
-    case EXCEPTION_INT_DIVIDE_BY_ZERO:
-    case EXCEPTION_INT_OVERFLOW:
-      return common_flags()->handle_sigfpe;
-  }
-  return false;
-}
-
 // Return the textual name for this exception.
 const char *DescribeSignalOrException(int signo) {
   unsigned code = signo;
   // Get the string description of the exception if this is a known deadly
   // exception.
   switch (code) {
     case EXCEPTION_ACCESS_VIOLATION: return "access-violation";
     case EXCEPTION_ARRAY_BOUNDS_EXCEEDED: return "array-bounds-exceeded";

 }
 
 extern "C" SANITIZER_INTERFACE_ATTRIBUTE
 long __asan_unhandled_exception_filter(EXCEPTION_POINTERS *info) {
   EXCEPTION_RECORD *exception_record = info->ExceptionRecord;
   CONTEXT *context = info->ContextRecord;
 
   // Continue the search if the signal wasn't deadly.
-  if (!ShouldReportDeadlyException(exception_record->ExceptionCode))
+  if (!IsHandledDeadlyException(exception_record->ExceptionCode))
     return EXCEPTION_CONTINUE_SEARCH;
   // FIXME: Handle EXCEPTION_STACK_OVERFLOW here.
 
   SignalContext sig = SignalContext::Create(exception_record, context);
   ReportDeadlySignal(exception_record->ExceptionCode, sig);
   UNREACHABLE("returned from reporting deadly signal");
 }
 

compiler-rt/trunk/lib/sanitizer_common/CMakeLists.txt

set(SANITIZER_HEADERS
   sanitizer_suppressions.h
   sanitizer_symbolizer.h
   sanitizer_symbolizer_internal.h
   sanitizer_symbolizer_libbacktrace.h
   sanitizer_symbolizer_mac.h
   sanitizer_syscall_generic.inc
   sanitizer_syscall_linux_x86_64.inc
   sanitizer_syscall_linux_aarch64.inc
-  sanitizer_thread_registry.h)
+  sanitizer_thread_registry.h
+  sanitizer_win.h)
 
 include_directories(..)
 
 set(SANITIZER_COMMON_DEFINITIONS)
 
 include(CheckIncludeFile)
 append_have_file_definition(rpc/xdr.h HAVE_RPC_XDR_H SANITIZER_COMMON_DEFINITIONS)
 append_have_file_definition(tirpc/rpc/xdr.h HAVE_TIRPC_RPC_XDR_H SANITIZER_COMMON_DEFINITIONS)

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

+//===-- sanitizer_win.h -----------------------------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// Windows-specific declarations.
+//
+//===----------------------------------------------------------------------===//
+#ifndef SANITIZER_WIN_H
+#define SANITIZER_WIN_H
+
+#include "sanitizer_platform.h"
+#if SANITIZER_WINDOWS
+#include "sanitizer_internal_defs.h"
+
+namespace __sanitizer {
+// Check based on flags if we should handle the exception.
+bool IsHandledDeadlyException(DWORD exceptionCode);
+}  // namespace __sanitizer
+
+#endif  // SANITIZER_WINDOWS
+#endif  // SANITIZER_WIN_H

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

void InstallDeadlySignalHandlers(SignalHandlerType handler) {
   // FIXME: Decide what to do on Windows.
 }
 
 bool IsHandledDeadlySignal(int signum) {
   // FIXME: Decide what to do on Windows.
   return false;
 }
 
+// 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:
+    case EXCEPTION_DATATYPE_MISALIGNMENT:
+    case EXCEPTION_IN_PAGE_ERROR:
+      return common_flags()->handle_segv;
+    case EXCEPTION_ILLEGAL_INSTRUCTION:
+    case EXCEPTION_PRIV_INSTRUCTION:
+    case EXCEPTION_BREAKPOINT:
+      return common_flags()->handle_sigill;
+    case EXCEPTION_FLT_DENORMAL_OPERAND:
+    case EXCEPTION_FLT_DIVIDE_BY_ZERO:
+    case EXCEPTION_FLT_INEXACT_RESULT:
+    case EXCEPTION_FLT_INVALID_OPERATION:
+    case EXCEPTION_FLT_OVERFLOW:
+    case EXCEPTION_FLT_STACK_CHECK:
+    case EXCEPTION_FLT_UNDERFLOW:
+    case EXCEPTION_INT_DIVIDE_BY_ZERO:
+    case EXCEPTION_INT_OVERFLOW:
+      return common_flags()->handle_sigfpe;
+  }
+  return false;
+}
+
 bool IsAccessibleMemoryRange(uptr beg, uptr size) {
   SYSTEM_INFO si;
   GetNativeSystemInfo(&si);
   uptr page_size = si.dwPageSize;
   uptr page_mask = ~(page_size - 1);
 
   for (uptr page = beg & page_mask, end = (beg + size - 1) & page_mask;
        page <= end;) {