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