r311589 - [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 13:12:40 PDT 2017


Thanks for the revert; looks like there was another bug in the interaction
of lambdas an UBSan 'this' sanitization that was exposed by this (the
lambda static invoker calls the operator() with a null this pointer, and
the sanitizer doesn't know that's actually OK). Should be fixed in r311695.

On 24 August 2017 at 11:20, Adrian Prantl via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> I temporarily reverted the commit in r311680 to get the bots going again.
>
> -- adrian
>
> > On Aug 24, 2017, at 11:12 AM, Adrian Prantl via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
> >
> > It looks like this broke / found errors on the green dragon bot:
> >
> > http://green.lab.llvm.org/green/job/clang-stage2-cmake-
> RgSan_check/4115/consoleFull#15752874848254eaf0-7326-4999-
> 85b0-388101f2d404
> >
> > ******************** TEST 'LLVM-Unit :: ADT/./ADTTests/
> FilterIteratorTest.FunctionPointer' FAILED ********************
> >
> > Note: Google Test filter = FilterIteratorTest.FunctionPointer
> > [==========] Running 1 test from 1 test case.
> > [----------] Global test environment set-up.
> > [----------] 1 test from FilterIteratorTest
> > [ RUN      ] FilterIteratorTest.FunctionPointer
> > /Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-RgSan at 2
> /llvm/unittests/ADT/IteratorTest.cpp:160:24: runtime error: load of null
> pointer of type 'const (lambda at /Users/buildslave/jenkins/
> sharedspace/clang-stage2-cmake-RgSan at 2/llvm/unittests/ADT/IteratorTest.cpp:160:24)
> *'
> > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
> /Users/buildslave/jenkins/sharedspace/clang-stage2-cmake-RgSan at 2
> /llvm/unittests/ADT/IteratorTest.cpp:160:24 in
> >
> > ********************
> >
> > -- adrian
> >> On Aug 23, 2017, at 12:39 PM, Richard Smith via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
> >>
> >> Author: rsmith
> >> Date: Wed Aug 23 12:39:04 2017
> >> New Revision: 311589
> >>
> >> URL: http://llvm.org/viewvc/llvm-project?rev=311589&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).
> >>
> >> 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=311589&r1=311588&r2=311589&view=diff
> >> ============================================================
> ==================
> >> --- cfe/trunk/include/clang/AST/DeclCXX.h (original)
> >> +++ cfe/trunk/include/clang/AST/DeclCXX.h Wed Aug 23 12:39:04 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=311589&r1=311588&r2=311589&view=diff
> >> ============================================================
> ==================
> >> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
> >> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Wed Aug 23 12:39:04 2017
> >> @@ -1014,11 +1014,11 @@ 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,
> >> +      EmitTypeCheck(TCK_Load, 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=311589&r1=
> 311588&r2=311589&view=diff
> >> ============================================================
> ==================
> >> --- cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp (original)
> >> +++ cfe/trunk/test/CodeGenCXX/catch-undef-behavior.cpp Wed Aug 23
> 12:39:04 2017
> >> @@ -449,6 +449,27 @@ void upcast_to_vbase() {
> >> }
> >> }
> >>
> >> +struct ThisAlign {
> >> +  void this_align_lambda();
> >> +};
> >> +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
> >>
> >>
> >> _______________________________________________
> >> cfe-commits mailing list
> >> cfe-commits at lists.llvm.org
> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
> _______________________________________________
> 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/01cb9973/attachment-0001.html>


More information about the cfe-commits mailing list