[testsuite] Require int32plus for builtin-sprintf-warn-1.c

Submitted by Senthil Kumar Selvaraj on Sept. 27, 2016, 11:10 a.m.

Details

Message ID 87oa398uox.fsf@atmel.com
State New
Headers show

Commit Message

Senthil Kumar Selvaraj Sept. 27, 2016, 11:10 a.m.
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.

Regards
Senthil

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

Comments

James Greenhalgh Sept. 27, 2016, 4:39 p.m.
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
>
Jeff Law Sept. 27, 2016, 6:38 p.m.
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
Christophe Lyon Sept. 27, 2016, 9:14 p.m.
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
Mike Stump Sept. 27, 2016, 9:41 p.m.
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.
Jeff Law Sept. 28, 2016, 4:41 a.m.
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
Senthil Kumar Selvaraj Sept. 28, 2016, 6:28 a.m.
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
>>

Patch hide | download patch | download mbox

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