Re: [PATCH] Fix -Os related -Werror failures.

[Andrew Pinski <pinskia at gmail dot com>]

On Fri, Oct 28, 2016 at 1:12 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday, October 28, 2016 12:44:32 AM CEST Jeff Law wrote:
>> On 10/28/2016 12:32 AM, Florian Weimer wrote:
>> > On 10/28/2016 06:46 AM, Carlos O'Donell wrote:
>> >> +/* With GCC 5.3 when compiling with -Os the compiler emits a warning
>> >> +   that buf[0] and buf[1] may be used uninitialized.  This can only
>> >> +   happen in the case where tmpbuf[3] is used, and in that case the
>> >> +   write to the tmpbuf[1] and tmpbuf[2] was assured because
>> >> +   ucs4_to_cns11643 would have filled in those entries.  The difficulty
>> >> +   is in getting the compiler to see this logic because tmpbuf[0] is
>> >> +   involved in determining the code page and is the indicator that
>> >> +   tmpbuf[2] is initialized.  */
>> >> +DIAG_PUSH_NEEDS_COMMENT;
>> >> +DIAG_IGNORE_NEEDS_COMMENT (5.3, "-Wmaybe-uninitialized");
>> >
>> > This hides the warning for -O2 builds as well, so I don't think this is
>> > a good idea.
>> >
>> > Those who want to build with -Os or other special compiler flags should
>> > just configure with --disable-werror.  We can't account for every
>> > optimization someone might want to disable in their build.
>> That'd be my recommendation.
>>
>> What often happens in these cases is the compiler in its default mode of
>> operation is able to statically eliminate a conditional branch on a
>> particular path.  However, to do so the compiler has to duplicate code.
>>
>> Not surprisingly, there's a cost/benefit tradeoff here and the
>> heuristics are largely driven by the real or estimated profile data as
>> well as the coarser "optimize for code space".  So changing flags
>> changes the output of those heuristics and ultimately can result in
>> leaving paths in the CFG that can not be executed -- and that often
>> leads to false positive may-be-uninitialized warnings and such.
>>
>> Long term I would like to find a good way to mark paths that are not
>> executable, but are not profitable to eliminate, then utilize that
>> information to prune various "may" warnings.  That would make those kind
>> of warnings more stable across different optimization levels as well as
>> more stable release-to-release.  But that's definitely in the "future
>> work" area.
>
> I've spent a lot of time trying to eliminate -Wmaybe-uninitialized
> warnings from the Linux kernel. Here are some data points that you
> may find useful too:
>
> - Building with -Os causes many false positives starting with gcc-4.9,
>   and I have disabled the warning for this specific flag. I believe
>   this is due to the lack of the "-fschedule-insns" optimization step

No this is false.  It is usually due to jump threading is not as
aggressive at -O2 than -Os due to -Os not wanting to increase code
size.

Thanks,
Andrew

> - Building with -O3 apparently also causes some false positives, but
>   we don't normally do that in the kernel, and the only architecture
>   port that does it also disables the warnings
> - Two more gcc options that cause false positives are -fprofile-arcs
>   and some of the -fsanitize=... options
> - overall, gcc-4.9 improved much over gcc-4.8 in these warnings,
>   but they have a different set of false-positives. As gcc-4.8 is
>   getting old, I'm pushing a patch to also disable the warning
>   for all 4.8 builds. Prior to v4.8, there was no option to disable
>   maybe-uninitialized warnings.
> - gcc-5 and gcc-6 appear to be slightly better than gcc-4.9 but also
>   introduce a small number of additional false-positive warnings,
>   apparently this happens mostly because they make different
>   inlining decisions, not because something fundamentally changed.
>   Generally speaking, if any of 4.9, 4.x or 5.x produce a warning
>   in some configurations, it's likely that the other ones will
>   do the same, depending on a combination target architecture and
>   optimization flags that impact inlining.
> - I found that most often when gcc is confused about whether a
>   variable is uninitialized or not, the source code tends to be
>   confusing to a human reader as well and rewriting it differently
>   results in better readability and better object code while
>   avoiding the warning. There are always other cases in which
>   this is not possible though.
>
>         Arnd