r311799. Thanks!
On Thu, Aug 24, 2017 at 7:14 PM, Richard Smith <rich...@metafoo.co.uk> wrote: > 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