[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
Sirraide wrote: It seems that a parameter to `getAddress()` that wasn’t being used has been removed, which caused builds to break after I merged this. My bad; I’ve already pushed a fix for this: #93086. I didn’t wait for CI this time because it was passing before (except for an unrelated issue), and I figured it wouldn’t be a problem because that was 2 days ago (it usually isn’t a problem in my experience at least), but the timing here was a bit unfortunate this time... https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4693,6 +4694,17 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + const CXXCastPath &BasePathArray = getContext().LambdaCastPaths.at(MD); + Address Base = GetAddressOfBaseClass( + LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(), + BasePathArray.end(), /*NullCheckValue=*/false, SourceLocation()); nickdesaulniers wrote: https://github.com/llvm/llvm-project/commit/da88e7fbd74cef33411bb5115f20e4b474d2d8b1 https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4693,6 +4694,17 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + const CXXCastPath &BasePathArray = getContext().LambdaCastPaths.at(MD); + Address Base = GetAddressOfBaseClass( + LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(), + BasePathArray.end(), /*NullCheckValue=*/false, SourceLocation()); Sirraide wrote: Fixed https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4693,6 +4694,17 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + const CXXCastPath &BasePathArray = getContext().LambdaCastPaths.at(MD); + Address Base = GetAddressOfBaseClass( + LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(), + BasePathArray.end(), /*NullCheckValue=*/false, SourceLocation()); Sirraide wrote: Already on it https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4693,6 +4694,17 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + const CXXCastPath &BasePathArray = getContext().LambdaCastPaths.at(MD); + Address Base = GetAddressOfBaseClass( + LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(), + BasePathArray.end(), /*NullCheckValue=*/false, SourceLocation()); nickdesaulniers wrote: error: too many arguments to function call, expected 0, have 1 LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(), ~~~ ^ https://lab.llvm.org/buildbot/#/builders/225/builds/36615/steps/6/logs/stdio https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
https://github.com/Sirraide closed https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
Sirraide wrote: > Should I just run make_cxx_dr_status again then and commit the result? I ended up figuring it out after talking to @Endilll about it. Also, the only CI failure earlier was something unrelated (CoverageMapping/mcdc-system-headers.cpp). https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/89828 >From b5422012a65165f27bb31be7e9490892f663acfe Mon Sep 17 00:00:00 2001 From: Sirraide Date: Tue, 23 Apr 2024 22:45:29 +0200 Subject: [PATCH 1/6] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda --- clang/docs/ReleaseNotes.rst | 3 + clang/lib/CodeGen/CGExpr.cpp | 23 +++ clang/test/CodeGenCXX/cxx2b-deducing-this.cpp | 63 +++ 3 files changed, 89 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d1f7293a842bb..34aad4abf3961 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -562,6 +562,9 @@ Bug Fixes to C++ Support - Fixed a crash when trying to evaluate a user-defined ``static_assert`` message whose ``size()`` function returns a large or negative value. Fixes (#GH89407). - Fixed a use-after-free bug in parsing of type constraints with default arguments that involve lambdas. (#GH67235) +- Fixed a crash when trying to emit captures in a lambda call operator with an explicit object + parameter that is called on a derived type of the lambda. + Fixes (#GH87210), (GH89541). Bug Fixes to AST Handling ^ diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 931cb391342ea..33795d7d4d192 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); + + const CXXBasePath *Path = &Paths.front(); + CXXCastPath BasePathArray; + for (unsigned I = 0, E = Path->size(); I != E; ++I) +BasePathArray.push_back( +const_cast((*Path)[I].Base)); + + Address Base = GetAddressOfBaseClass( + LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(), + BasePathArray.end(), /*NullCheckValue=*/false, SourceLocation()); + + LambdaLV = MakeAddrLValue(Base, QualType{LambdaTy->getTypeForDecl(), 0}); +} } else { QualType LambdaTagType = getContext().getTagDeclType(Field->getParent()); LambdaLV = MakeNaturalAlignAddrLValue(ThisValue, LambdaTagType); diff --git a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp index b755e80db35a1..649fe2afbf4e9 100644 --- a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp +++ b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp @@ -182,3 +182,66 @@ auto dothing(int num) fun(); } } + +namespace GH87210 { +template +struct Overloaded : Ts... { + using Ts::operator()...; +}; + +template +Overloaded(Ts...) -> Overloaded; + +// CHECK-LABEL: define dso_local void @_ZN7GH872101fEv() +// CHECK-NEXT: entry: +// CHECK-NEXT:[[X:%.*]] = alloca i32 +// CHECK-NEXT:[[Over:%.*]] = alloca %"{{.*}}Overloaded" +// CHECK: call noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EDaRT_"(ptr {{.*}} [[Over]]) +void f() { + int x; + Overloaded o { +// CHECK: define internal noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EDaRT_"(ptr {{.*}} [[Self:%.*]]) +// CHECK-NEXT: entry: +// CHECK-NEXT:[[SelfAddr:%.*]] = alloca ptr +// CHECK-NEXT:store ptr [[Self]], ptr [[SelfAddr]] +// CHECK-NEXT:[[SelfPtr:%.*]] = load ptr, ptr [[SelfAddr]] +// CHECK-NEXT:[[XRef:%.*]] = getelementptr inbounds %{{.*}}, ptr [[SelfPtr]], i32 0, i32 0 +// CHECK-NEXT:[[X:%.*]] = load ptr, ptr [[XRef]] +// CHECK-NEXT:ret ptr [[X]] +[&](this auto& self) { + return &x; +} + }; + o(); +} + +void g() { + int x; + Overloaded o { +[=](this auto& self) { + return x; +} + }; + o(); +} +} + +namespace GH89541 { +// Same as above; just check that this doesn't crash. +int one = 1; +auto factory(int& x = one) { + return [&](this auto self) { +x; + }; +}; + +using Base = decltype(factory()); +struct Derived : Base { + Derived() : Base(factory()) {} +}; + +void f() { + Derived d; + d(); +} +} >From 90d73ea88016307532bb38c4b2e8fa8f082bea75 Mon Sep 17 00:00:00 2001 From: Sirraide Date: Thu, 25 Apr 2024 01:01:18 +0200 Subject: [PATCH 2/6] [Clang] Tentative implementation of CWG 2881 --- clang/include/clang/AST/ASTContext.h | 9 +++ .../c
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
Sirraide wrote: > Endilll just updated the list of DRs to avoid unrelated changes, so by fixing > the conflict, only changes to cwg2881 should be in your PR Should I just run `make_cxx_dr_status` again then and commit the result? https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
https://github.com/cor3ntin approved this pull request. I think this makes sense. Thanks! @Endilll just updated the list of DRs to avoid unrelated changes, so by fixing the conflict, only changes to cwg2881 should be in your PR https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/89828 >From b5422012a65165f27bb31be7e9490892f663acfe Mon Sep 17 00:00:00 2001 From: Sirraide Date: Tue, 23 Apr 2024 22:45:29 +0200 Subject: [PATCH 1/5] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda --- clang/docs/ReleaseNotes.rst | 3 + clang/lib/CodeGen/CGExpr.cpp | 23 +++ clang/test/CodeGenCXX/cxx2b-deducing-this.cpp | 63 +++ 3 files changed, 89 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d1f7293a842bb..34aad4abf3961 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -562,6 +562,9 @@ Bug Fixes to C++ Support - Fixed a crash when trying to evaluate a user-defined ``static_assert`` message whose ``size()`` function returns a large or negative value. Fixes (#GH89407). - Fixed a use-after-free bug in parsing of type constraints with default arguments that involve lambdas. (#GH67235) +- Fixed a crash when trying to emit captures in a lambda call operator with an explicit object + parameter that is called on a derived type of the lambda. + Fixes (#GH87210), (GH89541). Bug Fixes to AST Handling ^ diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 931cb391342ea..33795d7d4d192 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); + + const CXXBasePath *Path = &Paths.front(); + CXXCastPath BasePathArray; + for (unsigned I = 0, E = Path->size(); I != E; ++I) +BasePathArray.push_back( +const_cast((*Path)[I].Base)); + + Address Base = GetAddressOfBaseClass( + LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(), + BasePathArray.end(), /*NullCheckValue=*/false, SourceLocation()); + + LambdaLV = MakeAddrLValue(Base, QualType{LambdaTy->getTypeForDecl(), 0}); +} } else { QualType LambdaTagType = getContext().getTagDeclType(Field->getParent()); LambdaLV = MakeNaturalAlignAddrLValue(ThisValue, LambdaTagType); diff --git a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp index b755e80db35a1..649fe2afbf4e9 100644 --- a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp +++ b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp @@ -182,3 +182,66 @@ auto dothing(int num) fun(); } } + +namespace GH87210 { +template +struct Overloaded : Ts... { + using Ts::operator()...; +}; + +template +Overloaded(Ts...) -> Overloaded; + +// CHECK-LABEL: define dso_local void @_ZN7GH872101fEv() +// CHECK-NEXT: entry: +// CHECK-NEXT:[[X:%.*]] = alloca i32 +// CHECK-NEXT:[[Over:%.*]] = alloca %"{{.*}}Overloaded" +// CHECK: call noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EDaRT_"(ptr {{.*}} [[Over]]) +void f() { + int x; + Overloaded o { +// CHECK: define internal noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EDaRT_"(ptr {{.*}} [[Self:%.*]]) +// CHECK-NEXT: entry: +// CHECK-NEXT:[[SelfAddr:%.*]] = alloca ptr +// CHECK-NEXT:store ptr [[Self]], ptr [[SelfAddr]] +// CHECK-NEXT:[[SelfPtr:%.*]] = load ptr, ptr [[SelfAddr]] +// CHECK-NEXT:[[XRef:%.*]] = getelementptr inbounds %{{.*}}, ptr [[SelfPtr]], i32 0, i32 0 +// CHECK-NEXT:[[X:%.*]] = load ptr, ptr [[XRef]] +// CHECK-NEXT:ret ptr [[X]] +[&](this auto& self) { + return &x; +} + }; + o(); +} + +void g() { + int x; + Overloaded o { +[=](this auto& self) { + return x; +} + }; + o(); +} +} + +namespace GH89541 { +// Same as above; just check that this doesn't crash. +int one = 1; +auto factory(int& x = one) { + return [&](this auto self) { +x; + }; +}; + +using Base = decltype(factory()); +struct Derived : Base { + Derived() : Base(factory()) {} +}; + +void f() { + Derived d; + d(); +} +} >From 90d73ea88016307532bb38c4b2e8fa8f082bea75 Mon Sep 17 00:00:00 2001 From: Sirraide Date: Thu, 25 Apr 2024 01:01:18 +0200 Subject: [PATCH 2/5] [Clang] Tentative implementation of CWG 2881 --- clang/include/clang/AST/ASTContext.h | 9 +++ .../c
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
Sirraide wrote: > You should run make_cxx_drs, otherwise I think this looks in good shape. Will do. > I think we should diag earlier (because if the lambda is not use we should > still signal it's broken) but doing that as a follow up PR seems reasonable. > > Less redundant diags is better but if we can't avoid it, i think it's a > reasonable compromise Alright; I’ll leave it as-is for this pr then. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
cor3ntin wrote: You should run make_cxx_drs, otherwise I think this looks in good shape. I think we should diag earlier (because if the lambda is not use we should still signal it's broken) but doing that as a follow up PR seems reasonable. Less redundant diags is better but if we can't avoid it, i think it's a reasonable compromise https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
Sirraide wrote: Ok, I’ve looked into this a bit more, and it seems that checking for this at instantiation time ends up being more complicated and produces more diagnostics than just checking it at the call site; I’ve added a comment about that. Other than that, is there anything else that we should look into here? https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/89828 >From b5422012a65165f27bb31be7e9490892f663acfe Mon Sep 17 00:00:00 2001 From: Sirraide Date: Tue, 23 Apr 2024 22:45:29 +0200 Subject: [PATCH 1/4] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda --- clang/docs/ReleaseNotes.rst | 3 + clang/lib/CodeGen/CGExpr.cpp | 23 +++ clang/test/CodeGenCXX/cxx2b-deducing-this.cpp | 63 +++ 3 files changed, 89 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d1f7293a842bb..34aad4abf3961 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -562,6 +562,9 @@ Bug Fixes to C++ Support - Fixed a crash when trying to evaluate a user-defined ``static_assert`` message whose ``size()`` function returns a large or negative value. Fixes (#GH89407). - Fixed a use-after-free bug in parsing of type constraints with default arguments that involve lambdas. (#GH67235) +- Fixed a crash when trying to emit captures in a lambda call operator with an explicit object + parameter that is called on a derived type of the lambda. + Fixes (#GH87210), (GH89541). Bug Fixes to AST Handling ^ diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 931cb391342ea..33795d7d4d192 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); + + const CXXBasePath *Path = &Paths.front(); + CXXCastPath BasePathArray; + for (unsigned I = 0, E = Path->size(); I != E; ++I) +BasePathArray.push_back( +const_cast((*Path)[I].Base)); + + Address Base = GetAddressOfBaseClass( + LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(), + BasePathArray.end(), /*NullCheckValue=*/false, SourceLocation()); + + LambdaLV = MakeAddrLValue(Base, QualType{LambdaTy->getTypeForDecl(), 0}); +} } else { QualType LambdaTagType = getContext().getTagDeclType(Field->getParent()); LambdaLV = MakeNaturalAlignAddrLValue(ThisValue, LambdaTagType); diff --git a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp index b755e80db35a1..649fe2afbf4e9 100644 --- a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp +++ b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp @@ -182,3 +182,66 @@ auto dothing(int num) fun(); } } + +namespace GH87210 { +template +struct Overloaded : Ts... { + using Ts::operator()...; +}; + +template +Overloaded(Ts...) -> Overloaded; + +// CHECK-LABEL: define dso_local void @_ZN7GH872101fEv() +// CHECK-NEXT: entry: +// CHECK-NEXT:[[X:%.*]] = alloca i32 +// CHECK-NEXT:[[Over:%.*]] = alloca %"{{.*}}Overloaded" +// CHECK: call noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EDaRT_"(ptr {{.*}} [[Over]]) +void f() { + int x; + Overloaded o { +// CHECK: define internal noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EDaRT_"(ptr {{.*}} [[Self:%.*]]) +// CHECK-NEXT: entry: +// CHECK-NEXT:[[SelfAddr:%.*]] = alloca ptr +// CHECK-NEXT:store ptr [[Self]], ptr [[SelfAddr]] +// CHECK-NEXT:[[SelfPtr:%.*]] = load ptr, ptr [[SelfAddr]] +// CHECK-NEXT:[[XRef:%.*]] = getelementptr inbounds %{{.*}}, ptr [[SelfPtr]], i32 0, i32 0 +// CHECK-NEXT:[[X:%.*]] = load ptr, ptr [[XRef]] +// CHECK-NEXT:ret ptr [[X]] +[&](this auto& self) { + return &x; +} + }; + o(); +} + +void g() { + int x; + Overloaded o { +[=](this auto& self) { + return x; +} + }; + o(); +} +} + +namespace GH89541 { +// Same as above; just check that this doesn't crash. +int one = 1; +auto factory(int& x = one) { + return [&](this auto self) { +x; + }; +}; + +using Base = decltype(factory()); +struct Derived : Base { + Derived() : Base(factory()) {} +}; + +void f() { + Derived d; + d(); +} +} >From 90d73ea88016307532bb38c4b2e8fa8f082bea75 Mon Sep 17 00:00:00 2001 From: Sirraide Date: Thu, 25 Apr 2024 01:01:18 +0200 Subject: [PATCH 2/4] [Clang] Tentative implementation of CWG 2881 --- clang/include/clang/AST/ASTContext.h | 9 +++ .../c
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
cor3ntin wrote: > @cor3ntin There already was a FIXME to consider moving this check to where > the call operator is first instantiated; should I look into that as well or > is it fine to leave this here? Asking because I’m assuming there’s a reason > why it wasn’t done that way. If you can do it easily, you should! I don't remember the details sadly, It's been over a year! https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
Sirraide wrote: @cor3ntin There already was a FIXME to consider moving this check to where the call operator is first instantiated; should I look into that as well or is it fine to leave this here? Asking because I’m assuming there’s a reason why it wasn’t done that way. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/89828 >From b5422012a65165f27bb31be7e9490892f663acfe Mon Sep 17 00:00:00 2001 From: Sirraide Date: Tue, 23 Apr 2024 22:45:29 +0200 Subject: [PATCH 1/3] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda --- clang/docs/ReleaseNotes.rst | 3 + clang/lib/CodeGen/CGExpr.cpp | 23 +++ clang/test/CodeGenCXX/cxx2b-deducing-this.cpp | 63 +++ 3 files changed, 89 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d1f7293a842bb6..34aad4abf39619 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -562,6 +562,9 @@ Bug Fixes to C++ Support - Fixed a crash when trying to evaluate a user-defined ``static_assert`` message whose ``size()`` function returns a large or negative value. Fixes (#GH89407). - Fixed a use-after-free bug in parsing of type constraints with default arguments that involve lambdas. (#GH67235) +- Fixed a crash when trying to emit captures in a lambda call operator with an explicit object + parameter that is called on a derived type of the lambda. + Fixes (#GH87210), (GH89541). Bug Fixes to AST Handling ^ diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 931cb391342ea2..33795d7d4d1921 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); + + const CXXBasePath *Path = &Paths.front(); + CXXCastPath BasePathArray; + for (unsigned I = 0, E = Path->size(); I != E; ++I) +BasePathArray.push_back( +const_cast((*Path)[I].Base)); + + Address Base = GetAddressOfBaseClass( + LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(), + BasePathArray.end(), /*NullCheckValue=*/false, SourceLocation()); + + LambdaLV = MakeAddrLValue(Base, QualType{LambdaTy->getTypeForDecl(), 0}); +} } else { QualType LambdaTagType = getContext().getTagDeclType(Field->getParent()); LambdaLV = MakeNaturalAlignAddrLValue(ThisValue, LambdaTagType); diff --git a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp index b755e80db35a12..649fe2afbf4e91 100644 --- a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp +++ b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp @@ -182,3 +182,66 @@ auto dothing(int num) fun(); } } + +namespace GH87210 { +template +struct Overloaded : Ts... { + using Ts::operator()...; +}; + +template +Overloaded(Ts...) -> Overloaded; + +// CHECK-LABEL: define dso_local void @_ZN7GH872101fEv() +// CHECK-NEXT: entry: +// CHECK-NEXT:[[X:%.*]] = alloca i32 +// CHECK-NEXT:[[Over:%.*]] = alloca %"{{.*}}Overloaded" +// CHECK: call noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EDaRT_"(ptr {{.*}} [[Over]]) +void f() { + int x; + Overloaded o { +// CHECK: define internal noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EDaRT_"(ptr {{.*}} [[Self:%.*]]) +// CHECK-NEXT: entry: +// CHECK-NEXT:[[SelfAddr:%.*]] = alloca ptr +// CHECK-NEXT:store ptr [[Self]], ptr [[SelfAddr]] +// CHECK-NEXT:[[SelfPtr:%.*]] = load ptr, ptr [[SelfAddr]] +// CHECK-NEXT:[[XRef:%.*]] = getelementptr inbounds %{{.*}}, ptr [[SelfPtr]], i32 0, i32 0 +// CHECK-NEXT:[[X:%.*]] = load ptr, ptr [[XRef]] +// CHECK-NEXT:ret ptr [[X]] +[&](this auto& self) { + return &x; +} + }; + o(); +} + +void g() { + int x; + Overloaded o { +[=](this auto& self) { + return x; +} + }; + o(); +} +} + +namespace GH89541 { +// Same as above; just check that this doesn't crash. +int one = 1; +auto factory(int& x = one) { + return [&](this auto self) { +x; + }; +}; + +using Base = decltype(factory()); +struct Derived : Base { + Derived() : Base(factory()) {} +}; + +void f() { + Derived d; + d(); +} +} >From 90d73ea88016307532bb38c4b2e8fa8f082bea75 Mon Sep 17 00:00:00 2001 From: Sirraide Date: Thu, 25 Apr 2024 01:01:18 +0200 Subject: [PATCH 2/3] [Clang] Tentative implementation of CWG 2881 --- clang/include/clang/AST/ASTContext.h | 9 +++
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
Sirraide wrote: > @Sirraide See example of > > https://github.com/llvm/llvm-project/blob/662ef8604268b207910225ecca90daf30a46720b/clang/test/CXX/drs/dr25xx.cpp#L148 > > > In this case, the date should be 2024-04-19. Ah, thanks. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
Endilll wrote: @Sirraide See example of https://github.com/llvm/llvm-project/blob/662ef8604268b207910225ecca90daf30a46720b/clang/test/CXX/drs/dr25xx.cpp#L148 In this case, the date should be 2024-04-19. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
Sirraide wrote: I’m also not sure if the `dr28xx.cpp` file is meant for open issues, but I’ve put the tests for this there for now. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
Sirraide wrote: Ok, I’ve added a check for CWG 2881 to sema, but there’s at least 2 things I still want to take a look at: - [ ] Consider emitting the upcast once in the function prologue instead of every time a variable is accessed. - [ ] Check for invalid explicit object parameters when the lambda is instantiated rather than when it is called. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
https://github.com/Sirraide updated https://github.com/llvm/llvm-project/pull/89828 >From b5422012a65165f27bb31be7e9490892f663acfe Mon Sep 17 00:00:00 2001 From: Sirraide Date: Tue, 23 Apr 2024 22:45:29 +0200 Subject: [PATCH 1/2] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda --- clang/docs/ReleaseNotes.rst | 3 + clang/lib/CodeGen/CGExpr.cpp | 23 +++ clang/test/CodeGenCXX/cxx2b-deducing-this.cpp | 63 +++ 3 files changed, 89 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d1f7293a842bb6..34aad4abf39619 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -562,6 +562,9 @@ Bug Fixes to C++ Support - Fixed a crash when trying to evaluate a user-defined ``static_assert`` message whose ``size()`` function returns a large or negative value. Fixes (#GH89407). - Fixed a use-after-free bug in parsing of type constraints with default arguments that involve lambdas. (#GH67235) +- Fixed a crash when trying to emit captures in a lambda call operator with an explicit object + parameter that is called on a derived type of the lambda. + Fixes (#GH87210), (GH89541). Bug Fixes to AST Handling ^ diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 931cb391342ea2..33795d7d4d1921 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); + + const CXXBasePath *Path = &Paths.front(); + CXXCastPath BasePathArray; + for (unsigned I = 0, E = Path->size(); I != E; ++I) +BasePathArray.push_back( +const_cast((*Path)[I].Base)); + + Address Base = GetAddressOfBaseClass( + LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(), + BasePathArray.end(), /*NullCheckValue=*/false, SourceLocation()); + + LambdaLV = MakeAddrLValue(Base, QualType{LambdaTy->getTypeForDecl(), 0}); +} } else { QualType LambdaTagType = getContext().getTagDeclType(Field->getParent()); LambdaLV = MakeNaturalAlignAddrLValue(ThisValue, LambdaTagType); diff --git a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp index b755e80db35a12..649fe2afbf4e91 100644 --- a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp +++ b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp @@ -182,3 +182,66 @@ auto dothing(int num) fun(); } } + +namespace GH87210 { +template +struct Overloaded : Ts... { + using Ts::operator()...; +}; + +template +Overloaded(Ts...) -> Overloaded; + +// CHECK-LABEL: define dso_local void @_ZN7GH872101fEv() +// CHECK-NEXT: entry: +// CHECK-NEXT:[[X:%.*]] = alloca i32 +// CHECK-NEXT:[[Over:%.*]] = alloca %"{{.*}}Overloaded" +// CHECK: call noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EDaRT_"(ptr {{.*}} [[Over]]) +void f() { + int x; + Overloaded o { +// CHECK: define internal noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EDaRT_"(ptr {{.*}} [[Self:%.*]]) +// CHECK-NEXT: entry: +// CHECK-NEXT:[[SelfAddr:%.*]] = alloca ptr +// CHECK-NEXT:store ptr [[Self]], ptr [[SelfAddr]] +// CHECK-NEXT:[[SelfPtr:%.*]] = load ptr, ptr [[SelfAddr]] +// CHECK-NEXT:[[XRef:%.*]] = getelementptr inbounds %{{.*}}, ptr [[SelfPtr]], i32 0, i32 0 +// CHECK-NEXT:[[X:%.*]] = load ptr, ptr [[XRef]] +// CHECK-NEXT:ret ptr [[X]] +[&](this auto& self) { + return &x; +} + }; + o(); +} + +void g() { + int x; + Overloaded o { +[=](this auto& self) { + return x; +} + }; + o(); +} +} + +namespace GH89541 { +// Same as above; just check that this doesn't crash. +int one = 1; +auto factory(int& x = one) { + return [&](this auto self) { +x; + }; +}; + +using Base = decltype(factory()); +struct Derived : Base { + Derived() : Base(factory()) {} +}; + +void f() { + Derived d; + d(); +} +} >From 90d73ea88016307532bb38c4b2e8fa8f082bea75 Mon Sep 17 00:00:00 2001 From: Sirraide Date: Thu, 25 Apr 2024 01:01:18 +0200 Subject: [PATCH 2/2] [Clang] Tentative implementation of CWG 2881 --- clang/include/clang/AST/ASTContext.h | 9 +++
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); rjmccall wrote: We could do either. Doing it once in the prologue makes some sense to me; in the (very unlikely) case that it's not just a static offset, that would probably lead to better code. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); Sirraide wrote: On that note, as for actually emitting the cast, is it worth it doing that once when we start emitting the lambda and then just reusing the computed lvalue instead of emitting the cast every time we need to access a capture? Or do we just let the optimiser clean that up? Asking because I’m not too familiar w/ how situations like these are typically handled in codegen. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); Sirraide wrote: > We'd presumably store it with a side-table in the ASTContext, which we'd only > populate for lambdas with a derived object parameter. I was just thinking about storing it in the call operator but I wasn’t sure where, but this makes sense. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); rjmccall wrote: If we were going to put it in the AST, the right place would be on the call operator FD rather than in every DRE for a capture. We'd presumably store it with a side-table in the ASTContext, which we'd only populate for lambdas with a derived object parameter. It's probably not an actual problem to repeatedly recompute it as long as we have that restriction, though. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); Sirraide wrote: Ah, good to know. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); cor3ntin wrote: Richard got you beat by a few days https://cplusplus.github.io/CWG/issues/2881.html https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); Sirraide wrote: > Also, maybe we should record the correct path in the AST somewhere? That would probably be less of a hack than doing this here, but the main problem I see is that all we have here is a DRE; it just seems a bit strange to store cast paths in one—at least to me. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); efriedma-quic wrote: https://isocpp.org/std/submit-issue for defect reports Also, maybe we should record the correct path in the AST somewhere? https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); Sirraide wrote: > We can manually resolve between the `operator()`s at the call site with a > qualified member name Ah, that’s what I was missing. > I think you need to open a DR with the committee. That sounds reasonable, but I candidly have no idea how to do that. > Please go ahead and diagnose that condition in Sema. Will do. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); rjmccall wrote: We can manually resolve between the `operator()`s at the call site with a qualified member name, and the `operator()` should still get called with the original `Overloaded` without any hint about which subobject it was called on. We could also have `Overloaded` just pick one of its bases to `using` the `operator()` from, but then I suppose someone might argue that the operator ought to pick up on that to decide the base path. (That would be absurd, IMO.) Example of the first: https://godbolt.org/z/rqvKGfevY I think you need to open a DR with the committee. It just needs to be ill-formed for a lambda call operator with an explicit `this` parameter to be instantiated such that the `this` parameter has a type with multiple base subobjects of the lambda type. I can't imagine any other resolution short of something absurd like adding an implicit offset parameter, which would then be breaking in other ways. Please go ahead and diagnose that condition in Sema. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); Sirraide wrote: That’s something I’m not entirely sure about, but I couldn’t come up with a way to actually end up with more than one path from a derived class to the lambda’s type such that both would still be accessible. https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
@@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); rjmccall wrote: Is there a reason there has to be a unique base subobject of the lambda type? https://github.com/llvm/llvm-project/pull/89828 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
llvmbot wrote: @llvm/pr-subscribers-clang-codegen Author: None (Sirraide) Changes Consider this code: ```c++ templatestruct Overloaded : Ts... { using Ts::operator()...; }; template Overloaded(Ts...) -> Overloaded ; void f() { int x; Overloaded o { [&](this auto& self) { return &x; } }; o(); } ``` To access `x` in the lambda, we need to perform derived-to-base conversion on `self` (since the type of `self` is not the lambda type, but rather `Overloaded<(lambda type)>`). We were previously missing this step, causing us to attempt to load the entire lambda (as the base class, it would end up being the ‘field’ with index `0` here), which would then assert later on in codegen. This fixes #87210 and fixes #89541. --- Full diff: https://github.com/llvm/llvm-project/pull/89828.diff 3 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+3) - (modified) clang/lib/CodeGen/CGExpr.cpp (+23) - (modified) clang/test/CodeGenCXX/cxx2b-deducing-this.cpp (+63) ``diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d1f7293a842bb6..34aad4abf39619 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -562,6 +562,9 @@ Bug Fixes to C++ Support - Fixed a crash when trying to evaluate a user-defined ``static_assert`` message whose ``size()`` function returns a large or negative value. Fixes (#GH89407). - Fixed a use-after-free bug in parsing of type constraints with default arguments that involve lambdas. (#GH67235) +- Fixed a crash when trying to emit captures in a lambda call operator with an explicit object + parameter that is called on a derived type of the lambda. + Fixes (#GH87210), (GH89541). Bug Fixes to AST Handling ^ diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 931cb391342ea2..33795d7d4d1921 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); + + const CXXBasePath *Path = &Paths.front(); + CXXCastPath BasePathArray; + for (unsigned I = 0, E = Path->size(); I != E; ++I) +BasePathArray.push_back( +const_cast((*Path)[I].Base)); + + Address Base = GetAddressOfBaseClass( + LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(), + BasePathArray.end(), /*NullCheckValue=*/false, SourceLocation()); + + LambdaLV = MakeAddrLValue(Base, QualType{LambdaTy->getTypeForDecl(), 0}); +} } else { QualType LambdaTagType = getContext().getTagDeclType(Field->getParent()); LambdaLV = MakeNaturalAlignAddrLValue(ThisValue, LambdaTagType); diff --git a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp index b755e80db35a12..649fe2afbf4e91 100644 --- a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp +++ b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp @@ -182,3 +182,66 @@ auto dothing(int num) fun(); } } + +namespace GH87210 { +template +struct Overloaded : Ts... { + using Ts::operator()...; +}; + +template +Overloaded(Ts...) -> Overloaded; + +// CHECK-LABEL: define dso_local void @_ZN7GH872101fEv() +// CHECK-NEXT: entry: +// CHECK-NEXT:[[X:%.*]] = alloca i32 +// CHECK-NEXT:[[Over:%.*]] = alloca %"{{.*}}Overloaded" +// CHECK: call noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EDaRT_"(ptr {{.*}} [[Over]]) +void f() { + int x; + Overloaded o { +// CHECK: define internal noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EDaRT_"(ptr {{.*}} [[Self:%.*]]) +// CHECK-NEXT: entry: +// CHECK-NEXT:[[SelfAddr:%.*]] = alloca ptr +// CHECK-NEXT:store ptr [[Self]], ptr [[SelfAddr]] +// CHECK-NEXT:[[SelfPtr:%.*]] = load ptr, ptr [[SelfAddr]] +// CHECK-NEXT:[[XRef:%.*]] = getelementptr inbounds %{{.*}}, ptr [[SelfPtr]], i32 0, i32 0 +// CHECK-NEXT:[[X:%.*]] = load ptr, ptr [[XRef]] +// CHECK-NEXT:ret ptr [[X]] +[&](this auto& self) { + return &x; +} + }; + o(); +} + +void g() { + int x; + Overloaded o { +[=](this auto& self) { + return x; +} + }; + o(); +} +} + +namespace GH89541 { +// Same as above; just check t
[clang] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda (PR #89828)
https://github.com/Sirraide created https://github.com/llvm/llvm-project/pull/89828 Consider this code: ```c++ template struct Overloaded : Ts... { using Ts::operator()...; }; template Overloaded(Ts...) -> Overloaded; void f() { int x; Overloaded o { [&](this auto& self) { return &x; } }; o(); } ``` To access `x` in the lambda, we need to perform derived-to-base conversion on `self` (since the type of `self` is not the lambda type, but rather `Overloaded<(lambda type)>`). We were previously missing this step, causing us to attempt to load the entire lambda (as the base class, it would end up being the ‘field’ with index `0` here), which would then assert later on in codegen. This fixes #87210 and fixes #89541. >From b5422012a65165f27bb31be7e9490892f663acfe Mon Sep 17 00:00:00 2001 From: Sirraide Date: Tue, 23 Apr 2024 22:45:29 +0200 Subject: [PATCH] [Clang] [CodeGen] Perform derived-to-base conversion on explicit object parameter in lambda --- clang/docs/ReleaseNotes.rst | 3 + clang/lib/CodeGen/CGExpr.cpp | 23 +++ clang/test/CodeGenCXX/cxx2b-deducing-this.cpp | 63 +++ 3 files changed, 89 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index d1f7293a842bb6..34aad4abf39619 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -562,6 +562,9 @@ Bug Fixes to C++ Support - Fixed a crash when trying to evaluate a user-defined ``static_assert`` message whose ``size()`` function returns a large or negative value. Fixes (#GH89407). - Fixed a use-after-free bug in parsing of type constraints with default arguments that involve lambdas. (#GH67235) +- Fixed a crash when trying to emit captures in a lambda call operator with an explicit object + parameter that is called on a derived type of the lambda. + Fixes (#GH87210), (GH89541). Bug Fixes to AST Handling ^ diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 931cb391342ea2..33795d7d4d1921 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -4684,6 +4684,29 @@ LValue CodeGenFunction::EmitLValueForLambdaField(const FieldDecl *Field, else LambdaLV = MakeAddrLValue(AddrOfExplicitObject, D->getType().getNonReferenceType()); + +// Make sure we have an lvalue to the lambda itself and not a derived class. +auto *ThisTy = D->getType().getNonReferenceType()->getAsCXXRecordDecl(); +auto *LambdaTy = cast(Field->getParent()); +if (ThisTy != LambdaTy) { + CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/true, + /*DetectVirtual=*/false); + + [[maybe_unused]] bool Derived = ThisTy->isDerivedFrom(LambdaTy, Paths); + assert(Derived && "Type not derived from lambda type?"); + + const CXXBasePath *Path = &Paths.front(); + CXXCastPath BasePathArray; + for (unsigned I = 0, E = Path->size(); I != E; ++I) +BasePathArray.push_back( +const_cast((*Path)[I].Base)); + + Address Base = GetAddressOfBaseClass( + LambdaLV.getAddress(*this), ThisTy, BasePathArray.begin(), + BasePathArray.end(), /*NullCheckValue=*/false, SourceLocation()); + + LambdaLV = MakeAddrLValue(Base, QualType{LambdaTy->getTypeForDecl(), 0}); +} } else { QualType LambdaTagType = getContext().getTagDeclType(Field->getParent()); LambdaLV = MakeNaturalAlignAddrLValue(ThisValue, LambdaTagType); diff --git a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp index b755e80db35a12..649fe2afbf4e91 100644 --- a/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp +++ b/clang/test/CodeGenCXX/cxx2b-deducing-this.cpp @@ -182,3 +182,66 @@ auto dothing(int num) fun(); } } + +namespace GH87210 { +template +struct Overloaded : Ts... { + using Ts::operator()...; +}; + +template +Overloaded(Ts...) -> Overloaded; + +// CHECK-LABEL: define dso_local void @_ZN7GH872101fEv() +// CHECK-NEXT: entry: +// CHECK-NEXT:[[X:%.*]] = alloca i32 +// CHECK-NEXT:[[Over:%.*]] = alloca %"{{.*}}Overloaded" +// CHECK: call noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EDaRT_"(ptr {{.*}} [[Over]]) +void f() { + int x; + Overloaded o { +// CHECK: define internal noundef ptr @"_ZZN7GH872101fEvENH3$_0clINS_10OverloadedIJS0_EDaRT_"(ptr {{.*}} [[Self:%.*]]) +// CHECK-NEXT: entry: +// CHECK-NEXT:[[SelfAddr:%.*]] = alloca ptr +// CHECK-NEXT:store ptr [[Self]], ptr [[SelfAddr]] +// CHECK-NEXT:[[SelfPtr:%.*]] = load ptr, ptr [[SelfAddr]] +// CHECK-NEXT:[[XRef:%.*]] = getelementptr inbounds %{{.*}}, ptr [[SelfPtr]], i32 0, i32 0 +// CHECK-NEXT:[[X:%.*]] = load ptr, ptr [[XRef]] +// CHECK-NEXT:ret ptr [[X]] +[&](this auto& self) { + return &x; +} + }; + o(); +} + +void g() { +