diff options
author | Vedant Kumar <vsk@apple.com> | 2017-08-02 18:10:31 +0000 |
---|---|---|
committer | Vedant Kumar <vsk@apple.com> | 2017-08-02 18:10:31 +0000 |
commit | 63a4e484860278f8a187fea35563d9a0ceed2db1 (patch) | |
tree | 405da97822ee96acf444033a6b8cd654b8e7978d | |
parent | d9009e558ab4d45eefae9d11d24951a61c3bb54b (diff) |
[ubsan] Have -fsanitize=vptr emit a null check if -fsanitize=null isn't available
In r309007, I made -fsanitize=null a hard prerequisite for -fsanitize=vptr. I
did not see the need for the two checks to have separate null checking logic
for the same pointer. I expected the two checks to either always be enabled
together, or to be mutually compatible.
In the mailing list discussion re: r309007 it became clear that that isn't the
case. If a codebase is -fsanitize=vptr clean but not -fsanitize=null clean,
it's useful to have -fsanitize=vptr emit its own null check. That's what this
patch does: with it, -fsanitize=vptr can be used without -fsanitize=null.
Differential Revision: https://reviews.llvm.org/D36112
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@309846 91177308-0d34-0410-b5e6-96231b3b80d8
-rw-r--r-- | docs/ReleaseNotes.rst | 4 | ||||
-rw-r--r-- | docs/UndefinedBehaviorSanitizer.rst | 8 | ||||
-rw-r--r-- | include/clang/Basic/DiagnosticDriverKinds.td | 3 | ||||
-rw-r--r-- | lib/CodeGen/CGExpr.cpp | 8 | ||||
-rw-r--r-- | lib/Driver/SanitizerArgs.cpp | 7 | ||||
-rw-r--r-- | test/CodeGenCXX/catch-undef-behavior.cpp | 4 | ||||
-rw-r--r-- | test/CodeGenCXX/ubsan-type-checks.cpp | 15 | ||||
-rw-r--r-- | test/Driver/fsanitize.c | 8 | ||||
-rw-r--r-- | test/Driver/rtti-options.cpp | 12 |
9 files changed, 29 insertions, 40 deletions
diff --git a/docs/ReleaseNotes.rst b/docs/ReleaseNotes.rst index 49ed87d9a9..08f9b8606d 100644 --- a/docs/ReleaseNotes.rst +++ b/docs/ReleaseNotes.rst @@ -198,9 +198,7 @@ Static Analyzer Undefined Behavior Sanitizer (UBSan) ------------------------------------ -The C++ dynamic type check now requires run-time null checking (i.e, -`-fsanitize=vptr` cannot be used without `-fsanitize=null`). This change does -not impact users who rely on UBSan check groups (e.g `-fsanitize=undefined`). +... Core Analysis Improvements ========================== diff --git a/docs/UndefinedBehaviorSanitizer.rst b/docs/UndefinedBehaviorSanitizer.rst index 6274054a46..aa52f2d155 100644 --- a/docs/UndefinedBehaviorSanitizer.rst +++ b/docs/UndefinedBehaviorSanitizer.rst @@ -132,10 +132,10 @@ Available checks are: - ``-fsanitize=vla-bound``: A variable-length array whose bound does not evaluate to a positive value. - ``-fsanitize=vptr``: Use of an object whose vptr indicates that it is of - the wrong dynamic type, or that its lifetime has not begun or has ended. - Incompatible with ``-fno-rtti`` and ``-fno-sanitize=null``. Link must be - performed by ``clang++``, not ``clang``, to make sure C++-specific parts of - the runtime library and C++ standard libraries are present. + the wrong dynamic type, or that its lifetime has not begun or has ended. + Incompatible with ``-fno-rtti``. Link must be performed by ``clang++``, not + ``clang``, to make sure C++-specific parts of the runtime library and C++ + standard libraries are present. You can also use the following check groups: - ``-fsanitize=undefined``: All of the checks listed above other than diff --git a/include/clang/Basic/DiagnosticDriverKinds.td b/include/clang/Basic/DiagnosticDriverKinds.td index 2a057f8050..d461e5e4ee 100644 --- a/include/clang/Basic/DiagnosticDriverKinds.td +++ b/include/clang/Basic/DiagnosticDriverKinds.td @@ -232,9 +232,6 @@ 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<AutoDisableVptrSanitizer>; -def warn_drv_disabling_vptr_no_null_check : Warning< - "implicitly disabling vptr sanitizer because null checking wasn't enabled">, - InGroup<AutoDisableVptrSanitizer>; def warn_drv_object_size_disabled_O0 : Warning< "the object size sanitizer has no effect at -O0, but is explicitly enabled: %0">, InGroup<InvalidCommandLineArgument>; diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp index e9ad05cf66..ec48b2cc86 100644 --- a/lib/CodeGen/CGExpr.cpp +++ b/lib/CodeGen/CGExpr.cpp @@ -694,17 +694,17 @@ void CodeGenFunction::EmitTypeCheck(TypeCheckKind TCK, SourceLocation Loc, // -- the [pointer or glvalue] is used to access a non-static data member // or call a non-static member function CXXRecordDecl *RD = Ty->getAsCXXRecordDecl(); - bool HasNullCheck = IsGuaranteedNonNull || IsNonNull; if (SanOpts.has(SanitizerKind::Vptr) && - !SkippedChecks.has(SanitizerKind::Vptr) && HasNullCheck && + !SkippedChecks.has(SanitizerKind::Vptr) && (TCK == TCK_MemberAccess || TCK == TCK_MemberCall || TCK == TCK_DowncastPointer || TCK == TCK_DowncastReference || TCK == TCK_UpcastToVirtualBase) && RD && RD->hasDefinition() && RD->isDynamicClass()) { // Ensure that the pointer is non-null before loading it. If there is no - // compile-time guarantee, reuse the run-time null check. + // compile-time guarantee, reuse the run-time null check or emit a new one. if (!IsGuaranteedNonNull) { - assert(IsNonNull && "Missing run-time null check"); + if (!IsNonNull) + IsNonNull = Builder.CreateIsNotNull(Ptr); if (!Done) Done = createBasicBlock("vptr.null"); llvm::BasicBlock *VptrNotNull = createBasicBlock("vptr.not.null"); diff --git a/lib/Driver/SanitizerArgs.cpp b/lib/Driver/SanitizerArgs.cpp index f4fe06af25..2a3ae51734 100644 --- a/lib/Driver/SanitizerArgs.cpp +++ b/lib/Driver/SanitizerArgs.cpp @@ -307,13 +307,6 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, Kinds &= ~Vptr; } - // Disable -fsanitize=vptr if -fsanitize=null is not enabled (the vptr - // instrumentation is broken without run-time null checks). - if ((Kinds & Vptr) && !(Kinds & Null)) { - Kinds &= ~Vptr; - D.Diag(diag::warn_drv_disabling_vptr_no_null_check); - } - // Check that LTO is enabled if we need it. if ((Kinds & NeedsLTO) && !D.isUsingLTO()) { D.Diag(diag::err_drv_argument_only_allowed_with) diff --git a/test/CodeGenCXX/catch-undef-behavior.cpp b/test/CodeGenCXX/catch-undef-behavior.cpp index d95a5370fb..179c334122 100644 --- a/test/CodeGenCXX/catch-undef-behavior.cpp +++ b/test/CodeGenCXX/catch-undef-behavior.cpp @@ -1,6 +1,6 @@ // RUN: %clang_cc1 -std=c++11 -fsanitize=signed-integer-overflow,integer-divide-by-zero,float-divide-by-zero,shift-base,shift-exponent,unreachable,return,vla-bound,alignment,null,vptr,object-size,float-cast-overflow,bool,enum,array-bounds,function -fsanitize-recover=signed-integer-overflow,integer-divide-by-zero,float-divide-by-zero,shift-base,shift-exponent,vla-bound,alignment,null,vptr,object-size,float-cast-overflow,bool,enum,array-bounds,function -emit-llvm %s -o - -triple x86_64-linux-gnu | opt -instnamer -S | FileCheck %s -// RUN: %clang_cc1 -std=c++11 -fsanitize=null,vptr,address -fsanitize-recover=null,vptr,address -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK-ASAN -// RUN: %clang_cc1 -std=c++11 -fsanitize=null,vptr -fsanitize-recover=null,vptr -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=DOWNCAST-NULL +// RUN: %clang_cc1 -std=c++11 -fsanitize=vptr,address -fsanitize-recover=vptr,address -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=CHECK-ASAN +// RUN: %clang_cc1 -std=c++11 -fsanitize=vptr -fsanitize-recover=vptr -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s --check-prefix=DOWNCAST-NULL // RUN: %clang_cc1 -std=c++11 -fsanitize=function -emit-llvm %s -o - -triple x86_64-linux-gnux32 | FileCheck %s --check-prefix=CHECK-X32 // RUN: %clang_cc1 -std=c++11 -fsanitize=function -emit-llvm %s -o - -triple i386-linux-gnu | FileCheck %s --check-prefix=CHECK-X86 diff --git a/test/CodeGenCXX/ubsan-type-checks.cpp b/test/CodeGenCXX/ubsan-type-checks.cpp index cc2d4d6e79..e53ab2466e 100644 --- a/test/CodeGenCXX/ubsan-type-checks.cpp +++ b/test/CodeGenCXX/ubsan-type-checks.cpp @@ -44,17 +44,22 @@ struct Dog : Animal { // VPTR-LABEL: define void @_Z12invalid_castP3Cat void invalid_cast(Cat *cat = nullptr) { - // First, null check the pointer: + // If -fsanitize=null is available, we'll reuse its check: // // VPTR: [[ICMP:%.*]] = icmp ne %struct.Dog* {{.*}}, null // VPTR-NEXT: br i1 [[ICMP]] // VPTR: call void @__ubsan_handle_type_mismatch - // - // Once we're done emitting the null check, reuse the check to see if we can - // proceed to the vptr check: - // + // VPTR-NOT: icmp ne %struct.Dog* {{.*}}, null // VPTR: br i1 [[ICMP]] // VPTR: call void @__ubsan_handle_dynamic_type_cache_miss + // + // Fall back to the vptr sanitizer's null check when -fsanitize=null isn't + // available. + // + // VPTR_NO_NULL-NOT: call void @__ubsan_handle_type_mismatch + // VPTR_NO_NULL: [[ICMP:%.*]] = icmp ne %struct.Dog* {{.*}}, null + // VPTR_NO_NULL-NEXT: br i1 [[ICMP]] + // VPTR_NO_NULL: call void @__ubsan_handle_dynamic_type_cache_miss auto *badDog = reinterpret_cast<Dog *>(cat); badDog->speak(); } diff --git a/test/Driver/fsanitize.c b/test/Driver/fsanitize.c index 3218079701..0aa289b50b 100644 --- a/test/Driver/fsanitize.c +++ b/test/Driver/fsanitize.c @@ -58,10 +58,6 @@ // RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -fno-rtti %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-UNDEFINED-NO-RTTI // CHECK-UNDEFINED-NO-RTTI-NOT: vptr -// RUN: %clang -target x86_64-linux-gnu -fsanitize=undefined -fno-sanitize=null %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-NO-NULL -// RUN: %clang -target x86_64-linux-gnu -fsanitize=vptr %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-NO-NULL -// CHECK-VPTR-NO-NULL: warning: implicitly disabling vptr sanitizer because null checking wasn't enabled - // RUN: %clang -target x86_64-linux-gnu -fsanitize=address,thread -fno-rtti %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-SANA-SANT // CHECK-SANA-SANT: '-fsanitize=address' not allowed with '-fsanitize=thread' @@ -366,8 +362,8 @@ // RUN: %clang -target x86_64-apple-darwin10 -mmacosx-version-min=10.8 -fsanitize=vptr %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-DARWIN-OLD // CHECK-VPTR-DARWIN-OLD: unsupported option '-fsanitize=vptr' for target 'x86_64-apple-darwin10' -// RUN: %clang -target x86_64-apple-darwin10 -mmacosx-version-min=10.9 -fsanitize=alignment,null,vptr %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-DARWIN-NEW -// CHECK-VPTR-DARWIN-NEW: -fsanitize=alignment,null,vptr +// RUN: %clang -target x86_64-apple-darwin10 -mmacosx-version-min=10.9 -fsanitize=alignment,vptr %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-VPTR-DARWIN-NEW +// CHECK-VPTR-DARWIN-NEW: -fsanitize=alignment,vptr // RUN: %clang -target armv7-apple-ios7 -miphoneos-version-min=7.0 -fsanitize=address %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-ASAN-IOS // CHECK-ASAN-IOS: -fsanitize=address diff --git a/test/Driver/rtti-options.cpp b/test/Driver/rtti-options.cpp index e7b0ff48a9..bb2958d845 100644 --- a/test/Driver/rtti-options.cpp +++ b/test/Driver/rtti-options.cpp @@ -16,14 +16,14 @@ // Make sure we only error/warn once, when trying to enable vptr and // undefined and have -fno-rtti // RUN: %clang -### -c -target x86_64-unknown-linux -fsanitize=undefined -fsanitize=vptr -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-ERROR -check-prefix=CHECK-OK %s -// RUN: %clang -### -c -target x86_64-unknown-linux -fsanitize=null,vptr %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s -// RUN: %clang -### -c -target x86_64-unknown-linux -fsanitize=null,vptr -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s -// RUN: %clang -### -c -target x86_64-unknown-linux -fsanitize=null,vptr -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-ERROR %s +// RUN: %clang -### -c -target x86_64-unknown-linux -fsanitize=vptr %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s +// RUN: %clang -### -c -target x86_64-unknown-linux -fsanitize=vptr -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s +// RUN: %clang -### -c -target x86_64-unknown-linux -fsanitize=vptr -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-ERROR %s // RUN: %clang -### -c -target x86_64-unknown-linux -fsanitize=undefined %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s // RUN: %clang -### -c -target x86_64-unknown-linux -fsanitize=undefined -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s -// RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=null,vptr %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-WARN %s -// RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=null,vptr -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s -// RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=null,vptr -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-ERROR %s +// RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=vptr %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-WARN %s +// RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=vptr -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s +// RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=vptr -fno-rtti %s 2>&1 | FileCheck -check-prefix=CHECK-SAN-ERROR %s // RUN: %clang -### -c -target x86_64-scei-ps4 -fsanitize=undefined -frtti %s 2>&1 | FileCheck -check-prefix=CHECK-OK %s // Exceptions + no/default rtti |