r311695 - [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).
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 24 19:14:17 PDT 2017
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 at 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170824/7077fa94/attachment.html>
More information about the cfe-commits
mailing list