On Mon, 2017-12-04 at 12:18 -0700, Martin Sebor wrote:
> On 12/04/2017 05:41 AM, Rainer Orth wrote:
> > Within test last week, 64-bit Solaris/SPARC bootstrap began to
> > fail:
> >
> > /vol/gcc/src/hg/trunk/local/gcc/dbxout.c: In function 'bool
> > dbxout_block(tree, int, tree, int)':
> > /vol/gcc/src/hg/trunk/local/gcc/dbxout.c:3767:1: error: '%lu'
> > directive writing between 1 and 20 bytes into a region of size 14
> > [-Werror=format-overflow=]
> > dbxout_block (tree block, int depth, tree args, int
> > parent_blocknum)
> > ^~~~~~~~~~~~
> > /vol/gcc/src/hg/trunk/local/gcc/dbxout.c:3767:1: note: directive
> > argument in the range [0, 18446744073709551614]
> > In file included from ./tm.h:26,
> > from /vol/gcc/src/hg/trunk/local/gcc/target.h:52,
> > from /vol/gcc/src/hg/trunk/local/gcc/dbxout.c:72:
> > /vol/gcc/src/hg/trunk/local/gcc/config/sparc/sol2.h:353:11: note:
> > 'std::sprintf' output between 8 and 27 bytes into a destination of
> > size 20
> > sprintf ((LABEL), "*.L%s%lu", (PREFIX), (unsigned long)(NUM))
> > ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > /vol/gcc/src/hg/trunk/local/gcc/dbxout.c:3855:5: note: in expansion
> > of macro 'ASM_GENERATE_INTERNAL_LABEL'
> > ASM_GENERATE_INTERNAL_LABEL (buf, "LBB", parent_blocknum);
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> > The line numbers are extremely confusing, to say the least, though:
> > the
> > one in the error and the first note refer to the begin of the
> > function
> > definition and only the third note refers to the line of the actual
> > error.
>
> I agree it looks confusing. It's the result of the interaction
> between macro tracking and inlining.
>
> I think it's a general problem that affects many (though not all)
> warnings emitted out of the middle-end. For instance, the example
> below results in similar output. The output changes depending on
> whether or not _FORTIFY_SOURCE is defined.
>
> #include <string.h>
>
> #define FOO(d, s) strcpy (d, s)
>
> void foo (char *d, const char *s) { FOO (d, s); }
>
> void sink (char*);
>
> void bar (void)
> {
> char a[3];
>
> foo (a, "1234567"); // bug here
>
> sink (a);
> }
>
> Without _FORTIFY_SOURCE:
>
> d.c: In function ‘void bar()’:
> d.c:3:26: warning: ‘void* __builtin_memcpy(void*, const void*, long
> unsigned int)’ writing 8 bytes into a region of size 3 overflows the
> destination [-Wstringop-overflow=]
> #define FOO(d, s) strcpy (d, s)
> ~~~~~~~^~~~~~
> d.c:5:37: note: in expansion of macro ‘FOO’
> void foo (char *d, const char *s) { FOO (d, s); }
> ^~~
>
> If bar() were a bigger function with multiple calls to foo() it
> could be tricky to tell which of them caused the warning.
>
> With _FORTIFY_SOURCE:
>
> In file included from /usr/include/string.h:635,
> from d.c:1:
> In function ‘char* strcpy(char*, const char*)’,
> inlined from ‘void bar()’ at d.c:5:37:
> /usr/include/bits/string3.h:110:33: warning: ‘void*
> __builtin___memcpy_chk(void*, const void*, long unsigned int, long
> unsigned int)’ writing 8 bytes into a region of size 3 overflows the
> destination [-Wstringop-overflow=]
> return __builtin___strcpy_chk (__dest, __src, __bos (__dest));
> ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> This suffers from the same problem as the first case: the line
> number of the offending call in bar() is missing.
>
> In both cases the problem is compounded by the diagnostic, as
> a result of folding, referring to __builtin___memcpy_chk while
> the source code contains a call to strcpy.
>
> I don't know nearly enough about the internals of the diagnostic
> subsystem to tell how difficult it might be to change the output
> to make it more readable. David Malcolm is the expert on this
> area so he might have an idea what it would take.
[Did you mean to CC me on this, rather than dje?]
I'm not as familiar as I could be on the existing inlining-tracking
implementation - so much so that, in ignorance, I wrote my own version
of it earlier this year (doh!).
The warning implementation reads:
3151 warning_at (loc, opt,
3152 (integer_onep (range[0])
3153 ? G_("%K%qD writing %E byte into a region "
3154 "of size %E overflows the destination")
3155 : G_("%K%qD writing %E bytes into a region "
3156 "of size %E overflows the destination")),
3157 exp, get_callee_fndecl (exp), range[0],
objsize);
The inlining information is coming from that "%K" in the string, which
leads to tree-pretty-print.c's percent_K_format being called for "exp".
This overwrites the "loc" provided for the warning_at with
EXPR_LOCATION of exp, and looks at TREE_BLOCK (exp), finding the
outermost containing block within its function, and writing it into the
diagnostic_info's x_data.
This block is used by lhd_print_error_function (and C++'s
cp_print_error_function) to print the "inlined from" information.
This has several limitations:
(a) It would be better for the user to identify a specific callsite,
rather than a function (or block), and print the source of the
callsite; we would then print something like:
(without _FORTIFY_SOURCE)
d.c: In function ‘void foo(char *d, const char *s)’,
inlined from ‘bar’ at d.c:13:5:
foo (a, "1234567"); // bug here
~~~~^~~~~~~~~~~~~~
d.c:3:26: warning: ‘void* __builtin_memcpy(void*, const void*, long unsigned
int)’ writing 8 bytes into a region of size 3 overflows the destination
[-Wstringop-overflow=]
#define FOO(d, s) strcpy (d, s)
~~~~~~~^~~~~~
d.c:5:37: note: in expansion of macro ‘FOO’
void foo (char *d, const char *s) { FOO (d, s); }
^~~
(with _FORTIFY_SOURCE)
In file included from /usr/include/string.h:636,
from d.c:1:
In function ‘strcpy’,
inlined from ‘foo’ at d.c:5:37,
void foo (char *d, const char *s) { FOO (d, s); }
^~~
inlined from ‘bar’ at d.c:13:5:
foo (a, "1234567"); // bug here
~~~~^~~~~~~~~~~~~~
/usr/include/bits/string3.h:104:10: warning: ‘__builtin___memcpy_chk’ writing 8
bytes into a region of size 3 overflows the destination [-Wstringop-overflow=]
return __builtin___strcpy_chk (__dest, __src, __bos (__dest));
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
How ought we to store the location of the callsite that's inlined?
(e.g. Would it be reasonable to add a TREE_BLOCK around a callsite
that's inlined, for that purpose?
(b) That "%K" seems like something of a wart. Would a family of
overloads like:
warning_at (tree exp, int option, const char *etc, ...);
be more appropriate? (though that requires locations for every
expression, I guess).
etc.
Dave
