r193205 - Split -fsanitize=bounds to -fsanitize=array-bounds (for thefrontend-inserted

Nuno Lopes nunoplopes at sapo.pt
Wed Oct 23 14:58:53 PDT 2013


Let me disagree with this change:
 - I do not agree that '-fsanitize=local-bounds' is not a good compromise. 
It can detect overflows on globals, heap, and stack, including arbitrary 
pointer arithmetic, and therefore it subsumes '-fsanitize=array-bounds'. 
The advantage of the latter is that it produces nice diagnostics, while the 
former simply crashes the program when it detects an overflow. Therefore, I 
think there's value in keeping *both* instrumentations 
in -fsanitize=undefined.  BTW, '-fsanitize=local-bounds' properly inserts 
debug information in the trap, and therefore you can use a debugger to 
pinpoint the offending program statement.
 - '-fsanitize=local-bounds' does not have false positives by design. 
PR17653 is a bug which I'll fix soon.  This pass was designed to have zero 
false positives.
 - Also, I don't recall any previous discussion on this change..

Nuno

----- Original Message ----- 
From: "Richard Smith" <richard-llvm at metafoo.co.uk>
Sent: Tuesday, October 22, 2013 11:51 PM
Subject: r193205 - Split -fsanitize=bounds to -fsanitize=array-bounds (for 
thefrontend-inserted


> Author: rsmith
> Date: Tue Oct 22 17:51:04 2013
> New Revision: 193205
>
> URL: http://llvm.org/viewvc/llvm-project?rev=193205&view=rev
> Log:
> Split -fsanitize=bounds to -fsanitize=array-bounds (for the 
> frontend-inserted
> check using the ubsan runtime) and -fsanitize=local-bounds (for the 
> middle-end
> check which inserts traps).
>
> Remove -fsanitize=local-bounds from -fsanitize=undefined. It does not 
> produce
> useful diagnostics and has false positives (PR17635), and is not a good
> compromise position between UBSan's checks and ASan's checks.
>
> Map -fbounds-checking to -fsanitize=local-bounds to restore Clang's 
> historical
> behavior for that flag.
>
> Modified:
>    cfe/trunk/include/clang/Basic/Sanitizers.def
>    cfe/trunk/lib/CodeGen/BackendUtil.cpp
>    cfe/trunk/lib/CodeGen/CGExpr.cpp
>    cfe/trunk/lib/CodeGen/CGExprScalar.cpp
>    cfe/trunk/lib/Driver/SanitizerArgs.cpp
>    cfe/trunk/test/CodeGen/bounds-checking.c
>    cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
>    cfe/trunk/test/Driver/bounds-checking.c
>    cfe/trunk/test/Driver/fsanitize.c
>
> Modified: cfe/trunk/include/clang/Basic/Sanitizers.def
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Sanitizers.def?rev=193205&r1=193204&r2=193205&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/Sanitizers.def (original)
> +++ cfe/trunk/include/clang/Basic/Sanitizers.def Tue Oct 22 17:51:04 2013
> @@ -59,8 +59,8 @@ SANITIZER("leak", Leak)
>
> // UndefinedBehaviorSanitizer
> SANITIZER("alignment", Alignment)
> +SANITIZER("array-bounds", ArrayBounds)
> SANITIZER("bool", Bool)
> -SANITIZER("bounds", Bounds)
> SANITIZER("enum", Enum)
> SANITIZER("float-cast-overflow", FloatCastOverflow)
> SANITIZER("float-divide-by-zero", FloatDivideByZero)
> @@ -84,7 +84,7 @@ SANITIZER("dataflow", DataFlow)
> // -fsanitize=undefined includes all the sanitizers which have low 
> overhead, no
> // ABI or address space layout implications, and only catch undefined 
> behavior.
> SANITIZER_GROUP("undefined", Undefined,
> -                Alignment | Bool | Bounds | Enum | FloatCastOverflow |
> +                Alignment | Bool | ArrayBounds | Enum | FloatCastOverflow 
> |
>                 FloatDivideByZero | Function | IntegerDivideByZero | Null 
> |
>                 ObjectSize | Return | Shift | SignedIntegerOverflow |
>                 Unreachable | VLABound | Vptr)
> @@ -94,7 +94,7 @@ SANITIZER_GROUP("undefined", Undefined,
> // runtime support.  This group is generally used in conjunction with the
> // -fsanitize-undefined-trap-on-error flag.
> SANITIZER_GROUP("undefined-trap", UndefinedTrap,
> -                Alignment | Bool | Bounds | Enum | FloatCastOverflow |
> +                Alignment | Bool | ArrayBounds | Enum | FloatCastOverflow 
> |
>                 FloatDivideByZero | IntegerDivideByZero | Null | 
> ObjectSize |
>                 Return | Shift | SignedIntegerOverflow | Unreachable |
>                 VLABound)
> @@ -103,5 +103,9 @@ SANITIZER_GROUP("integer", Integer,
>                 SignedIntegerOverflow | UnsignedIntegerOverflow | Shift |
>                 IntegerDivideByZero)
>
> +// -fbounds-checking
> +SANITIZER("local-bounds", LocalBounds)
> +SANITIZER_GROUP("bounds", Bounds, ArrayBounds | LocalBounds)
> +
> #undef SANITIZER
> #undef SANITIZER_GROUP
>
> Modified: cfe/trunk/lib/CodeGen/BackendUtil.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/BackendUtil.cpp?rev=193205&r1=193204&r2=193205&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/BackendUtil.cpp (original)
> +++ cfe/trunk/lib/CodeGen/BackendUtil.cpp Tue Oct 22 17:51:04 2013
> @@ -245,7 +245,7 @@ void EmitAssemblyHelper::CreatePasses(Ta
>                            addObjCARCOptPass);
>   }
>
> -  if (LangOpts.Sanitize.Bounds) {
> +  if (LangOpts.Sanitize.LocalBounds) {
>     PMBuilder.addExtension(PassManagerBuilder::EP_ScalarOptimizerLate,
>                            addBoundsCheckingPass);
>     PMBuilder.addExtension(PassManagerBuilder::EP_EnabledOnOptLevel0,
>
> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=193205&r1=193204&r2=193205&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Tue Oct 22 17:51:04 2013
> @@ -641,7 +641,8 @@ static llvm::Value *getArrayIndexingBoun
> void CodeGenFunction::EmitBoundsCheck(const Expr *E, const Expr *Base,
>                                       llvm::Value *Index, QualType 
> IndexType,
>                                       bool Accessed) {
> -  assert(SanOpts->Bounds && "should not be called unless adding bounds 
> checks");
> +  assert(SanOpts->ArrayBounds &&
> +         "should not be called unless adding bounds checks");
>
>   QualType IndexedType;
>   llvm::Value *Bound = getArrayIndexingBound(*this, Base, IndexedType);
> @@ -742,7 +743,7 @@ LValue CodeGenFunction::EmitUnsupportedL
>
> LValue CodeGenFunction::EmitCheckedLValue(const Expr *E, TypeCheckKind 
> TCK) {
>   LValue LV;
> -  if (SanOpts->Bounds && isa<ArraySubscriptExpr>(E))
> +  if (SanOpts->ArrayBounds && isa<ArraySubscriptExpr>(E))
>     LV = EmitArraySubscriptExpr(cast<ArraySubscriptExpr>(E), 
> /*Accessed*/true);
>   else
>     LV = EmitLValue(E);
> @@ -2233,7 +2234,7 @@ LValue CodeGenFunction::EmitArraySubscri
>   QualType IdxTy  = E->getIdx()->getType();
>   bool IdxSigned = IdxTy->isSignedIntegerOrEnumerationType();
>
> -  if (SanOpts->Bounds)
> +  if (SanOpts->ArrayBounds)
>     EmitBoundsCheck(E, E->getBase(), Idx, IdxTy, Accessed);
>
>   // If the base is a vector type, then we are forming a vector element 
> lvalue
>
> Modified: cfe/trunk/lib/CodeGen/CGExprScalar.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprScalar.cpp?rev=193205&r1=193204&r2=193205&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGExprScalar.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExprScalar.cpp Tue Oct 22 17:51:04 2013
> @@ -1073,7 +1073,7 @@ Value *ScalarExprEmitter::VisitArraySubs
>   Value *Idx  = Visit(E->getIdx());
>   QualType IdxTy = E->getIdx()->getType();
>
> -  if (CGF.SanOpts->Bounds)
> +  if (CGF.SanOpts->ArrayBounds)
>     CGF.EmitBoundsCheck(E, E->getBase(), Idx, IdxTy, /*Accessed*/true);
>
>   bool IdxSigned = IdxTy->isSignedIntegerOrEnumerationType();
> @@ -2314,7 +2314,7 @@ static Value *emitPointerArithmetic(Code
>   if (isSubtraction)
>     index = CGF.Builder.CreateNeg(index, "idx.neg");
>
> -  if (CGF.SanOpts->Bounds)
> +  if (CGF.SanOpts->ArrayBounds)
>     CGF.EmitBoundsCheck(op.E, pointerOperand, index, 
> indexOperand->getType(),
>                         /*Accessed*/ false);
>
>
> Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=193205&r1=193204&r2=193205&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original)
> +++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Tue Oct 22 17:51:04 2013
> @@ -256,8 +256,8 @@ bool SanitizerArgs::parse(const Driver &
>       "-fsanitize=undefined-trap -fsanitize-undefined-trap-on-error";
>   } else if (A->getOption().matches(options::OPT_fbounds_checking) ||
>              A->getOption().matches(options::OPT_fbounds_checking_EQ)) {
> -    Add = Bounds;
> -    DeprecatedReplacement = "-fsanitize=bounds";
> +    Add = LocalBounds;
> +    DeprecatedReplacement = "-fsanitize=local-bounds";
>   } else if (A->getOption().matches(options::OPT_fsanitize_EQ)) {
>     Add = parse(D, A, DiagnoseErrors);
>   } else if (A->getOption().matches(options::OPT_fno_sanitize_EQ)) {
>
> Modified: cfe/trunk/test/CodeGen/bounds-checking.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/bounds-checking.c?rev=193205&r1=193204&r2=193205&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGen/bounds-checking.c (original)
> +++ cfe/trunk/test/CodeGen/bounds-checking.c Tue Oct 22 17:51:04 2013
> @@ -1,26 +1,29 @@
> -// RUN: %clang_cc1 -fsanitize=bounds -emit-llvm -triple 
> x86_64-apple-darwin10 < %s | FileCheck %s
> +// RUN: %clang_cc1 -fsanitize=local-bounds -emit-llvm -triple 
> x86_64-apple-darwin10 %s -o - | FileCheck %s
> +// RUN: 
> %clang_cc1 -fsanitize=array-bounds -O -fsanitize-undefined-trap-on-error -emit-llvm 
>  -triple x86_64-apple-darwin10 -DNO_DYNAMIC %s -o - | FileCheck %s
>
> -// CHECK: @f
> +// CHECK-LABEL: @f
> double f(int b, int i) {
>   double a[b];
> -  // CHECK: trap
> +  // CHECK: call {{.*}} @llvm.trap
>   return a[i];
> }
>
> -// CHECK: @f2
> +// CHECK-LABEL: @f2
> void f2() {
>   // everything is constant; no trap possible
> -  // CHECK-NOT: trap
> +  // CHECK-NOT: call {{.*}} @llvm.trap
>   int a[2];
>   a[1] = 42;
> -
> +
> +#ifndef NO_DYNAMIC
>   short *b = malloc(64);
>   b[5] = *a + a[1] + 2;
> +#endif
> }
>
> -// CHECK: @f3
> +// CHECK-LABEL: @f3
> void f3() {
>   int a[1];
> -  // CHECK: trap
> +  // CHECK: call {{.*}} @llvm.trap
>   a[2] = 1;
> }
>
> Modified: cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp?rev=193205&r1=193204&r2=193205&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp Tue Oct 22 17:51:04 
> 2013
> @@ -1,4 +1,4 @@
> -// RUN: 
> %clang_cc1 -std=c++11 -fsanitize=signed-integer-overflow,integer-divide-by-zero,float-divide-by-zero,shift,unreachable,return,vla-bound,alignment,null,vptr,object-size,float-cast-overflow,bool,enum,bounds,function 
>  -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
> +// RUN: 
> %clang_cc1 -std=c++11 -fsanitize=signed-integer-overflow,integer-divide-by-zero,float-divide-by-zero,shift,unreachable,return,vla-bound,alignment,null,vptr,object-size,float-cast-overflow,bool,enum,array-bounds,function 
>  -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
>
> struct S {
>   double d;
>
> Modified: cfe/trunk/test/Driver/bounds-checking.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/bounds-checking.c?rev=193205&r1=193204&r2=193205&view=diff
> ==============================================================================
> --- cfe/trunk/test/Driver/bounds-checking.c (original)
> +++ cfe/trunk/test/Driver/bounds-checking.c Tue Oct 22 17:51:04 2013
> @@ -1,11 +1,11 @@
> // RUN: %clang -fsanitize=bounds -### -fsyntax-only %s 2> %t
> // RUN: FileCheck -check-prefix=CHECK < %t %s
> -// CHECK: "-fsanitize=bounds"
> +// CHECK: "-fsanitize=array-bounds,local-bounds"
>
> // RUN: %clang -fbounds-checking -### -fsyntax-only %s 2> %t
> // RUN: FileCheck -check-prefix=CHECK-OLD < %t %s
> -// CHECK-OLD: "-fsanitize=bounds"
> +// CHECK-OLD: "-fsanitize=local-bounds"
>
> // RUN: %clang -fbounds-checking=3 -### -fsyntax-only %s 2> %t
> // RUN: FileCheck -check-prefix=CHECK-OLD2 < %t %s
> -// CHECK-OLD2: "-fsanitize=bounds"
> +// CHECK-OLD2: "-fsanitize=local-bounds"
>
> Modified: cfe/trunk/test/Driver/fsanitize.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/fsanitize.c?rev=193205&r1=193204&r2=193205&view=diff
> ==============================================================================
> --- cfe/trunk/test/Driver/fsanitize.c (original)
> +++ cfe/trunk/test/Driver/fsanitize.c Tue Oct 22 17:51:04 2013
> @@ -1,17 +1,17 @@
> // RUN: %clang -target x86_64-linux-gnu -fcatch-undefined-behavior %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|unreachable|return|vla-bound|alignment|null|object-size|float-cast-overflow|bounds|enum|bool),?){14}"}}
> +// CHECK-UNDEFINED-TRAP: 
> "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|shift|unreachable|return|vla-bound|alignment|null|object-size|float-cast-overflow|array-bounds|enum|bool),?){14}"}}
> // CHECK-UNDEFINED-TRAP: "-fsanitize-undefined-trap-on-error"
>
> // 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|unreachable|return|vla-bound|alignment|null|vptr|object-size|float-cast-overflow|bounds|enum|bool),?){16}"}}
> +// CHECK-UNDEFINED: 
> "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift|unreachable|return|vla-bound|alignment|null|vptr|object-size|float-cast-overflow|array-bounds|enum|bool),?){16}"}}
>
> // RUN: %clang -target x86_64-linux-gnu -fsanitize=integer %s -### 2>&1 | 
> FileCheck %s --check-prefix=CHECK-INTEGER
> // CHECK-INTEGER: 
> "-fsanitize={{((signed-integer-overflow|unsigned-integer-overflow|integer-divide-by-zero|shift),?){4}"}}
>
> // RUN: %clang -target 
> x86_64-linux-gnu -fsanitize=thread,undefined -fno-thread-sanitizer -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|unreachable|return|vla-bound|alignment|null|object-size|bounds),?){12}"}}
> +// CHECK-PARTIAL-UNDEFINED: 
> "-fsanitize={{((signed-integer-overflow|integer-divide-by-zero|float-divide-by-zero|function|shift|unreachable|return|vla-bound|alignment|null|object-size|array-bounds),?){12}"}}
>
> // RUN: %clang -target x86_64-linux-gnu -fsanitize=address-full %s -### 
> 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-FULL
> // CHECK-ASAN-FULL: 
> "-fsanitize={{((address|init-order|use-after-return|use-after-scope),?){4}"}}
> @@ -101,7 +101,7 @@
> // CHECK-DEPRECATED: argument '-fno-thread-sanitizer' is deprecated, use 
> '-fno-sanitize=thread' instead
> // CHECK-DEPRECATED: argument '-faddress-sanitizer' is deprecated, use 
> '-fsanitize=address' instead
> // CHECK-DEPRECATED: argument '-fno-address-sanitizer' is deprecated, use 
> '-fno-sanitize=address' instead
> -// CHECK-DEPRECATED: argument '-fbounds-checking' is deprecated, use 
> '-fsanitize=bounds' instead
> +// CHECK-DEPRECATED: argument '-fbounds-checking' is deprecated, use 
> '-fsanitize=local-bounds' instead
>
> // RUN: %clang -target x86_64-linux-gnu -fsanitize=thread %s -### 2>&1 | 
> FileCheck %s --check-prefix=CHECK-TSAN-NO-PIE
> // CHECK-TSAN-NO-PIE: "-mrelocation-model" "pic" "-pic-level" "2" 
> "-pie-level" "2"
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits 




More information about the cfe-commits mailing list