https://github.com/TilakChad updated https://github.com/llvm/llvm-project/pull/120849
>From f35f0712ea3fcb8c053e7f0405b606c01450ad57 Mon Sep 17 00:00:00 2001 From: Tilak Chad <tilakchad...@gmail.com> Date: Sat, 21 Dec 2024 19:22:37 +0545 Subject: [PATCH 1/2] [Clang] Prevent assignment to captured structured bindings inside immutable lambda --- clang/docs/ReleaseNotes.rst | 1 + clang/lib/Sema/SemaExpr.cpp | 25 ++++++++++++++-------- clang/test/SemaCXX/cxx20-decomposition.cpp | 23 ++++++++++++++++++++ 3 files changed, 40 insertions(+), 9 deletions(-) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 6b9e1109f3906e..8b79ab4f551a8a 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -884,6 +884,7 @@ Bug Fixes to C++ Support - Fixed recognition of ``std::initializer_list`` when it's surrounded with ``extern "C++"`` and exported out of a module (which is the case e.g. in MSVC's implementation of ``std`` module). (#GH118218) - Fixed a pack expansion issue in checking unexpanded parameter sizes. (#GH17042) +- Fixed a bug where captured structured bindings were modifiable inside non-mutable lambda (#GH95081) Bug Fixes to AST Handling ^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index 24f7d27c691154..b5e247644ef36e 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -3352,6 +3352,7 @@ ExprResult Sema::BuildDeclarationNameExpr( case Decl::VarTemplateSpecialization: case Decl::VarTemplatePartialSpecialization: case Decl::Decomposition: + case Decl::Binding: case Decl::OMPCapturedExpr: // In C, "extern void blah;" is valid and is an r-value. if (!getLangOpts().CPlusPlus && !type.hasQualifiers() && @@ -3371,20 +3372,13 @@ ExprResult Sema::BuildDeclarationNameExpr( // potentially-evaluated contexts? Since the variable isn't actually // captured in an unevaluated context, it seems that the answer is no. if (!isUnevaluatedContext()) { - QualType CapturedType = getCapturedDeclRefType(cast<VarDecl>(VD), Loc); + QualType CapturedType = getCapturedDeclRefType(cast<ValueDecl>(VD), Loc); if (!CapturedType.isNull()) type = CapturedType; } - break; } - case Decl::Binding: - // These are always lvalues. - valueKind = VK_LValue; - type = type.getNonReferenceType(); - break; - case Decl::Function: { if (unsigned BID = cast<FunctionDecl>(VD)->getBuiltinID()) { if (!Context.BuiltinInfo.isDirectlyAddressable(BID)) { @@ -13299,7 +13293,18 @@ static NonConstCaptureKind isReferenceToNonConstCapture(Sema &S, Expr *E) { // The declaration must be a variable which is not declared 'const'. VarDecl *var = dyn_cast<VarDecl>(DRE->getDecl()); - if (!var) return NCCK_None; + if (!var) { + // Bindings also can be captured by lambda in C++ + BindingDecl *binding = dyn_cast<BindingDecl>(DRE->getDecl()); + if (!binding || binding->getType().isConstQualified()) + return NCCK_None; + + assert(S.getLangOpts().CPlusPlus && "BindingDecl outside of C++?"); + assert(!isa<BlockDecl>(binding->getDeclContext())); + + return NCCK_Lambda; + } + if (var->getType().isConstQualified()) return NCCK_None; assert(var->hasLocalStorage() && "capture added 'const' to non-local?"); @@ -19247,6 +19252,8 @@ bool Sema::NeedToCaptureVariable(ValueDecl *Var, SourceLocation Loc) { } QualType Sema::getCapturedDeclRefType(ValueDecl *Var, SourceLocation Loc) { + assert(Var && "Null value cannot be captured"); + QualType CaptureType; QualType DeclRefType; diff --git a/clang/test/SemaCXX/cxx20-decomposition.cpp b/clang/test/SemaCXX/cxx20-decomposition.cpp index 430a158ff458ed..ccc1af5898059f 100644 --- a/clang/test/SemaCXX/cxx20-decomposition.cpp +++ b/clang/test/SemaCXX/cxx20-decomposition.cpp @@ -183,3 +183,26 @@ namespace ODRUseTests { }(0); }(0); // expected-note 2{{in instantiation}} } } + + +namespace GH95081 { + void prevent_assignment_check() { + int arr[] = {1,2}; + auto [e1, e2] = arr; + + auto lambda = [e1] { + e1 = 42; // expected-error {{cannot assign to a variable captured by copy in a non-mutable lambda}} + }; + } + + void f(int&) = delete; + void f(const int&); + + int arr[1]; + void foo() { + auto [x] = arr; + [x]() { + f(x); // deleted f(int&) used to be picked up erroneously + } (); + } +} >From fda25e8e97ffb6f5038c73e204114e81fc862c5c Mon Sep 17 00:00:00 2001 From: Tilak Chad <tilakchad...@gmail.com> Date: Tue, 24 Dec 2024 00:55:56 +0545 Subject: [PATCH 2/2] [Clang] Refactored to use common code and updated variables name --- clang/lib/Sema/SemaExpr.cpp | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index b5e247644ef36e..562c98c6babe04 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -13291,22 +13291,24 @@ static NonConstCaptureKind isReferenceToNonConstCapture(Sema &S, Expr *E) { if (!DRE) return NCCK_None; if (!DRE->refersToEnclosingVariableOrCapture()) return NCCK_None; - // The declaration must be a variable which is not declared 'const'. - VarDecl *var = dyn_cast<VarDecl>(DRE->getDecl()); - if (!var) { - // Bindings also can be captured by lambda in C++ - BindingDecl *binding = dyn_cast<BindingDecl>(DRE->getDecl()); - if (!binding || binding->getType().isConstQualified()) - return NCCK_None; + ValueDecl *Value = dyn_cast<ValueDecl>(DRE->getDecl()); - assert(S.getLangOpts().CPlusPlus && "BindingDecl outside of C++?"); - assert(!isa<BlockDecl>(binding->getDeclContext())); + // The declaration must be a value which is not declared 'const'. + if (!Value || Value->getType().isConstQualified()) + return NCCK_None; + BindingDecl *Binding = dyn_cast<BindingDecl>(Value); + if (Binding) { + assert(S.getLangOpts().CPlusPlus && "BindingDecl outside of C++?"); + assert(!isa<BlockDecl>(Binding->getDeclContext())); return NCCK_Lambda; } - if (var->getType().isConstQualified()) return NCCK_None; - assert(var->hasLocalStorage() && "capture added 'const' to non-local?"); + VarDecl *Var = dyn_cast<VarDecl>(Value); + if (!Var) + return NCCK_None; + + assert(Var->hasLocalStorage() && "capture added 'const' to non-local?"); // Decide whether the first capture was for a block or a lambda. DeclContext *DC = S.CurContext, *Prev = nullptr; @@ -13315,16 +13317,16 @@ static NonConstCaptureKind isReferenceToNonConstCapture(Sema &S, Expr *E) { // For init-capture, it is possible that the variable belongs to the // template pattern of the current context. if (auto *FD = dyn_cast<FunctionDecl>(DC)) - if (var->isInitCapture() && - FD->getTemplateInstantiationPattern() == var->getDeclContext()) + if (Var->isInitCapture() && + FD->getTemplateInstantiationPattern() == Var->getDeclContext()) break; - if (DC == var->getDeclContext()) + if (DC == Var->getDeclContext()) break; Prev = DC; DC = DC->getParent(); } // Unless we have an init-capture, we've gone one step too far. - if (!var->isInitCapture()) + if (!Var->isInitCapture()) DC = Prev; return (isa<BlockDecl>(DC) ? NCCK_Block : NCCK_Lambda); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits