| Message ID | 8949a6b5-7fda-0a4a-4039-f4f8e183871c@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | include more detail in -Warray-bounds (PR 86650) | expand |
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
On 07/23/2018 07:20 PM, David Malcolm wrote: > 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...); Yes. > > 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). Would having the pretty printer restore the location and the block after it's done printing the context and before processing the rest of the format string fix it? (I have only a vague idea how this all works so I'm not sure if this even makes sense.) > > [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]. That would make sense to me. I think that's also what we agreed would be the way forward the last time we discussed this. > > It looks like the old location was (a), and now you're seeing (b), > since (b) is the location of the statement. Yes, I think that's it. > > 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). It looks like this: e.c: In function ‘f’: e.c:5:9: warning: array subscript -1 is below array bounds of ‘int[10]’ [-Warray-bounds] a[-1] = 0; ~~~~~~^~~ e.c:1:5: note: while referencing ‘a’ int a[10]; ^ Martin
On Mon, 2018-07-23 at 20:56 -0600, Martin Sebor wrote: > On 07/23/2018 07:20 PM, David Malcolm wrote: > > 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...); > > Yes. > > > > > 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). > > Would having the pretty printer restore the location and > the block after it's done printing the context and before > processing the rest of the format string fix it? (I have > only a vague idea how this all works so I'm not sure if > this even makes sense.) Structurally, it looks like this: Temporaries during the emission of | Long-lived stuff: the diagnostic: | | +---------+ +----------------+ | |global_dc| |diagnostic_info | | +---------+ |+------------+ | | ||text_info: | | | || m_richloc-+--+---> rich_location | || x_data----+--+-------------------+--> block (via pp_ti_abstract_origin) |+------------+ | | +----------------+ | | The location_t of the diagnostic is stored in the rich_location. Calling: warning_at (location) creates a rich_location wrapping "location" and uses it as above. During formatting, the %K/%G codes set text_info.x_data via pp_ti_abstract_origin and overwrite the location_t in the rich_location. So in theory we could have a format code that sets the block and doesn't touch the rich_location. But that seems like overkill to me. > > > > [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]. > > That would make sense to me. I think that's also what we > agreed would be the way forward the last time we discussed > this. (nods) > > > > It looks like the old location was (a), and now you're seeing (b), > > since (b) is the location of the statement. > > Yes, I think that's it. > > > > > 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). > > It looks like this: > > e.c: In function ‘f’: > e.c:5:9: warning: array subscript -1 is below array bounds of > ‘int[10]’ > [-Warray-bounds] > a[-1] = 0; > ~~~~~~^~~ > e.c:1:5: note: while referencing ‘a’ > int a[10]; > ^ FWIW, either of the locations/underlining above seem fine to me, but I feel the wording of the diagnostic could be improved. When I first read: array subscript -1 is below array bounds of ‘int[10]’ I found myself pausing to think about the message; the "10" in the message confused me for a moment, and then I realized that it's referring to the *lower* bound of the array. I don't know how "standardese" refers to this, but I think about it as the "start" of the array. So maybe the messages could read something like: out of bounds access to array ‘int[10]’: subscript -1 is before the start of the array or something. In a similar vein, is it easy to distinguish reads from writes, which might give: out of bounds write to array ‘int[10]’: subscript -1 writes to before the start of the array vs e.g.: out of bounds read from array 'int[10]': subscript 10 reads from after 'a[9]', the final element of the array and similar (I'm brainstorming here). Dave
On 07/24/2018 11:05 AM, David Malcolm wrote: > On Mon, 2018-07-23 at 20:56 -0600, Martin Sebor wrote: >> On 07/23/2018 07:20 PM, David Malcolm wrote: >>> 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...); >> >> Yes. >> >>> >>> 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). >> >> Would having the pretty printer restore the location and >> the block after it's done printing the context and before >> processing the rest of the format string fix it? (I have >> only a vague idea how this all works so I'm not sure if >> this even makes sense.) > > Structurally, it looks like this: > > Temporaries during the emission of | Long-lived stuff: > the diagnostic: | > | +---------+ > +----------------+ | |global_dc| > |diagnostic_info | | +---------+ > |+------------+ | | > ||text_info: | | | > || m_richloc-+--+---> rich_location | > || x_data----+--+-------------------+--> block (via pp_ti_abstract_origin) > |+------------+ | | > +----------------+ | > | > > The location_t of the diagnostic is stored in the rich_location. > > Calling: > warning_at (location) > creates a rich_location wrapping "location" and uses it as above. > > During formatting, the %K/%G codes set text_info.x_data via > pp_ti_abstract_origin and overwrite the location_t in the > rich_location. > > So in theory we could have a format code that sets the block and > doesn't touch the rich_location. But that seems like overkill to me. I wasn't thinking of a new format. Rather, I thought the %K would save the current block and location (set by the location argument to warning_at), then after printing the inlining stack but before printing the rest of the diagnostic the printer would restore the saved block and location. I still don't know enough to tell if it would work. In any event, if it's easier to always print the inlining stack and get rid of %K and %G then that would be preferable. I don't think they are used for any other purpose (i.e., they are always used as the first directive in a format string). >>> [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]. >> >> That would make sense to me. I think that's also what we >> agreed would be the way forward the last time we discussed >> this. > > (nods) So how do we go about making this happen? Somewhat selfishly I was sort of waiting for you to take the lead on it since you're much more familiar with the code than I am :) But that doesn't mean I can't try to tackle it myself. If it seems like something you can fit into your schedule sometime soonish let me know. Otherwise I'll see if I can figure it out myself. In the meantime, I'll proceed with the rest of this patch sans the %G bits. >>> It looks like the old location was (a), and now you're seeing (b), >>> since (b) is the location of the statement. >> >> Yes, I think that's it. >> >>> >>> 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). >> >> It looks like this: >> >> e.c: In function ‘f’: >> e.c:5:9: warning: array subscript -1 is below array bounds of >> ‘int[10]’ >> [-Warray-bounds] >> a[-1] = 0; >> ~~~~~~^~~ >> e.c:1:5: note: while referencing ‘a’ >> int a[10]; >> ^ > > FWIW, either of the locations/underlining above seem fine to me, The underling in the warning isn't right. It doesn't matter so much in the straightforward case of a one-dimensional array but it becomes a problem when the warning involves a mult-dimensional array. Contrast the output for this code: const int i0 = 5, k0 = 0, i1 = 0, k1 = 5; int a[2][3]; void f (void) { a[i0][k0] = 0; a[i1][k1] = 0; } With my patch (without the note) we get: e.c:7:13: warning: array subscript 5 is above array bounds of ‘int[2][3]’ [-Warray-bounds] a[i0][k0] = 0; ~~~~~~~~~~^~~ e.c:8:13: warning: array subscript 5 is above array bounds of ‘int[3]’ [-Warray-bounds] a[i1][k1] = 0; ~~~~~~~~~~^~~ when without the patch we get: e.c:7:4: warning: array subscript 5 is above array bounds of ‘int[2][3]’ [-Warray-bounds] a[i0][k0] = 0; ~^~~~ e.c:8:8: warning: array subscript 5 is above array bounds of ‘int[3]’ [-Warray-bounds] a[i1][k1] = 0; ~~~~~^~~~ With the patch the warning doesn't clearly indicate which of the two indices is out of bounds. I don't think we should lose that feature. > but I > feel the wording of the diagnostic could be improved. When I first > read: > > array subscript -1 is below array bounds of ‘int[10]’ > > I found myself pausing to think about the message; the "10" in the > message confused me for a moment, and then I realized that it's > referring to the *lower* bound of the array. I don't know how > "standardese" refers to this, but I think about it as the "start" of > the array. So maybe the messages could read something like: > > out of bounds access to array ‘int[10]’: subscript -1 is before the > start of the array I agree that the warnings could be tweaked. With the inclusion of the value of the out-of-bounds index (or its range) and with the type of the referenced array in the diagnostic it's no longer necessary to mention whether the index is above or below bounds. It's enough to say that it's out of bounds. It will not only obviate questions like the one above but also simplify the code (and also simplify writing tests). > > or something. In a similar vein, is it easy to distinguish reads from > writes, which might give: > > out of bounds write to array ‘int[10]’: subscript -1 writes to before > the start of the array > > vs e.g.: > > out of bounds read from array 'int[10]': subscript 10 reads from > after 'a[9]', the final element of the array > > and similar (I'm brainstorming here). Thanks for your comments -- they're always helpful! Martin
Attached is a much scaled-down patch that only adds a note to the -Warray-bounds warnings mentioning the declaration to which the out-of-bounds index or offset applies. Printing the inlining context needs a bit more work but this small improvement can be made independently of it. On 07/23/2018 05:49 PM, 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? > > Martin PR tree-optimization/86650 - -Warray-bounds missing inlining context gcc/ChangeLog: PR tree-optimization/86650 * tree-vrp.c (vrp_prop::check_array_ref): Print an inform message. (vrp_prop::check_mem_ref): Same. gcc/testsuite/ChangeLog: PR tree-optimization/86650 * gcc.dg/Warray-bounds-33.c: New test. diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index 7ab8898..e553f3a 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -4838,14 +4838,13 @@ vrp_prop::check_array_ref (location_t location, tree ref, tree artype = TREE_TYPE (TREE_OPERAND (ref, 0)); + bool warned = false; + /* Empty array. */ if (up_bound && tree_int_cst_equal (low_bound, up_bound_p1)) - { - warning_at (location, OPT_Warray_bounds, - "array subscript %E is above array bounds of %qT", - low_bound, artype); - TREE_NO_WARNING (ref) = 1; - } + warned = warning_at (location, OPT_Warray_bounds, + "array subscript %E is above array bounds of %qT", + low_bound, artype); if (TREE_CODE (low_sub) == SSA_NAME) { @@ -4866,12 +4865,10 @@ vrp_prop::check_array_ref (location_t location, tree ref, : tree_int_cst_le (up_bound, up_sub)) && TREE_CODE (low_sub) == INTEGER_CST && tree_int_cst_le (low_sub, low_bound)) - { - warning_at (location, OPT_Warray_bounds, - "array subscript [%E, %E] is outside array bounds of %qT", - low_sub, up_sub, artype); - TREE_NO_WARNING (ref) = 1; - } + warned = warning_at (location, OPT_Warray_bounds, + "array subscript [%E, %E] is outside " + "array bounds of %qT", + low_sub, up_sub, artype); } else if (up_bound && TREE_CODE (up_sub) == INTEGER_CST @@ -4885,10 +4882,9 @@ vrp_prop::check_array_ref (location_t location, tree ref, dump_generic_expr (MSG_NOTE, TDF_SLIM, ref); fprintf (dump_file, "\n"); } - warning_at (location, OPT_Warray_bounds, - "array subscript %E is above array bounds of %qT", - up_sub, artype); - TREE_NO_WARNING (ref) = 1; + warned = warning_at (location, OPT_Warray_bounds, + "array subscript %E is above array bounds of %qT", + up_sub, artype); } else if (TREE_CODE (low_sub) == INTEGER_CST && tree_int_cst_lt (low_sub, low_bound)) @@ -4899,9 +4895,18 @@ vrp_prop::check_array_ref (location_t location, tree ref, dump_generic_expr (MSG_NOTE, TDF_SLIM, ref); fprintf (dump_file, "\n"); } - warning_at (location, OPT_Warray_bounds, - "array subscript %E is below array bounds of %qT", - low_sub, artype); + warned = warning_at (location, OPT_Warray_bounds, + "array subscript %E is below array bounds of %qT", + low_sub, artype); + } + + if (warned) + { + ref = TREE_OPERAND (ref, 0); + + if (DECL_P (ref)) + inform (DECL_SOURCE_LOCATION (ref), "while referencing %qD", ref); + TREE_NO_WARNING (ref) = 1; } } @@ -4916,7 +4921,8 @@ vrp_prop::check_array_ref (location_t location, tree ref, the address of the just-past-the-end element of an array). */ void -vrp_prop::check_mem_ref (location_t location, tree ref, bool ignore_off_by_one) +vrp_prop::check_mem_ref (location_t location, tree ref, + bool ignore_off_by_one) { if (TREE_NO_WARNING (ref)) return; @@ -5134,16 +5140,21 @@ vrp_prop::check_mem_ref (location_t location, tree ref, bool ignore_off_by_one) offrange[1] = offrange[1] / eltsize; } + bool warned; if (offrange[0] == offrange[1]) - warning_at (location, OPT_Warray_bounds, - "array subscript %wi is outside array bounds " - "of %qT", - offrange[0].to_shwi (), reftype); + warned = warning_at (location, OPT_Warray_bounds, + "array subscript %wi is outside array bounds " + "of %qT", + offrange[0].to_shwi (), reftype); else - warning_at (location, OPT_Warray_bounds, - "array subscript [%wi, %wi] is outside array bounds " - "of %qT", - offrange[0].to_shwi (), offrange[1].to_shwi (), reftype); + warned = warning_at (location, OPT_Warray_bounds, + "array subscript [%wi, %wi] is outside " + "array bounds of %qT", + offrange[0].to_shwi (), + offrange[1].to_shwi (), reftype); + if (warned && DECL_P (arg)) + inform (DECL_SOURCE_LOCATION (arg), "while referencing %qD", arg); + TREE_NO_WARNING (ref) = 1; return; } @@ -5183,60 +5194,69 @@ vrp_prop::search_for_addr_array (tree t, location_t location) } while (handled_component_p (t) || TREE_CODE (t) == MEM_REF); - if (TREE_CODE (t) == MEM_REF - && TREE_CODE (TREE_OPERAND (t, 0)) == ADDR_EXPR - && !TREE_NO_WARNING (t)) - { - tree tem = TREE_OPERAND (TREE_OPERAND (t, 0), 0); - tree low_bound, up_bound, el_sz; - offset_int idx; - if (TREE_CODE (TREE_TYPE (tem)) != ARRAY_TYPE - || TREE_CODE (TREE_TYPE (TREE_TYPE (tem))) == ARRAY_TYPE - || !TYPE_DOMAIN (TREE_TYPE (tem))) - return; + if (TREE_CODE (t) != MEM_REF + || TREE_CODE (TREE_OPERAND (t, 0)) != ADDR_EXPR + || TREE_NO_WARNING (t)) + return; - low_bound = TYPE_MIN_VALUE (TYPE_DOMAIN (TREE_TYPE (tem))); - up_bound = TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (tem))); - el_sz = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (tem))); - if (!low_bound - || TREE_CODE (low_bound) != INTEGER_CST - || !up_bound - || TREE_CODE (up_bound) != INTEGER_CST - || !el_sz - || TREE_CODE (el_sz) != INTEGER_CST) - return; + tree tem = TREE_OPERAND (TREE_OPERAND (t, 0), 0); + tree low_bound, up_bound, el_sz; + if (TREE_CODE (TREE_TYPE (tem)) != ARRAY_TYPE + || TREE_CODE (TREE_TYPE (TREE_TYPE (tem))) == ARRAY_TYPE + || !TYPE_DOMAIN (TREE_TYPE (tem))) + return; - if (!mem_ref_offset (t).is_constant (&idx)) - return; + low_bound = TYPE_MIN_VALUE (TYPE_DOMAIN (TREE_TYPE (tem))); + up_bound = TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (tem))); + el_sz = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (tem))); + if (!low_bound + || TREE_CODE (low_bound) != INTEGER_CST + || !up_bound + || TREE_CODE (up_bound) != INTEGER_CST + || !el_sz + || TREE_CODE (el_sz) != INTEGER_CST) + return; - idx = wi::sdiv_trunc (idx, wi::to_offset (el_sz)); - if (idx < 0) + offset_int idx; + if (!mem_ref_offset (t).is_constant (&idx)) + return; + + bool warned = false; + idx = wi::sdiv_trunc (idx, wi::to_offset (el_sz)); + if (idx < 0) + { + if (dump_file && (dump_flags & TDF_DETAILS)) { - if (dump_file && (dump_flags & TDF_DETAILS)) - { - fprintf (dump_file, "Array bound warning for "); - dump_generic_expr (MSG_NOTE, TDF_SLIM, t); - fprintf (dump_file, "\n"); - } - warning_at (location, OPT_Warray_bounds, - "array subscript %wi is below array bounds of %qT", - idx.to_shwi (), TREE_TYPE (tem)); - TREE_NO_WARNING (t) = 1; + fprintf (dump_file, "Array bound warning for "); + dump_generic_expr (MSG_NOTE, TDF_SLIM, t); + fprintf (dump_file, "\n"); } - else if (idx > (wi::to_offset (up_bound) - - wi::to_offset (low_bound) + 1)) + warned = warning_at (location, OPT_Warray_bounds, + "array subscript %wi is below " + "array bounds of %qT", + idx.to_shwi (), TREE_TYPE (tem)); + } + else if (idx > (wi::to_offset (up_bound) + - wi::to_offset (low_bound) + 1)) + { + if (dump_file && (dump_flags & TDF_DETAILS)) { - if (dump_file && (dump_flags & TDF_DETAILS)) - { - fprintf (dump_file, "Array bound warning for "); - dump_generic_expr (MSG_NOTE, TDF_SLIM, t); - fprintf (dump_file, "\n"); - } - warning_at (location, OPT_Warray_bounds, - "array subscript %wu is above array bounds of %qT", - idx.to_uhwi (), TREE_TYPE (tem)); - TREE_NO_WARNING (t) = 1; + fprintf (dump_file, "Array bound warning for "); + dump_generic_expr (MSG_NOTE, TDF_SLIM, t); + fprintf (dump_file, "\n"); } + warned = warning_at (location, OPT_Warray_bounds, + "array subscript %wu is above " + "array bounds of %qT", + idx.to_uhwi (), TREE_TYPE (tem)); + } + + if (warned) + { + if (DECL_P (t)) + inform (DECL_SOURCE_LOCATION (t), "while referencing %qD", t); + + TREE_NO_WARNING (t) = 1; } } diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-33.c b/gcc/testsuite/gcc.dg/Warray-bounds-33.c new file mode 100644 index 0000000..cea7c4b --- /dev/null +++ b/gcc/testsuite/gcc.dg/Warray-bounds-33.c @@ -0,0 +1,18 @@ +/* PR tree-optimization/86650 - -Warray-bounds missing inlining context + { dg-do compile } + { dg-options "-O2 -Warray-bounds" } */ + +int a[3]; /* { dg-message "while referencing .a." } */ +int x; + +inline void foo (int i) +{ + a[i + 1] = 123; /* { dg-warning "\\\[-Warray-bounds]" } */ +} + +int bar (void) +{ + foo (3); + + return x; +}
On 07/31/2018 12:50 PM, Martin Sebor wrote: > Attached is a much scaled-down patch that only adds a note > to the -Warray-bounds warnings mentioning the declaration > to which the out-of-bounds index or offset applies. > > Printing the inlining context needs a bit more work but > this small improvement can be made independently of it. > > On 07/23/2018 05:49 PM, 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? >> >> Martin > > > gcc-86650.diff > > > PR tree-optimization/86650 - -Warray-bounds missing inlining context > > gcc/ChangeLog: > > PR tree-optimization/86650 > * tree-vrp.c (vrp_prop::check_array_ref): Print an inform message. > (vrp_prop::check_mem_ref): Same. > > gcc/testsuite/ChangeLog: > > PR tree-optimization/86650 > * gcc.dg/Warray-bounds-33.c: New test. OK. jeff
PR tree-optimization/86650 - -Warray-bounds missing inlining context gcc/c/ChangeLog: PR tree-optimization/86650 * c-objc-common.c (c_tree_printer): Adjust. gcc/c-family/ChangeLog: PR tree-optimization/86650 * c-format.c (local_gcall_ptr_node): Rename... (local_gimple_ptr_node): ...to this. * c-format.h (T89_G): Adjust. gcc/cp/ChangeLog: PR tree-optimization/86650 * error.c (cp_printer): Adjust. gcc/ChangeLog: PR tree-optimization/86650 * gimple-pretty-print.c (percent_G_format): Simplify. * tree-diagnostic.c (default_tree_printer): Adjust. * tree-pretty-print.c (percent_K_format): Add argument. * tree-pretty-print.h: Add argument. * tree-vrp.c (vrp_prop::check_array_ref): Add argument. Print an inform message. (vrp_prop::check_mem_ref): Same. (check_array_bounds): Pass gimple statement to callees. * gimple-fold.c (gimple_fold_builtin_strncpy): Adjust. * gimple-ssa-warn-restrict.h (check_bounds_or_overlap): Replace gcall* argument with gimple*. * gimple-ssa-warn-restrict.c (check_call): Same. (wrestrict_dom_walker::before_dom_children): Same. (builtin_access::builtin_access): Same. (check_bounds_or_overlap): Same. * tree-ssa-ccp.c (pass_post_ipa_warn::execute): Adjust. * tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Adjust. gcc/testsuite/ChangeLog: PR tree-optimization/86650 * gcc.dg/Warray-bounds-33.c: New test. * gcc.dg/format/gcc_diag-10.c: Adjust. diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c index a0192dd..705bffb 100644 --- a/gcc/c-family/c-format.c +++ b/gcc/c-family/c-format.c @@ -56,7 +56,7 @@ struct function_format_info /* Initialized in init_dynamic_diag_info. */ static GTY(()) tree local_tree_type_node; -static GTY(()) tree local_gcall_ptr_node; +static GTY(()) tree local_gimple_ptr_node; static GTY(()) tree locus; static bool decode_format_attr (tree, function_format_info *, int); @@ -691,7 +691,7 @@ static const format_char_info gcc_diag_char_table[] = /* Custom conversion specifiers. */ - /* G requires a "gcall*" argument at runtime. */ + /* G requires a "gimple*" argument at runtime. */ { "G", 1, STD_C89, { T89_G, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"", NULL }, /* K requires a "tree" argument at runtime. */ { "K", 1, STD_C89, { T89_T, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"", NULL }, @@ -722,7 +722,7 @@ static const format_char_info gcc_tdiag_char_table[] = { "E", 1, STD_C89, { T89_T, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q+", "", NULL }, { "K", 1, STD_C89, { T89_T, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"", NULL }, - /* G requires a "gcall*" argument at runtime. */ + /* G requires a "gimple*" argument at runtime. */ { "G", 1, STD_C89, { T89_G, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"", NULL }, { "v", 0, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q#", "", NULL }, @@ -754,7 +754,7 @@ static const format_char_info gcc_cdiag_char_table[] = { "E", 1, STD_C89, { T89_T, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q+", "", NULL }, { "K", 1, STD_C89, { T89_T, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"", NULL }, - /* G requires a "gcall*" argument at runtime. */ + /* G requires a "gimple*" argument at runtime. */ { "G", 1, STD_C89, { T89_G, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"", NULL }, { "v", 0, STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q#", "", NULL }, @@ -787,7 +787,7 @@ static const format_char_info gcc_cxxdiag_char_table[] = { "K", 1, STD_C89,{ T89_T, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"", NULL }, { "v", 0,STD_C89, { T89_I, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "q#", "", NULL }, - /* G requires a "gcall*" argument at runtime. */ + /* G requires a "gimple*" argument at runtime. */ { "G", 1, STD_C89,{ T89_G, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }, "", "\"", NULL }, /* These accept either an 'int' or an 'enum tree_code' (which is handled as an 'int'.) */ @@ -3875,27 +3875,27 @@ init_dynamic_diag_info (void) local_tree_type_node = void_type_node; } - /* Similar to the above but for gcall*. */ - if (!local_gcall_ptr_node - || local_gcall_ptr_node == void_type_node) + /* Similar to the above but for gimple*. */ + if (!local_gimple_ptr_node + || local_gimple_ptr_node == void_type_node) { - if ((local_gcall_ptr_node = maybe_get_identifier ("gcall"))) + if ((local_gimple_ptr_node = maybe_get_identifier ("gimple"))) { - local_gcall_ptr_node - = identifier_global_value (local_gcall_ptr_node); - if (local_gcall_ptr_node) + local_gimple_ptr_node + = identifier_global_value (local_gimple_ptr_node); + if (local_gimple_ptr_node) { - if (TREE_CODE (local_gcall_ptr_node) != TYPE_DECL) + if (TREE_CODE (local_gimple_ptr_node) != TYPE_DECL) { - error ("%<gcall%> is not defined as a type"); - local_gcall_ptr_node = 0; + error ("%<gimple%> is not defined as a type"); + local_gimple_ptr_node = 0; } else - local_gcall_ptr_node = TREE_TYPE (local_gcall_ptr_node); + local_gimple_ptr_node = TREE_TYPE (local_gimple_ptr_node); } } else - local_gcall_ptr_node = void_type_node; + local_gimple_ptr_node = void_type_node; } static tree hwi; diff --git a/gcc/c-family/c-format.h b/gcc/c-family/c-format.h index f828e77..d984d10 100644 --- a/gcc/c-family/c-format.h +++ b/gcc/c-family/c-format.h @@ -298,7 +298,7 @@ struct format_kind_info #define T_UC &unsigned_char_type_node #define T99_UC { STD_C99, NULL, T_UC } #define T_V &void_type_node -#define T89_G { STD_C89, NULL, &local_gcall_ptr_node } +#define T89_G { STD_C89, NULL, &local_gimple_ptr_node } #define T89_T { STD_C89, NULL, &local_tree_type_node } #define T89_V { STD_C89, NULL, T_V } #define T_W &wchar_type_node diff --git a/gcc/c/c-objc-common.c b/gcc/c/c-objc-common.c index a841bc1..ddbd60c 100644 --- a/gcc/c/c-objc-common.c +++ b/gcc/c/c-objc-common.c @@ -67,7 +67,7 @@ c_objc_common_init (void) %D: a general decl, %E: an identifier or expression, %F: a function declaration, - %G: a Gimple call statement, + %G: a Gimple statement, %K: a CALL_EXPR, %T: a type. %V: a list of type qualifiers from a tree. @@ -99,7 +99,7 @@ c_tree_printer (pretty_printer *pp, text_info *text, const char *spec, if (*spec == 'K') { t = va_arg (*text->args_ptr, tree); - percent_K_format (text, t); + percent_K_format (text, EXPR_LOCATION (t), TREE_BLOCK (t)); return true; } diff --git a/gcc/cp/error.c b/gcc/cp/error.c index b0d8e32..6496256 100644 --- a/gcc/cp/error.c +++ b/gcc/cp/error.c @@ -4096,7 +4096,7 @@ cp_printer (pretty_printer *pp, text_info *text, const char *spec, case 'K': t = va_arg (*text->args_ptr, tree); - percent_K_format (text, t); + percent_K_format (text, EXPR_LOCATION (t), TREE_BLOCK (t)); return true; case 'H': diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c index c3fa570..506a296 100644 --- a/gcc/gimple-fold.c +++ b/gcc/gimple-fold.c @@ -1657,7 +1657,6 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi, if (!nonstring) { tree fndecl = gimple_call_fndecl (stmt); - gcall *call = as_a <gcall *> (stmt); /* Warn about the lack of nul termination: the result is not a (nul-terminated) string. */ @@ -1666,11 +1665,11 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi, warning_at (loc, OPT_Wstringop_truncation, "%G%qD destination unchanged after copying no bytes " "from a string of length %E", - call, fndecl, slen); + stmt, fndecl, slen); else warning_at (loc, OPT_Wstringop_truncation, "%G%qD destination unchanged after copying no bytes", - call, fndecl); + stmt, fndecl); } replace_call_with_value (gsi, dest); @@ -1991,7 +1990,7 @@ gimple_fold_builtin_strcat_chk (gimple_stmt_iterator *gsi) static bool gimple_fold_builtin_strncat (gimple_stmt_iterator *gsi) { - gcall *stmt = as_a <gcall *> (gsi_stmt (*gsi)); + gimple *stmt = gsi_stmt (*gsi); tree dst = gimple_call_arg (stmt, 0); tree src = gimple_call_arg (stmt, 1); tree len = gimple_call_arg (stmt, 2); diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index 19cdb40..d3c5ec6 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -2887,20 +2887,13 @@ gimple_dump_bb_for_graph (pretty_printer *pp, basic_block bb) /* Handle the %G format for TEXT. Same as %K in handle_K_format in - tree-pretty-print.c but with a Gimple call statement as an argument. */ + tree-pretty-print.c but with a Gimple statement as an argument. */ void percent_G_format (text_info *text) { - gcall *stmt = va_arg (*text->args_ptr, gcall*); + gimple *stmt = va_arg (*text->args_ptr, gimple*); - /* Build a call expression from the Gimple call statement and - pass it to the K formatter that knows how to format it. */ - tree exp = build_vl_exp (CALL_EXPR, gimple_call_num_args (stmt) + 3); - CALL_EXPR_FN (exp) = gimple_call_fn (stmt); - TREE_TYPE (exp) = gimple_call_return_type (stmt); - CALL_EXPR_STATIC_CHAIN (exp) = gimple_call_chain (stmt); - SET_EXPR_LOCATION (exp, gimple_location (stmt)); - - percent_K_format (text, exp); + tree block = gimple_block (stmt); + percent_K_format (text, gimple_location (stmt), block); } diff --git a/gcc/gimple-ssa-warn-restrict.c b/gcc/gimple-ssa-warn-restrict.c index 7dedb24..01503d6 100644 --- a/gcc/gimple-ssa-warn-restrict.c +++ b/gcc/gimple-ssa-warn-restrict.c @@ -88,7 +88,7 @@ class wrestrict_dom_walker : public dom_walker bool handle_gimple_call (gimple_stmt_iterator *); private: - void check_call (gcall *); + void check_call (gimple *); }; edge @@ -102,8 +102,7 @@ wrestrict_dom_walker::before_dom_children (basic_block bb) if (!is_gimple_call (stmt)) continue; - if (gcall *call = as_a <gcall *> (stmt)) - check_call (call); + check_call (stmt); } return NULL; @@ -192,7 +191,7 @@ class builtin_access return detect_overlap != &builtin_access::generic_overlap; } - builtin_access (gcall *, builtin_memref &, builtin_memref &); + builtin_access (gimple *, builtin_memref &, builtin_memref &); /* Entry point to determine overlap. */ bool overlap (); @@ -563,7 +562,7 @@ builtin_memref::offset_out_of_bounds (int strict, offset_int ooboff[2]) const /* Create an association between the memory references DST and SRC for access by a call EXPR to a memory or string built-in funtion. */ -builtin_access::builtin_access (gcall *call, builtin_memref &dst, +builtin_access::builtin_access (gimple *call, builtin_memref &dst, builtin_memref &src) : dstref (&dst), srcref (&src), sizrange (), ovloff (), ovlsiz (), dstoff (), srcoff (), dstsiz (), srcsiz () @@ -1324,7 +1323,7 @@ builtin_access::overlap () Return true when one has been detected, false otherwise. */ static bool -maybe_diag_overlap (location_t loc, gcall *call, builtin_access &acs) +maybe_diag_overlap (location_t loc, gimple *call, builtin_access &acs) { if (!acs.overlap ()) return false; @@ -1577,7 +1576,7 @@ maybe_diag_overlap (location_t loc, gcall *call, builtin_access &acs) has been issued. */ static bool -maybe_diag_offset_bounds (location_t loc, gcall *call, tree func, int strict, +maybe_diag_offset_bounds (location_t loc, gimple *call, tree func, int strict, tree expr, const builtin_memref &ref) { if (!warn_array_bounds) @@ -1722,7 +1721,7 @@ maybe_diag_offset_bounds (location_t loc, gcall *call, tree func, int strict, if/when appropriate. */ void -wrestrict_dom_walker::check_call (gcall *call) +wrestrict_dom_walker::check_call (gimple *call) { /* Avoid checking the call if it has already been diagnosed for some reason. */ @@ -1822,7 +1821,7 @@ wrestrict_dom_walker::check_call (gcall *call) detected and diagnosed, true otherwise. */ bool -check_bounds_or_overlap (gcall *call, tree dst, tree src, tree dstsize, +check_bounds_or_overlap (gimple *call, tree dst, tree src, tree dstsize, tree srcsize, bool bounds_only /* = false */) { location_t loc = gimple_nonartificial_location (call); diff --git a/gcc/gimple-ssa-warn-restrict.h b/gcc/gimple-ssa-warn-restrict.h index 9bf84a3..c624589 100644 --- a/gcc/gimple-ssa-warn-restrict.h +++ b/gcc/gimple-ssa-warn-restrict.h @@ -20,7 +20,7 @@ #ifndef GIMPLE_SSA_WARN_RESTRICT_H -extern bool check_bounds_or_overlap (gcall *, tree, tree, tree, tree, +extern bool check_bounds_or_overlap (gimple *, tree, tree, tree, tree, bool = false); #endif /* GIMPLE_SSA_WARN_RESTRICT_H */ diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-33.c b/gcc/testsuite/gcc.dg/Warray-bounds-33.c new file mode 100644 index 0000000..471c216 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Warray-bounds-33.c @@ -0,0 +1,22 @@ +/* PR tree-optimization/86650 - -Warray-bounds missing inlining context + { dg-do compile } + { dg-options "-O2 -Warray-bounds" } */ + +int a[3]; /* { dg-message "while referencing .a." } */ +int x; + +inline void foo (int i) +{ + a[i + 1] = 123; /* { dg-warning "\\\[-Warray-bounds]" } */ +} + +int bar (void) +{ + foo (3); + + return x; +} + +/* Verify that the inlining context is printed. (For some bizarre + reason, checking for "In function ..." fails in DejaGnu.) + { dg-message " function .foo.* inlined from .bar." "inlining stack" { target *-*-* } 0 } */ diff --git a/gcc/testsuite/gcc.dg/format/gcc_diag-10.c b/gcc/testsuite/gcc.dg/format/gcc_diag-10.c index 9bda73b..d724231 100644 --- a/gcc/testsuite/gcc.dg/format/gcc_diag-10.c +++ b/gcc/testsuite/gcc.dg/format/gcc_diag-10.c @@ -15,9 +15,9 @@ typedef struct location_s union tree_node; typedef union tree_node *tree; -/* Define gcall as a dummy type. The typedef must be provided for +/* Define gimple as a dummy type. The typedef must be provided for the C test to find the symbol. */ -typedef struct gcall gcall; +typedef struct gimple gimple; #define FORMAT(kind) __attribute__ ((format (__gcc_## kind ##__, 1, 2))) @@ -26,7 +26,7 @@ void cdiag (const char*, ...) FORMAT (cdiag); void tdiag (const char*, ...) FORMAT (tdiag); void cxxdiag (const char*, ...) FORMAT (cxxdiag); -void test_diag (tree t, gcall *gc) +void test_diag (tree t, gimple *gc) { diag ("%<"); /* { dg-warning "unterminated quoting directive" } */ diag ("%>"); /* { dg-warning "unmatched quoting directive " } */ @@ -50,7 +50,7 @@ void test_diag (tree t, gcall *gc) diag ("%<%r%R%>", ""); } -void test_cdiag (tree t, gcall *gc) +void test_cdiag (tree t, gimple *gc) { cdiag ("%<"); /* { dg-warning "unterminated quoting directive" } */ cdiag ("%>"); /* { dg-warning "unmatched quoting directive " } */ @@ -90,7 +90,7 @@ void test_cdiag (tree t, gcall *gc) cdiag ("%<%qT%>", t); /* { dg-warning ".q. flag used within a quoted sequence" } */ } -void test_tdiag (tree t, gcall *gc) +void test_tdiag (tree t, gimple *gc) { tdiag ("%<"); /* { dg-warning "unterminated quoting directive" } */ tdiag ("%>"); /* { dg-warning "unmatched quoting directive " } */ @@ -127,7 +127,7 @@ void test_tdiag (tree t, gcall *gc) tdiag ("%<%qT%>", t); /* { dg-warning ".q. flag used within a quoted sequence" } */ } -void test_cxxdiag (tree t, gcall *gc) +void test_cxxdiag (tree t, gimple *gc) { cxxdiag ("%A", t); /* { dg-warning ".A. conversion used unquoted" } */ cxxdiag ("%D", t); /* { dg-warning ".D. conversion used unquoted" } */ diff --git a/gcc/tree-diagnostic.c b/gcc/tree-diagnostic.c index 9814d02..6b03b31 100644 --- a/gcc/tree-diagnostic.c +++ b/gcc/tree-diagnostic.c @@ -282,7 +282,7 @@ default_tree_printer (pretty_printer *pp, text_info *text, const char *spec, case 'K': t = va_arg (*text->args_ptr, tree); - percent_K_format (text, t); + percent_K_format (text, EXPR_LOCATION (t), TREE_BLOCK (t)); return true; default: diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c index e65c40a..627d8d7 100644 --- a/gcc/tree-pretty-print.c +++ b/gcc/tree-pretty-print.c @@ -3980,15 +3980,14 @@ newline_and_indent (pretty_printer *pp, int spc) /* Handle the %K format for TEXT. Separate from default_tree_printer so it can also be used in front ends. - Argument is a statement from which EXPR_LOCATION and TREE_BLOCK will - be recorded. */ + The location LOC and BLOCK are expected to be extracted by the caller + from the %K argument arg via EXPR_LOCATION(arg) and TREE_BLOCK(arg). */ void -percent_K_format (text_info *text, tree t) +percent_K_format (text_info *text, location_t loc, tree block) { - text->set_location (0, EXPR_LOCATION (t), true); + text->set_location (0, loc, true); gcc_assert (pp_ti_abstract_origin (text) != NULL); - tree block = TREE_BLOCK (t); *pp_ti_abstract_origin (text) = NULL; if (in_lto_p) diff --git a/gcc/tree-pretty-print.h b/gcc/tree-pretty-print.h index cf2427f..adfc77b 100644 --- a/gcc/tree-pretty-print.h +++ b/gcc/tree-pretty-print.h @@ -45,7 +45,7 @@ extern int op_code_prio (enum tree_code); extern int op_prio (const_tree); extern const char *op_symbol_code (enum tree_code); extern void print_call_name (pretty_printer *, tree, dump_flags_t); -extern void percent_K_format (text_info *, tree); +extern void percent_K_format (text_info *, location_t, tree); extern void pp_tree_identifier (pretty_printer *, tree); extern void dump_function_header (FILE *, tree, dump_flags_t); extern void pp_double_int (pretty_printer *pp, double_int d, bool uns); diff --git a/gcc/tree-ssa-ccp.c b/gcc/tree-ssa-ccp.c index 2f0e542..63c9531 100644 --- a/gcc/tree-ssa-ccp.c +++ b/gcc/tree-ssa-ccp.c @@ -3456,7 +3456,7 @@ pass_post_ipa_warn::execute (function *fun) location_t loc = gimple_location (stmt); if (warning_at (loc, OPT_Wnonnull, "%Gargument %u null where non-null " - "expected", as_a <gcall *>(stmt), i + 1)) + "expected", stmt, i + 1)) { tree fndecl = gimple_call_fndecl (stmt); if (fndecl && DECL_IS_BUILTIN (fndecl)) diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c index 736e2d9..43cdba8 100644 --- a/gcc/tree-ssa-strlen.c +++ b/gcc/tree-ssa-strlen.c @@ -1627,8 +1627,7 @@ handle_builtin_strcpy (enum built_in_function bcode, gimple_stmt_iterator *gsi) tree type = TREE_TYPE (oldlen); oldlen = fold_build2 (PLUS_EXPR, type, oldlen, build_int_cst (type, 1)); - check_bounds_or_overlap (as_a <gcall *>(stmt), olddsi->ptr, src, - oldlen, NULL_TREE); + check_bounds_or_overlap (stmt, olddsi->ptr, src, oldlen, NULL_TREE); } return; @@ -1715,8 +1714,7 @@ handle_builtin_strcpy (enum built_in_function bcode, gimple_stmt_iterator *gsi) if (const strinfo *chksi = olddsi ? olddsi : dsi) if (si - && !check_bounds_or_overlap (as_a <gcall *>(stmt), chksi->ptr, si->ptr, - NULL_TREE, len)) + && !check_bounds_or_overlap (stmt, chksi->ptr, si->ptr, NULL_TREE, len)) { gimple_set_no_warning (stmt, true); set_no_warning = true; @@ -2031,8 +2029,6 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt) lenrange[0] = wi::shwi (0, prec); } - gcall *call = as_a <gcall *> (stmt); - /* Set to true for strncat whose bound is derived from the length of the destination (the expected usage pattern). */ bool cat_dstlen_bounded = false; @@ -2048,7 +2044,7 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt) "%G%qD output truncated before terminating nul " "copying %E bytes from a string of the same " "length", - call, func, cnt); + stmt, func, cnt); else if (!cat_dstlen_bounded) { if (wi::geu_p (lenrange[0], cntrange[1])) @@ -2062,12 +2058,12 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt) "from a string of length %wu", "%G%qD output truncated copying %E bytes " "from a string of length %wu", - call, func, cnt, lenrange[0].to_uhwi ()); + stmt, func, cnt, lenrange[0].to_uhwi ()); return warning_at (callloc, OPT_Wstringop_truncation, "%G%qD output truncated copying between %wu " "and %wu bytes from a string of length %wu", - call, func, cntrange[0].to_uhwi (), + stmt, func, cntrange[0].to_uhwi (), cntrange[1].to_uhwi (), lenrange[0].to_uhwi ()); } else if (wi::geu_p (lenrange[1], cntrange[1])) @@ -2081,12 +2077,12 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt) "byte from a string of length %wu", "%G%qD output may be truncated copying %E " "bytes from a string of length %wu", - call, func, cnt, lenrange[1].to_uhwi ()); + stmt, func, cnt, lenrange[1].to_uhwi ()); return warning_at (callloc, OPT_Wstringop_truncation, "%G%qD output may be truncated copying between " "%wu and %wu bytes from a string of length %wu", - call, func, cntrange[0].to_uhwi (), + stmt, func, cntrange[0].to_uhwi (), cntrange[1].to_uhwi (), lenrange[1].to_uhwi ()); } } @@ -2102,7 +2098,7 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt) return warning_at (callloc, OPT_Wstringop_truncation, "%G%qD output may be truncated copying between " "%wu and %wu bytes from a string of length %wu", - call, func, cntrange[0].to_uhwi (), + stmt, func, cntrange[0].to_uhwi (), cntrange[1].to_uhwi (), lenrange[0].to_uhwi ()); } } @@ -2122,7 +2118,7 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt) if (cntrange[0] == cntrange[1]) return warning_at (callloc, OPT_Wstringop_truncation, "%G%qD specified bound %E equals destination size", - as_a <gcall *> (stmt), func, cnt); + stmt, func, cnt); } return false; @@ -2180,8 +2176,7 @@ handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *gsi) else srcsize = NULL_TREE; - if (!check_bounds_or_overlap (as_a <gcall *>(stmt), dst, src, - dstsize, srcsize)) + if (!check_bounds_or_overlap (stmt, dst, src, dstsize, srcsize)) { gimple_set_no_warning (stmt, true); return; @@ -2222,13 +2217,13 @@ handle_builtin_stxncpy (built_in_function, gimple_stmt_iterator *gsi) && warning_at (callloc, OPT_Wstringop_truncation, "%G%qD output truncated before terminating nul " "copying as many bytes from a string as its length", - as_a <gcall *>(stmt), func)) + stmt, func)) warned = true; else if (silen && is_strlen_related_p (src, silen->ptr)) warned = warning_at (callloc, OPT_Wstringop_overflow_, "%G%qD specified bound depends on the length " "of the source argument", - as_a <gcall *>(stmt), func); + stmt, func); if (warned) { location_t strlenloc = pss->second; @@ -2467,8 +2462,7 @@ handle_builtin_strcat (enum built_in_function bcode, gimple_stmt_iterator *gsi) tree sptr = si && si->ptr ? si->ptr : src; - if (!check_bounds_or_overlap (as_a <gcall *>(stmt), dst, sptr, - NULL_TREE, slen)) + if (!check_bounds_or_overlap (stmt, dst, sptr, NULL_TREE, slen)) { gimple_set_no_warning (stmt, true); set_no_warning = true; @@ -2578,8 +2572,7 @@ handle_builtin_strcat (enum built_in_function bcode, gimple_stmt_iterator *gsi) tree sptr = si && si->ptr ? si->ptr : src; - if (!check_bounds_or_overlap (as_a <gcall *>(stmt), dst, sptr, - dstlen, srcsize)) + if (!check_bounds_or_overlap (stmt, dst, sptr, dstlen, srcsize)) { gimple_set_no_warning (stmt, true); set_no_warning = true; diff --git a/gcc/tree-vrp.c b/gcc/tree-vrp.c index 7ab8898..ba856c1 100644 --- a/gcc/tree-vrp.c +++ b/gcc/tree-vrp.c @@ -4752,9 +4752,9 @@ class vrp_prop : public ssa_propagation_engine void vrp_initialize (void); void vrp_finalize (bool); void check_all_array_refs (void); - void check_array_ref (location_t, tree, bool); - void check_mem_ref (location_t, tree, bool); - void search_for_addr_array (tree, location_t); + void check_array_ref (location_t, gimple *, tree, bool); + void check_mem_ref (location_t, gimple *, tree, bool); + void search_for_addr_array (tree, gimple *, location_t); class vr_values vr_values; /* Temporary delegator to minimize code churn. */ @@ -4780,7 +4780,7 @@ class vrp_prop : public ssa_propagation_engine IGNORE_OFF_BY_ONE is true if the ARRAY_REF is inside a ADDR_EXPR. */ void -vrp_prop::check_array_ref (location_t location, tree ref, +vrp_prop::check_array_ref (location_t location, gimple *stmt, tree ref, bool ignore_off_by_one) { value_range *vr = NULL; @@ -4838,14 +4838,13 @@ vrp_prop::check_array_ref (location_t location, tree ref, tree artype = TREE_TYPE (TREE_OPERAND (ref, 0)); + bool warned = false; + /* Empty array. */ if (up_bound && tree_int_cst_equal (low_bound, up_bound_p1)) - { - warning_at (location, OPT_Warray_bounds, - "array subscript %E is above array bounds of %qT", - low_bound, artype); - TREE_NO_WARNING (ref) = 1; - } + warned = warning_at (location, OPT_Warray_bounds, + "%Garray subscript %E is above array bounds of %qT", + stmt, low_bound, artype); if (TREE_CODE (low_sub) == SSA_NAME) { @@ -4866,12 +4865,10 @@ vrp_prop::check_array_ref (location_t location, tree ref, : tree_int_cst_le (up_bound, up_sub)) && TREE_CODE (low_sub) == INTEGER_CST && tree_int_cst_le (low_sub, low_bound)) - { - warning_at (location, OPT_Warray_bounds, - "array subscript [%E, %E] is outside array bounds of %qT", - low_sub, up_sub, artype); - TREE_NO_WARNING (ref) = 1; - } + warned = warning_at (location, OPT_Warray_bounds, + "%Garray subscript [%E, %E] is outside " + "array bounds of %qT", + stmt, low_sub, up_sub, artype); } else if (up_bound && TREE_CODE (up_sub) == INTEGER_CST @@ -4885,10 +4882,9 @@ vrp_prop::check_array_ref (location_t location, tree ref, dump_generic_expr (MSG_NOTE, TDF_SLIM, ref); fprintf (dump_file, "\n"); } - warning_at (location, OPT_Warray_bounds, - "array subscript %E is above array bounds of %qT", - up_sub, artype); - TREE_NO_WARNING (ref) = 1; + warned = warning_at (location, OPT_Warray_bounds, + "%Garray subscript %E is above array bounds of %qT", + stmt, up_sub, artype); } else if (TREE_CODE (low_sub) == INTEGER_CST && tree_int_cst_lt (low_sub, low_bound)) @@ -4899,9 +4895,18 @@ vrp_prop::check_array_ref (location_t location, tree ref, dump_generic_expr (MSG_NOTE, TDF_SLIM, ref); fprintf (dump_file, "\n"); } - warning_at (location, OPT_Warray_bounds, - "array subscript %E is below array bounds of %qT", - low_sub, artype); + warned = warning_at (location, OPT_Warray_bounds, + "%Garray subscript %E is below array bounds of %qT", + stmt, low_sub, artype); + } + + if (warned) + { + ref = TREE_OPERAND (ref, 0); + + if (DECL_P (ref)) + inform (DECL_SOURCE_LOCATION (ref), "while referencing %qD", ref); + TREE_NO_WARNING (ref) = 1; } } @@ -4916,7 +4921,8 @@ vrp_prop::check_array_ref (location_t location, tree ref, the address of the just-past-the-end element of an array). */ void -vrp_prop::check_mem_ref (location_t location, tree ref, bool ignore_off_by_one) +vrp_prop::check_mem_ref (location_t location, gimple *stmt, tree ref, + bool ignore_off_by_one) { if (TREE_NO_WARNING (ref)) return; @@ -5134,16 +5140,21 @@ vrp_prop::check_mem_ref (location_t location, tree ref, bool ignore_off_by_one) offrange[1] = offrange[1] / eltsize; } + bool warned; if (offrange[0] == offrange[1]) - warning_at (location, OPT_Warray_bounds, - "array subscript %wi is outside array bounds " - "of %qT", - offrange[0].to_shwi (), reftype); + warned = warning_at (location, OPT_Warray_bounds, + "%Garray subscript %wi is outside array bounds " + "of %qT", + stmt, offrange[0].to_shwi (), reftype); else - warning_at (location, OPT_Warray_bounds, - "array subscript [%wi, %wi] is outside array bounds " - "of %qT", - offrange[0].to_shwi (), offrange[1].to_shwi (), reftype); + warned = warning_at (location, OPT_Warray_bounds, + "%Garray subscript [%wi, %wi] is outside " + "array bounds of %qT", + stmt, offrange[0].to_shwi (), + offrange[1].to_shwi (), reftype); + if (warned && DECL_P (arg)) + inform (DECL_SOURCE_LOCATION (arg), "while referencing %qD", arg); + TREE_NO_WARNING (ref) = 1; return; } @@ -5158,9 +5169,9 @@ vrp_prop::check_mem_ref (location_t location, tree ref, bool ignore_off_by_one) HOST_WIDE_INT tmpidx = extrema[i].to_shwi () / eltsize.to_shwi (); warning_at (location, OPT_Warray_bounds, - "intermediate array offset %wi is outside array bounds " + "%Gintermediate array offset %wi is outside array bounds " "of %qT", - tmpidx, reftype); + stmt, tmpidx, reftype); TREE_NO_WARNING (ref) = 1; } } @@ -5169,74 +5180,83 @@ vrp_prop::check_mem_ref (location_t location, tree ref, bool ignore_off_by_one) address of an ARRAY_REF, and call check_array_ref on it. */ void -vrp_prop::search_for_addr_array (tree t, location_t location) +vrp_prop::search_for_addr_array (tree t, gimple *stmt, location_t location) { /* Check each ARRAY_REF and MEM_REF in the reference chain. */ do { if (TREE_CODE (t) == ARRAY_REF) - check_array_ref (location, t, true /*ignore_off_by_one*/); + check_array_ref (location, stmt, t, true /*ignore_off_by_one*/); else if (TREE_CODE (t) == MEM_REF) - check_mem_ref (location, t, true /*ignore_off_by_one*/); + check_mem_ref (location, stmt, t, true /*ignore_off_by_one*/); t = TREE_OPERAND (t, 0); } while (handled_component_p (t) || TREE_CODE (t) == MEM_REF); - if (TREE_CODE (t) == MEM_REF - && TREE_CODE (TREE_OPERAND (t, 0)) == ADDR_EXPR - && !TREE_NO_WARNING (t)) - { - tree tem = TREE_OPERAND (TREE_OPERAND (t, 0), 0); - tree low_bound, up_bound, el_sz; - offset_int idx; - if (TREE_CODE (TREE_TYPE (tem)) != ARRAY_TYPE - || TREE_CODE (TREE_TYPE (TREE_TYPE (tem))) == ARRAY_TYPE - || !TYPE_DOMAIN (TREE_TYPE (tem))) - return; + if (TREE_CODE (t) != MEM_REF + || TREE_CODE (TREE_OPERAND (t, 0)) != ADDR_EXPR + || TREE_NO_WARNING (t)) + return; - low_bound = TYPE_MIN_VALUE (TYPE_DOMAIN (TREE_TYPE (tem))); - up_bound = TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (tem))); - el_sz = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (tem))); - if (!low_bound - || TREE_CODE (low_bound) != INTEGER_CST - || !up_bound - || TREE_CODE (up_bound) != INTEGER_CST - || !el_sz - || TREE_CODE (el_sz) != INTEGER_CST) - return; + tree tem = TREE_OPERAND (TREE_OPERAND (t, 0), 0); + tree low_bound, up_bound, el_sz; + if (TREE_CODE (TREE_TYPE (tem)) != ARRAY_TYPE + || TREE_CODE (TREE_TYPE (TREE_TYPE (tem))) == ARRAY_TYPE + || !TYPE_DOMAIN (TREE_TYPE (tem))) + return; - if (!mem_ref_offset (t).is_constant (&idx)) - return; + low_bound = TYPE_MIN_VALUE (TYPE_DOMAIN (TREE_TYPE (tem))); + up_bound = TYPE_MAX_VALUE (TYPE_DOMAIN (TREE_TYPE (tem))); + el_sz = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (tem))); + if (!low_bound + || TREE_CODE (low_bound) != INTEGER_CST + || !up_bound + || TREE_CODE (up_bound) != INTEGER_CST + || !el_sz + || TREE_CODE (el_sz) != INTEGER_CST) + return; - idx = wi::sdiv_trunc (idx, wi::to_offset (el_sz)); - if (idx < 0) + offset_int idx; + if (!mem_ref_offset (t).is_constant (&idx)) + return; + + bool warned = false; + idx = wi::sdiv_trunc (idx, wi::to_offset (el_sz)); + if (idx < 0) + { + if (dump_file && (dump_flags & TDF_DETAILS)) { - if (dump_file && (dump_flags & TDF_DETAILS)) - { - fprintf (dump_file, "Array bound warning for "); - dump_generic_expr (MSG_NOTE, TDF_SLIM, t); - fprintf (dump_file, "\n"); - } - warning_at (location, OPT_Warray_bounds, - "array subscript %wi is below array bounds of %qT", - idx.to_shwi (), TREE_TYPE (tem)); - TREE_NO_WARNING (t) = 1; + fprintf (dump_file, "Array bound warning for "); + dump_generic_expr (MSG_NOTE, TDF_SLIM, t); + fprintf (dump_file, "\n"); } - else if (idx > (wi::to_offset (up_bound) - - wi::to_offset (low_bound) + 1)) + warned = warning_at (location, OPT_Warray_bounds, + "%Garray subscript %wi is below " + "array bounds of %qT", + stmt, idx.to_shwi (), TREE_TYPE (tem)); + } + else if (idx > (wi::to_offset (up_bound) + - wi::to_offset (low_bound) + 1)) + { + if (dump_file && (dump_flags & TDF_DETAILS)) { - if (dump_file && (dump_flags & TDF_DETAILS)) - { - fprintf (dump_file, "Array bound warning for "); - dump_generic_expr (MSG_NOTE, TDF_SLIM, t); - fprintf (dump_file, "\n"); - } - warning_at (location, OPT_Warray_bounds, - "array subscript %wu is above array bounds of %qT", - idx.to_uhwi (), TREE_TYPE (tem)); - TREE_NO_WARNING (t) = 1; + fprintf (dump_file, "Array bound warning for "); + dump_generic_expr (MSG_NOTE, TDF_SLIM, t); + fprintf (dump_file, "\n"); } + warned = warning_at (location, OPT_Warray_bounds, + "%Garray subscript %wu is above " + "array bounds of %qT", + stmt, idx.to_uhwi (), TREE_TYPE (tem)); + } + + if (warned) + { + if (DECL_P (t)) + inform (DECL_SOURCE_LOCATION (t), "while referencing %qD", t); + + TREE_NO_WARNING (t) = 1; } } @@ -5262,12 +5282,13 @@ check_array_bounds (tree *tp, int *walk_subtree, void *data) vrp_prop *vrp_prop = (class vrp_prop *)wi->info; if (TREE_CODE (t) == ARRAY_REF) - vrp_prop->check_array_ref (location, t, false /*ignore_off_by_one*/); + vrp_prop->check_array_ref (location, wi->stmt, t, + false /*ignore_off_by_one*/); else if (TREE_CODE (t) == MEM_REF) - vrp_prop->check_mem_ref (location, t, false /*ignore_off_by_one*/); + vrp_prop->check_mem_ref (location, wi->stmt, t, false /*ignore_off_by_one*/); else if (TREE_CODE (t) == ADDR_EXPR) { - vrp_prop->search_for_addr_array (t, location); + vrp_prop->search_for_addr_array (t, wi->stmt, location); *walk_subtree = FALSE; }
(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? Martin