r293199 - Turn on -Wblock-capture-autoreleasing by default.

[Akira Hatanaka via cfe-commits]

Hi Nico,

The documentation might confuse people now that the warning is on by default, but it’s correct as clang still qualifies indirect parameters with __autoreleasing. We had to add this warning and turn it on by default because a lot of people capture indirect parameters in their code not realizing the object assigned might get released by an autorelease pool.

Perhaps we can change the warning message or the documentation to make it clear that the parameter has been implicitly marked __autoreleasing regardless.

> On Feb 11, 2017, at 11:54 AM, Nico Weber <thakis at chromium.org> wrote:
> 
> Hi Akira,
> 
> this fires in internal chrome/ios code like so:
> 
> somefile.m: error: block captures an autoreleasing out-parameter, which may result in use-after-free bugs [-Werror,-Wblock-capture-autoreleasing]
>           if (error) {
>               ^
> somefile.m: note: declare the parameter __autoreleasing explicitly to suppress this warning
> - (NSDictionary *)fooWithError:(NSError **)error {
>                                                   ^
>                                                   __autoreleasing
> somefile.m: note: declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools
> 
> 
> A colleague points out that http://clang.llvm.org/docs/AutomaticReferenceCounting.html#indirect-parameters <http://clang.llvm.org/docs/AutomaticReferenceCounting.html#indirect-parameters> suggests NSError ** should "be implicitly qualified with __autoreleasing." Is that a bug in the implementation of the warning, is AutomaticReferenceCounting.html incorrect, or are we just misunderstanding that document?
> 
> Thanks,
> Nico
> 
> On Thu, Jan 26, 2017 at 1:51 PM, Akira Hatanaka via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
> Author: ahatanak
> Date: Thu Jan 26 12:51:10 2017
> New Revision: 293199
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=293199&view=rev <http://llvm.org/viewvc/llvm-project?rev=293199&view=rev>
> Log:
> Turn on -Wblock-capture-autoreleasing by default.
> 
> Turning on the warning by default helps the users as it's a common
> mistake to capture out-parameters in a block without ensuring the object
> assigned doesn't get released.
> 
> rdar://problem/30200058
> 
> Modified:
>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>     cfe/trunk/test/SemaObjC/arc.m
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=293199&r1=293198&r2=293199&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=293199&r1=293198&r2=293199&view=diff>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Jan 26 12:51:10 2017
> @@ -5185,7 +5185,7 @@ def err_arc_inconsistent_property_owners
>  def warn_block_capture_autoreleasing : Warning<
>    "block captures an autoreleasing out-parameter, which may result in "
>    "use-after-free bugs">,
> -  InGroup<BlockCaptureAutoReleasing>, DefaultIgnore;
> +  InGroup<BlockCaptureAutoReleasing>;
>  def note_declare_parameter_autoreleasing : Note<
>    "declare the parameter __autoreleasing explicitly to suppress this warning">;
>  def note_declare_parameter_strong : Note<
> 
> Modified: cfe/trunk/test/SemaObjC/arc.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc.m?rev=293199&r1=293198&r2=293199&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc.m?rev=293199&r1=293198&r2=293199&view=diff>
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/arc.m (original)
> +++ cfe/trunk/test/SemaObjC/arc.m Thu Jan 26 12:51:10 2017
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -fblocks -verify -Wno-objc-root-class -Wblock-capture-autoreleasing %s
> +// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -fblocks -verify -Wno-objc-root-class %s
> 
>  typedef unsigned long NSUInteger;
>  typedef const void * CFTypeRef;
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <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/20170212/dde4244c/attachment.html>