[PATCH] D64317: [Driver] Add float-divide-by-zero back to supported sanitizers after D63793/rC365272

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 8 12:33:21 PDT 2019


rsmith added inline comments.


================
Comment at: lib/Driver/SanitizerArgs.cpp:27-30
 static const SanitizerMask NeedsUbsanRt =
     SanitizerKind::Undefined | SanitizerKind::Integer |
     SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
+    SanitizerKind::CFI | SanitizerKind::FloatDivideByZero;
----------------
Duplicating this list of "all sanitizers covered by the ubsan runtime" in many places is leading to lots of bugs.

This change misses `getPPTransparentSanitizers()` in `include/clang/Basic/Sanitizers.h`. Previous changes forgot to add `UnsignedIntegerOverflow` and `LocalBounds` to that function and also here and in `SupportsCoverage` and `RecoverableByDefault` below (but did update `TrappingSupported` and `getSupportedSanitizers`).

A better approach would be to change `Sanitizers.def` to specify the relevant properties of the sanitizer (whether it's in the ubsan runtime, whether it's recoverable, whether it supports coverage, whether it supports trapping, whether it affects preprocessing) along with its definition.


================
Comment at: test/Driver/fsanitize.c:844
+
+// RUN: %clang -target x86_64-linux -fsanitize=float-divide-by-zero %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-DIVBYZERO,CHECK-DIVBYZERO-RECOVER
+// RUN: %clang -target x86_64-linux -fsanitize=float-divide-by-zero %s -fno-sanitize-recover=float-divide-by-zero -### 2>&1 | FileCheck %s --check-prefixes=CHECK-DIVBYZERO,CHECK-DIVBYZERO-NORECOVER
----------------
I think these tests should be target-independent. We should support this sanitizer (and indeed almost all of ubsan) regardless of the target. (Currently this is true on all targets except Myriad, where the latter is disallowed only due to a bug in r281071.)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64317/new/

https://reviews.llvm.org/D64317





More information about the cfe-commits mailing list