[libcxx] r280335 - Fix libc++ configuration with -fsanitize-coverage
Author: krasin Date: Wed Aug 31 20:38:32 2016 New Revision: 280335 URL: http://llvm.org/viewvc/llvm-project?rev=280335&view=rev Log: Fix libc++ configuration with -fsanitize-coverage Summary: a recent change (r280015) in libc++ configuration broke LibFuzzer bot: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fuzzer/builds/12245 It's not restricted just to that bot; any code that uses the sanitize coverage and configures libc++ hits it. This CL fixes the issue. Reviewers: compnerd Subscribers: aizatsky Differential Revision: https://reviews.llvm.org/D24116 Modified: libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake libcxx/trunk/cmake/config-ix.cmake Modified: libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake?rev=280335&r1=280334&r2=280335&view=diff == --- libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake (original) +++ libcxx/trunk/cmake/Modules/CheckLibcxxAtomic.cmake Wed Aug 31 20:38:32 2016 @@ -16,6 +16,9 @@ function(check_cxx_atomics varname) if (CMAKE_C_FLAGS MATCHES -fsanitize OR CMAKE_CXX_FLAGS MATCHES -fsanitize) set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize=all") endif() + if (CMAKE_C_FLAGS MATCHES -fsanitize-coverage OR CMAKE_CXX_FLAGS MATCHES -fsanitize-coverage) +set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize-coverage=edge,trace-cmp,indirect-calls,8bit-counters") + endif() check_cxx_source_compiles(" #include #include Modified: libcxx/trunk/cmake/config-ix.cmake URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/cmake/config-ix.cmake?rev=280335&r1=280334&r2=280335&view=diff == --- libcxx/trunk/cmake/config-ix.cmake (original) +++ libcxx/trunk/cmake/config-ix.cmake Wed Aug 31 20:38:32 2016 @@ -27,6 +27,9 @@ if (LIBCXX_SUPPORTS_NODEFAULTLIBS_FLAG) if (CMAKE_C_FLAGS MATCHES -fsanitize OR CMAKE_CXX_FLAGS MATCHES -fsanitize) set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize=all") endif () + if (CMAKE_C_FLAGS MATCHES -fsanitize-coverage OR CMAKE_CXX_FLAGS MATCHES -fsanitize-coverage) +set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -fno-sanitize-coverage=edge,trace-cmp,indirect-calls,8bit-counters") + endif () endif () include(CheckLibcxxAtomic) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[libunwind] r299666 - Fix unused typedef. Follow up to r299575.
Author: krasin Date: Thu Apr 6 12:35:35 2017 New Revision: 299666 URL: http://llvm.org/viewvc/llvm-project?rev=299666&view=rev Log: Fix unused typedef. Follow up to r299575. Modified: libunwind/trunk/src/AddressSpace.hpp Modified: libunwind/trunk/src/AddressSpace.hpp URL: http://llvm.org/viewvc/llvm-project/libunwind/trunk/src/AddressSpace.hpp?rev=299666&r1=299665&r2=299666&view=diff == --- libunwind/trunk/src/AddressSpace.hpp (original) +++ libunwind/trunk/src/AddressSpace.hpp Thu Apr 6 12:35:35 2017 @@ -383,7 +383,7 @@ inline bool LocalAddressSpace::findUnwin #if !defined(Elf_Phdr) typedef ElfW(Phdr) Elf_Phdr; #endif -#if !defined(Elf_Addr) +#if !defined(Elf_Addr) && defined(__ANDROID__) typedef ElfW(Addr) Elf_Addr; #endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r299671 - Fix unused lambda capture. Follow up to r299653.
Author: krasin Date: Thu Apr 6 12:42:05 2017 New Revision: 299671 URL: http://llvm.org/viewvc/llvm-project?rev=299671&view=rev Log: Fix unused lambda capture. Follow up to r299653. Modified: cfe/trunk/lib/Analysis/CloneDetection.cpp Modified: cfe/trunk/lib/Analysis/CloneDetection.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CloneDetection.cpp?rev=299671&r1=299670&r2=299671&view=diff == --- cfe/trunk/lib/Analysis/CloneDetection.cpp (original) +++ cfe/trunk/lib/Analysis/CloneDetection.cpp Thu Apr 6 12:42:05 2017 @@ -492,7 +492,7 @@ void RecursiveCloneTypeIIConstraint::con // Sort hash_codes in StmtsByHash. std::stable_sort(StmtsByHash.begin(), StmtsByHash.end(), - [this](std::pair LHS, + [](std::pair LHS, std::pair RHS) { return LHS.first < RHS.first; }); ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r296374 - [ubsan] Factor out logic to emit a range check. NFC.
Hi Vedant, not on top of my head. Dmitriy, can you please take a look? krasin On Mon, Feb 27, 2017 at 12:43 PM, Vedant Kumar wrote: > Hi Ivan, > > I saw a bot failure on your job after this commit: > > http://lab.llvm.org:8011/builders/sanitizer-x86_64- > linux-autoconf/builds/5467/steps/tsan%20analyze/logs/stdio > http://lab.llvm.org:8011/builders/sanitizer-x86_64- > linux-autoconf/builds/5467/steps/build%20release%20tsan% > 20with%20clang/logs/stdio > > However, I cannot reproduce it locally with a stage2 TSAN build. > > After staring at my diff I couldn't find anything that would explain the > failure. No other bots seem upset. > > Do you have any idea about what's going on? Let me know if you want me to > revert... > > vedant > > > On Feb 27, 2017, at 11:46 AM, Vedant Kumar via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > > > Author: vedantk > > Date: Mon Feb 27 13:46:19 2017 > > New Revision: 296374 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=296374&view=rev > > Log: > > [ubsan] Factor out logic to emit a range check. NFC. > > > > This is a readability improvement, but it will also help prep an > > upcoming patch to detect UB loads from bitfields. > > > > Modified: > >cfe/trunk/lib/CodeGen/CGExpr.cpp > >cfe/trunk/lib/CodeGen/CodeGenFunction.h > > > > Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp > > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ > CGExpr.cpp?rev=296374&r1=296373&r2=296374&view=diff > > > == > > --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original) > > +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Mon Feb 27 13:46:19 2017 > > @@ -1301,6 +1301,46 @@ llvm::MDNode *CodeGenFunction::getRangeF > > return MDHelper.createRange(Min, End); > > } > > > > +bool CodeGenFunction::EmitScalarRangeCheck(llvm::Value *Value, > QualType Ty, > > + SourceLocation Loc) { > > + bool HasBoolCheck = SanOpts.has(SanitizerKind::Bool); > > + bool HasEnumCheck = SanOpts.has(SanitizerKind::Enum); > > + if (!HasBoolCheck && !HasEnumCheck) > > +return false; > > + > > + bool IsBool = hasBooleanRepresentation(Ty) || > > +NSAPI(CGM.getContext()).isObjCBOOLType(Ty); > > + bool NeedsBoolCheck = HasBoolCheck && IsBool; > > + bool NeedsEnumCheck = HasEnumCheck && Ty->getAs(); > > + if (!NeedsBoolCheck && !NeedsEnumCheck) > > +return false; > > + > > + llvm::APInt Min, End; > > + if (!getRangeForType(*this, Ty, Min, End, /*StrictEnums=*/true, > IsBool)) > > +return true; > > + > > + SanitizerScope SanScope(this); > > + llvm::Value *Check; > > + --End; > > + if (!Min) { > > +Check = Builder.CreateICmpULE( > > +Value, llvm::ConstantInt::get(getLLVMContext(), End)); > > + } else { > > +llvm::Value *Upper = Builder.CreateICmpSLE( > > +Value, llvm::ConstantInt::get(getLLVMContext(), End)); > > +llvm::Value *Lower = Builder.CreateICmpSGE( > > +Value, llvm::ConstantInt::get(getLLVMContext(), Min)); > > +Check = Builder.CreateAnd(Upper, Lower); > > + } > > + llvm::Constant *StaticArgs[] = {EmitCheckSourceLocation(Loc), > > + EmitCheckTypeDescriptor(Ty)}; > > + SanitizerMask Kind = > > + NeedsEnumCheck ? SanitizerKind::Enum : SanitizerKind::Bool; > > + EmitCheck(std::make_pair(Check, Kind), SanitizerHandler:: > LoadInvalidValue, > > +StaticArgs, EmitCheckValue(Value)); > > + return true; > > +} > > + > > llvm::Value *CodeGenFunction::EmitLoadOfScalar(Address Addr, bool > Volatile, > >QualType Ty, > >SourceLocation Loc, > > @@ -1353,35 +1393,9 @@ llvm::Value *CodeGenFunction::EmitLoadOf > > false /*ConvertTypeToTag*/); > > } > > > > - bool IsBool = hasBooleanRepresentation(Ty) || > > -NSAPI(CGM.getContext()).isObjCBOOLType(Ty); > > - bool NeedsBoolCheck = SanOpts.has(SanitizerKind::Bool) && IsBool; > > - bool NeedsEnumCheck = > > - SanOpts.has(SanitizerKind::Enum) && Ty->getAs(); > > - if (NeedsBoolCheck || NeedsEnumCheck) { > > -SanitizerScope SanScope(this); > > -llvm::APInt Min, End; > > -if (getRangeForType(*this, Ty, Min, End, /*StrictEnums=*/true, > IsBool)) { > > - --End; > > - llvm::Value *Check; > > - if (!Min) > > -Check = Builder.CreateICmpULE( > > - Load, llvm::ConstantInt::get(getLLVMContext(), End)); > > - else { > > -llvm::Value *Upper = Builder.CreateICmpSLE( > > - Load, llvm::ConstantInt::get(getLLVMContext(), End)); > > -llvm::Value *Lower = Builder.CreateICmpSGE( > > - Load, llvm::ConstantInt::get(getLLVMContext(), Min)); > > -Check = Builder.CreateAnd(Upper, Lower); > > - } > > - llvm::Constant *StaticArgs[] = { > > -EmitCheckSourceLocation(Loc), > > -Em
[PATCH] D26559: Insert a type check before reading vtable.
krasin updated this revision to Diff 77868. krasin added a comment. Add a regression test. https://reviews.llvm.org/D26559 Files: lib/CodeGen/CGExprCXX.cpp test/CodeGenCXX/ubsan-null.cpp Index: test/CodeGenCXX/ubsan-null.cpp === --- /dev/null +++ test/CodeGenCXX/ubsan-null.cpp @@ -0,0 +1,17 @@ +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s +struct T { + virtual int v() { return 1; } +}; + +struct U : T { + virtual int v() { return 2; } +}; + +// CHECK: define i32 @_Z5get_vP1T +int get_v(T* t) { + // CHECK: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null + // CHECK-NEXT: br i1 [[UBSAN_CMP_RES]], label %cont, label %handler.type_mismatch + // CHECK: call void @__ubsan_handle_type_mismatch_abort + // CHECK: load i32 (%struct.T*)**, i32 (%struct.T*)*** + return t->v(); +} Index: lib/CodeGen/CGExprCXX.cpp === --- lib/CodeGen/CGExprCXX.cpp +++ lib/CodeGen/CGExprCXX.cpp @@ -35,17 +35,6 @@ "Trying to emit a member or operator call expr on a static method!"); ASTContext &C = CGF.getContext(); - // C++11 [class.mfct.non-static]p2: - // If a non-static member function of a class X is called for an object that - // is not of type X, or of a type derived from X, the behavior is undefined. - SourceLocation CallLoc; - if (CE) -CallLoc = CE->getExprLoc(); - CGF.EmitTypeCheck(isa(MD) -? CodeGenFunction::TCK_ConstructorCall -: CodeGenFunction::TCK_MemberCall, -CallLoc, This, C.getRecordType(MD->getParent())); - // Push the this ptr. const CXXRecordDecl *RD = CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD); @@ -96,6 +85,14 @@ const CXXDestructorDecl *DD, const CGCallee &Callee, llvm::Value *This, llvm::Value *ImplicitParam, QualType ImplicitParamTy, const CallExpr *CE, StructorType Type) { +SourceLocation CallLoc; + ASTContext &C = getContext(); + if (CE) +CallLoc = CE->getExprLoc(); + + EmitTypeCheck(CodeGenFunction::TCK_MemberCall, +CallLoc, This, C.getRecordType(DD->getParent())); + CallArgList Args; commonEmitCXXMemberOrOperatorCall(*this, DD, This, ImplicitParam, ImplicitParamTy, CE, Args, nullptr); @@ -293,6 +290,19 @@ llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo); + // C++11 [class.mfct.non-static]p2: + // If a non-static member function of a class X is called for an object that + // is not of type X, or of a type derived from X, the behavior is undefined. + SourceLocation CallLoc; + ASTContext &C = getContext(); + if (CE) +CallLoc = CE->getExprLoc(); + + EmitTypeCheck(isa(CalleeDecl) +? CodeGenFunction::TCK_ConstructorCall +: CodeGenFunction::TCK_MemberCall, +CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent())); + // FIXME: Uses of 'MD' past this point need to be audited. We may need to use // 'CalleeDecl' instead. Index: test/CodeGenCXX/ubsan-null.cpp === --- /dev/null +++ test/CodeGenCXX/ubsan-null.cpp @@ -0,0 +1,17 @@ +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s +struct T { + virtual int v() { return 1; } +}; + +struct U : T { + virtual int v() { return 2; } +}; + +// CHECK: define i32 @_Z5get_vP1T +int get_v(T* t) { + // CHECK: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null + // CHECK-NEXT: br i1 [[UBSAN_CMP_RES]], label %cont, label %handler.type_mismatch + // CHECK: call void @__ubsan_handle_type_mismatch_abort + // CHECK: load i32 (%struct.T*)**, i32 (%struct.T*)*** + return t->v(); +} Index: lib/CodeGen/CGExprCXX.cpp === --- lib/CodeGen/CGExprCXX.cpp +++ lib/CodeGen/CGExprCXX.cpp @@ -35,17 +35,6 @@ "Trying to emit a member or operator call expr on a static method!"); ASTContext &C = CGF.getContext(); - // C++11 [class.mfct.non-static]p2: - // If a non-static member function of a class X is called for an object that - // is not of type X, or of a type derived from X, the behavior is undefined. - SourceLocation CallLoc; - if (CE) -CallLoc = CE->getExprLoc(); - CGF.EmitTypeCheck(isa(MD) -? CodeGenFunction::TCK_ConstructorCall -: CodeGenFunction::TCK_MemberCall, -CallLoc, This, C.getRecordType(MD->getParent())); - // Push the this ptr. const CXXRecordDecl *RD = CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD); @@ -96,6 +85,14 @@ const CXXDestructorDecl *DD, const CGCallee &Callee, llvm::Value *This, llvm::Value *ImplicitParam, QualTyp
[PATCH] D26559: Insert a type check before reading vtable.
krasin added inline comments. Comment at: lib/CodeGen/CGExprCXX.cpp:93 + + EmitTypeCheck(CodeGenFunction::TCK_MemberCall, +CallLoc, This, C.getRecordType(DD->getParent())); pcc wrote: > pcc wrote: > > Is it correct to emit a type check at this point? Looking at [0] it looks > > like this function is only called from the Microsoft C++ ABI after we have > > already resolved the virtual function pointer. > > > > [0] > > http://llvm-cs.pcc.me.uk/tools/clang/lib/CodeGen/CGExprCXX.cpp/rEmitCXXDestructorCall > What about this comment? Sorry, I have missed the comment. I have added this check here to preserve the existing behavior. From what you describe, there could be a very similar issue related to the Microsoft C++ ABI to the one that I fix here. I am okay to file a bug or add a note about this, your choice. https://reviews.llvm.org/D26559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26559: Insert a type check before reading vtable.
krasin updated this revision to Diff 77941. krasin added a comment. Do better job with destructors. https://reviews.llvm.org/D26559 Files: lib/CodeGen/CGExprCXX.cpp lib/CodeGen/MicrosoftCXXABI.cpp test/CodeGenCXX/ubsan-vtable-checks.cpp Index: test/CodeGenCXX/ubsan-vtable-checks.cpp === --- /dev/null +++ test/CodeGenCXX/ubsan-vtable-checks.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI +struct T { + virtual ~T() {} + virtual int v() { return 1; } +}; + +struct U : T { + ~U(); + virtual int v() { return 2; } +}; + +U::~U() {} + +// ITANIUM: define i32 @_Z5get_vP1T +// MSABI: define i32 @"\01?get_v +int get_v(T* t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null + // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %cont, label %handler.type_mismatch + // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort + // Second, we check that vtable is actually loaded once the type check is done. + // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + return t->v(); +} + +// ITANIUM: define void @_Z9delete_itP1T +// MSABI: define void @"\01?delete_it +void delete_it(T *t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-VPTR: br i1 {{.*}} label %handler.dynamic_type_cache_miss + // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort + // Second, we check that vtable is actually loaded once the type check is done. + // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + delete t; +} Index: lib/CodeGen/MicrosoftCXXABI.cpp === --- lib/CodeGen/MicrosoftCXXABI.cpp +++ lib/CodeGen/MicrosoftCXXABI.cpp @@ -1817,16 +1817,21 @@ assert(CE == nullptr || CE->arg_begin() == CE->arg_end()); assert(DtorType == Dtor_Deleting || DtorType == Dtor_Complete); + ASTContext &Context = getContext(); + SourceLocation CallLoc = CE ? CE->getLocStart() : SourceLocation(); + CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall, +CallLoc, This.getPointer(), +Context.getRecordType(Dtor->getParent())); + // We have only one destructor in the vftable but can get both behaviors // by passing an implicit int parameter. GlobalDecl GD(Dtor, Dtor_Deleting); const CGFunctionInfo *FInfo = &CGM.getTypes().arrangeCXXStructorDeclaration( Dtor, StructorType::Deleting); llvm::Type *Ty = CGF.CGM.getTypes().GetFunctionType(*FInfo); CGCallee Callee = getVirtualFunctionPointer( - CGF, GD, This, Ty, CE ? CE->getLocStart() : SourceLocation()); + CGF, GD, This, Ty, CallLoc); - ASTContext &Context = getContext(); llvm::Value *ImplicitParam = llvm::ConstantInt::get( llvm::IntegerType::getInt32Ty(CGF.getLLVMContext()), DtorType == Dtor_Deleting); Index: lib/CodeGen/CGExprCXX.cpp === --- lib/CodeGen/CGExprCXX.cpp +++ lib/CodeGen/CGExprCXX.cpp @@ -35,17 +35,6 @@ "Trying to emit a member or operator call expr on a static method!"); ASTContext &C = CGF.getContext(); - // C++11 [class.mfct.non-static]p2: - // If a non-static member function of a class X is called for an object that - // is not of type X, or of a type derived from X, the behavior is undefined. - SourceLocation CallLoc; - if (CE) -CallLoc = CE->getExprLoc(); - CGF.EmitTypeCheck(isa(MD) -? CodeGenFunction::TCK_ConstructorCall -: CodeGenFunction::TCK_MemberCall, -CallLoc, This, C.getRecordType(MD->getParent())); - // Push the this ptr. const CXXRecordDecl *RD = CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD); @@ -293,6 +282,19 @@ llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo); + // C++11 [class.mfct.non-static]p2: + // If a non-static member function of a class X is called for an object that + // is not of type X, or
[PATCH] D26559: Insert a type check before reading vtable.
krasin added inline comments. Comment at: lib/CodeGen/CGExprCXX.cpp:93 + + EmitTypeCheck(CodeGenFunction::TCK_MemberCall, +CallLoc, This, C.getRecordType(DD->getParent())); pcc wrote: > krasin wrote: > > pcc wrote: > > > pcc wrote: > > > > Is it correct to emit a type check at this point? Looking at [0] it > > > > looks like this function is only called from the Microsoft C++ ABI > > > > after we have already resolved the virtual function pointer. > > > > > > > > [0] > > > > http://llvm-cs.pcc.me.uk/tools/clang/lib/CodeGen/CGExprCXX.cpp/rEmitCXXDestructorCall > > > What about this comment? > > Sorry, I have missed the comment. I have added this check here to preserve > > the existing behavior. > > > > From what you describe, there could be a very similar issue related to the > > Microsoft C++ ABI to the one that I fix here. I am okay to file a bug or > > add a note about this, your choice. > Have you looked at how hard it would be to fix this? Done. Note: since I don't have an ability to test on Windows, this CL is now high-risk. If it breaks any Windows-specific tests, when submitted, I will revert the CL and undo the MSABI change. After all, the previous state was to keep things working as they were before. https://reviews.llvm.org/D26559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26559: Insert a type check before reading vtable.
krasin added a comment. Friendly ping. https://reviews.llvm.org/D26559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26559: Insert a type check before reading vtable.
krasin updated this revision to Diff 78104. krasin added a comment. Address comments. https://reviews.llvm.org/D26559 Files: lib/CodeGen/CGExprCXX.cpp test/CodeGenCXX/ubsan-vtable-checks.cpp Index: test/CodeGenCXX/ubsan-vtable-checks.cpp === --- /dev/null +++ test/CodeGenCXX/ubsan-vtable-checks.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI +struct T { + virtual ~T() {} + virtual int v() { return 1; } +}; + +struct U : T { + ~U(); + virtual int v() { return 2; } +}; + +U::~U() {} + +// ITANIUM: define i32 @_Z5get_vP1T +// MSABI: define i32 @"\01?get_v +int get_v(T* t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null + // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %cont, label %handler.type_mismatch + // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort + // Second, we check that vtable is actually loaded once the type check is done. + // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + return t->v(); +} + +// ITANIUM: define void @_Z9delete_itP1T +// MSABI: define void @"\01?delete_it +void delete_it(T *t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-VPTR: br i1 {{.*}} label %handler.dynamic_type_cache_miss + // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort + // Second, we check that vtable is actually loaded once the type check is done. + // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + delete t; +} Index: lib/CodeGen/CGExprCXX.cpp === --- lib/CodeGen/CGExprCXX.cpp +++ lib/CodeGen/CGExprCXX.cpp @@ -35,17 +35,6 @@ "Trying to emit a member or operator call expr on a static method!"); ASTContext &C = CGF.getContext(); - // C++11 [class.mfct.non-static]p2: - // If a non-static member function of a class X is called for an object that - // is not of type X, or of a type derived from X, the behavior is undefined. - SourceLocation CallLoc; - if (CE) -CallLoc = CE->getExprLoc(); - CGF.EmitTypeCheck(isa(MD) -? CodeGenFunction::TCK_ConstructorCall -: CodeGenFunction::TCK_MemberCall, -CallLoc, This, C.getRecordType(MD->getParent())); - // Push the this ptr. const CXXRecordDecl *RD = CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD); @@ -293,6 +282,19 @@ llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo); + // C++11 [class.mfct.non-static]p2: + // If a non-static member function of a class X is called for an object that + // is not of type X, or of a type derived from X, the behavior is undefined. + SourceLocation CallLoc; + ASTContext &C = getContext(); + if (CE) +CallLoc = CE->getExprLoc(); + + EmitTypeCheck(isa(CalleeDecl) +? CodeGenFunction::TCK_ConstructorCall +: CodeGenFunction::TCK_MemberCall, +CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent())); + // FIXME: Uses of 'MD' past this point need to be audited. We may need to use // 'CalleeDecl' instead. @@ -1763,6 +1765,13 @@ const CXXDeleteExpr *DE, Address Ptr, QualType ElementType) { + SourceLocation CallLoc; + if (DE) +CallLoc = DE->getExprLoc(); + CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall, +CallLoc, Ptr.getPointer(), +ElementType); + // Find the destructor for the type, if applicable. If the // destructor is virtual, we'll just emit the vcall and return. const CXXDestructorDecl *Dtor = nullptr; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26559: Insert a type check before reading vtable.
krasin marked 2 inline comments as done. krasin added inline comments. Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:1820-1833 + ASTContext &Context = getContext(); + SourceLocation CallLoc = CE ? CE->getLocStart() : SourceLocation(); + CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall, +CallLoc, This.getPointer(), +Context.getRecordType(Dtor->getParent())); + // We have only one destructor in the vftable but can get both behaviors pcc wrote: > If you undo this part do the tests still pass? Thank you for the catch. Yes, the other check covers this case too. Reverted this file. https://reviews.llvm.org/D26559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D26559: Insert a type check before reading vtable.
Thank you, Richard. Shall I merge the newly introduced test/CodeGenCXX/ubsan-vtable-checks.cpp into catch-undef-behavior.cpp or it's more clear when it's standalone? On Tue, Nov 15, 2016 at 5:51 PM, Richard Smith wrote: > On Fri, Nov 11, 2016 at 3:02 PM, Ivan Krasin via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> krasin added a comment. >> >> Small correction: all UBSan type checks tests live in compiler-rt. > > > Actually, most of the UBSan tests live in > test/CodeGenCXX/catch-undef-behavior.cpp > in Clang; the compiler-rt tests are merely aiming to test that the runtime > produces the correct diagnostics. There are a few more test files testing > UBSan besides that one; you can find the tests by grepping for "fsanitize=" > in Clang's test/ directory. > > >> There is a test for UBsan + devirtualization inside tools/clang. My point >> still stands. >> >> >> https://reviews.llvm.org/D26559 >> >> >> >> ___ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D26559: Insert a type check before reading vtable.
Okay, I will keep it as a separate file then. It's very likely that I will soon send a follow up, as there's another very similar case known to not work properly (based on the experience with UBSan Vptr bot in Chromium) On Tue, Nov 15, 2016 at 6:27 PM, Richard Smith wrote: > On Tue, Nov 15, 2016 at 6:20 PM, Ivan Krasin wrote: > >> Thank you, Richard. >> >> Shall I merge the newly introduced test/CodeGenCXX/ubsan-vtable-checks.cpp >> into catch-undef-behavior.cpp or it's more clear when it's standalone? >> > > Up to you. catch-undef-behavior.cpp is getting unwieldy, so a separate > test file doesn't seem like a bad thing. > > >> On Tue, Nov 15, 2016 at 5:51 PM, Richard Smith >> wrote: >> >>> On Fri, Nov 11, 2016 at 3:02 PM, Ivan Krasin via cfe-commits < >>> cfe-commits@lists.llvm.org> wrote: >>> >>>> krasin added a comment. >>>> >>>> Small correction: all UBSan type checks tests live in compiler-rt. >>> >>> >>> Actually, most of the UBSan tests live in >>> test/CodeGenCXX/catch-undef-behavior.cpp >>> in Clang; the compiler-rt tests are merely aiming to test that the runtime >>> produces the correct diagnostics. There are a few more test files testing >>> UBSan besides that one; you can find the tests by grepping for "fsanitize=" >>> in Clang's test/ directory. >>> >>> >>>> There is a test for UBsan + devirtualization inside tools/clang. My >>>> point still stands. >>>> >>>> >>>> https://reviews.llvm.org/D26559 >>>> >>>> >>>> >>>> ___ >>>> cfe-commits mailing list >>>> cfe-commits@lists.llvm.org >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>> >>> >>> >> > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D26559: Insert a type check before reading vtable.
.. but the follow up will be in a separate CL, as I have not even started to work on it yet. On Tue, Nov 15, 2016 at 6:31 PM, Ivan Krasin wrote: > Okay, I will keep it as a separate file then. It's very likely that I will > soon send a follow up, as there's another very similar case known to not > work properly (based on the experience with UBSan Vptr bot in Chromium) > > On Tue, Nov 15, 2016 at 6:27 PM, Richard Smith > wrote: > >> On Tue, Nov 15, 2016 at 6:20 PM, Ivan Krasin wrote: >> >>> Thank you, Richard. >>> >>> Shall I merge the newly introduced test/CodeGenCXX/ubsan-vtable-checks.cpp >>> into catch-undef-behavior.cpp or it's more clear when it's standalone? >>> >> >> Up to you. catch-undef-behavior.cpp is getting unwieldy, so a separate >> test file doesn't seem like a bad thing. >> >> >>> On Tue, Nov 15, 2016 at 5:51 PM, Richard Smith >>> wrote: >>> >>>> On Fri, Nov 11, 2016 at 3:02 PM, Ivan Krasin via cfe-commits < >>>> cfe-commits@lists.llvm.org> wrote: >>>> >>>>> krasin added a comment. >>>>> >>>>> Small correction: all UBSan type checks tests live in compiler-rt. >>>> >>>> >>>> Actually, most of the UBSan tests live in >>>> test/CodeGenCXX/catch-undef-behavior.cpp >>>> in Clang; the compiler-rt tests are merely aiming to test that the runtime >>>> produces the correct diagnostics. There are a few more test files testing >>>> UBSan besides that one; you can find the tests by grepping for "fsanitize=" >>>> in Clang's test/ directory. >>>> >>>> >>>>> There is a test for UBsan + devirtualization inside tools/clang. My >>>>> point still stands. >>>>> >>>>> >>>>> https://reviews.llvm.org/D26559 >>>>> >>>>> >>>>> >>>>> ___ >>>>> cfe-commits mailing list >>>>> cfe-commits@lists.llvm.org >>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>>> >>>> >>>> >>> >> > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26559: Insert a type check before reading vtable.
krasin updated this revision to Diff 78240. krasin marked an inline comment as done. krasin added a comment. Sync to Clang ToT. https://reviews.llvm.org/D26559 Files: lib/CodeGen/CGExprCXX.cpp test/CodeGenCXX/ubsan-vtable-checks.cpp Index: test/CodeGenCXX/ubsan-vtable-checks.cpp === --- /dev/null +++ test/CodeGenCXX/ubsan-vtable-checks.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI +struct T { + virtual ~T() {} + virtual int v() { return 1; } +}; + +struct U : T { + ~U(); + virtual int v() { return 2; } +}; + +U::~U() {} + +// ITANIUM: define i32 @_Z5get_vP1T +// MSABI: define i32 @"\01?get_v +int get_v(T* t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null + // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %cont, label %handler.type_mismatch + // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort + // Second, we check that vtable is actually loaded once the type check is done. + // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + return t->v(); +} + +// ITANIUM: define void @_Z9delete_itP1T +// MSABI: define void @"\01?delete_it +void delete_it(T *t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-VPTR: br i1 {{.*}} label %handler.dynamic_type_cache_miss + // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort + // Second, we check that vtable is actually loaded once the type check is done. + // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + delete t; +} Index: lib/CodeGen/CGExprCXX.cpp === --- lib/CodeGen/CGExprCXX.cpp +++ lib/CodeGen/CGExprCXX.cpp @@ -35,17 +35,6 @@ "Trying to emit a member or operator call expr on a static method!"); ASTContext &C = CGF.getContext(); - // C++11 [class.mfct.non-static]p2: - // If a non-static member function of a class X is called for an object that - // is not of type X, or of a type derived from X, the behavior is undefined. - SourceLocation CallLoc; - if (CE) -CallLoc = CE->getExprLoc(); - CGF.EmitTypeCheck(isa(MD) -? CodeGenFunction::TCK_ConstructorCall -: CodeGenFunction::TCK_MemberCall, -CallLoc, This, C.getRecordType(MD->getParent())); - // Push the this ptr. const CXXRecordDecl *RD = CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD); @@ -293,6 +282,19 @@ llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo); + // C++11 [class.mfct.non-static]p2: + // If a non-static member function of a class X is called for an object that + // is not of type X, or of a type derived from X, the behavior is undefined. + SourceLocation CallLoc; + ASTContext &C = getContext(); + if (CE) +CallLoc = CE->getExprLoc(); + + EmitTypeCheck(isa(CalleeDecl) +? CodeGenFunction::TCK_ConstructorCall +: CodeGenFunction::TCK_MemberCall, +CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent())); + // FIXME: Uses of 'MD' past this point need to be audited. We may need to use // 'CalleeDecl' instead. @@ -1763,6 +1765,13 @@ const CXXDeleteExpr *DE, Address Ptr, QualType ElementType) { + SourceLocation CallLoc; + if (DE) +CallLoc = DE->getExprLoc(); + CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall, +CallLoc, Ptr.getPointer(), +ElementType); + // Find the destructor for the type, if applicable. If the // destructor is virtual, we'll just emit the vcall and return. const CXXDestructorDecl *Dtor = nullptr; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26559: Insert a type check before reading vtable.
krasin updated this revision to Diff 78276. krasin added a comment. Fix the test under -Asserts build. https://reviews.llvm.org/D26559 Files: lib/CodeGen/CGExprCXX.cpp test/CodeGenCXX/ubsan-vtable-checks.cpp Index: test/CodeGenCXX/ubsan-vtable-checks.cpp === --- /dev/null +++ test/CodeGenCXX/ubsan-vtable-checks.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI +struct T { + virtual ~T() {} + virtual int v() { return 1; } +}; + +struct U : T { + ~U(); + virtual int v() { return 2; } +}; + +U::~U() {} + +// ITANIUM: define i32 @_Z5get_vP1T +// MSABI: define i32 @"\01?get_v +int get_v(T* t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null + // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %{{.*}}, label %{{.*}} + // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort + // Second, we check that vtable is actually loaded once the type check is done. + // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + return t->v(); +} + +// ITANIUM: define void @_Z9delete_itP1T +// MSABI: define void @"\01?delete_it +void delete_it(T *t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-VPTR: br i1 {{.*}} label %{{.*}} + // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort + // Second, we check that vtable is actually loaded once the type check is done. + // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + delete t; +} Index: lib/CodeGen/CGExprCXX.cpp === --- lib/CodeGen/CGExprCXX.cpp +++ lib/CodeGen/CGExprCXX.cpp @@ -35,17 +35,6 @@ "Trying to emit a member or operator call expr on a static method!"); ASTContext &C = CGF.getContext(); - // C++11 [class.mfct.non-static]p2: - // If a non-static member function of a class X is called for an object that - // is not of type X, or of a type derived from X, the behavior is undefined. - SourceLocation CallLoc; - if (CE) -CallLoc = CE->getExprLoc(); - CGF.EmitTypeCheck(isa(MD) -? CodeGenFunction::TCK_ConstructorCall -: CodeGenFunction::TCK_MemberCall, -CallLoc, This, C.getRecordType(MD->getParent())); - // Push the this ptr. const CXXRecordDecl *RD = CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD); @@ -293,6 +282,19 @@ llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo); + // C++11 [class.mfct.non-static]p2: + // If a non-static member function of a class X is called for an object that + // is not of type X, or of a type derived from X, the behavior is undefined. + SourceLocation CallLoc; + ASTContext &C = getContext(); + if (CE) +CallLoc = CE->getExprLoc(); + + EmitTypeCheck(isa(CalleeDecl) +? CodeGenFunction::TCK_ConstructorCall +: CodeGenFunction::TCK_MemberCall, +CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent())); + // FIXME: Uses of 'MD' past this point need to be audited. We may need to use // 'CalleeDecl' instead. @@ -1763,6 +1765,13 @@ const CXXDeleteExpr *DE, Address Ptr, QualType ElementType) { + SourceLocation CallLoc; + if (DE) +CallLoc = DE->getExprLoc(); + CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall, +CallLoc, Ptr.getPointer(), +ElementType); + // Find the destructor for the type, if applicable. If the // destructor is virtual, we'll just emit the vcall and return. const CXXDestructorDecl *Dtor = nullptr; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26559: Insert a type check before reading vtable.
krasin marked an inline comment as done. krasin added inline comments. Comment at: test/CodeGenCXX/ubsan-vtable-checks.cpp:23 + // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null + // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %cont, label %handler.type_mismatch + // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort pcc wrote: > Does this test pass in a -Asserts build? Fixed. https://reviews.llvm.org/D26559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26559: Insert a type check before reading vtable.
krasin updated this revision to Diff 78277. krasin added a comment. Address minor comment. https://reviews.llvm.org/D26559 Files: lib/CodeGen/CGExprCXX.cpp test/CodeGenCXX/ubsan-vtable-checks.cpp Index: test/CodeGenCXX/ubsan-vtable-checks.cpp === --- /dev/null +++ test/CodeGenCXX/ubsan-vtable-checks.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI +struct T { + virtual ~T() {} + virtual int v() { return 1; } +}; + +struct U : T { + ~U(); + virtual int v() { return 2; } +}; + +U::~U() {} + +// ITANIUM: define i32 @_Z5get_vP1T +// MSABI: define i32 @"\01?get_v +int get_v(T* t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null + // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %{{.*}}, label %{{.*}} + // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort + // Second, we check that vtable is actually loaded once the type check is done. + // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + return t->v(); +} + +// ITANIUM: define void @_Z9delete_itP1T +// MSABI: define void @"\01?delete_it +void delete_it(T *t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-VPTR: br i1 {{.*}} label %{{.*}} + // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort + // Second, we check that vtable is actually loaded once the type check is done. + // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + delete t; +} Index: lib/CodeGen/CGExprCXX.cpp === --- lib/CodeGen/CGExprCXX.cpp +++ lib/CodeGen/CGExprCXX.cpp @@ -35,17 +35,6 @@ "Trying to emit a member or operator call expr on a static method!"); ASTContext &C = CGF.getContext(); - // C++11 [class.mfct.non-static]p2: - // If a non-static member function of a class X is called for an object that - // is not of type X, or of a type derived from X, the behavior is undefined. - SourceLocation CallLoc; - if (CE) -CallLoc = CE->getExprLoc(); - CGF.EmitTypeCheck(isa(MD) -? CodeGenFunction::TCK_ConstructorCall -: CodeGenFunction::TCK_MemberCall, -CallLoc, This, C.getRecordType(MD->getParent())); - // Push the this ptr. const CXXRecordDecl *RD = CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD); @@ -293,6 +282,19 @@ llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo); + // C++11 [class.mfct.non-static]p2: + // If a non-static member function of a class X is called for an object that + // is not of type X, or of a type derived from X, the behavior is undefined. + SourceLocation CallLoc; + ASTContext &C = getContext(); + if (CE) +CallLoc = CE->getExprLoc(); + + EmitTypeCheck(isa(CalleeDecl) +? CodeGenFunction::TCK_ConstructorCall +: CodeGenFunction::TCK_MemberCall, +CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent())); + // FIXME: Uses of 'MD' past this point need to be audited. We may need to use // 'CalleeDecl' instead. @@ -1763,6 +1765,12 @@ const CXXDeleteExpr *DE, Address Ptr, QualType ElementType) { + SourceLocation CallLoc; + CallLoc = DE->getExprLoc(); + CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall, +CallLoc, Ptr.getPointer(), +ElementType); + // Find the destructor for the type, if applicable. If the // destructor is virtual, we'll just emit the vcall and return. const CXXDestructorDecl *Dtor = nullptr; Index: test/CodeGenCXX/ubsan-vtable-checks.cpp === --- /dev/null +++ test/CodeGenCXX/ubsan-vtable-checks.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefi
[PATCH] D26559: Insert a type check before reading vtable.
krasin updated this revision to Diff 78279. krasin added a comment. inline https://reviews.llvm.org/D26559 Files: lib/CodeGen/CGExprCXX.cpp test/CodeGenCXX/ubsan-vtable-checks.cpp Index: test/CodeGenCXX/ubsan-vtable-checks.cpp === --- /dev/null +++ test/CodeGenCXX/ubsan-vtable-checks.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI +struct T { + virtual ~T() {} + virtual int v() { return 1; } +}; + +struct U : T { + ~U(); + virtual int v() { return 2; } +}; + +U::~U() {} + +// ITANIUM: define i32 @_Z5get_vP1T +// MSABI: define i32 @"\01?get_v +int get_v(T* t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null + // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %{{.*}}, label %{{.*}} + // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort + // Second, we check that vtable is actually loaded once the type check is done. + // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + return t->v(); +} + +// ITANIUM: define void @_Z9delete_itP1T +// MSABI: define void @"\01?delete_it +void delete_it(T *t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-VPTR: br i1 {{.*}} label %{{.*}} + // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort + // Second, we check that vtable is actually loaded once the type check is done. + // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + delete t; +} Index: lib/CodeGen/CGExprCXX.cpp === --- lib/CodeGen/CGExprCXX.cpp +++ lib/CodeGen/CGExprCXX.cpp @@ -35,17 +35,6 @@ "Trying to emit a member or operator call expr on a static method!"); ASTContext &C = CGF.getContext(); - // C++11 [class.mfct.non-static]p2: - // If a non-static member function of a class X is called for an object that - // is not of type X, or of a type derived from X, the behavior is undefined. - SourceLocation CallLoc; - if (CE) -CallLoc = CE->getExprLoc(); - CGF.EmitTypeCheck(isa(MD) -? CodeGenFunction::TCK_ConstructorCall -: CodeGenFunction::TCK_MemberCall, -CallLoc, This, C.getRecordType(MD->getParent())); - // Push the this ptr. const CXXRecordDecl *RD = CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD); @@ -293,6 +282,19 @@ llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo); + // C++11 [class.mfct.non-static]p2: + // If a non-static member function of a class X is called for an object that + // is not of type X, or of a type derived from X, the behavior is undefined. + SourceLocation CallLoc; + ASTContext &C = getContext(); + if (CE) +CallLoc = CE->getExprLoc(); + + EmitTypeCheck(isa(CalleeDecl) +? CodeGenFunction::TCK_ConstructorCall +: CodeGenFunction::TCK_MemberCall, +CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent())); + // FIXME: Uses of 'MD' past this point need to be audited. We may need to use // 'CalleeDecl' instead. @@ -1763,6 +1765,10 @@ const CXXDeleteExpr *DE, Address Ptr, QualType ElementType) { + CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall, +DE->getExprLoc(), Ptr.getPointer(), +ElementType); + // Find the destructor for the type, if applicable. If the // destructor is virtual, we'll just emit the vcall and return. const CXXDestructorDecl *Dtor = nullptr; Index: test/CodeGenCXX/ubsan-vtable-checks.cpp === --- /dev/null +++ test/CodeGenCXX/ubsan-vtable-checks.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM +// RUN:
r287181 - Insert a type check before reading vtable.
Author: krasin Date: Wed Nov 16 18:39:48 2016 New Revision: 287181 URL: http://llvm.org/viewvc/llvm-project?rev=287181&view=rev Log: Insert a type check before reading vtable. Summary: this is to prevent a situation when a pointer is invalid or null, but we get to reading from vtable before we can check that (possibly causing a segfault without a good diagnostics). Reviewers: pcc Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D26559 Added: cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp Modified: cfe/trunk/lib/CodeGen/CGExprCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprCXX.cpp?rev=287181&r1=287180&r2=287181&view=diff == --- cfe/trunk/lib/CodeGen/CGExprCXX.cpp (original) +++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp Wed Nov 16 18:39:48 2016 @@ -35,17 +35,6 @@ commonEmitCXXMemberOrOperatorCall(CodeGe "Trying to emit a member or operator call expr on a static method!"); ASTContext &C = CGF.getContext(); - // C++11 [class.mfct.non-static]p2: - // If a non-static member function of a class X is called for an object that - // is not of type X, or of a type derived from X, the behavior is undefined. - SourceLocation CallLoc; - if (CE) -CallLoc = CE->getExprLoc(); - CGF.EmitTypeCheck(isa(MD) -? CodeGenFunction::TCK_ConstructorCall -: CodeGenFunction::TCK_MemberCall, -CallLoc, This, C.getRecordType(MD->getParent())); - // Push the this ptr. const CXXRecordDecl *RD = CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD); @@ -293,6 +282,19 @@ RValue CodeGenFunction::EmitCXXMemberOrO llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo); + // C++11 [class.mfct.non-static]p2: + // If a non-static member function of a class X is called for an object that + // is not of type X, or of a type derived from X, the behavior is undefined. + SourceLocation CallLoc; + ASTContext &C = getContext(); + if (CE) +CallLoc = CE->getExprLoc(); + + EmitTypeCheck(isa(CalleeDecl) +? CodeGenFunction::TCK_ConstructorCall +: CodeGenFunction::TCK_MemberCall, +CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent())); + // FIXME: Uses of 'MD' past this point need to be audited. We may need to use // 'CalleeDecl' instead. @@ -1763,6 +1765,15 @@ static void EmitObjectDelete(CodeGenFunc const CXXDeleteExpr *DE, Address Ptr, QualType ElementType) { + // C++11 [expr.delete]p3: + // If the static type of the object to be deleted is different from its + // dynamic type, the static type shall be a base class of the dynamic type + // of the object to be deleted and the static type shall have a virtual + // destructor or the behavior is undefined. + CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall, +DE->getExprLoc(), Ptr.getPointer(), +ElementType); + // Find the destructor for the type, if applicable. If the // destructor is virtual, we'll just emit the vcall and return. const CXXDestructorDecl *Dtor = nullptr; Added: cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp?rev=287181&view=auto == --- cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp (added) +++ cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp Wed Nov 16 18:39:48 2016 @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI +struct T { + virtual ~T() {} + virtual int v() { return 1; } +}; + +struct U : T { + ~U(); + virtual int v() { return 2; } +}; + +U::~U() {} + +// ITANIUM: define i32 @_Z5get_vP1T +// MSABI: define i32 @"\01?get_v +int get_v(T* t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null + // CHECK-NULL-NEXT: br
[PATCH] D26559: Insert a type check before reading vtable.
This revision was automatically updated to reflect the committed changes. Closed by commit rL287181: Insert a type check before reading vtable. (authored by krasin). Changed prior to commit: https://reviews.llvm.org/D26559?vs=78279&id=78288#toc Repository: rL LLVM https://reviews.llvm.org/D26559 Files: cfe/trunk/lib/CodeGen/CGExprCXX.cpp cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp Index: cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp === --- cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp +++ cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI +// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI +struct T { + virtual ~T() {} + virtual int v() { return 1; } +}; + +struct U : T { + ~U(); + virtual int v() { return 2; } +}; + +U::~U() {} + +// ITANIUM: define i32 @_Z5get_vP1T +// MSABI: define i32 @"\01?get_v +int get_v(T* t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-NULL-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-NULL: [[UBSAN_CMP_RES:%[0-9]+]] = icmp ne %struct.T* %{{[_a-z0-9]+}}, null + // CHECK-NULL-NEXT: br i1 [[UBSAN_CMP_RES]], label %{{.*}}, label %{{.*}} + // CHECK-NULL: call void @__ubsan_handle_type_mismatch_abort + // Second, we check that vtable is actually loaded once the type check is done. + // CHECK-NULL: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + return t->v(); +} + +// ITANIUM: define void @_Z9delete_itP1T +// MSABI: define void @"\01?delete_it +void delete_it(T *t) { + // First, we check that vtable is not loaded before a type check. + // CHECK-VPTR-NOT: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + // CHECK-VPTR: br i1 {{.*}} label %{{.*}} + // CHECK-VPTR: call void @__ubsan_handle_dynamic_type_cache_miss_abort + // Second, we check that vtable is actually loaded once the type check is done. + // CHECK-VPTR: load {{.*}} (%struct.T*{{.*}})**, {{.*}} (%struct.T*{{.*}})*** + delete t; +} Index: cfe/trunk/lib/CodeGen/CGExprCXX.cpp === --- cfe/trunk/lib/CodeGen/CGExprCXX.cpp +++ cfe/trunk/lib/CodeGen/CGExprCXX.cpp @@ -35,17 +35,6 @@ "Trying to emit a member or operator call expr on a static method!"); ASTContext &C = CGF.getContext(); - // C++11 [class.mfct.non-static]p2: - // If a non-static member function of a class X is called for an object that - // is not of type X, or of a type derived from X, the behavior is undefined. - SourceLocation CallLoc; - if (CE) -CallLoc = CE->getExprLoc(); - CGF.EmitTypeCheck(isa(MD) -? CodeGenFunction::TCK_ConstructorCall -: CodeGenFunction::TCK_MemberCall, -CallLoc, This, C.getRecordType(MD->getParent())); - // Push the this ptr. const CXXRecordDecl *RD = CGF.CGM.getCXXABI().getThisArgumentTypeForMethod(MD); @@ -293,6 +282,19 @@ llvm::FunctionType *Ty = CGM.getTypes().GetFunctionType(*FInfo); + // C++11 [class.mfct.non-static]p2: + // If a non-static member function of a class X is called for an object that + // is not of type X, or of a type derived from X, the behavior is undefined. + SourceLocation CallLoc; + ASTContext &C = getContext(); + if (CE) +CallLoc = CE->getExprLoc(); + + EmitTypeCheck(isa(CalleeDecl) +? CodeGenFunction::TCK_ConstructorCall +: CodeGenFunction::TCK_MemberCall, +CallLoc, This.getPointer(), C.getRecordType(CalleeDecl->getParent())); + // FIXME: Uses of 'MD' past this point need to be audited. We may need to use // 'CalleeDecl' instead. @@ -1763,6 +1765,15 @@ const CXXDeleteExpr *DE, Address Ptr, QualType ElementType) { + // C++11 [expr.delete]p3: + // If the static type of the object to be deleted is different from its + // dynamic type, the static type shall be a base class of the dynamic type + // of the object to be deleted and the static type shall have a virtual + // destructor or the behavior is undefined. + CGF.EmitTypeCheck(CodeGenFunction::TCK_MemberCall, +DE->getExprLoc(), Ptr.getPoi
r287185 - Explicitly specify that ubsan-vtable-checks is x86-64.
Author: krasin Date: Wed Nov 16 19:09:04 2016 New Revision: 287185 URL: http://llvm.org/viewvc/llvm-project?rev=287185&view=rev Log: Explicitly specify that ubsan-vtable-checks is x86-64. This should fix a failure on PowerPC introduced by r287181. Modified: cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp Modified: cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp?rev=287185&r1=287184&r2=287185&view=diff == --- cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp (original) +++ cfe/trunk/test/CodeGenCXX/ubsan-vtable-checks.cpp Wed Nov 16 19:09:04 2016 @@ -1,6 +1,6 @@ -// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=ITANIUM // RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=null %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NULL --check-prefix=MSABI -// RUN: %clang_cc1 -std=c++11 -triple %itanium_abi_triple -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM +// RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=ITANIUM // RUN: %clang_cc1 -std=c++11 -triple x86_64-windows -emit-llvm -fsanitize=vptr %s -o - | FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-VPTR --check-prefix=MSABI struct T { virtual ~T() {} ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26560: Add a test for vcall on a null ptr.
krasin updated this revision to Diff 78405. krasin added a comment. sync https://reviews.llvm.org/D26560 Files: test/ubsan/TestCases/TypeCheck/null.cpp Index: test/ubsan/TestCases/TypeCheck/null.cpp === --- test/ubsan/TestCases/TypeCheck/null.cpp +++ test/ubsan/TestCases/TypeCheck/null.cpp @@ -1,20 +1,50 @@ -// RUN: %clangxx -fsanitize=null %s -O3 -o %t -// RUN: %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD -// RUN: %expect_crash %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE -// RUN: %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE -// RUN: %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER -// RUN: %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN +// RUN: %clangxx -fsanitize=null -fno-sanitize-recover=null -g %s -O3 -o %t +// RUN: not %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD +// RUN: not %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE +// RUN: not %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE +// RUN: not %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER +// RUN: not %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN +// RUN: not %run %t t 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL +// RUN: not %run %t u 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL2 + +#include struct S { int f() { return 0; } int k; }; +struct T { + virtual int v() { return 1; } +}; + +struct U : T { + virtual int v() { return 2; } +}; + +static inline void break_optimization(void *arg) { + __asm__ __volatile__("" : : "r" (arg) : "memory"); +} + int main(int, char **argv) { int *p = 0; S *s = 0; + T *t = 0; + U *u = 0; + + if (argv[1][0] == 'T') { +t = new T; + } + if (argv[1][0] == 'U') { +u = new U; + } + + break_optimization(s); + break_optimization(t); + break_optimization(u); (void)*p; // ok! + (void)*t; // ok! switch (argv[1][0]) { case 'l': @@ -34,5 +64,13 @@ case 'f': // CHECK-MEMFUN: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'S' return s->f(); + case 't': + case 'T': +// CHECK-VCALL: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'T' +return t->v(); + case 'u': + case 'U': +// CHECK-VCALL2: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'U' +return u->v(); } } Index: test/ubsan/TestCases/TypeCheck/null.cpp === --- test/ubsan/TestCases/TypeCheck/null.cpp +++ test/ubsan/TestCases/TypeCheck/null.cpp @@ -1,20 +1,50 @@ -// RUN: %clangxx -fsanitize=null %s -O3 -o %t -// RUN: %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD -// RUN: %expect_crash %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE -// RUN: %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE -// RUN: %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER -// RUN: %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN +// RUN: %clangxx -fsanitize=null -fno-sanitize-recover=null -g %s -O3 -o %t +// RUN: not %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD +// RUN: not %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE +// RUN: not %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE +// RUN: not %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER +// RUN: not %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN +// RUN: not %run %t t 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL +// RUN: not %run %t u 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL2 + +#include struct S { int f() { return 0; } int k; }; +struct T { + virtual int v() { return 1; } +}; + +struct U : T { + virtual int v() { return 2; } +}; + +static inline void break_optimization(void *arg) { + __asm__ __volatile__("" : : "r" (arg) : "memory"); +} + int main(int, char **argv) { int *p = 0; S *s = 0; + T *t = 0; + U *u = 0; + + if (argv[1][0] == 'T') { +t = new T; + } + if (argv[1][0] == 'U') { +u = new U; + } + + break_optimization(s); + break_optimization(t); + break_optimization(u); (void)*p; // ok! + (void)*t; // ok! switch (argv[1][0]) { case 'l': @@ -34,5 +64,13 @@ case 'f': // CHECK-MEMFUN: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'S' return s->f(); + case 't': + case 'T': +// CHECK-VCALL: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'T' +return t->v(); + case 'u': + case 'U': +// CHECK-VCALL2: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'U' +return u->v(); } } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26560: Add a test for vcall on a null ptr.
krasin added inline comments. Comment at: test/ubsan/TestCases/TypeCheck/null.cpp:1 -// RUN: %clangxx -fsanitize=null %s -O3 -o %t -// RUN: %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD -// RUN: %expect_crash %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE -// RUN: %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE -// RUN: %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER -// RUN: %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN +// RUN: %clangxx -fsanitize=null -fno-sanitize-recover=null -g %s -O3 -o %t +// RUN: not %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD pcc wrote: > Why add the -g? It's a debug left over. Thank you for the catch. Comment at: test/ubsan/TestCases/TypeCheck/null.cpp:10 + +#include pcc wrote: > Is this #include needed? Debug leftover. Removed. Thank you for spotting this. Comment at: test/ubsan/TestCases/TypeCheck/null.cpp:35 + + if (argv[1][0] == 'T') { +t = new T; pcc wrote: > Did you intend to add tests for these cases? Actually, the real reason for adding these is that break_optimization didn't really fool the compiler, and I had to add some more logic to avoid letting it know that the pointer is always null => it's undefined behavior. In my case, I saw my return being ignored and two switch statements executed together. I can't currently reproduce it now, most likely, because the fix has eliminated the virtual call on a pointer that is guaranteed to be null. So, I have removed these as well as break_optimization calls. https://reviews.llvm.org/D26560 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26560: Add a test for vcall on a null ptr.
krasin updated this revision to Diff 78557. krasin added a comment. sync & address the comments. https://reviews.llvm.org/D26560 Files: test/ubsan/TestCases/TypeCheck/null.cpp Index: test/ubsan/TestCases/TypeCheck/null.cpp === --- test/ubsan/TestCases/TypeCheck/null.cpp +++ test/ubsan/TestCases/TypeCheck/null.cpp @@ -1,20 +1,34 @@ -// RUN: %clangxx -fsanitize=null %s -O3 -o %t -// RUN: %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD -// RUN: %expect_crash %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE -// RUN: %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE -// RUN: %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER -// RUN: %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN +// RUN: %clangxx -fsanitize=null -fno-sanitize-recover=null %s -O3 -o %t +// RUN: not %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD +// RUN: not %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE +// RUN: not %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE +// RUN: not %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER +// RUN: not %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN +// RUN: not %run %t t 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL +// RUN: not %run %t u 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL2 struct S { int f() { return 0; } int k; }; +struct T { + virtual int v() { return 1; } +}; + +struct U : T { + virtual int v() { return 2; } +}; + int main(int, char **argv) { int *p = 0; S *s = 0; + T *t = 0; + U *u = 0; (void)*p; // ok! + (void)*t; // ok! + (void)*u; // ok! switch (argv[1][0]) { case 'l': @@ -34,5 +48,11 @@ case 'f': // CHECK-MEMFUN: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'S' return s->f(); + case 't': +// CHECK-VCALL: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'T' +return t->v(); + case 'u': +// CHECK-VCALL2: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'U' +return u->v(); } } Index: test/ubsan/TestCases/TypeCheck/null.cpp === --- test/ubsan/TestCases/TypeCheck/null.cpp +++ test/ubsan/TestCases/TypeCheck/null.cpp @@ -1,20 +1,34 @@ -// RUN: %clangxx -fsanitize=null %s -O3 -o %t -// RUN: %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD -// RUN: %expect_crash %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE -// RUN: %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE -// RUN: %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER -// RUN: %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN +// RUN: %clangxx -fsanitize=null -fno-sanitize-recover=null %s -O3 -o %t +// RUN: not %run %t l 2>&1 | FileCheck %s --check-prefix=CHECK-LOAD +// RUN: not %run %t s 2>&1 | FileCheck %s --check-prefix=CHECK-STORE +// RUN: not %run %t r 2>&1 | FileCheck %s --check-prefix=CHECK-REFERENCE +// RUN: not %run %t m 2>&1 | FileCheck %s --check-prefix=CHECK-MEMBER +// RUN: not %run %t f 2>&1 | FileCheck %s --check-prefix=CHECK-MEMFUN +// RUN: not %run %t t 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL +// RUN: not %run %t u 2>&1 | FileCheck %s --check-prefix=CHECK-VCALL2 struct S { int f() { return 0; } int k; }; +struct T { + virtual int v() { return 1; } +}; + +struct U : T { + virtual int v() { return 2; } +}; + int main(int, char **argv) { int *p = 0; S *s = 0; + T *t = 0; + U *u = 0; (void)*p; // ok! + (void)*t; // ok! + (void)*u; // ok! switch (argv[1][0]) { case 'l': @@ -34,5 +48,11 @@ case 'f': // CHECK-MEMFUN: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'S' return s->f(); + case 't': +// CHECK-VCALL: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'T' +return t->v(); + case 'u': +// CHECK-VCALL2: null.cpp:[[@LINE+1]]:15: runtime error: member call on null pointer of type 'U' +return u->v(); } } ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26560: Add a test for vcall on a null ptr.
krasin added a comment. Ping. https://reviews.llvm.org/D26560 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26559: Insert a type check before reading vtable.
krasin added a comment. All UBSan test cases happen to live in compiler-rt. I have added a couple of test cases in https://reviews.llvm.org/D26560. Also, I am not against adding Clang counterparts for them which would test IL instead of the output from running. But it probably deserves a separate CL, as it has nothing to do with fixing this particular bug. https://reviews.llvm.org/D26559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26559: Insert a type check before reading vtable.
krasin added a comment. Small correction: all UBSan type checks tests live in compiler-rt. There is a test for UBsan + devirtualization inside tools/clang. My point still stands. https://reviews.llvm.org/D26559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D26559: Insert a type check before reading vtable.
krasin added a comment. Sounds reasonable. Will do on Monday. https://reviews.llvm.org/D26559 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D11968: Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
krasin created this revision. krasin added subscribers: cfe-commits, pcc. Clang sanitizers, such as AddressSanitizer, ThreadSanitizer, MemorySanitizer, Control Flow Integrity and others, use blacklists to specify which types / functions should not be instrumented to avoid false positives or suppress known failures. This change adds the blacklist filenames to the list of dependencies of the rules, generated with -M/-MM/-MD/-MMD. This lets CMake/Ninja recognize that certain C/C++/ObjC files need to be recompiled (if a blacklist is updated). http://reviews.llvm.org/D11968 Files: include/clang/Frontend/DependencyOutputOptions.h lib/Frontend/CompilerInvocation.cpp lib/Frontend/DependencyFile.cpp test/Frontend/dependency-gen.c Index: test/Frontend/dependency-gen.c === --- test/Frontend/dependency-gen.c +++ test/Frontend/dependency-gen.c @@ -20,6 +20,10 @@ // RUN: cd a/b // RUN: %clang -MD -MF - %s -fsyntax-only -I ./ | FileCheck -check-prefix=CHECK-SIX %s // CHECK-SIX: {{ }}x.h +// RUN: echo "fun:foo" > %t.blacklist +// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s +// CHECK-SEVEN: {{ }}x.h +// CHECK-SEVEN: .blacklist #ifndef INCLUDE_FLAG_TEST #include Index: lib/Frontend/DependencyFile.cpp === --- lib/Frontend/DependencyFile.cpp +++ lib/Frontend/DependencyFile.cpp @@ -162,6 +162,7 @@ const Preprocessor *PP; std::string OutputFile; std::vector Targets; + std::vector ExtraDeps; bool IncludeSystemHeaders; bool PhonyTarget; bool AddMissingHeaderDeps; @@ -177,6 +178,7 @@ public: DFGImpl(const Preprocessor *_PP, const DependencyOutputOptions &Opts) : PP(_PP), OutputFile(Opts.OutputFile), Targets(Opts.Targets), + ExtraDeps(Opts.ExtraDeps), IncludeSystemHeaders(Opts.IncludeSystemHeaders), PhonyTarget(Opts.UsePhonyTargets), AddMissingHeaderDeps(Opts.AddMissingHeaderDeps), @@ -411,6 +413,11 @@ return; } + // Add extra dependencies to the end of the list. + for (auto ExtraDep : ExtraDeps) { +AddFilename(ExtraDep); + } + // Write out the dependency targets, trying to avoid overly long // lines when possible. We try our best to emit exactly the same // dependency file as GCC (4.2), assuming the included files are the Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -705,6 +705,10 @@ Args.getLastArgValue(OPT_module_dependency_dir); if (Args.hasArg(OPT_MV)) Opts.OutputFormat = DependencyOutputFormat::NMake; + // Add sanitizer blacklists as extra dependencies. + // They won't be discovered by the regular preprocessor, so + // we let make / ninja to know about this implicit dependency. + Opts.ExtraDeps = Args.getAllArgValues(OPT_fsanitize_blacklist); } bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args, Index: include/clang/Frontend/DependencyOutputOptions.h === --- include/clang/Frontend/DependencyOutputOptions.h +++ include/clang/Frontend/DependencyOutputOptions.h @@ -47,6 +47,9 @@ /// must contain at least one entry. std::vector Targets; + /// A list of filenames to be used as extra dependencies for every target. + std::vector ExtraDeps; + /// \brief The file to write GraphViz-formatted header dependencies to. std::string DOTOutputFile; Index: test/Frontend/dependency-gen.c === --- test/Frontend/dependency-gen.c +++ test/Frontend/dependency-gen.c @@ -20,6 +20,10 @@ // RUN: cd a/b // RUN: %clang -MD -MF - %s -fsyntax-only -I ./ | FileCheck -check-prefix=CHECK-SIX %s // CHECK-SIX: {{ }}x.h +// RUN: echo "fun:foo" > %t.blacklist +// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s +// CHECK-SEVEN: {{ }}x.h +// CHECK-SEVEN: .blacklist #ifndef INCLUDE_FLAG_TEST #include Index: lib/Frontend/DependencyFile.cpp === --- lib/Frontend/DependencyFile.cpp +++ lib/Frontend/DependencyFile.cpp @@ -162,6 +162,7 @@ const Preprocessor *PP; std::string OutputFile; std::vector Targets; + std::vector ExtraDeps; bool IncludeSystemHeaders; bool PhonyTarget; bool AddMissingHeaderDeps; @@ -177,6 +178,7 @@ public: DFGImpl(const Preprocessor *_PP, const DependencyOutputOptions &Opts) : PP(_PP), OutputFile(Opts.OutputFile), Targets(Opts.Targets), + ExtraDeps(Opts.ExtraDeps), IncludeSystemHeaders(Opts.IncludeSystemHeaders), PhonyTarget(Opts.UsePhonyTargets), AddMissingHeaderDep
Re: [PATCH] D11968: Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
krasin added a comment. In http://reviews.llvm.org/D11968#222338, @pcc wrote: > We should also make blacklists appear in the `--show-includes` output. Is it about Windows compatibility? Do you mean that the output of bin/clang-cl /Zs /showIncludes ~/lala.cc -fsanitize=address should include the default blacklist (and any explicit blacklists, if they were specified)? Comment at: lib/Frontend/DependencyFile.cpp:416-420 @@ -413,2 +415,7 @@ + // Add extra dependencies to the end of the list. + for (auto ExtraDep : ExtraDeps) { +AddFilename(ExtraDep); + } + // Write out the dependency targets, trying to avoid overly long pcc wrote: > If you move this code to the constructor you won't need to add an extra data > member to this class. I considered that. In this case, extra deps will appear even before the source file itself. Right now, for my local test code, it generates: ``` lala.o: /usr/local/google/home/krasin/lala.cc \ /usr/local/google/home/krasin/foo.h \ /usr/local/google/home/krasin/bar.h \ /usr/local/google/home/krasin/blacklist.txt ``` If moved to the constructor, it will become: ``` lala.o: /usr/local/google/home/krasin/blacklist.txt \ /usr/local/google/home/krasin/lala.cc \ /usr/local/google/home/krasin/foo.h \ /usr/local/google/home/krasin/bar.h ``` This decreases the readability of the generated rules, as the main source becomes harder to discover. Please, let me know, if you think it's not a problem; I will move the code to the constructor and eliminate the extra member. http://reviews.llvm.org/D11968 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11968: Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
krasin updated this revision to Diff 31895. krasin marked an inline comment as done. krasin added a comment. Add more test cases. http://reviews.llvm.org/D11968 Files: include/clang/Frontend/DependencyOutputOptions.h lib/Frontend/CompilerInvocation.cpp lib/Frontend/DependencyFile.cpp test/Frontend/dependency-gen.c Index: test/Frontend/dependency-gen.c === --- test/Frontend/dependency-gen.c +++ test/Frontend/dependency-gen.c @@ -20,7 +20,16 @@ // RUN: cd a/b // RUN: %clang -MD -MF - %s -fsyntax-only -I ./ | FileCheck -check-prefix=CHECK-SIX %s // CHECK-SIX: {{ }}x.h - +// RUN: echo "fun:foo" > %t.blacklist +// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s +// CHECK-SEVEN: {{ }}x.h +// CHECK-SEVEN: .blacklist +// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . | FileCheck -check-prefix=CHECK-EIGHT %s +// CHECK-EIGHT: {{ }}x.h +// CHECK-EIGHT: asan_blacklist.txt +// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . -fno-sanitize-blacklist | FileCheck -check-prefix=CHECK-NINE %s +// CHECK-NINE: {{ }}x.h +// CHECK-NINE-NOT: asan_blacklist.txt #ifndef INCLUDE_FLAG_TEST #include #endif Index: lib/Frontend/DependencyFile.cpp === --- lib/Frontend/DependencyFile.cpp +++ lib/Frontend/DependencyFile.cpp @@ -162,6 +162,7 @@ const Preprocessor *PP; std::string OutputFile; std::vector Targets; + std::vector ExtraDeps; bool IncludeSystemHeaders; bool PhonyTarget; bool AddMissingHeaderDeps; @@ -177,6 +178,7 @@ public: DFGImpl(const Preprocessor *_PP, const DependencyOutputOptions &Opts) : PP(_PP), OutputFile(Opts.OutputFile), Targets(Opts.Targets), + ExtraDeps(Opts.ExtraDeps), IncludeSystemHeaders(Opts.IncludeSystemHeaders), PhonyTarget(Opts.UsePhonyTargets), AddMissingHeaderDeps(Opts.AddMissingHeaderDeps), @@ -411,6 +413,11 @@ return; } + // Add extra dependencies to the end of the list. + for (auto ExtraDep : ExtraDeps) { +AddFilename(ExtraDep); + } + // Write out the dependency targets, trying to avoid overly long // lines when possible. We try our best to emit exactly the same // dependency file as GCC (4.2), assuming the included files are the Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -705,6 +705,10 @@ Args.getLastArgValue(OPT_module_dependency_dir); if (Args.hasArg(OPT_MV)) Opts.OutputFormat = DependencyOutputFormat::NMake; + // Add sanitizer blacklists as extra dependencies. + // They won't be discovered by the regular preprocessor, so + // we let make / ninja to know about this implicit dependency. + Opts.ExtraDeps = Args.getAllArgValues(OPT_fsanitize_blacklist); } bool clang::ParseDiagnosticArgs(DiagnosticOptions &Opts, ArgList &Args, Index: include/clang/Frontend/DependencyOutputOptions.h === --- include/clang/Frontend/DependencyOutputOptions.h +++ include/clang/Frontend/DependencyOutputOptions.h @@ -47,6 +47,9 @@ /// must contain at least one entry. std::vector Targets; + /// A list of filenames to be used as extra dependencies for every target. + std::vector ExtraDeps; + /// \brief The file to write GraphViz-formatted header dependencies to. std::string DOTOutputFile; Index: test/Frontend/dependency-gen.c === --- test/Frontend/dependency-gen.c +++ test/Frontend/dependency-gen.c @@ -20,7 +20,16 @@ // RUN: cd a/b // RUN: %clang -MD -MF - %s -fsyntax-only -I ./ | FileCheck -check-prefix=CHECK-SIX %s // CHECK-SIX: {{ }}x.h - +// RUN: echo "fun:foo" > %t.blacklist +// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s +// CHECK-SEVEN: {{ }}x.h +// CHECK-SEVEN: .blacklist +// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . | FileCheck -check-prefix=CHECK-EIGHT %s +// CHECK-EIGHT: {{ }}x.h +// CHECK-EIGHT: asan_blacklist.txt +// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . -fno-sanitize-blacklist | FileCheck -check-prefix=CHECK-NINE %s +// CHECK-NINE: {{ }}x.h +// CHECK-NINE-NOT: asan_blacklist.txt #ifndef INCLUDE_FLAG_TEST #include #endif Index: lib/Frontend/DependencyFile.cpp === --- lib/Frontend/DependencyFile.cpp +++ lib/Frontend/DependencyFile.cpp @@ -162,6 +162,7 @@ const Preprocessor *PP; std::string OutputFile; std::vector Targets; + std::vector ExtraDeps; bool IncludeSystemHeaders; bool PhonyTarget;
Re: [PATCH] D11968: Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
krasin updated this revision to Diff 31979. krasin added a comment. Windows compat http://reviews.llvm.org/D11968 Files: include/clang/Frontend/DependencyOutputOptions.h include/clang/Frontend/Utils.h lib/Frontend/CompilerInstance.cpp lib/Frontend/CompilerInvocation.cpp lib/Frontend/DependencyFile.cpp lib/Frontend/HeaderIncludeGen.cpp test/Frontend/dependency-gen.c Index: test/Frontend/dependency-gen.c === --- test/Frontend/dependency-gen.c +++ test/Frontend/dependency-gen.c @@ -20,7 +20,16 @@ // RUN: cd a/b // RUN: %clang -MD -MF - %s -fsyntax-only -I ./ | FileCheck -check-prefix=CHECK-SIX %s // CHECK-SIX: {{ }}x.h - +// RUN: echo "fun:foo" > %t.blacklist +// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s +// CHECK-SEVEN: {{ }}x.h +// CHECK-SEVEN: .blacklist +// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . | FileCheck -check-prefix=CHECK-EIGHT %s +// CHECK-EIGHT: {{ }}x.h +// CHECK-EIGHT: asan_blacklist.txt +// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . -fno-sanitize-blacklist | FileCheck -check-prefix=CHECK-NINE %s +// CHECK-NINE: {{ }}x.h +// CHECK-NINE-NOT: asan_blacklist.txt #ifndef INCLUDE_FLAG_TEST #include #endif Index: lib/Frontend/HeaderIncludeGen.cpp === --- lib/Frontend/HeaderIncludeGen.cpp +++ lib/Frontend/HeaderIncludeGen.cpp @@ -46,7 +46,36 @@ }; } -void clang::AttachHeaderIncludeGen(Preprocessor &PP, bool ShowAllHeaders, +static void PrintHeaderInfo(raw_ostream *OutputFile, const char* Filename, +bool ShowDepth, unsigned CurrentIncludeDepth, +bool MSStyle) { +// Write to a temporary string to avoid unnecessary flushing on errs(). +SmallString<512> Pathname(Filename); +if (!MSStyle) + Lexer::Stringify(Pathname); + +SmallString<256> Msg; +if (MSStyle) + Msg += "Note: including file:"; + +if (ShowDepth) { + // The main source file is at depth 1, so skip one dot. + for (unsigned i = 1; i != CurrentIncludeDepth; ++i) +Msg += MSStyle ? ' ' : '.'; + + if (!MSStyle) +Msg += ' '; +} +Msg += Pathname; +Msg += '\n'; + +OutputFile->write(Msg.data(), Msg.size()); +OutputFile->flush(); +} + +void clang::AttachHeaderIncludeGen(Preprocessor &PP, + const std::vector &ExtraHeaders, + bool ShowAllHeaders, StringRef OutputPath, bool ShowDepth, bool MSStyle) { raw_ostream *OutputFile = MSStyle ? &llvm::outs() : &llvm::errs(); @@ -69,6 +98,14 @@ } } + // Print header info for extra headers, pretending they were discovered + // by the regular preprocessor. The primary use case is to support + // proper generation of Make / Ninja file dependencies for implicit includes, + // such as sanitizer blacklists. It's only important for cl.exe + // compatibility, the GNU way to generate rules is -M / -MM / -MD / -MMD. + for (auto Header : ExtraHeaders) { +PrintHeaderInfo(OutputFile, Header.c_str(), ShowDepth, 2, MSStyle); + } PP.addPPCallbacks(llvm::make_unique(&PP, ShowAllHeaders, OutputFile, @@ -112,27 +149,7 @@ // Dump the header include information we are past the predefines buffer or // are showing all headers. if (ShowHeader && Reason == PPCallbacks::EnterFile) { -// Write to a temporary string to avoid unnecessary flushing on errs(). -SmallString<512> Filename(UserLoc.getFilename()); -if (!MSStyle) - Lexer::Stringify(Filename); - -SmallString<256> Msg; -if (MSStyle) - Msg += "Note: including file:"; - -if (ShowDepth) { - // The main source file is at depth 1, so skip one dot. - for (unsigned i = 1; i != CurrentIncludeDepth; ++i) -Msg += MSStyle ? ' ' : '.'; - - if (!MSStyle) -Msg += ' '; -} -Msg += Filename; -Msg += '\n'; - -OutputFile->write(Msg.data(), Msg.size()); -OutputFile->flush(); +PrintHeaderInfo(OutputFile, UserLoc.getFilename(), +ShowDepth, CurrentIncludeDepth, MSStyle); } } Index: lib/Frontend/DependencyFile.cpp === --- lib/Frontend/DependencyFile.cpp +++ lib/Frontend/DependencyFile.cpp @@ -182,7 +182,11 @@ AddMissingHeaderDeps(Opts.AddMissingHeaderDeps), SeenMissingHeader(false), IncludeModuleFiles(Opts.IncludeModuleFiles), - OutputFormat(Opts.OutputFormat) {} + OutputFormat(Opts.OutputFormat) { +for (auto ExtraDep : Opts.ExtraDeps) { + AddFilename(Extra
Re: [PATCH] D11968: Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
krasin marked an inline comment as done. krasin added a comment. Please, take another look. The test for --show-includes is on the way. http://reviews.llvm.org/D11968 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11968: Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
krasin updated this revision to Diff 31983. krasin added a comment. More tests / fix tests. http://reviews.llvm.org/D11968 Files: include/clang/Frontend/DependencyOutputOptions.h include/clang/Frontend/Utils.h lib/Frontend/CompilerInstance.cpp lib/Frontend/CompilerInvocation.cpp lib/Frontend/DependencyFile.cpp lib/Frontend/HeaderIncludeGen.cpp test/Frontend/dependency-gen.c test/Frontend/print-header-includes.c Index: test/Frontend/print-header-includes.c === --- test/Frontend/print-header-includes.c +++ test/Frontend/print-header-includes.c @@ -13,4 +13,12 @@ // MS: Note: including file: {{.*test2.h}} // MS-NOT: Note +// RUN: echo "fun:foo" > %t.blacklist +// RUN: %clang_cc1 -fsanitize=address -fsanitize-blacklist=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout +// RUN: FileCheck --check-prefix=MS-BLACKLIST < %t.stdout %s +// MS-BLACKLIST: Note: including file: {{.*\.blacklist}} +// MS-BLACKLIST: Note: including file: {{.*test.h}} +// MS-BLACKLIST: Note: including file: {{.*test2.h}} +// MS-BLACKLIST-NOT: Note + #include "Inputs/test.h" Index: test/Frontend/dependency-gen.c === --- test/Frontend/dependency-gen.c +++ test/Frontend/dependency-gen.c @@ -20,7 +20,16 @@ // RUN: cd a/b // RUN: %clang -MD -MF - %s -fsyntax-only -I ./ | FileCheck -check-prefix=CHECK-SIX %s // CHECK-SIX: {{ }}x.h - +// RUN: echo "fun:foo" > %t.blacklist +// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s +// CHECK-SEVEN: .blacklist +// CHECK-SEVEN: {{ }}x.h +// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . | FileCheck -check-prefix=CHECK-EIGHT %s +// CHECK-EIGHT: asan_blacklist.txt +// CHECK-EIGHT: {{ }}x.h +// RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . -fno-sanitize-blacklist | FileCheck -check-prefix=CHECK-NINE %s +// CHECK-NINE-NOT: asan_blacklist.txt +// CHECK-NINE: {{ }}x.h #ifndef INCLUDE_FLAG_TEST #include #endif Index: lib/Frontend/HeaderIncludeGen.cpp === --- lib/Frontend/HeaderIncludeGen.cpp +++ lib/Frontend/HeaderIncludeGen.cpp @@ -46,7 +46,36 @@ }; } -void clang::AttachHeaderIncludeGen(Preprocessor &PP, bool ShowAllHeaders, +static void PrintHeaderInfo(raw_ostream *OutputFile, const char* Filename, +bool ShowDepth, unsigned CurrentIncludeDepth, +bool MSStyle) { +// Write to a temporary string to avoid unnecessary flushing on errs(). +SmallString<512> Pathname(Filename); +if (!MSStyle) + Lexer::Stringify(Pathname); + +SmallString<256> Msg; +if (MSStyle) + Msg += "Note: including file:"; + +if (ShowDepth) { + // The main source file is at depth 1, so skip one dot. + for (unsigned i = 1; i != CurrentIncludeDepth; ++i) +Msg += MSStyle ? ' ' : '.'; + + if (!MSStyle) +Msg += ' '; +} +Msg += Pathname; +Msg += '\n'; + +OutputFile->write(Msg.data(), Msg.size()); +OutputFile->flush(); +} + +void clang::AttachHeaderIncludeGen(Preprocessor &PP, + const std::vector &ExtraHeaders, + bool ShowAllHeaders, StringRef OutputPath, bool ShowDepth, bool MSStyle) { raw_ostream *OutputFile = MSStyle ? &llvm::outs() : &llvm::errs(); @@ -69,6 +98,14 @@ } } + // Print header info for extra headers, pretending they were discovered + // by the regular preprocessor. The primary use case is to support + // proper generation of Make / Ninja file dependencies for implicit includes, + // such as sanitizer blacklists. It's only important for cl.exe + // compatibility, the GNU way to generate rules is -M / -MM / -MD / -MMD. + for (auto Header : ExtraHeaders) { +PrintHeaderInfo(OutputFile, Header.c_str(), ShowDepth, 2, MSStyle); + } PP.addPPCallbacks(llvm::make_unique(&PP, ShowAllHeaders, OutputFile, @@ -112,27 +149,7 @@ // Dump the header include information we are past the predefines buffer or // are showing all headers. if (ShowHeader && Reason == PPCallbacks::EnterFile) { -// Write to a temporary string to avoid unnecessary flushing on errs(). -SmallString<512> Filename(UserLoc.getFilename()); -if (!MSStyle) - Lexer::Stringify(Filename); - -SmallString<256> Msg; -if (MSStyle) - Msg += "Note: including file:"; - -if (ShowDepth) { - // The main source file is at depth 1, so skip one dot. - for (unsigned i = 1; i != CurrentIncludeDepth; ++i) -Msg += MSStyle ? ' ' : '.'; - - if (!MSSt
Re: [PATCH] D11968: Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
krasin added a comment. Thank you, Peter. I will commit once I have restored my password (the email to Chris is sent) http://reviews.llvm.org/D11968 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r244867 - Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
Author: krasin Date: Wed Aug 12 23:04:37 2015 New Revision: 244867 URL: http://llvm.org/viewvc/llvm-project?rev=244867&view=rev Log: Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD. Summary: Clang sanitizers, such as AddressSanitizer, ThreadSanitizer, MemorySanitizer, Control Flow Integrity and others, use blacklists to specify which types / functions should not be instrumented to avoid false positives or suppress known failures. This change adds the blacklist filenames to the list of dependencies of the rules, generated with -M/-MM/-MD/-MMD. This lets CMake/Ninja recognize that certain C/C++/ObjC files need to be recompiled (if a blacklist is updated). Reviewers: pcc Subscribers: rsmith, honggyu.kim, pcc, cfe-commits Differential Revision: http://reviews.llvm.org/D11968 Modified: cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h cfe/trunk/include/clang/Frontend/Utils.h cfe/trunk/lib/Frontend/CompilerInstance.cpp cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/lib/Frontend/DependencyFile.cpp cfe/trunk/lib/Frontend/HeaderIncludeGen.cpp cfe/trunk/test/Frontend/dependency-gen.c cfe/trunk/test/Frontend/print-header-includes.c Modified: cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h?rev=244867&r1=244866&r2=244867&view=diff == --- cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h (original) +++ cfe/trunk/include/clang/Frontend/DependencyOutputOptions.h Wed Aug 12 23:04:37 2015 @@ -47,6 +47,9 @@ public: /// must contain at least one entry. std::vector Targets; + /// A list of filenames to be used as extra dependencies for every target. + std::vector ExtraDeps; + /// \brief The file to write GraphViz-formatted header dependencies to. std::string DOTOutputFile; Modified: cfe/trunk/include/clang/Frontend/Utils.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/Utils.h?rev=244867&r1=244866&r2=244867&view=diff == --- cfe/trunk/include/clang/Frontend/Utils.h (original) +++ cfe/trunk/include/clang/Frontend/Utils.h Wed Aug 12 23:04:37 2015 @@ -148,6 +148,9 @@ public: /// AttachHeaderIncludeGen - Create a header include list generator, and attach /// it to the given preprocessor. /// +/// \param ExtraHeaders - If not empty, will write the header filenames, just +/// like they were included during a regular preprocessing. Useful for +/// implicit include dependencies, like sanitizer blacklists. /// \param ShowAllHeaders - If true, show all header information instead of just /// headers following the predefines buffer. This is useful for making sure /// includes mentioned on the command line are also reported, but differs from @@ -156,7 +159,9 @@ public: /// information to, instead of writing to stderr. /// \param ShowDepth - Whether to indent to show the nesting of the includes. /// \param MSStyle - Whether to print in cl.exe /showIncludes style. -void AttachHeaderIncludeGen(Preprocessor &PP, bool ShowAllHeaders = false, +void AttachHeaderIncludeGen(Preprocessor &PP, +const std::vector &ExtraHeaders, +bool ShowAllHeaders = false, StringRef OutputPath = "", bool ShowDepth = true, bool MSStyle = false); Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=244867&r1=244866&r2=244867&view=diff == --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original) +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Wed Aug 12 23:04:37 2015 @@ -354,17 +354,19 @@ void CompilerInstance::createPreprocesso // Handle generating header include information, if requested. if (DepOpts.ShowHeaderIncludes) -AttachHeaderIncludeGen(*PP); +AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps); if (!DepOpts.HeaderIncludeOutputFile.empty()) { StringRef OutputPath = DepOpts.HeaderIncludeOutputFile; if (OutputPath == "-") OutputPath = ""; -AttachHeaderIncludeGen(*PP, /*ShowAllHeaders=*/true, OutputPath, +AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps, + /*ShowAllHeaders=*/true, OutputPath, /*ShowDepth=*/false); } if (DepOpts.PrintShowIncludes) { -AttachHeaderIncludeGen(*PP, /*ShowAllHeaders=*/false, /*OutputPath=*/"", +AttachHeaderIncludeGen(*PP, DepOpts.ExtraDeps, + /*ShowAllHeaders=*/false, /*OutputPath=*/"", /*ShowDepth=*/true, /*MSStyle=*/true); } } Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp URL: ht
Re: r244867 - Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
Thank you, Yaron. Sorry for breaking the build, it obviously passed locally and I didn't think about all the variety of the supported configs. :( Does LLVM have try bots, so that I can run the tests with my patch before committing it? krasin On Thu, Aug 13, 2015 at 3:40 AM, Renato Golin wrote: > On 13 August 2015 at 07:15, Yaron Keren via cfe-commits > wrote: > > CHECK-EIGHT is failing bots, see > > > > > http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/24306/steps/check-all/logs/FAIL%3A%20Clang%3A%3Adependency-gen.c > > > That check only works if compiler-rt is built in, and supports the > target. I wouldn't put that in a Clang test. > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r244867 - Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
Thank you, Yaron. Understood. On Thu, Aug 13, 2015 at 11:04 AM, Yaron Keren wrote: > There is great variety of OSs, compilers, configs in the bots. You can't > get the smae with try bots unless everything is duplicated which is a > waste. You test locally, commit, watch the bots here: > > http://lab.llvm.org:8011/grid > > or wait for buildbot failure e-mails which arrive a bit slower. > Breaks may happen even if you test locally, so it's critical be around > after the commit to to resolve such issues. > > http://llvm.org/docs/DeveloperPolicy.html#quality > > > > 2015-08-13 20:51 GMT+03:00 Ivan Krasin : > >> Thank you, Yaron. >> >> Sorry for breaking the build, it obviously passed locally and I didn't >> think about all the variety of the supported configs. :( >> >> Does LLVM have try bots, so that I can run the tests with my patch before >> committing it? >> >> krasin >> > > On Thu, Aug 13, 2015 at 3:40 AM, Renato Golin > wrote: > >> On 13 August 2015 at 07:15, Yaron Keren via cfe-commits >> wrote: >> > CHECK-EIGHT is failing bots, see >> > >> > >> http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/24306/steps/check-all/logs/FAIL%3A%20Clang%3A%3Adependency-gen.c >> >> >> That check only works if compiler-rt is built in, and supports the >> target. I wouldn't put that in a Clang test. >> > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r244867 - Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
Hi there, I will remove both test cases, which rely on the existence of the default blacklist. It's handled by the driver anyway, and the code I changed is in the frontend; at this point, there's no such a thing, as a default blacklist, all of them are explicit. Just a sec. krasin On Thu, Aug 13, 2015 at 4:09 PM, NAKAMURA Takumi wrote: > Tweaked a test in r244970. > Could you split it out if you would like to run it for the default target? > On Fri, Aug 14, 2015 at 5:07 AM Ivan Krasin via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Thank you, Yaron. Understood. >> >> On Thu, Aug 13, 2015 at 11:04 AM, Yaron Keren >> wrote: >> >>> There is great variety of OSs, compilers, configs in the bots. You can't >>> get the smae with try bots unless everything is duplicated which is a >>> waste. You test locally, commit, watch the bots here: >>> >>> http://lab.llvm.org:8011/grid >>> >>> or wait for buildbot failure e-mails which arrive a bit slower. >>> Breaks may happen even if you test locally, so it's critical be around >>> after the commit to to resolve such issues. >>> >>> http://llvm.org/docs/DeveloperPolicy.html#quality >>> >>> >>> >>> 2015-08-13 20:51 GMT+03:00 Ivan Krasin : >>> >>>> Thank you, Yaron. >>>> >>>> Sorry for breaking the build, it obviously passed locally and I didn't >>>> think about all the variety of the supported configs. :( >>>> >>>> Does LLVM have try bots, so that I can run the tests with my patch >>>> before committing it? >>>> >>>> krasin >>>> >>> >>> On Thu, Aug 13, 2015 at 3:40 AM, Renato Golin >>> wrote: >>> >>>> On 13 August 2015 at 07:15, Yaron Keren via cfe-commits >>>> wrote: >>>> > CHECK-EIGHT is failing bots, see >>>> > >>>> > >>>> http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/24306/steps/check-all/logs/FAIL%3A%20Clang%3A%3Adependency-gen.c >>>> >>>> >>>> That check only works if compiler-rt is built in, and supports the >>>> target. I wouldn't put that in a Clang test. >>>> >>> >>> >> ___ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D12021: Remove test cases, which rely on the default sanitizer blacklists.The default blacklists may vary across different architectures andconfigurations. It was not wise to include into http
krasin created this revision. krasin added a reviewer: chapuni. krasin added subscribers: pcc, cfe-commits. http://reviews.llvm.org/D12021 Files: test/Frontend/dependency-gen.c Index: test/Frontend/dependency-gen.c === --- test/Frontend/dependency-gen.c +++ test/Frontend/dependency-gen.c @@ -24,11 +24,6 @@ // RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s // CHECK-SEVEN: .blacklist // CHECK-SEVEN: {{ }}x.h -// RUN: %clang -target x86_64-linux-gnu -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . | FileCheck -check-prefix=CHECK-EIGHT %s -// CHECK-EIGHT: {{ }}x.h -// RUN: %clang -target x86_64-linux-gnu -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . -fno-sanitize-blacklist | FileCheck -check-prefix=CHECK-NINE %s -// CHECK-NINE-NOT: asan_blacklist.txt -// CHECK-NINE: {{ }}x.h #ifndef INCLUDE_FLAG_TEST #include #endif Index: test/Frontend/dependency-gen.c === --- test/Frontend/dependency-gen.c +++ test/Frontend/dependency-gen.c @@ -24,11 +24,6 @@ // RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s // CHECK-SEVEN: .blacklist // CHECK-SEVEN: {{ }}x.h -// RUN: %clang -target x86_64-linux-gnu -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . | FileCheck -check-prefix=CHECK-EIGHT %s -// CHECK-EIGHT: {{ }}x.h -// RUN: %clang -target x86_64-linux-gnu -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . -fno-sanitize-blacklist | FileCheck -check-prefix=CHECK-NINE %s -// CHECK-NINE-NOT: asan_blacklist.txt -// CHECK-NINE: {{ }}x.h #ifndef INCLUDE_FLAG_TEST #include #endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r244867 - Add sanitizer blacklists to the rules generated with -M/-MM/-MD/-MMD.
I have created http://reviews.llvm.org/D12021, please take a look. On Thu, Aug 13, 2015 at 4:13 PM, Ivan Krasin wrote: > Hi there, > > I will remove both test cases, which rely on the existence of the default > blacklist. It's handled by the driver anyway, and the code I changed is in > the frontend; at this point, there's no such a thing, as a default > blacklist, all of them are explicit. > Just a sec. > > krasin > > On Thu, Aug 13, 2015 at 4:09 PM, NAKAMURA Takumi > wrote: > >> Tweaked a test in r244970. >> Could you split it out if you would like to run it for the default target? >> On Fri, Aug 14, 2015 at 5:07 AM Ivan Krasin via cfe-commits < >> cfe-commits@lists.llvm.org> wrote: >> >>> Thank you, Yaron. Understood. >>> >>> On Thu, Aug 13, 2015 at 11:04 AM, Yaron Keren >>> wrote: >>> >>>> There is great variety of OSs, compilers, configs in the bots. You >>>> can't get the smae with try bots unless everything is duplicated which is a >>>> waste. You test locally, commit, watch the bots here: >>>> >>>> http://lab.llvm.org:8011/grid >>>> >>>> or wait for buildbot failure e-mails which arrive a bit slower. >>>> Breaks may happen even if you test locally, so it's critical be around >>>> after the commit to to resolve such issues. >>>> >>>> http://llvm.org/docs/DeveloperPolicy.html#quality >>>> >>>> >>>> >>>> 2015-08-13 20:51 GMT+03:00 Ivan Krasin : >>>> >>>>> Thank you, Yaron. >>>>> >>>>> Sorry for breaking the build, it obviously passed locally and I didn't >>>>> think about all the variety of the supported configs. :( >>>>> >>>>> Does LLVM have try bots, so that I can run the tests with my patch >>>>> before committing it? >>>>> >>>>> krasin >>>>> >>>> >>>> On Thu, Aug 13, 2015 at 3:40 AM, Renato Golin >>>> wrote: >>>> >>>>> On 13 August 2015 at 07:15, Yaron Keren via cfe-commits >>>>> wrote: >>>>> > CHECK-EIGHT is failing bots, see >>>>> > >>>>> > >>>>> http://lab.llvm.org:8011/builders/clang-x86_64-ubuntu-gdb-75/builds/24306/steps/check-all/logs/FAIL%3A%20Clang%3A%3Adependency-gen.c >>>>> >>>>> >>>>> That check only works if compiler-rt is built in, and supports the >>>>> target. I wouldn't put that in a Clang test. >>>>> >>>> >>>> >>> ___ >>> cfe-commits mailing list >>> cfe-commits@lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>> >> > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12021: Remove test cases, which rely on the default sanitizer blacklists.
krasin added a comment. In http://reviews.llvm.org/D12021#224102, @pcc wrote: > You could create a fake resource directory which contains only the > `asan_blacklist.txt` file. See `test/Driver/Inputs/resource_dir` for an > example of this kind of thing. I believe it's unnecessary. The default blacklists are handled in https://github.com/llvm-mirror/clang/blob/82a8792ad361e9443615fc81ee8dd37c76e64dbd/lib/Driver/SanitizerArgs.cpp#L82, and the code I added is in the frontend. The tests I am removing are not really testing that blacklists get into the deps. They test that default blacklists are explicitly passed from the toolchain to the frontend. If there's no such a test, I could add it in a separate CL. http://reviews.llvm.org/D12021 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r244985 - Remove test cases, which rely on the default sanitizer blacklists.
Author: krasin Date: Thu Aug 13 18:37:28 2015 New Revision: 244985 URL: http://llvm.org/viewvc/llvm-project?rev=244985&view=rev Log: Remove test cases, which rely on the default sanitizer blacklists. Summary: The default blacklists may vary across different architectures and configurations. It was not wise to include into http://reviews.llvm.org/D11968 Reviewers: chapuni, pcc Subscribers: cfe-commits, pcc Differential Revision: http://reviews.llvm.org/D12021 Modified: cfe/trunk/test/Frontend/dependency-gen.c Modified: cfe/trunk/test/Frontend/dependency-gen.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/dependency-gen.c?rev=244985&r1=244984&r2=244985&view=diff == --- cfe/trunk/test/Frontend/dependency-gen.c (original) +++ cfe/trunk/test/Frontend/dependency-gen.c Thu Aug 13 18:37:28 2015 @@ -24,11 +24,6 @@ // RUN: %clang -MD -MF - %s -fsyntax-only -fsanitize=cfi-vcall -flto -fsanitize-blacklist=%t.blacklist -I ./ | FileCheck -check-prefix=CHECK-SEVEN %s // CHECK-SEVEN: .blacklist // CHECK-SEVEN: {{ }}x.h -// RUN: %clang -target x86_64-linux-gnu -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . | FileCheck -check-prefix=CHECK-EIGHT %s -// CHECK-EIGHT: {{ }}x.h -// RUN: %clang -target x86_64-linux-gnu -MD -MF - %s -fsyntax-only -fsanitize=address -flto -I . -fno-sanitize-blacklist | FileCheck -check-prefix=CHECK-NINE %s -// CHECK-NINE-NOT: asan_blacklist.txt -// CHECK-NINE: {{ }}x.h #ifndef INCLUDE_FLAG_TEST #include #endif ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D12544: Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.
krasin created this revision. krasin added reviewers: pcc, rsmith. krasin added a subscriber: cfe-commits. Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output. Introduce a frontend option -fdepfile-entry, and only insert them for the user-defined sanitizer blacklists. In frontend, grab ExtraDeps from -fdepfile-entry, instead of -fsanitize-blacklist. http://reviews.llvm.org/D12544 Files: include/clang/Driver/Options.td include/clang/Driver/SanitizerArgs.h lib/Driver/SanitizerArgs.cpp lib/Frontend/CompilerInvocation.cpp test/Driver/fsanitize-blacklist.c test/Frontend/print-header-includes.c Index: test/Frontend/print-header-includes.c === --- test/Frontend/print-header-includes.c +++ test/Frontend/print-header-includes.c @@ -14,7 +14,7 @@ // MS-NOT: Note // RUN: echo "fun:foo" > %t.blacklist -// RUN: %clang_cc1 -fsanitize=address -fsanitize-blacklist=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout +// RUN: %clang_cc1 -fsanitize=address -fdepfile-entry=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout // RUN: FileCheck --check-prefix=MS-BLACKLIST < %t.stdout %s // MS-BLACKLIST: Note: including file: {{.*\.blacklist}} // MS-BLACKLIST: Note: including file: {{.*test.h}} Index: test/Driver/fsanitize-blacklist.c === --- test/Driver/fsanitize-blacklist.c +++ test/Driver/fsanitize-blacklist.c @@ -13,8 +13,7 @@ // RUN: echo "badline" > %t.bad // RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST -// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good -// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.second +// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good" "-fsanitize-blacklist={{.*}}.second // Ignore -fsanitize-blacklist flag if there is no -fsanitize flag. // RUN: %clang -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE --check-prefix=DELIMITERS Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -709,7 +709,7 @@ // Add sanitizer blacklists as extra dependencies. // They won't be discovered by the regular preprocessor, so // we let make / ninja to know about this implicit dependency. - Opts.ExtraDeps = Args.getAllArgValues(OPT_fsanitize_blacklist); + Opts.ExtraDeps = Args.getAllArgValues(OPT_fdepfile_entry); auto ModuleFiles = Args.getAllArgValues(OPT_fmodule_file); Opts.ExtraDeps.insert(Opts.ExtraDeps.end(), ModuleFiles.begin(), ModuleFiles.end()); Index: lib/Driver/SanitizerArgs.cpp === --- lib/Driver/SanitizerArgs.cpp +++ lib/Driver/SanitizerArgs.cpp @@ -176,6 +176,7 @@ RecoverableSanitizers.clear(); TrapSanitizers.clear(); BlacklistFiles.clear(); + ExtraDeps.clear(); CoverageFeatures = 0; MsanTrackOrigins = 0; MsanUseAfterDtor = false; @@ -383,13 +384,16 @@ if (Arg->getOption().matches(options::OPT_fsanitize_blacklist)) { Arg->claim(); std::string BLPath = Arg->getValue(); - if (llvm::sys::fs::exists(BLPath)) + if (llvm::sys::fs::exists(BLPath)) { BlacklistFiles.push_back(BLPath); - else +ExtraDeps.push_back(BLPath); + } else D.Diag(clang::diag::err_drv_no_such_file) << BLPath; + } else if (Arg->getOption().matches(options::OPT_fno_sanitize_blacklist)) { Arg->claim(); BlacklistFiles.clear(); + ExtraDeps.clear(); } } // Validate blacklists format. @@ -563,6 +567,11 @@ BlacklistOpt += BLPath; CmdArgs.push_back(Args.MakeArgString(BlacklistOpt)); } + for (const auto &Dep : ExtraDeps) { +SmallString<64> ExtraDepOpt("-fdepfile-entry="); +ExtraDepOpt += Dep; +CmdArgs.push_back(Args.MakeArgString(ExtraDepOpt)); + } if (MsanTrackOrigins) CmdArgs.push_back(Args.MakeArgString("-fsanitize-memory-track-origins=" + Index: include/clang/Driver/SanitizerArgs.h === --- include/clang/Driver/SanitizerArgs.h +++ include/clang/Driver/SanitizerArgs.h @@ -27,6 +27,7 @@ SanitizerSet TrapSanitizers; std::vector BlacklistFiles; + std::vector ExtraDeps; int CoverageFeatures; int MsanTrackOrigins; bool MsanUseAfterDtor; Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -495,6 +495,8 @@ Flags<[DriverOption]>; def fdebug_pass_arguments : Flag<["-"], "fdebug-pass-arguments">, Group; def fdebug_pass_structure : Flag<["-"], "fdebug-pass-structure">, Group; +def fdepfile_entry : Joined<["-"], "fdepfile-
Re: [PATCH] D12544: Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.
krasin updated this revision to Diff 33764. krasin added a comment. Add a test for default blacklist. http://reviews.llvm.org/D12544 Files: include/clang/Driver/Options.td include/clang/Driver/SanitizerArgs.h lib/Driver/SanitizerArgs.cpp lib/Frontend/CompilerInvocation.cpp test/Driver/Inputs/resource_dir/asan_blacklist.txt test/Driver/fsanitize-blacklist.c test/Frontend/print-header-includes.c Index: test/Frontend/print-header-includes.c === --- test/Frontend/print-header-includes.c +++ test/Frontend/print-header-includes.c @@ -14,7 +14,7 @@ // MS-NOT: Note // RUN: echo "fun:foo" > %t.blacklist -// RUN: %clang_cc1 -fsanitize=address -fsanitize-blacklist=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout +// RUN: %clang_cc1 -fsanitize=address -fdepfile-entry=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout // RUN: FileCheck --check-prefix=MS-BLACKLIST < %t.stdout %s // MS-BLACKLIST: Note: including file: {{.*\.blacklist}} // MS-BLACKLIST: Note: including file: {{.*test.h}} Index: test/Driver/fsanitize-blacklist.c === --- test/Driver/fsanitize-blacklist.c +++ test/Driver/fsanitize-blacklist.c @@ -13,13 +13,26 @@ // RUN: echo "badline" > %t.bad // RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST -// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good -// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.second +// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good" "-fsanitize-blacklist={{.*}}.second + +// Now, check for -fdepfile-entry flags. +// RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST2 +// CHECK-BLACKLIST2: -fdepfile-entry={{.*}}.good" "-fdepfile-entry={{.*}}.second + +// Check that the default blacklist is not added as an extra dependency. +// RUN: %clang -fsanitize=address -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-BLACKLIST +// CHECK-DEFAULT-BLACKLIST: -fsanitize-blacklist={{.*}}asan_blacklist.txt +// CHECK-DEFAULT-BLACKLIST-NOT: -fdepfile-entry // Ignore -fsanitize-blacklist flag if there is no -fsanitize flag. // RUN: %clang -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE --check-prefix=DELIMITERS // CHECK-NO-SANITIZE-NOT: -fsanitize-blacklist +// Ignore -fsanitize-blacklist flag if there is no -fsanitize flag. +// Now, check for the absense of -fdepfile-entry flags. +// RUN: %clang -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE2 --check-prefix=DELIMITERS +// CHECK-NO-SANITIZE2-NOT: -fdepfile-entry + // Flag -fno-sanitize-blacklist wins if it is specified later. // RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fno-sanitize-blacklist %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-BLACKLIST --check-prefix=DELIMITERS // CHECK-NO-BLACKLIST-NOT: -fsanitize-blacklist Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -709,7 +709,7 @@ // Add sanitizer blacklists as extra dependencies. // They won't be discovered by the regular preprocessor, so // we let make / ninja to know about this implicit dependency. - Opts.ExtraDeps = Args.getAllArgValues(OPT_fsanitize_blacklist); + Opts.ExtraDeps = Args.getAllArgValues(OPT_fdepfile_entry); auto ModuleFiles = Args.getAllArgValues(OPT_fmodule_file); Opts.ExtraDeps.insert(Opts.ExtraDeps.end(), ModuleFiles.begin(), ModuleFiles.end()); Index: lib/Driver/SanitizerArgs.cpp === --- lib/Driver/SanitizerArgs.cpp +++ lib/Driver/SanitizerArgs.cpp @@ -176,6 +176,7 @@ RecoverableSanitizers.clear(); TrapSanitizers.clear(); BlacklistFiles.clear(); + ExtraDeps.clear(); CoverageFeatures = 0; MsanTrackOrigins = 0; MsanUseAfterDtor = false; @@ -383,13 +384,16 @@ if (Arg->getOption().matches(options::OPT_fsanitize_blacklist)) { Arg->claim(); std::string BLPath = Arg->getValue(); - if (llvm::sys::fs::exists(BLPath)) + if (llvm::sys::fs::exists(BLPath)) { BlacklistFiles.push_back(BLPath); - else +ExtraDeps.push_back(BLPath); + } else D.Diag(clang::diag::err_drv_no_such_file) << BLPath; + } else if (Arg->getOption().matches(options::OPT_fno_sanitize_blacklist)) { Arg->claim(); BlacklistFiles.clear(); + ExtraDeps.clear(); } } // Validate blacklists format. @@ -563,6 +567,11 @@ BlacklistOpt += BLPath; CmdArgs.push_back(Args.MakeArgString(BlacklistOpt)); } + for (const auto &Dep : ExtraDeps
Re: [PATCH] D12544: Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.
krasin added a comment. In http://reviews.llvm.org/D12544#237819, @pcc wrote: > It's probably time to add something like the driver test I was talking about > in http://reviews.llvm.org/D12021. Without that, you will have no test > coverage for the functional change here. Done, please, take a look. http://reviews.llvm.org/D12544 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12544: Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.
krasin updated this revision to Diff 33849. krasin added a comment. Addressed a nit. http://reviews.llvm.org/D12544 Files: include/clang/Driver/Options.td include/clang/Driver/SanitizerArgs.h lib/Driver/SanitizerArgs.cpp lib/Frontend/CompilerInvocation.cpp test/Driver/Inputs/resource_dir/asan_blacklist.txt test/Driver/fsanitize-blacklist.c test/Frontend/print-header-includes.c Index: test/Frontend/print-header-includes.c === --- test/Frontend/print-header-includes.c +++ test/Frontend/print-header-includes.c @@ -14,7 +14,7 @@ // MS-NOT: Note // RUN: echo "fun:foo" > %t.blacklist -// RUN: %clang_cc1 -fsanitize=address -fsanitize-blacklist=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout +// RUN: %clang_cc1 -fsanitize=address -fdepfile-entry=%t.blacklist -E --show-includes -o %t.out %s > %t.stdout // RUN: FileCheck --check-prefix=MS-BLACKLIST < %t.stdout %s // MS-BLACKLIST: Note: including file: {{.*\.blacklist}} // MS-BLACKLIST: Note: including file: {{.*test.h}} Index: test/Driver/fsanitize-blacklist.c === --- test/Driver/fsanitize-blacklist.c +++ test/Driver/fsanitize-blacklist.c @@ -13,13 +13,25 @@ // RUN: echo "badline" > %t.bad // RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST -// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good -// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.second +// CHECK-BLACKLIST: -fsanitize-blacklist={{.*}}.good" "-fsanitize-blacklist={{.*}}.second + +// Now, check for -fdepfile-entry flags. +// RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fsanitize-blacklist=%t.second %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-BLACKLIST2 +// CHECK-BLACKLIST2: -fdepfile-entry={{.*}}.good" "-fdepfile-entry={{.*}}.second + +// Check that the default blacklist is not added as an extra dependency. +// RUN: %clang -fsanitize=address -resource-dir=%S/Inputs/resource_dir %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-DEFAULT-BLACKLIST --implicit-check-not=fdepfile-entry +// CHECK-DEFAULT-BLACKLIST: -fsanitize-blacklist={{.*}}asan_blacklist.txt // Ignore -fsanitize-blacklist flag if there is no -fsanitize flag. // RUN: %clang -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE --check-prefix=DELIMITERS // CHECK-NO-SANITIZE-NOT: -fsanitize-blacklist +// Ignore -fsanitize-blacklist flag if there is no -fsanitize flag. +// Now, check for the absense of -fdepfile-entry flags. +// RUN: %clang -fsanitize-blacklist=%t.good %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-SANITIZE2 --check-prefix=DELIMITERS +// CHECK-NO-SANITIZE2-NOT: -fdepfile-entry + // Flag -fno-sanitize-blacklist wins if it is specified later. // RUN: %clang -fsanitize=address -fsanitize-blacklist=%t.good -fno-sanitize-blacklist %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-BLACKLIST --check-prefix=DELIMITERS // CHECK-NO-BLACKLIST-NOT: -fsanitize-blacklist Index: lib/Frontend/CompilerInvocation.cpp === --- lib/Frontend/CompilerInvocation.cpp +++ lib/Frontend/CompilerInvocation.cpp @@ -709,7 +709,7 @@ // Add sanitizer blacklists as extra dependencies. // They won't be discovered by the regular preprocessor, so // we let make / ninja to know about this implicit dependency. - Opts.ExtraDeps = Args.getAllArgValues(OPT_fsanitize_blacklist); + Opts.ExtraDeps = Args.getAllArgValues(OPT_fdepfile_entry); auto ModuleFiles = Args.getAllArgValues(OPT_fmodule_file); Opts.ExtraDeps.insert(Opts.ExtraDeps.end(), ModuleFiles.begin(), ModuleFiles.end()); Index: lib/Driver/SanitizerArgs.cpp === --- lib/Driver/SanitizerArgs.cpp +++ lib/Driver/SanitizerArgs.cpp @@ -176,6 +176,7 @@ RecoverableSanitizers.clear(); TrapSanitizers.clear(); BlacklistFiles.clear(); + ExtraDeps.clear(); CoverageFeatures = 0; MsanTrackOrigins = 0; MsanUseAfterDtor = false; @@ -383,13 +384,16 @@ if (Arg->getOption().matches(options::OPT_fsanitize_blacklist)) { Arg->claim(); std::string BLPath = Arg->getValue(); - if (llvm::sys::fs::exists(BLPath)) + if (llvm::sys::fs::exists(BLPath)) { BlacklistFiles.push_back(BLPath); - else +ExtraDeps.push_back(BLPath); + } else D.Diag(clang::diag::err_drv_no_such_file) << BLPath; + } else if (Arg->getOption().matches(options::OPT_fno_sanitize_blacklist)) { Arg->claim(); BlacklistFiles.clear(); + ExtraDeps.clear(); } } // Validate blacklists format. @@ -563,6 +567,11 @@ BlacklistOpt += BLPath; CmdArgs.push_back(Args.MakeArgString(BlacklistOpt)); } + for (const auto &Dep : ExtraDeps) { +SmallString<64> Extra
Re: [PATCH] D12544: Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.
krasin added inline comments. Comment at: test/Driver/fsanitize-blacklist.c:25 @@ -18,2 +24,3 @@ +// CHECK-DEFAULT-BLACKLIST: -fsanitize-blacklist={{.*}}asan_blacklist.txt // Ignore -fsanitize-blacklist flag if there is no -fsanitize flag. Good idea. Done. http://reviews.llvm.org/D12544 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
r246700 - Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output.
Author: krasin Date: Wed Sep 2 15:02:38 2015 New Revision: 246700 URL: http://llvm.org/viewvc/llvm-project?rev=246700&view=rev Log: Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output. Summary: Do not include default sanitizer blacklists into -M/-MM/-MD/-MMD output. Introduce a frontend option -fdepfile-entry, and only insert them for the user-defined sanitizer blacklists. In frontend, grab ExtraDeps from -fdepfile-entry, instead of -fsanitize-blacklist. Reviewers: rsmith, pcc Subscribers: cfe-commits Differential Revision: http://reviews.llvm.org/D12544 Added: cfe/trunk/test/Driver/Inputs/resource_dir/asan_blacklist.txt Modified: cfe/trunk/include/clang/Driver/Options.td cfe/trunk/include/clang/Driver/SanitizerArgs.h cfe/trunk/lib/Driver/SanitizerArgs.cpp cfe/trunk/lib/Frontend/CompilerInvocation.cpp cfe/trunk/test/Driver/fsanitize-blacklist.c cfe/trunk/test/Frontend/print-header-includes.c Modified: cfe/trunk/include/clang/Driver/Options.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=246700&r1=246699&r2=246700&view=diff == --- cfe/trunk/include/clang/Driver/Options.td (original) +++ cfe/trunk/include/clang/Driver/Options.td Wed Sep 2 15:02:38 2015 @@ -495,6 +495,8 @@ def fcxx_modules : Flag <["-"], "fcxx-mo Flags<[DriverOption]>; def fdebug_pass_arguments : Flag<["-"], "fdebug-pass-arguments">, Group; def fdebug_pass_structure : Flag<["-"], "fdebug-pass-structure">, Group; +def fdepfile_entry : Joined<["-"], "fdepfile-entry=">, +Group, Flags<[CC1Option]>; def fdiagnostics_fixit_info : Flag<["-"], "fdiagnostics-fixit-info">, Group; def fdiagnostics_parseable_fixits : Flag<["-"], "fdiagnostics-parseable-fixits">, Group, Flags<[CoreOption, CC1Option]>, HelpText<"Print fix-its in machine parseable form">; Modified: cfe/trunk/include/clang/Driver/SanitizerArgs.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/SanitizerArgs.h?rev=246700&r1=246699&r2=246700&view=diff == --- cfe/trunk/include/clang/Driver/SanitizerArgs.h (original) +++ cfe/trunk/include/clang/Driver/SanitizerArgs.h Wed Sep 2 15:02:38 2015 @@ -27,6 +27,7 @@ class SanitizerArgs { SanitizerSet TrapSanitizers; std::vector BlacklistFiles; + std::vector ExtraDeps; int CoverageFeatures; int MsanTrackOrigins; bool MsanUseAfterDtor; Modified: cfe/trunk/lib/Driver/SanitizerArgs.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/SanitizerArgs.cpp?rev=246700&r1=246699&r2=246700&view=diff == --- cfe/trunk/lib/Driver/SanitizerArgs.cpp (original) +++ cfe/trunk/lib/Driver/SanitizerArgs.cpp Wed Sep 2 15:02:38 2015 @@ -176,6 +176,7 @@ void SanitizerArgs::clear() { RecoverableSanitizers.clear(); TrapSanitizers.clear(); BlacklistFiles.clear(); + ExtraDeps.clear(); CoverageFeatures = 0; MsanTrackOrigins = 0; MsanUseAfterDtor = false; @@ -383,13 +384,16 @@ SanitizerArgs::SanitizerArgs(const ToolC if (Arg->getOption().matches(options::OPT_fsanitize_blacklist)) { Arg->claim(); std::string BLPath = Arg->getValue(); - if (llvm::sys::fs::exists(BLPath)) + if (llvm::sys::fs::exists(BLPath)) { BlacklistFiles.push_back(BLPath); - else +ExtraDeps.push_back(BLPath); + } else D.Diag(clang::diag::err_drv_no_such_file) << BLPath; + } else if (Arg->getOption().matches(options::OPT_fno_sanitize_blacklist)) { Arg->claim(); BlacklistFiles.clear(); + ExtraDeps.clear(); } } // Validate blacklists format. @@ -563,6 +567,11 @@ void SanitizerArgs::addArgs(const ToolCh BlacklistOpt += BLPath; CmdArgs.push_back(Args.MakeArgString(BlacklistOpt)); } + for (const auto &Dep : ExtraDeps) { +SmallString<64> ExtraDepOpt("-fdepfile-entry="); +ExtraDepOpt += Dep; +CmdArgs.push_back(Args.MakeArgString(ExtraDepOpt)); + } if (MsanTrackOrigins) CmdArgs.push_back(Args.MakeArgString("-fsanitize-memory-track-origins=" + Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=246700&r1=246699&r2=246700&view=diff == --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original) +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Wed Sep 2 15:02:38 2015 @@ -709,7 +709,7 @@ static void ParseDependencyOutputArgs(De // Add sanitizer blacklists as extra dependencies. // They won't be discovered by the regular preprocessor, so // we let make / ninja to know about this implicit dependency. - Opts.ExtraDeps = Args.getAllArgValues(OPT_fsanitize_blacklist); + Opts.ExtraDeps = Args.getA
Re: [PATCH] D16175: Introduce -fsanitize-stats flag.
krasin1 added a subscriber: krasin1. krasin1 accepted this revision. krasin1 added a reviewer: krasin1. krasin1 added a comment. This revision is now accepted and ready to land. LGTM Comment at: lib/Driver/SanitizerArgs.cpp:560 @@ +559,3 @@ + LinkerOptionFlag = "--linker-option=/include:"; + if (TC.getTriple().getArch() == llvm::Triple::x86) +LinkerOptionFlag += '_'; A comment about why does this underscore is needed for x86 is appreciated. http://reviews.llvm.org/D16175 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15921: [analyzer] Utility to match function calls.
krasin added a subscriber: krasin. krasin added a comment. FYI: this revision has likely broken the build: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/9561 FAIL: Clang :: Analysis/simple-stream-checks.c (367 of 8927) - TEST 'Clang :: Analysis/simple-stream-checks.c' FAILED Script: --- /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_asan/./bin/clang -cc1 -internal-isystem /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm_build_asan/bin/../lib/clang/3.9.0/include -nostdsysteminc -analyze -analyzer-checker=core,alpha.unix.SimpleStream -verify /mnt/b/sanitizer-buildbot3/sanitizer-x86_64-linux-fast/build/llvm/tools/clang/test/Analysis/simple-stream-checks.c -- Repository: rL LLVM http://reviews.llvm.org/D15921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits