On Tue, 1 Feb 2022 at 23:55, Martin Sebor via Gcc-help
<gcc-help@xxxxxxxxxxx> wrote:
>
> On 1/31/22 22:18, Richard Sandiford wrote:
> > I agree that changing the wording doesn't solve the whole problem,
> > but I think it does solve something. At the moment, we're effectively
> > asking each individual user to be aware of the context above, to know what
> > meaning is being attached to the present tense. Making the message more
> > equivocal would at least suggest that the compiler doesn't “know”.
>
> The compiler can never "know" for certain if any statement will be
> executed. Every warning message that involves control or data flow
> must be interpreted in the context of the surrounding code, whether
> it's an expression, statement, or the whole function. Every warning
> message must necessarily also be understood as only suggesting that
> "there may be a problem" in the program rather than "there definitely
> is a bug."
>
> There's plenty of literature out there that explains this, including
> the GCC manual, so I'd expect most C/C++ programmers to understand
> that.
I disagree. So does the manual:
-Warray-bounds
-Warray-bounds=n
This option is only active when -ftree-vrp is active
(default for -O2 and above). It
warns about subscripts to arrays that are always out of
bounds. This warning is
enabled by -Wall.
If we're going to claim that it's common knowledge that warnings are
always contextual and not definite, can we not use language like
"always out of bounds"? How else am I supposed to read that other than
"always"? Always, under specific conditions? That's not what the word
means.
And the problem with saying they must be interpreted in the context of
the surrounding code is that a compiler can completely restructure the
code, and create unreachable paths that are not present in the
surrounding code. We have numerous examples where the surrounding code
makes it very clear that the problem cannot happen, and yet GCC warns,
e.g. https://gcc.gnu.org/PR104017#c1
So really you have to interpret the warning in the context of some
*different* piece of code that was dreamt up by the compiler, which is
not at all obvious unless you know how to read the entrails in a GCC
dump file.
I don't think that rewording every warning message just to
> drill that message home and without addressing the bigger problem
> would make enough of a difference to justify the effort. (Users
> don't feel any better about -Wmaybe-uninitialized false positives
> than about -Wuninitalized, and they've periodically argued to
> remove the former from -Wall despite its equivocal phrasing.)
>
> (That said, with my tongue in cheek, the original phrasing of these
> warnings (up to GCC 6) was:
>
> call to 'memcpy' will always overflow destination buffer
>
> so some progress has been made on this front ;-)
>
> >
> > It's not the pass's “fault” that it can't tell how closely the IL
> > it sees matches the original code. But it is GCC's “fault” as a whole,
> > not the user's, and I think that's what matters here.
>
> I agree. I think there are at least two underlying problems: users
> expecting every warning message to point out an actual bug in their
> code, and GCC failing to somehow indicate the conditional nature of
> the problems in the messages.
>
> >> What I think is needed here is to mention the conditions
> >> under which the invalid statement is executed. With that, we should
> >> be able to print the same context as what the static analyzer prints:
> >>
> >> warning: dereference of NULL ‘0’ [CWE-476] [-Wanalyzer-null-dereference]
> >> 43 | h2->h21 = 0;
> >> | ~~~~~~~~^~~
> >> ‘foo’: events 1-3
> >> |
> >> | 39 | *l4data = p.l4_ofs != UINT16_MAX ? (char *) p.base_ +
> >> p.l4_ofs : NULL;
> >> | |
> >> ^
> >> | |
> >> |
> >> | |
> >> (1) following ‘false’ branch...
> >>
> >> |......
> >> | 42 | py(h2 + 1, &somedata[0], 6);
> >> | | ~~~~~~
> >>
> >> | | |
> >> | | (2) ...to here
> >>
> >> | 43 | h21 = 0;
> >> | | ~~~~~~~
> >>
> >> | | |
> >> | | (3) dereference of NULL ‘<unknown>’
> >
> > I agree this would be better, but I don't think it should hold back
> > tweaks to the error messages in the meantime. (And if the tracing
> > was an optional feature, we'd still want wording that makes sense
> > when the feature is turned off.)
> >
> > I realise this is rehashing an old discussion, sorry. But it seems
> > like it's a discussion that gets rehashed precisely because it's
> > about an issue that users keep hitting.
>
> It's an old issue that users are running into increasingly often
> as the middle end warnings keep getting "smarter." Although that's
> not the case here, the switch to Ranger in GCC 12 in particular has
> made the data flow analysis used by warnings like -Wstringop-overflow
> much more accurate. That's exactly what we wanted; what we
> underestimated is the extent to which the warnings expose unreachable
> statements, or those that are reachable under complex conditions, or
> that are synthesized by GCC seemingly out of thin air. All these are
> problematic, especially when the conditions involve layers of libstdc++
> (or other third party) template code. From a user's point of view,
> there's no difference between the two.
>
> Indicating the flow like in the above isn't too difficult. I put
> together a prototype that does that for some warnings, including
> -Wmaybe-uninitialized and a subset of -Wstringop-overflow. What
> it doesn't capture (and what I think is essential) is conditions
> involved in determining ranges of values like pointer offsets,
> array bounds, or memcpy sizes. To capture those we either need
> to extend Ranger or develop a separate range analyzer just for
> these warnings.
>
> Martin
>
> >
> > Thanks,
> > Richard
>
