On Fri, 2016-11-18 at 09:51 -0700, Martin Sebor wrote: > > Martin: are the changes to your test cases OK by you, or is there > > a better way to rewrite them? > > Thanks for looking into it! > > Since the purpose of the test_sprintf_note function in the test is > to verify the location of the caret within the warnings I think we > should keep it if it's possible. Would either removing the P macro > or moving the function to a different file that doesn't use the > -ftrack-macro-expansion=0 option work?
To get substring locations with the proposed patch, that code will need to be in a file without -ftrack-macro-expansion=0. builtin-sprintf-warn-4.c seems to fit the bill, and has caret-printing enabled too, so here's another attempt at the patch, just covering the affected test cases, which moves test_sprintf_note to that file, and drops "P". The carets/underlines from the warnings look sane, and the test case verifies that via dg-begin/end-multiline-output directives. The test case also verifies the carets/underlins from the *notes*. [FWIW, I'm less convinced by the carets/underlines from the notes: they all underline the whole of the __builtin_sprintf expression, though looking at gimple-ssa-sprintf.c, I see: location_t callloc = gimple_location (info.callstmt); which is used for the "inform" calls in question. Hence I think it's faithfully printing what that code is asking it to. I'd prefer to not touch that location in this patch, since it seems orthogonal to fixing the PR preprocessor/78324; perhaps something to address as part of PR middle-end/77696 ?]. Martin: how does this look to you? Any objections to this change as part of my fix for PR preprocessor/78324? gcc/testsuite/ChangeLog: * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test_sprintf_note): Move to... * gcc.dg/tree-ssa/builtin-sprintf-warn-4.c: ...here. Drop -ftrack-macro-expansion=0. (test_sprintf_note): Remove "P" macro. Add dg-begin/end-multiline-output directives. (LINE, buffer, ptr): Copy from builtin-sprintf-warn-1.c. diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c index 5779a95..a24889b 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-1.c @@ -170,35 +170,6 @@ void test_sprintf_zero_length_array (void *p, int i) __builtin_sprintf (buffer (1), "%s", s [i].a); } -/* Verify that the note printed along with the diagnostic mentions - the correct sizes and refers to the location corresponding to - the affected directive. */ - -void test_sprintf_note (void) -{ -#define P __builtin_sprintf - - /* Diagnostic column numbers are 1-based. */ - - P (buffer (0), /* { dg-message "format output 4 bytes into a destination of size 0" } */ - "%c%s%i", '1', "2", 3); /* { dg-warning "7:.%c. directive writing 1 byte into a region of size 0" } */ - - P (buffer (1), /* { dg-message "format output 6 bytes into a destination of size 1" } */ - "%c%s%i", '1', "23", 45); /* { dg-warning "9:.%s. directive writing 2 bytes into a region of size 0" } */ - - P (buffer (2), /* { dg-message "format output 6 bytes into a destination of size 2" } */ - "%c%s%i", '1', "2", 345); /* { dg-warning "11:.%i. directive writing 3 bytes into a region of size 0" } */ - - /* It would be nice if the caret in the location range for the format - string below could be made to point at the closing quote of the format - string, like so: - sprintf (d, "%c%s%i", '1', "2", 3456); - ~~~~~~^ - Unfortunately, that doesn't work with the current setup. */ - P (buffer (6), /* { dg-message "format output 7 bytes into a destination of size 6" } */ - "%c%s%i", '1', "2", 3456); /* { dg-warning "writing a terminating nul past the end of the destination" } */ -} - #undef T #define T(size, fmt, ...) \ __builtin___sprintf_chk (buffer (size), 0, objsize (size), fmt, \ diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-4.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-4.c index faa5806..3b3fb68 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-4.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-4.c @@ -1,5 +1,5 @@ /* { dg-do compile } */ -/* { dg-options "-Wformat -Wformat-length=1 -fdiagnostics-show-caret -ftrack-macro-expansion=0" } */ +/* { dg-options "-Wformat -Wformat-length=1 -fdiagnostics-show-caret" } */ extern int sprintf (char*, const char*, ...); @@ -91,3 +91,81 @@ void test (void) } /* { dg-prune-output "too many arguments for format" } */ + +/* When debugging, define LINE to the line number of the test case to exercise + and avoid exercising any of the others. The buffer macro + below makes use of LINE to avoid warnings for other lines. */ +#ifndef LINE +# define LINE 0 +#endif + +char buffer [256]; +extern char *ptr; + +/* Evaluate to an array of SIZE characters when non-negative and LINE + is not set or set to the line the macro is on, or to a pointer to + an unknown object otherwise. */ +#define buffer(size) \ + (0 <= size && (!LINE || __LINE__ == LINE) \ + ? buffer + sizeof buffer - size : ptr) + +/* Verify that the note printed along with the diagnostic mentions + the correct sizes and refers to the location corresponding to + the affected directive. */ + +void test_sprintf_note (void) +{ + /* Diagnostic column numbers are 1-based. */ + + __builtin_sprintf (buffer (0), "%c%s%i", '1', "2", 3); + /* { dg-warning "35: .%c. directive writing 1 byte into a region of size 0" "" { target *-*-* } .-1 } + { dg-begin-multiline-output "" } + __builtin_sprintf (buffer (0), "%c%s%i", '1', "2", 3); + ^~ + { dg-end-multiline-output "" } + + { dg-message "format output 4 bytes into a destination of size 0" "" { target *-*-* } .-7 } + { dg-begin-multiline-output "" } + __builtin_sprintf (buffer (0), "%c%s%i", '1', "2", 3); + ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + { dg-end-multiline-output "" } */ + + __builtin_sprintf (buffer (1), "%c%s%i", '1', "23", 45); + /* { dg-warning "37: .%s. directive writing 2 bytes into a region of size 0" "" { target *-*-* } .-1 } + { dg-begin-multiline-output "" } + __builtin_sprintf (buffer (1), "%c%s%i", '1', "23", 45); + ^~ ~~~~ + { dg-end-multiline-output "" } + + { dg-message "format output 6 bytes into a destination of size 1" "" { target *-*-* } .-7 } + { dg-begin-multiline-output "" } + __builtin_sprintf (buffer (1), "%c%s%i", '1', "23", 45); + ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + { dg-end-multiline-output "" } */ + + __builtin_sprintf (buffer (2), "%c%s%i", '1', "2", 345); + /* { dg-warning "39: .%i. directive writing 3 bytes into a region of size 0" "" { target *-*-* } .-1 } + { dg-begin-multiline-output "" } + __builtin_sprintf (buffer (2), "%c%s%i", '1', "2", 345); + ^~ + { dg-end-multiline-output "" } + + { dg-message "format output 6 bytes into a destination of size 2" "" { target *-*-* } .-7 } + { dg-begin-multiline-output "" } + __builtin_sprintf (buffer (2), "%c%s%i", '1', "2", 345); + ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + { dg-end-multiline-output "" } */ + + __builtin_sprintf (buffer (6), "%c%s%i", '1', "2", 3456); + /* { dg-warning "41: writing a terminating nul past the end of the destination" "" { target *-*-* } .-1 } + { dg-begin-multiline-output "" } + __builtin_sprintf (buffer (6), "%c%s%i", '1', "2", 3456); + ~~~~~~^ + { dg-end-multiline-output "" } + + { dg-message "format output 7 bytes into a destination of size 6" "" { target *-*-* } .-7 } + { dg-begin-multiline-output "" } + __builtin_sprintf (buffer (6), "%c%s%i", '1', "2", 3456); + ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + { dg-end-multiline-output "" } */ +}