Hi Hans, This fixes a regression in ubsan's handling of lambda expressions. Seems like a reasonable candidate for Clang 5, but it is rather late in the day...
On 24 August 2017 at 13:10, Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: rsmith > Date: Thu Aug 24 13:10:33 2017 > New Revision: 311695 > > URL: http://llvm.org/viewvc/llvm-project?rev=311695&view=rev > Log: > [ubsan] PR34266: When sanitizing the 'this' value for a member function > that happens to be a lambda call operator, use the lambda's 'this' pointer, > not the captured enclosing 'this' pointer (if any). > > Do not sanitize the 'this' pointer of a member call operator for a lambda > with > no capture-default, since that call operator can legitimately be called > with a > null this pointer from the static invoker function. Any actual call with a > null > this pointer should still be caught in the caller (if it is being > sanitized). > > This reinstates r311589 (reverted in r311680) with the above fix. > > Modified: > cfe/trunk/include/clang/AST/DeclCXX.h > cfe/trunk/lib/CodeGen/CodeGenFunction.cpp > cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp > > Modified: cfe/trunk/include/clang/AST/DeclCXX.h > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/ > clang/AST/DeclCXX.h?rev=311695&r1=311694&r2=311695&view=diff > ============================================================ > ================== > --- cfe/trunk/include/clang/AST/DeclCXX.h (original) > +++ cfe/trunk/include/clang/AST/DeclCXX.h Thu Aug 24 13:10:33 2017 > @@ -2027,7 +2027,10 @@ public: > > /// \brief Returns the type of the \c this pointer. > /// > - /// Should only be called for instance (i.e., non-static) methods. > + /// Should only be called for instance (i.e., non-static) methods. Note > + /// that for the call operator of a lambda closure type, this returns > the > + /// desugared 'this' type (a pointer to the closure type), not the > captured > + /// 'this' type. > QualType getThisType(ASTContext &C) const; > > unsigned getTypeQualifiers() const { > > Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ > CodeGenFunction.cpp?rev=311695&r1=311694&r2=311695&view=diff > ============================================================ > ================== > --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) > +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Thu Aug 24 13:10:33 2017 > @@ -22,6 +22,7 @@ > #include "CodeGenPGO.h" > #include "TargetInfo.h" > #include "clang/AST/ASTContext.h" > +#include "clang/AST/ASTLambda.h" > #include "clang/AST/Decl.h" > #include "clang/AST/DeclCXX.h" > #include "clang/AST/StmtCXX.h" > @@ -1014,11 +1015,22 @@ void CodeGenFunction::StartFunction(Glob > } > > // Check the 'this' pointer once per function, if it's available. > - if (CXXThisValue) { > + if (CXXABIThisValue) { > SanitizerSet SkippedChecks; > SkippedChecks.set(SanitizerKind::ObjectSize, true); > QualType ThisTy = MD->getThisType(getContext()); > - EmitTypeCheck(TCK_Load, Loc, CXXThisValue, ThisTy, > + > + // If this is the call operator of a lambda with no > capture-default, it > + // may have a static invoker function, which may call this operator > with > + // a null 'this' pointer. > + if (isLambdaCallOperator(MD) && > + cast<CXXRecordDecl>(MD->getParent())->getLambdaCaptureDefault() > == > + LCD_None) > + SkippedChecks.set(SanitizerKind::Null, true); > + > + EmitTypeCheck(isa<CXXConstructorDecl>(MD) ? TCK_ConstructorCall > + : TCK_MemberCall, > + Loc, CXXABIThisValue, ThisTy, > getContext().getTypeAlignInChars(ThisTy-> > getPointeeType()), > SkippedChecks); > } > > Modified: cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ > CodeGenCXX/catch-undef-behavior.cpp?rev=311695&r1= > 311694&r2=311695&view=diff > ============================================================ > ================== > --- cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp (original) > +++ cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp Thu Aug 24 > 13:10:33 2017 > @@ -449,6 +449,28 @@ void upcast_to_vbase() { > } > } > > +struct ThisAlign { > + void this_align_lambda(); > + void this_align_lambda_2(); > +}; > +void ThisAlign::this_align_lambda() { > + // CHECK-LABEL: define {{.*}}@"_ZZN9ThisAlign17this_ > align_lambdaEvENK3$_0clEv" > + // CHECK-SAME: (%{{.*}}* %[[this:[^)]*]]) > + // CHECK: %[[this_addr:.*]] = alloca > + // CHECK: store %{{.*}}* %[[this]], %{{.*}}** %[[this_addr]], > + // CHECK: %[[this_inner:.*]] = load %{{.*}}*, %{{.*}}** %[[this_addr]], > + // CHECK: %[[this_outer_addr:.*]] = getelementptr inbounds %{{.*}}, > %{{.*}}* %[[this_inner]], i32 0, i32 0 > + // CHECK: %[[this_outer:.*]] = load %{{.*}}*, %{{.*}}** > %[[this_outer_addr]], > + // > + // CHECK: %[[this_inner_isnonnull:.*]] = icmp ne %{{.*}}* > %[[this_inner]], null > + // CHECK: %[[this_inner_asint:.*]] = ptrtoint %{{.*}}* %[[this_inner]] > to i > + // CHECK: %[[this_inner_misalignment:.*]] = and i{{32|64}} > %[[this_inner_asint]], {{3|7}}, > + // CHECK: %[[this_inner_isaligned:.*]] = icmp eq i{{32|64}} > %[[this_inner_misalignment]], 0 > + // CHECK: %[[this_inner_valid:.*]] = and i1 %[[this_inner_isnonnull]], > %[[this_inner_isaligned]], > + // CHECK: br i1 %[[this_inner_valid:.*]] > + [&] { return this; } (); > +} > + > namespace CopyValueRepresentation { > // CHECK-LABEL: define {{.*}} @_ZN23CopyValueRepresentation2S3aSERKS0_ > // CHECK-NOT: call {{.*}} @__ubsan_handle_load_invalid_value > @@ -532,4 +554,18 @@ namespace CopyValueRepresentation { > } > } > > +void ThisAlign::this_align_lambda_2() { > + // CHECK-LABEL: define {{.*}}@"_ZZN9ThisAlign19this_ > align_lambda_2EvENK3$_1clEv" > + // CHECK-SAME: (%{{.*}}* %[[this:[^)]*]]) > + // CHECK: %[[this_addr:.*]] = alloca > + // CHECK: store %{{.*}}* %[[this]], %{{.*}}** %[[this_addr]], > + // CHECK: %[[this_inner:.*]] = load %{{.*}}*, %{{.*}}** %[[this_addr]], > + // > + // Do not perform a null check on the 'this' pointer if the function > might be > + // called from a static invoker. > + // CHECK-NOT: icmp ne %{{.*}}* %[[this_inner]], null > + auto *p = +[] {}; > + p(); > +} > + > // CHECK: attributes [[NR_NUW]] = { noreturn nounwind } > > > _______________________________________________ > 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
