| Message ID | 87oa398uox.fsf@atmel.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Sep 27, 2016 at 04:40:22PM +0530, Senthil Kumar Selvaraj wrote: > Hi, > > This patch requires int32plus for > gcc.dg/tree-ssa/builtin-sprintf-warn-1.c, as it reports a bunch of > failures for a 16 bit int target like the avr. The "%u" format > specifier tests, for example, use int literals big enough to only fit > in a long int, and this causes unexpected warnings. > > Comitted to trunk. This change is obviously incomplete as it does not update the expected line numbers for warnings generated by this testcase. Found with my bisect robot: Failures: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c Bisected to: Author: saaadhu <saaadhu@138bc75d-0d04-0410-961f-82ee72b054a4> Date: Tue Sep 27 11:05:25 2016 +0000 Fix bogus test failure for avr The test has a bunch of hardcoded integer literals that would fit only in a 32 bits+ int, causing overflow warnings for a 16 bit int target like avr. gcc/testsuite/ChangeLog 2016-09-27 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Require int32plus. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@240528 FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (nil) (test for warnings, line 96) /* { dg-warning "nul past the end" "(nil)" { target *-linux-gnu *-*-uclinux } 96 } */ FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c Glibc %p (test for warnings, line 108) /* { dg-warning "nul past the end" "Glibc %p" { target *-linux-gnu } 108 } */ /* { dg-warning "nul past the end" "Generic %p" { target *-*-uclinux } 108 } */ FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for excess errors) The line numbers here need bumped to match the change you've made. Thanks, James > 2016-09-27 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Require int32plus. > > PR fortran/77666 > Index: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c > =================================================================== > --- gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (revision 240524) > +++ gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (working copy) > @@ -1,5 +1,6 @@ > /* { dg-do compile } */ > /* { dg-options "-std=c99 -Wformat -Wformat-length=1 -ftrack-macro-expansion=0" } */ > +/* { dg-require-effective-target int32plus } */ > > /* When debugging, define LINE to the line number of the test case to exercise > and avoid exercising any of the others. The buffer and objsize macros >
On 09/27/2016 10:39 AM, James Greenhalgh wrote: > On Tue, Sep 27, 2016 at 04:40:22PM +0530, Senthil Kumar Selvaraj wrote: >> Hi, >> >> This patch requires int32plus for >> gcc.dg/tree-ssa/builtin-sprintf-warn-1.c, as it reports a bunch of >> failures for a 16 bit int target like the avr. The "%u" format >> specifier tests, for example, use int literals big enough to only fit >> in a long int, and this causes unexpected warnings. >> >> Comitted to trunk. > > This change is obviously incomplete as it does not update the expected > line numbers for warnings generated by this testcase. Right. It does make me wonder if these directives could go at the bottom of the file so that adding/removing a directive doesn't require updating line #s in the file. jeff
On 27 September 2016 at 20:38, Jeff Law <law@redhat.com> wrote: > On 09/27/2016 10:39 AM, James Greenhalgh wrote: >> >> On Tue, Sep 27, 2016 at 04:40:22PM +0530, Senthil Kumar Selvaraj wrote: >>> >>> Hi, >>> >>> This patch requires int32plus for >>> gcc.dg/tree-ssa/builtin-sprintf-warn-1.c, as it reports a bunch of >>> failures for a 16 bit int target like the avr. The "%u" format >>> specifier tests, for example, use int literals big enough to only fit >>> in a long int, and this causes unexpected warnings. >>> >>> Comitted to trunk. >> >> >> This change is obviously incomplete as it does not update the expected >> line numbers for warnings generated by this testcase. > > Right. > > It does make me wonder if these directives could go at the bottom of the > file so that adding/removing a directive doesn't require updating line #s in > the file. > > jeff > I did observe regressions too, on aarch64: - PASS now FAIL [PASS => FAIL]: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (nil) (test for warnings, line 96) gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for excess errors) gcc.dg/tree-ssa/builtin-sprintf-warn-1.c Glibc %p (test for warnings, line 108) Christophe
On Sep 27, 2016, at 11:38 AM, Jeff Law <law@redhat.com> wrote: > > On 09/27/2016 10:39 AM, James Greenhalgh wrote: >> On Tue, Sep 27, 2016 at 04:40:22PM +0530, Senthil Kumar Selvaraj wrote: >>> Hi, >>> >>> This patch requires int32plus for >>> gcc.dg/tree-ssa/builtin-sprintf-warn-1.c, as it reports a bunch of >>> failures for a 16 bit int target like the avr. The "%u" format >>> specifier tests, for example, use int literals big enough to only fit >>> in a long int, and this causes unexpected warnings. >>> >>> Comitted to trunk. >> >> This change is obviously incomplete as it does not update the expected >> line numbers for warnings generated by this testcase. > Right. > > It does make me wonder if these directives could go at the bottom of the file so that adding/removing a directive doesn't require updating line #s in the file. We support relative numbers in some of the places now, right? :-) absolute line numbers should be recoded to relative numbers as people hit them.
On 09/27/2016 03:41 PM, Mike Stump wrote: > On Sep 27, 2016, at 11:38 AM, Jeff Law <law@redhat.com> wrote: >> >> On 09/27/2016 10:39 AM, James Greenhalgh wrote: >>> On Tue, Sep 27, 2016 at 04:40:22PM +0530, Senthil Kumar Selvaraj wrote: >>>> Hi, >>>> >>>> This patch requires int32plus for >>>> gcc.dg/tree-ssa/builtin-sprintf-warn-1.c, as it reports a bunch of >>>> failures for a 16 bit int target like the avr. The "%u" format >>>> specifier tests, for example, use int literals big enough to only fit >>>> in a long int, and this causes unexpected warnings. >>>> >>>> Comitted to trunk. >>> >>> This change is obviously incomplete as it does not update the expected >>> line numbers for warnings generated by this testcase. >> Right. >> >> It does make me wonder if these directives could go at the bottom of the file so that adding/removing a directive doesn't require updating line #s in the file. > > We support relative numbers in some of the places now, right? :-) absolute line numbers should be recoded to relative numbers as people hit them. > Yea, that's probably an even better solution. jeff
James Greenhalgh writes: > On Tue, Sep 27, 2016 at 04:40:22PM +0530, Senthil Kumar Selvaraj wrote: >> Hi, >> >> This patch requires int32plus for >> gcc.dg/tree-ssa/builtin-sprintf-warn-1.c, as it reports a bunch of >> failures for a 16 bit int target like the avr. The "%u" format >> specifier tests, for example, use int literals big enough to only fit >> in a long int, and this causes unexpected warnings. >> >> Comitted to trunk. > > This change is obviously incomplete as it does not update the expected > line numbers for warnings generated by this testcase. Sorry for the breakage. While I tested that it reports UNSUPPORTED for avr, I didn't test that it doesn't break other targets. I thought I'd just modified behavior to skip the test, didn't realize the side effect of adding a new line. Guess Martin already has a patch fixing this and a couple of other things (https://gcc.gnu.org/ml/gcc-patches/2016-09/msg02073.html). Thanks Martin! Regards Senthil > > Found with my bisect robot: > > Failures: > gcc.dg/tree-ssa/builtin-sprintf-warn-1.c > > Bisected to: > > Author: saaadhu <saaadhu@138bc75d-0d04-0410-961f-82ee72b054a4> > Date: Tue Sep 27 11:05:25 2016 +0000 > > Fix bogus test failure for avr > > The test has a bunch of hardcoded integer literals that would fit only in a > 32 bits+ int, causing overflow warnings for a 16 bit int target like avr. > > gcc/testsuite/ChangeLog > > 2016-09-27 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> > > * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Require int32plus. > > > git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@240528 > > > FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (nil) (test for warnings, line 96) > /* { dg-warning "nul past the end" "(nil)" { target *-linux-gnu *-*-uclinux } 96 } */ > > FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c Glibc %p (test for warnings, line 108) > /* { dg-warning "nul past the end" "Glibc %p" { target *-linux-gnu } 108 } */ > /* { dg-warning "nul past the end" "Generic %p" { target *-*-uclinux } 108 } */ > > FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (test for excess errors) > > The line numbers here need bumped to match the change you've made. > > Thanks, > James > > >> 2016-09-27 Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> >> >> * gcc.dg/tree-ssa/builtin-sprintf-warn-1.c: Require int32plus. >> >> PR fortran/77666 >> Index: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c >> =================================================================== >> --- gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (revision 240524) >> +++ gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (working copy) >> @@ -1,5 +1,6 @@ >> /* { dg-do compile } */ >> /* { dg-options "-std=c99 -Wformat -Wformat-length=1 -ftrack-macro-expansion=0" } */ >> +/* { dg-require-effective-target int32plus } */ >> >> /* When debugging, define LINE to the line number of the test case to exercise >> and avoid exercising any of the others. The buffer and objsize macros >>
Index: gcc.dg/tree-ssa/builtin-sprintf-warn-1.c =================================================================== --- gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (revision 240524) +++ gcc.dg/tree-ssa/builtin-sprintf-warn-1.c (working copy) @@ -1,5 +1,6 @@ /* { dg-do compile } */ /* { dg-options "-std=c99 -Wformat -Wformat-length=1 -ftrack-macro-expansion=0" } */ +/* { dg-require-effective-target int32plus } */ /* When debugging, define LINE to the line number of the test case to exercise and avoid exercising any of the others. The buffer and objsize macros