| Message ID | 5372720C.20404@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, May 13, 2014 at 9:27 PM, Florian Weimer <fweimer@redhat.com> wrote: > Patterns that trigger the optimization and warning can form after inlining, > and it can be rather difficult to figure out what exactly is causing the > warning. The inlining context at least provides additional hints, enabling > developers to substitute the arguments and discover what, precisely, is > happening. > > More context is provided with -g than without, but I think this is > acceptable. > > I bootstrapped and tested the attached patch on x86_64-redhat-linux-gnu, > with no new regressions. I think that your block walking code is bogus in that it looks at only BLOCK_SOURCE_LOCATION, exposing an implementation detail that should be hidden by using inlined_function_outer_scope_p. It also will print an unlimited call stack - isn't that too verbose? Thanks, Richard. > -- > Florian Weimer / Red Hat Product Security Team
On 05/14/2014 11:34 AM, Richard Biener wrote: > On Tue, May 13, 2014 at 9:27 PM, Florian Weimer <fweimer@redhat.com> wrote: >> Patterns that trigger the optimization and warning can form after inlining, >> and it can be rather difficult to figure out what exactly is causing the >> warning. The inlining context at least provides additional hints, enabling >> developers to substitute the arguments and discover what, precisely, is >> happening. >> >> More context is provided with -g than without, but I think this is >> acceptable. >> >> I bootstrapped and tested the attached patch on x86_64-redhat-linux-gnu, >> with no new regressions. > > I think that your block walking code is bogus in that it looks at > only BLOCK_SOURCE_LOCATION, exposing an implementation > detail that should be hidden by using inlined_function_outer_scope_p. Do you mean that I should replace the condition with inlined_function_outer_scope_p (block)? I've made this change. > It also will print an unlimited call stack - isn't that too verbose? Isn't the call stack limited by inlining parameters? I don't see a good way to prune it. When investigating instances of this warning, you might need all frames. I could add a parameter and set it to some arbitrary default, like 5, and print a warning if the call stack is longer. But I don't know if this is really worth the effort, considering that the warning is somewhat rare to begin with.
On Tue, May 13, 2014 at 09:27:08PM +0200, Florian Weimer wrote: > Patterns that trigger the optimization and warning can form after > inlining, and it can be rather difficult to figure out what exactly > is causing the warning. The inlining context at least provides > additional hints, enabling developers to substitute the arguments > and discover what, precisely, is happening. > > More context is provided with -g than without, but I think this is > acceptable. > > I bootstrapped and tested the attached patch on > x86_64-redhat-linux-gnu, with no new regressions. This looks wrong. If you want to print inline context, you just should use %K%s and pass a tree with the right EXPR_LOCATION and TREE_BLOCK. So perhaps: if (stmt == NULL) warning_at (input_location, OPT_Wstrict_overflow, "%s", warnmsg); else warning_at (gimple_location (stmt), OPT_Wstrict_overflow, "%K%s", build_empty_stmt (gimple_location (stmt)), warnmsg); (or add something similar to %K that will take location_t or change %K to take location_t instead of tree now that we have both a block and locus in location_t. Note, your patch would happily crash if stmt is NULL. Jakub
gcc/ 2014-05-13 Florian Weimer <fweimer@redhat.com> * fold-const.c (fold_undefer_overflow_warnings): Print notes with inlining information. gcc/testsuite/ 2014-05-13 Florian Weimer <fweimer@redhat.com> * c-c++-common/Wstrict-overflow-1.c: New test. diff --git a/gcc/fold-const.c b/gcc/fold-const.c index 0fcb87f..5f13992 100644 --- a/gcc/fold-const.c +++ b/gcc/fold-const.c @@ -268,6 +268,12 @@ fold_undefer_overflow_warnings (bool issue, const_gimple stmt, int code) else locus = gimple_location (stmt); warning_at (locus, OPT_Wstrict_overflow, "%s", warnmsg); + + /* Print notes with inlining information. */ + for (tree block = gimple_block (stmt); block && TREE_CODE (block) == BLOCK; + block = BLOCK_SUPERCONTEXT (block)) + if (BLOCK_SOURCE_LOCATION (block)) + inform (BLOCK_SOURCE_LOCATION (block), "inlined from here"); } /* Stop deferring overflow warnings, ignoring any deferred diff --git a/gcc/testsuite/c-c++-common/Wstrict-overflow-1.c b/gcc/testsuite/c-c++-common/Wstrict-overflow-1.c new file mode 100644 index 0000000..693c52b --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wstrict-overflow-1.c @@ -0,0 +1,21 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -g -Wstrict-overflow" } */ + +int f (void); + +static int g (int a, int b, int c) +{ + if (a + b < c) /* { dg-warning "assuming signed overflow" } */ + return -1; + return f (); +} + +static int h (int a, int b) +{ + return g(a, 1, b); /* { dg-message "note: inlined from here" } */ +} + +int k (int a) +{ + return h(a, a); /* { dg-message "note: inlined from here" } */ +}