On Mon, 2018-07-23 at 17:49 -0600, Martin Sebor wrote:
> (David, I'm hoping your your help here. Please see the end.)
>
> While looking into a recent -Warray-bounds instance in Glibc
> involving inlining of large functions it became apparent that
> GCC could do a better job of pinpointing the source of
> the problem.
>
> The attached patch makes a few adjustments to both
> the pretty printer infrastructure and to VRP to make this
> possible. The diagnostic pretty printer already has directives
> to print the inlining context for both tree and gcall* arguments,
> so most of the changes just adjust things to be able to pass in
> gimple* argument instead.
>
> The only slightly interesting change is to print the declaration
> to which the out-of-bounds array refers if one is known.
>
> Tested on x86_64-linux with one regression.
>
> The regression is in the gcc.dg/Warray-bounds.c test: the column
> numbers of the warnings are off. Adding the %G specifier to
> the array bounds warnings in VRP has the unexpected effect of
> expanding the extent of the underling. For instance, for a test
> case like this:
>
> int a[10];
>
> void f (void)
> {
> a[-1] = 0;
> }
>
> from the expected:
>
> a[-1] = 0;
> ~^~~~
>
> to this:
>
> a[-1] = 0;
> ~~~~~~^~~
>
> David, do you have any idea how to avoid this?
Are you referring to the the various places in your patch (in e.g.
vrp_prop::check_array_ref
vrp_prop::check_mem_ref
vrp_prop::search_for_addr_array
) where the patch changed things from this form:
warning_at (location, OPT_Warray_bounds,
"[...format string...]", ARGS...);
to this form:
warning_at (location, OPT_Warray_bounds,
"%G[...format string...]", stmt, ARGS...);
If so, there are two location_t values of interest here:
(a) the "location" value, and
(b) gimple_location (stmt)
My recollection is that %G and %K override the "location" value passed
in as the first param to the diagnostic call, overwriting it within the
diagnostic_info's text_info with the location value from the %K/%G
(which also set up the pp_ti_abstract_origin of the text_info from the
block information stashed in the ad-hoc data part of the location, so
that the pretty-printer prints the inlining chain).
[aside, why don't we always just print the inlining chain? IIRC, %K
and %G feel too much like having to jump through hoops to me, given
that gimple_block is looking at gimple_location anyway, why not just
use the location in the location_t's ad-hoc data; I have a feeling
there's a PR open about this, but I don't have it to hand right now].
It looks like the old location was (a), and now you're seeing (b),
since (b) is the location of the statement.
Whether or not this is a problem is difficult to tell; what does the
full diagnostic look like? (you only quoted the diagnostic_show_locus
part of it).
Hope this is helpful
Dave
