Commit 6493242f authored by Hans Wennborg's avatar Hans Wennborg
Browse files

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.

------------------------------------------------------------------------

llvm-svn: 311799
parent f84d026a

clang/include/clang/AST/DeclCXX.h

+4 −1
Original line number Diff line number Diff line
@@ -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 {

clang/lib/CodeGen/CodeGenFunction.cpp

+14 −2
Original line number Diff line number Diff line
@@ -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<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);

    }

clang/test/CodeGenCXX/catch-undef-behavior.cpp

+36 −0
Original line number Diff line number Diff line
@@ -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 }