Re: [PATCH] enhance buffer overflow warnings (and c/53562)

[Martin Sebor <msebor at gmail dot com>]

> Sure, they might and in that case the warning would be a false
> positive.  It wouldn't be the first such warning that wasn't 100%
> free of them.  But my testing with Binutils, GCC, and the Linux
> kernel has exposed only 10 instances of new warnings and I don't
> think I saw this idiom among them.  But even if some were, or if
> all of them were false positives I think that would be well within
> the acceptable rates.

I've gone through all the new warnings.  With one exception where
it's unclear what exactly is going on they all appear to be due
to the idiom of writing into a struct member to clear or copy
data into the member and those that follow to the end of the
struct, like so:

  struct S { int a, b, c } *s;
  memset (&s->b, 0, sizeof *s - offsetof (struct S, b));

It should be doable to recognize this idiom and suppress the
warning in this case.  Let me work on enhancing the patch to
do that.

> Here are the numbers of warnings for
> Binutils and the kernel:
>
> 113   -Wimplicit-fallthrough
>  38   -Wformat-length=
>  12   -Wunused-const-variable=
>  10   -Wstringop-overflow
>   2   -Wdangling-else
>   2   -Wframe-address
>   2   -Wint-in-bool-context
>   1   -Wbool-operation
>
> > Using some header structure at the
> > beginning and then conditionally on fields in that structure various
> > payloads occurs in many projects, starting with glibc, gcc, Linux kernel,
> > ... The warning really must not be detached from reality.
>
> That's an unfair assertion in light of the numbers above.
>
> > If you want a warning for suspicious calls, sure, but
> > 1) it has to be clearly worded significantly differently from how do you
> >    word it, so that users really understand you are warning about
> >    suspicious code (though, I really believe it is very common and there
> >    is really nothing the users can do about it)
> > 2) it really shouldn't be enabled in -Wall, and I think not even in
> > -Wextra
>
> As you have argued yourself recently in our discussion of
> -Wimplicit-fallthrough, warnings that aren't enabled by either
> of these options don't generally benefit users because very few
> turn them on explicitly.  I agree with that argument although I
> would be in favor of rolling out a new warning gradually over
> two or more releases if it were known to be prone to high rates
> of false positive. The -Wstringop-overflow warning clearly isn't
> in that category so there's no need for it.  My offer to do
> additional testing is still good if you'd like to see more data.
>
> As for the wording, I welcome suggestions for improvements.
>
> Martin