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

[MaskRay]

Summary

D63793 removed float-divide-by-zero from the "undefined" set but it
failed to add it to getSupportedSanitizers(), thus the sanitizer is
rejected by the driver:

clang-9: error: unsupported option '-fsanitize=float-divide-by-zero' for target 'x86_64-unknown-linux-gnu'

Also, add SanitizerMask::FloatDivideByZero to a few other masks to make -fsanitize-trap, -fsanitize-recover, -fsanitize-minimal-runtime and -fsanitize-coverage work.

[MaskRay]

This is incorrect. Should fix -fsanitize=float-divide-by-zero instead.

[MaskRay]

Fix -fsanitize=float-divide-by-zero

[MaskRay]

Remove a stray :

[MaskRay]

make various sanitizer features work

[MaskRay]

Fix sanitizer-ld.c test

[vitalybuka]

is the patch description still correct?

[MaskRay]

In D64317#1574007, @vitalybuka wrote:

is the patch description still correct?

Yes.

Also, add SanitizerMask::FloatDivideByZero to a few other masks to make -fsanitize-trap, -fsanitize-recover, -fsanitize-minimal-runtime and -fsanitize-coverage work.

-fsanitize=float-divide-by-zero was accidentally removed by D63793. This patch adds it back.

[rsmith]

lib/Driver/SanitizerArgs.cpp

27–30 ↗

(On Diff #208410)

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.

test/Driver/fsanitize.c

844 ↗

(On Diff #208410)

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.)

[MaskRay]

Fix getPPTransparentSanitizers (test/Modules/check-for-sanitizer-feature.cpp) & ubsan_blacklist.txt

[MaskRay]

lib/Driver/SanitizerArgs.cpp

27–30 ↗

(On Diff #208410)

This change misses getPPTransparentSanitizers() in include/clang/Basic/Sanitizers.h

Thanks for pointing this out! It also missed ubsan_blacklist.txt (I think it still makes sense to reuse the ubsan_blacklist.txt file, as "Integer" does that too)

It is not in static const SanitizerMask LegacyFsanitizeRecoverMask = SanitizerKind::Undefined | SanitizerKind::Integer; but since this mask is legacy, I guess we do not have to touch it. AFAICT, all places have been fixed.

A better approach would be to change Sanitizers.def to specify the relevant properties of the sanitizer

I'm strongly in favor of doing this, as I learned from the process how brittle it is to let these masks scatter over all places.. But, can we (1) bring back float-divide-by-zero first, and then (2) refactor tests and make them target-independent, (3) fix Sanitizers.def?

test/Driver/fsanitize.c

844 ↗

(On Diff #208410)

Changed -target x86_64-linux to -target %itanium_abi_triple.

It seems Myriad also works - I have tried -target sparcel-myriad

[MaskRay]

test/Driver/fsanitize-blacklist.c

32 ↗

(On Diff #208565)

These tests can probably be changed to use -target %itanium_abi_triple, too. I can do that if -target %itanium_abi_triple -fsanitize=float-divide-by-zero is proven to work...

[rsmith]

LGTM, thanks!

cfe/trunk/include/clang/Basic/Sanitizers.h

 /// For each sanitizer group bit set in \p Kinds, set the bits for sanitizers
 /// this group enables.
 SanitizerMask expandSanitizerGroups(SanitizerMask Kinds);
 
 /// Return the sanitizers which do not affect preprocessing.
 inline SanitizerMask getPPTransparentSanitizers() {
   return SanitizerKind::CFI | SanitizerKind::Integer |
          SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
-         SanitizerKind::Undefined;
+         SanitizerKind::Undefined | SanitizerKind::FloatDivideByZero;
 }
 
 } // namespace clang
 
 #endif // LLVM_CLANG_BASIC_SANITIZERS_H

cfe/trunk/lib/Driver/SanitizerArgs.cpp

 
 using namespace clang;
 using namespace clang::driver;
 using namespace llvm::opt;
 
 static const SanitizerMask NeedsUbsanRt =
     SanitizerKind::Undefined | SanitizerKind::Integer |
     SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
-    SanitizerKind::CFI;
+    SanitizerKind::CFI | SanitizerKind::FloatDivideByZero;
 static const SanitizerMask NeedsUbsanCxxRt =
     SanitizerKind::Vptr | SanitizerKind::CFI;
 static const SanitizerMask NotAllowedWithTrap = SanitizerKind::Vptr;
 static const SanitizerMask NotAllowedWithMinimalRuntime = SanitizerKind::Vptr;
 static const SanitizerMask RequiresPIE =
     SanitizerKind::DataFlow | SanitizerKind::HWAddress | SanitizerKind::Scudo;
 static const SanitizerMask NeedsUnwindTables =
     SanitizerKind::Address | SanitizerKind::HWAddress | SanitizerKind::Thread |
     SanitizerKind::Memory | SanitizerKind::DataFlow;
 static const SanitizerMask SupportsCoverage =
     SanitizerKind::Address | SanitizerKind::HWAddress |
     SanitizerKind::KernelAddress | SanitizerKind::KernelHWAddress |
     SanitizerKind::Memory | SanitizerKind::KernelMemory | SanitizerKind::Leak |
     SanitizerKind::Undefined | SanitizerKind::Integer |
     SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
     SanitizerKind::DataFlow | SanitizerKind::Fuzzer |
-    SanitizerKind::FuzzerNoLink;
+    SanitizerKind::FuzzerNoLink | SanitizerKind::FloatDivideByZero;
 static const SanitizerMask RecoverableByDefault =
     SanitizerKind::Undefined | SanitizerKind::Integer |
-    SanitizerKind::ImplicitConversion | SanitizerKind::Nullability;
+    SanitizerKind::ImplicitConversion | SanitizerKind::Nullability |
+    SanitizerKind::FloatDivideByZero;
 static const SanitizerMask Unrecoverable =
     SanitizerKind::Unreachable | SanitizerKind::Return;
 static const SanitizerMask AlwaysRecoverable =
     SanitizerKind::KernelAddress | SanitizerKind::KernelHWAddress;
 static const SanitizerMask LegacyFsanitizeRecoverMask =
     SanitizerKind::Undefined | SanitizerKind::Integer;
 static const SanitizerMask NeedsLTO = SanitizerKind::CFI;
 static const SanitizerMask TrappingSupported =
     (SanitizerKind::Undefined & ~SanitizerKind::Vptr) |
     SanitizerKind::UnsignedIntegerOverflow | SanitizerKind::ImplicitConversion |
     SanitizerKind::Nullability | SanitizerKind::LocalBounds |
-    SanitizerKind::CFI;
+    SanitizerKind::CFI | SanitizerKind::FloatDivideByZero;
 static const SanitizerMask TrappingDefault = SanitizerKind::CFI;
 static const SanitizerMask CFIClasses =
     SanitizerKind::CFIVCall | SanitizerKind::CFINVCall |
     SanitizerKind::CFIMFCall | SanitizerKind::CFIDerivedCast |
     SanitizerKind::CFIUnrelatedCast;
 static const SanitizerMask CompatibleWithMinimalRuntime =
     TrappingSupported | SanitizerKind::Scudo | SanitizerKind::ShadowCallStack;
 
struct Blacklist {
     const char *File;
     SanitizerMask Mask;
   } Blacklists[] = {{"asan_blacklist.txt", SanitizerKind::Address},
                     {"hwasan_blacklist.txt", SanitizerKind::HWAddress},
                     {"msan_blacklist.txt", SanitizerKind::Memory},
                     {"tsan_blacklist.txt", SanitizerKind::Thread},
                     {"dfsan_abilist.txt", SanitizerKind::DataFlow},
                     {"cfi_blacklist.txt", SanitizerKind::CFI},
-                    {"ubsan_blacklist.txt", SanitizerKind::Undefined |
-                                                SanitizerKind::Integer |
-                                                SanitizerKind::Nullability}};
+                    {"ubsan_blacklist.txt",
+                     SanitizerKind::Undefined | SanitizerKind::Integer |
+                         SanitizerKind::Nullability |
+                         SanitizerKind::FloatDivideByZero}};
 
   for (auto BL : Blacklists) {
     if (!(Kinds & BL.Mask))
       continue;
 
     clang::SmallString<64> Path(D.ResourceDir);
     llvm::sys::path::append(Path, "share", BL.File);
     if (llvm::sys::fs::exists(Path))

cfe/trunk/lib/Driver/ToolChain.cpp

 SanitizerMask ToolChain::getSupportedSanitizers() const {
   // Return sanitizers which don't require runtime support and are not
   // platform dependent.
 
   SanitizerMask Res = (SanitizerKind::Undefined & ~SanitizerKind::Vptr &
                        ~SanitizerKind::Function) |
                       (SanitizerKind::CFI & ~SanitizerKind::CFIICall) |
                       SanitizerKind::CFICastStrict |
+                      SanitizerKind::FloatDivideByZero |
                       SanitizerKind::UnsignedIntegerOverflow |
                       SanitizerKind::ImplicitConversion |
                       SanitizerKind::Nullability | SanitizerKind::LocalBounds;
   if (getTriple().getArch() == llvm::Triple::x86 ||
       getTriple().getArch() == llvm::Triple::x86_64 ||
       getTriple().getArch() == llvm::Triple::arm ||
       getTriple().getArch() == llvm::Triple::aarch64 ||
       getTriple().getArch() == llvm::Triple::wasm32 ||

cfe/trunk/test/Driver/fsanitize-blacklist.c

 // CHECK-DEFAULT-BLACKLIST-ASAN: -fsanitize-blacklist={{.*[^w]}}asan_blacklist.txt
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=hwaddress -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-BLACKLIST-HWASAN --implicit-check-not=fdepfile-entry --implicit-check-not=-fsanitize-blacklist=
 // CHECK-DEFAULT-BLACKLIST-HWASAN: -fsanitize-blacklist={{.*}}hwasan_blacklist.txt
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=integer -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-UBSAN-BLACKLIST --implicit-check-not=fdepfile-entry --implicit-check-not=-fsanitize-blacklist=
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=nullability -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-UBSAN-BLACKLIST --implicit-check-not=fdepfile-entry --implicit-check-not=-fsanitize-blacklist=
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-UBSAN-BLACKLIST --implicit-check-not=fdepfile-entry --implicit-check-not=-fsanitize-blacklist=
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=alignment -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-UBSAN-BLACKLIST --implicit-check-not=fdepfile-entry --implicit-check-not=-fsanitize-blacklist=
+// RUN: %clang -target %itanium_abi_triple -fsanitize=float-divide-by-zero -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-UBSAN-BLACKLIST --implicit-check-not=fdepfile-entry --implicit-check-not=-fsanitize-blacklist=
 // CHECK-DEFAULT-UBSAN-BLACKLIST: -fsanitize-blacklist={{.*}}ubsan_blacklist.txt
 
 // Check that combining ubsan and another sanitizer results in both blacklists being used.
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined,address -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-UBSAN-BLACKLIST --check-prefix=CHECK-DEFAULT-ASAN-BLACKLIST --implicit-check-not=fdepfile-entry --implicit-check-not=-fsanitize-blacklist=
 
 // Ignore -fsanitize-blacklist flag if there is no -fsanitize flag.
 // RUN: %clang -target x86_64-linux-gnu -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE --check-prefix=DELIMITERS
 // CHECK-NO-SANITIZE-NOT: -fsanitize-blacklist

cfe/trunk/test/Driver/fsanitize-coverage.c

 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-coverage=0 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-0
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-coverage=edge -fsanitize-coverage=0 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-0
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-0
 // CHECK-SANITIZE-COVERAGE-0-NOT: fsanitize-coverage-type
 // CHECK-SANITIZE-COVERAGE-0: -fsanitize=address
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=kernel-address -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=hwaddress -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=kernel-hwaddress -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=memory -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=kernel-memory -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=leak -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=bool -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=dataflow -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
+// RUN: %clang -target %itanium_abi_triple -fsanitize=float-divide-by-zero -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // RUN: %clang -target x86_64-linux-gnu                     -fsanitize-coverage=func,trace-pc %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-FUNC
 // CHECK-SANITIZE-COVERAGE-FUNC: fsanitize-coverage-type=1
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-coverage=bb %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-BB
 // CHECK-SANITIZE-COVERAGE-BB: fsanitize-coverage-type=2
 
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address -fsanitize-coverage=edge %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANITIZE-COVERAGE-EDGE
 // CHECK-SANITIZE-COVERAGE-EDGE: fsanitize-coverage-type=3

cfe/trunk/test/Driver/fsanitize.c

 // RUN: %clang -target x86_64-linux-gnu -fsanitize=pointer-subtract %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-POINTER-SUB-NEEDS-ADDRESS
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=pointer-subtract -fno-sanitize=pointer-subtract %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-POINTER-SUB
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=pointer-compare -fno-sanitize=pointer-compare %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-POINTER-CMP
 // CHECK-POINTER-ALL: -cc1{{.*}}-fsanitize={{[^"]*}}pointer-compare,pointer-subtract{{.*}}" {{.*}} "-mllvm" "-asan-detect-invalid-pointer-cmp" {{.*}}"-mllvm" "-asan-detect-invalid-pointer-sub"
 // CHECK-POINTER-CMP-NEEDS-ADDRESS: error: invalid argument '-fsanitize=pointer-compare' only allowed with '-fsanitize=address'
 // CHECK-POINTER-SUB-NEEDS-ADDRESS: error: invalid argument '-fsanitize=pointer-subtract' only allowed with '-fsanitize=address'
 // CHECK-NO-POINTER-SUB-NOT: {{.*}}asan-detect-invalid-pointer{{.*}}
 // CHECK-NO-POINTER-CMP-NOT: {{.*}}asan-detect-invalid-pointer{{.*}}
+
+// RUN: %clang -target %itanium_abi_triple -fsanitize=float-divide-by-zero %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-DIVBYZERO,CHECK-DIVBYZERO-RECOVER
+// RUN: %clang -target %itanium_abi_triple -fsanitize=float-divide-by-zero %s -fno-sanitize-recover=float-divide-by-zero -### 2>&1 | FileCheck %s --check-prefixes=CHECK-DIVBYZERO,CHECK-DIVBYZERO-NORECOVER
+// RUN: %clang -target %itanium_abi_triple -fsanitize=float-divide-by-zero -fsanitize-trap=float-divide-by-zero %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-DIVBYZERO,CHECK-DIVBYZERO-NORECOVER,CHECK-DIVBYZERO-TRAP
+// CHECK-DIVBYZERO: "-fsanitize=float-divide-by-zero"
+// CHECK-DIVBYZERO-RECOVER: "-fsanitize-recover=float-divide-by-zero"
+// CHECK-DIVBYZERO-NORECOVER-NOT: "-fsanitize-recover=float-divide-by-zero"
+// CHECK-DIVBYZERO-TRAP: "-fsanitize-trap=float-divide-by-zero"
+
+// RUN: %clang -target %itanium_abi_triple -fsanitize=float-divide-by-zero -fsanitize-minimal-runtime %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-DIVBYZERO,CHECK-DIVBYZERO-MINIMAL
+// CHECK-DIVBYZERO-MINIMAL: "-fsanitize-minimal-runtime"
+
+// RUN: %clang -target %itanium_abi_triple -fsanitize=undefined,float-divide-by-zero %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DIVBYZERO-UBSAN
+// CHECK-DIVBYZERO-UBSAN: "-fsanitize={{.*}},float-divide-by-zero,{{.*}}"

cfe/trunk/test/Driver/sanitizer-ld.c

 // CHECK-MSAN-LINUX-CXX: "-lrt"
 // CHECK-MSAN-LINUX-CXX: "-ldl"
 
 // RUN: %clang -fsanitize=undefined %s -### -o %t.o 2>&1 \
 // RUN:     -target i386-unknown-linux -fuse-ld=ld \
 // RUN:     --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:   | FileCheck --check-prefix=CHECK-UBSAN-LINUX %s
 
+// RUN: %clang -fsanitize=float-divide-by-zero %s -### -o %t.o 2>&1 \
+// RUN:     -target i386-unknown-linux -fuse-ld=ld \
+// RUN:     --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-UBSAN-LINUX %s
+
 // RUN: %clang -fsanitize=undefined %s -### -o %t.o 2>&1 \
 // RUN:     -target i386-unknown-linux -fuse-ld=ld \
 // RUN:     --sysroot=%S/Inputs/basic_linux_tree \
 // RUN:     -static-libsan \
 // RUN:   | FileCheck --check-prefix=CHECK-UBSAN-LINUX %s
 
 // CHECK-UBSAN-LINUX: "{{.*}}ld{{(.exe)?}}"
 // CHECK-UBSAN-LINUX-NOT: libclang_rt.asan

cfe/trunk/test/Modules/check-for-sanitizer-feature.cpp

 //
 // First, build without any sanitization.
 // RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t.2 \
 // RUN:   -fmodule-map-file=%S/Inputs/check-for-sanitizer-feature/map \
 // RUN:   -I %S/Inputs/check-for-sanitizer-feature -verify %s
 // RUN: ls %t.2 | count 2
 //
 // Next, build with sanitization, and check that a new module isn't built.
-// RUN: %clang_cc1 -fsanitize=cfi-vcall,unsigned-integer-overflow,nullability-arg,null -fmodules \
+// RUN: %clang_cc1 -fsanitize=cfi-vcall,float-divide-by-zero,unsigned-integer-overflow,nullability-arg,null -fmodules \
 // RUN:   -fmodules-cache-path=%t.2 \
 // RUN:   -fmodule-map-file=%S/Inputs/check-for-sanitizer-feature/map \
 // RUN:   -I %S/Inputs/check-for-sanitizer-feature -verify %s
 // RUN: ls %t.2 | count 2
 
 // Finally, test that including enabled sanitizers in the module hash isn't
 // required to ensure correctness of module imports.
 //