[PATCH] D34563: [ubsan] Disable the object-size check at -O0

Justin Bogner via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 23 16:01:46 PDT 2017


vsk at apple.com writes:
> Good point, it makes sense to hand users a warning when they explicitly enable
> -fsanitize=object-size at -O0. I've added in the diagnostic. PTAL.

LGTM.

> diff --git a/include/clang/Basic/DiagnosticDriverKinds.td b/include/clang/Basic/DiagnosticDriverKinds.td
> index 3833f0f28f..6ee061cca6 100644
> --- a/include/clang/Basic/DiagnosticDriverKinds.td
> +++ b/include/clang/Basic/DiagnosticDriverKinds.td
> @@ -226,6 +226,8 @@ def warn_drv_enabling_rtti_with_exceptions : Warning<
>  def warn_drv_disabling_vptr_no_rtti_default : Warning<
>    "implicitly disabling vptr sanitizer because rtti wasn't enabled">,
>    InGroup<DiagGroup<"auto-disable-vptr-sanitizer">>;
> +def warn_drv_object_size_disabled_O0 : Warning<
> +  "the object size sanitizer has no effect at -O0, but is explicitly enabled: %0">;
>  
>  def note_drv_command_failed_diag_msg : Note<
>    "diagnostic msg: %0">;
> diff --git a/lib/Driver/SanitizerArgs.cpp b/lib/Driver/SanitizerArgs.cpp
> index d6a9c35eda..d5c27ccddd 100644
> --- a/lib/Driver/SanitizerArgs.cpp
> +++ b/lib/Driver/SanitizerArgs.cpp
> @@ -208,12 +208,28 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
>    SanitizerMask TrappingKinds = parseSanitizeTrapArgs(D, Args);
>    SanitizerMask InvalidTrappingKinds = TrappingKinds & NotAllowedWithTrap;
>  
> +  // The object size sanitizer should not be enabled at -O0.
> +  Arg *OptLevel = Args.getLastArg(options::OPT_O_Group);
> +  bool RemoveObjectSizeAtO0 =
> +      !OptLevel || OptLevel->getOption().matches(options::OPT_O0);
> +
>    for (ArgList::const_reverse_iterator I = Args.rbegin(), E = Args.rend();
>         I != E; ++I) {
>      const auto *Arg = *I;
>      if (Arg->getOption().matches(options::OPT_fsanitize_EQ)) {
>        Arg->claim();
> -      SanitizerMask Add = parseArgValues(D, Arg, true);
> +      SanitizerMask Add = parseArgValues(D, Arg, /*AllowGroups=*/true);
> +
> +      if (RemoveObjectSizeAtO0) {
> +        AllRemove |= SanitizerKind::ObjectSize;
> +
> +        // The user explicitly enabled the object size check sanitizer. Warn
> +        // them that this does nothing at -O0.
> +        if (Add & SanitizerKind::ObjectSize)
> +          D.Diag(diag::warn_drv_object_size_disabled_O0)
> +              << Arg->getAsString(Args);
> +      }
> +
>        AllAddedKinds |= expandSanitizerGroups(Add);
>  
>        // Avoid diagnosing any sanitizer which is disabled later.
> diff --git a/test/Driver/fsanitize-object-size.c b/test/Driver/fsanitize-object-size.c
> new file mode 100644
> index 0000000000..ad8e27a9f4
> --- /dev/null
> +++ b/test/Driver/fsanitize-object-size.c
> @@ -0,0 +1,29 @@
> +// Check that the object size check is disabled at -O0.
> +//
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=object-size %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OSIZE
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=object-size %s -O0 -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OSIZE
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=null,object-size %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-OSIZE
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefixes=CHECK-NO-OSIZE-NO-WARNING
> +
> +// Check that the object size check is enabled at other optimization levels.
> +//
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -O1 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HAS-OSIZE
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=object-size -O2 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HAS-OSIZE
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=object-size -O3 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HAS-OSIZE
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=object-size -O4 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HAS-OSIZE
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=object-size -Ofast %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HAS-OSIZE
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=object-size -Os %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HAS-OSIZE
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=object-size -Oz %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HAS-OSIZE
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=object-size -Og %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HAS-OSIZE
> +
> +// Use of trap mode shouldn't affect the object size check.
> +//
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -fsanitize-trap=undefined -O1 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HAS-OSIZE
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined-trap -O1 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HAS-OSIZE
> +// RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined-trap -fsanitize-undefined-trap-on-error -O1 %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-HAS-OSIZE
> +
> +// CHECK-HAS-OSIZE: -fsanitize={{[^ ]*}}object-size
> +
> +// CHECK-NO-OSIZE: warning: the object size sanitizer has no effect at -O0, but is explicitly enabled: -fsanitize={{[^ ]*}}object-size
> +
> +// CHECK-NO-OSIZE-NO-WARNING-NOT: -fsanitize={{[^ ]*}}object-size
> diff --git a/test/Driver/fsanitize.c b/test/Driver/fsanitize.c
> index f14459df63..0752ef6df0 100644
> --- a/test/Driver/fsanitize.c
> +++ b/test/Driver/fsanitize.c
> @@ -3,18 +3,18 @@
>  // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -fsanitize-undefined-trap-on-error %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-TRAP
>  // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined-trap -fsanitize-undefined-trap-on-error %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-TRAP
>  // RUN: %clang -target x86_64-linux-gnu -fsanitize-undefined-trap-on-error -fsanitize=undefined-trap %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-TRAP
> -// CHECK-UNDEFINED-TRAP: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|object-size|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute|function),?){19}"}}
> -// CHECK-UNDEFINED-TRAP: "-fsanitize-trap=alignment,array-bounds,bool,enum,float-cast-overflow,float-divide-by-zero,function,integer-divide-by-zero,nonnull-attribute,null,object-size,pointer-overflow,return,returns-nonnull-attribute,shift-base,shift-exponent,signed-integer-overflow,unreachable,vla-bound"
> -// CHECK-UNDEFINED-TRAP2: "-fsanitize-trap=alignment,array-bounds,bool,enum,float-cast-overflow,float-divide-by-zero,function,integer-divide-by-zero,nonnull-attribute,null,object-size,pointer-overflow,return,returns-nonnull-attribute,shift-base,shift-exponent,unreachable,vla-bound"
> +// CHECK-UNDEFINED-TRAP: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute|function),?){18}"}}
> +// CHECK-UNDEFINED-TRAP: "-fsanitize-trap=alignment,array-bounds,bool,enum,float-cast-overflow,float-divide-by-zero,function,integer-divide-by-zero,nonnull-attribute,null,pointer-overflow,return,returns-nonnull-attribute,shift-base,shift-exponent,signed-integer-overflow,unreachable,vla-bound"
> +// CHECK-UNDEFINED-TRAP2: "-fsanitize-trap=alignment,array-bounds,bool,enum,float-cast-overflow,float-divide-by-zero,function,integer-divide-by-zero,nonnull-attribute,null,pointer-overflow,return,returns-nonnull-attribute,shift-base,shift-exponent,unreachable,vla-bound"
>  
>  // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED
> -// CHECK-UNDEFINED: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|vptr|object-size|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){20}"}}
> +// CHECK-UNDEFINED: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|vptr|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){19}"}}
>  
>  // RUN: %clang -target x86_64-apple-darwin10 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-DARWIN
> -// CHECK-UNDEFINED-DARWIN: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|object-size|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){18}"}}
> +// CHECK-UNDEFINED-DARWIN: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){17}"}}
>  
>  // RUN: %clang -target i386-unknown-openbsd -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-OPENBSD
> -// CHECK-UNDEFINED-OPENBSD: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|object-size|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){18}"}}
> +// CHECK-UNDEFINED-OPENBSD: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){17}"}}
>  
>  // RUN: %clang -target i386-pc-win32 -fsanitize=undefined %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-WIN --check-prefix=CHECK-UNDEFINED-WIN32
>  // RUN: %clang -target i386-pc-win32 -fsanitize=undefined -x c++ %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-WIN --check-prefix=CHECK-UNDEFINED-WIN32 --check-prefix=CHECK-UNDEFINED-WIN-CXX
> @@ -23,7 +23,7 @@
>  // CHECK-UNDEFINED-WIN32: "--dependent-lib={{[^"]*}}ubsan_standalone-i386.lib"
>  // CHECK-UNDEFINED-WIN64: "--dependent-lib={{[^"]*}}ubsan_standalone-x86_64.lib"
>  // CHECK-UNDEFINED-WIN-CXX: "--dependent-lib={{[^"]*}}ubsan_standalone_cxx{{[^"]*}}.lib"
> -// CHECK-UNDEFINED-WIN-SAME: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|object-size|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){18}"}}
> +// CHECK-UNDEFINED-WIN-SAME: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){17}"}}
>  
>  // RUN: %clang -target i386-pc-win32 -fsanitize-coverage=bb %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-COVERAGE-WIN32
>  // CHECK-COVERAGE-WIN32: "--dependent-lib={{[^"]*}}ubsan_standalone-i386.lib"
> @@ -43,7 +43,7 @@
>  // CHECK-FNO-SANITIZE-ALL: "-fsanitize=thread"
>  
>  // RUN: %clang -target x86_64-linux-gnu -fsanitize=thread,undefined -fno-sanitize=thread -fno-sanitize=float-cast-overflow,vptr,bool,enum %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-PARTIAL-UNDEFINED
> -// CHECK-PARTIAL-UNDEFINED: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|object-size|pointer-overflow|array-bounds|returns-nonnull-attribute|nonnull-attribute),?){16}"}}
> +// CHECK-PARTIAL-UNDEFINED: "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift-base|shift-exponent|unreachable|return|vla-bound|alignment|null|pointer-overflow|array-bounds|returns-nonnull-attribute|nonnull-attribute),?){15}"}}
>  
>  // RUN: %clang -target x86_64-linux-gnu -fsanitize=shift -fno-sanitize=shift-base %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-FSANITIZE-SHIFT-PARTIAL
>  // CHECK-FSANITIZE-SHIFT-PARTIAL: "-fsanitize=shift-exponent"
> @@ -217,11 +217,11 @@
>  // RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined -fno-sanitize-recover=undefined -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-RECOVER-UBSAN
>  // RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined -fno-sanitize-recover=all -fsanitize-recover=thread -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-RECOVER-UBSAN
>  // RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined -fsanitize-recover=all -fno-sanitize-recover=undefined -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-RECOVER-UBSAN
> -// CHECK-RECOVER-UBSAN: "-fsanitize-recover={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift-base|shift-exponent|vla-bound|alignment|null|vptr|object-size|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){18}"}}
> +// CHECK-RECOVER-UBSAN: "-fsanitize-recover={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift-base|shift-exponent|vla-bound|alignment|null|vptr|pointer-overflow|float-cast-overflow|array-bounds|enum|bool|returns-nonnull-attribute|nonnull-attribute),?){17}"}}
>  // CHECK-NO-RECOVER-UBSAN-NOT: sanitize-recover
>  
>  // RUN: %clang -target x86_64-linux-gnu %s -fsanitize=undefined -fno-sanitize-recover=all -fsanitize-recover=object-size,shift-base -### 2>&1 | FileCheck %s --check-prefix=CHECK-PARTIAL-RECOVER
> -// CHECK-PARTIAL-RECOVER: "-fsanitize-recover={{((object-size|shift-base),?){2}"}}
> +// CHECK-PARTIAL-RECOVER: "-fsanitize-recover={{((shift-base),?){1}"}}
>  
>  // RUN: %clang -target x86_64-linux-gnu %s -fsanitize=address -fsanitize-recover=all -### 2>&1 | FileCheck %s --check-prefix=CHECK-RECOVER-ASAN
>  // CHECK-RECOVER-ASAN: "-fsanitize-recover=address"
>
> vedant
>
>     On Jun 23, 2017, at 1:05 PM, Justin Bogner <mail at justinbogner.com> wrote:
>
>         +++ test/Driver/fsanitize-object-size.c
>         @@ -0,0 +1,25 @@
>         +// Check that the object size check is disabled at -O0.
>         +//
>         +// RUN: %clang -target x86_64-linux-gnu -fsanitize=object-size %s -##
>         # 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OSIZE
>         +// RUN: %clang -target x86_64-linux-gnu -fsanitize=object-size %s -O0
>         -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-OSIZE
>
>     This isn't great - the user explicitly opted in to object-size but they
>     don't get it. We should either diagnose this and behave as is, or just
>     enable the sanitizer even though it's not effective for forwards
>     compatibility.


More information about the cfe-commits mailing list