linux-kernel - Re: [patch] compiler, clang: suppress warning for unused static inline functions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.10.1705301704370.10695@chino.kir.corp.google.com>
Date:   Tue, 30 May 2017 17:10:10 -0700 (PDT)
From:   David Rientjes <rientjes@...gle.com>
To:     Doug Anderson <dianders@...omium.org>
cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        Christoph Lameter <cl@...ux.com>,
        Pekka Enberg <penberg@...nel.org>,
        Joonsoo Kim <iamjoonsoo.kim@....com>, linux-mm@...ck.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Mark Brown <broonie@...nel.org>,
        Ingo Molnar <mingo@...nel.org>,
        David Miller <davem@...emloft.net>
Subject: Re: [patch] compiler, clang: suppress warning for unused static
 inline functions

On Wed, 24 May 2017, Doug Anderson wrote:

> * Matthias has been sending out individual patches that take each
> particular case into account to try to remove the warnings.  In some
> cases this removes totally dead code.  In other cases this adds
> __maybe_unused.  ...and as a last resort it uses #ifdef.  In each of
> these individual patches we wouldn't want a list of all other patches,
> I think.
> 

Again, I defer to maintainers like Andrew and Ingo who have to deal with 
an enormous amount of patches on how they would like to handle it; I don't 
think myself or anybody else who doesn't deal with a large number of 
patches should be mandating how it's handled.

For reference, the patchset that this patch originated from added 8 lines 
and removed 1, so I disagree that this cleans anything up; in reality, it 
obfuscates the code and makes the #ifdef nesting more complex.

> If you just want a list of things in response to this thread...
> 
> Clang's behavior has found some dead code, as shown by:
> 
> * https://patchwork.kernel.org/patch/9732161/
>   ring-buffer: Remove unused function __rb_data_page_index()
> * https://patchwork.kernel.org/patch/9735027/
>   r8152: Remove unused function usb_ocp_read()
> * https://patchwork.kernel.org/patch/9735053/
>   net1080: Remove unused function nc_dump_ttl()
> * https://patchwork.kernel.org/patch/9741513/
>   crypto: rng: Remove unused function __crypto_rng_cast()
> * https://patchwork.kernel.org/patch/9741539/
>   x86/ioapic: Remove unused function IO_APIC_irq_trigger()
> * https://patchwork.kernel.org/patch/9741549/
>   ASoC: Intel: sst: Remove unused function sst_restore_shim64()
> * https://patchwork.kernel.org/patch/9743225/
>   ASoC: cht_bsw_max98090_ti: Remove unused function cht_get_codec_dai()
> 
> ...plus more examples...
> 

Let us please not confuse the matter by suggesting that you cannot 
continue to do this work by simply removing the __attribute__((unused)) 
and emailing kernel-janitors to cleanup unused code (which should already 
be significantly small by the sheer fact that it is inlined).

> However, clang's behavior has also led to patches that add a
> "__maybe_unused" attribute (usually no increase in LOC unless it
> causes word wrap) and also added a handful of #ifdefs, as you've
> pointed out.  The example we already talked about was:
> 

The good work to remove truly dead code may easily continue while not 
adding more and more LOC to suppress these warnings for a compiler that is 
very heavily in the minority.