r295150 - [Sema] Disallow returning a __block variable via a move.

[Akira Hatanaka via cfe-commits]

Hans,

Can this be merged to 4.0 too?

> On Feb 14, 2017, at 9:15 PM, Akira Hatanaka via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> 
> Author: ahatanak
> Date: Tue Feb 14 23:15:28 2017
> New Revision: 295150
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=295150&view=rev
> Log:
> [Sema] Disallow returning a __block variable via a move.
> 
> r274291 made changes to prefer calling a move constructor to calling a
> copy constructor when returning from a function. This caused programs to
> crash when a __block variable in the heap was moved out and used later.
> 
> This commit fixes the bug by disallowing moving out of __block variables
> implicitly.
> 
> rdar://problem/28181080
> 
> Differential Revision: https://reviews.llvm.org/D29908
> 
> Modified:
>    cfe/trunk/lib/Sema/SemaStmt.cpp
>    cfe/trunk/test/SemaObjCXX/blocks.mm
> 
> Modified: cfe/trunk/lib/Sema/SemaStmt.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=295150&r1=295149&r2=295150&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaStmt.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaStmt.cpp Tue Feb 14 23:15:28 2017
> @@ -2743,15 +2743,17 @@ bool Sema::isCopyElisionCandidate(QualTy
>   // ...automatic...
>   if (!VD->hasLocalStorage()) return false;
> 
> +  // Return false if VD is a __block variable. We don't want to implicitly move
> +  // out of a __block variable during a return because we cannot assume the
> +  // variable will no longer be used.
> +  if (VD->hasAttr<BlocksAttr>()) return false;
> +
>   if (AllowParamOrMoveConstructible)
>     return true;
> 
>   // ...non-volatile...
>   if (VD->getType().isVolatileQualified()) return false;
> 
> -  // __block variables can't be allocated in a way that permits NRVO.
> -  if (VD->hasAttr<BlocksAttr>()) return false;
> -
>   // Variables with higher required alignment than their type's ABI
>   // alignment cannot use NRVO.
>   if (!VD->getType()->isDependentType() && VD->hasAttr<AlignedAttr>() &&
> 
> Modified: cfe/trunk/test/SemaObjCXX/blocks.mm
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjCXX/blocks.mm?rev=295150&r1=295149&r2=295150&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaObjCXX/blocks.mm (original)
> +++ cfe/trunk/test/SemaObjCXX/blocks.mm Tue Feb 14 23:15:28 2017
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class %s
> +// RUN: %clang_cc1 -fsyntax-only -verify -fblocks -Wno-objc-root-class -std=c++11 %s
> @protocol NSObject;
> 
> void bar(id(^)(void));
> @@ -144,3 +144,17 @@ namespace DependentReturn {
> 
>   template void f<X>(X);
> }
> +
> +namespace MoveBlockVariable {
> +struct B0 {
> +};
> +
> +struct B1 { // expected-note 2 {{candidate constructor (the implicit}}
> +  B1(B0&&); // expected-note {{candidate constructor not viable}}
> +};
> +
> +B1 test_move() {
> +  __block B0 b;
> +  return b; // expected-error {{no viable conversion from returned value of type 'MoveBlockVariable::B0' to function return type 'MoveBlockVariable::B1'}}
> +}
> +}
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits