From b47c628b1c5c88a4db60d2dda6411a2365a45dd1 Mon Sep 17 00:00:00 2001 From: Hans Wennborg Date: Fri, 25 Aug 2017 20:27:58 +0000 Subject: Merging r311695: ------------------------------------------------------------------------ r311695 | rsmith | 2017-08-24 13:10:33 -0700 (Thu, 24 Aug 2017) | 9 lines [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. ------------------------------------------------------------------------ git-svn-id: https://llvm.org/svn/llvm-project/cfe/branches/release_50@311799 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/DeclCXX.h | 5 ++++- lib/CodeGen/CodeGenFunction.cpp | 16 ++++++++++++-- test/CodeGenCXX/catch-undef-behavior.cpp | 36 ++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 3 deletions(-) diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h index c39eaee9b1..2f735c5506 100644 --- a/include/clang/AST/DeclCXX.h +++ b/include/clang/AST/DeclCXX.h @@ -2028,7 +2028,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 { diff --git a/lib/CodeGen/CodeGenFunction.cpp b/lib/CodeGen/CodeGenFunction.cpp index 93a4a38661..c23b25ea46 100644 --- a/lib/CodeGen/CodeGenFunction.cpp +++ b/lib/CodeGen/CodeGenFunction.cpp @@ -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" @@ -983,11 +984,22 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, } // 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(MD->getParent())->getLambdaCaptureDefault() == + LCD_None) + SkippedChecks.set(SanitizerKind::Null, true); + + EmitTypeCheck(isa(MD) ? TCK_ConstructorCall + : TCK_MemberCall, + Loc, CXXABIThisValue, ThisTy, getContext().getTypeAlignInChars(ThisTy->getPointeeType()), SkippedChecks); } diff --git a/test/CodeGenCXX/catch-undef-behavior.cpp b/test/CodeGenCXX/catch-undef-behavior.cpp index 179c334122..d58853c47f 100644 --- a/test/CodeGenCXX/catch-undef-behavior.cpp +++ b/test/CodeGenCXX/catch-undef-behavior.cpp @@ -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 } -- cgit v1.2.3