[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

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

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!

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

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 x86_64-linux-gnu -fsanitize=float-divide-by-zero %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FLOAT-DIV-BY-ZERO:
+// CHECK-FLOAT-DIV-BY-ZERO: "-fsanitize=float-divide-by-zero"