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" <[email protected]>
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
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to