[Patch,testsuite]: Fix testcase that bangs long and int against void*

[Patch,testsuite]: Fix testcase that bangs long and int against void*

[Georg-Johann Lay]

This test case is obviously written for 32-bit platforms, thus added
"dg-require-effective-target ilp32" to ensure that the pointer mess won't lead
to FAILs because of

warning: cast to pointer from integer of different size

and

warning: cast from pointer to integer of different size

Ok to apply?

	* gcc.dg/lto/20091013-1_0.c: Add dg-require-effective-target ilp32.


Index: gcc.dg/lto/20091013-1_0.c
===================================================================
--- gcc.dg/lto/20091013-1_0.c   (revision 183150)
+++ gcc.dg/lto/20091013-1_0.c   (working copy)
@@ -1,5 +1,6 @@
 /* { dg-lto-do link } */
 /* { dg-require-effective-target fpic } */
+/* { dg-require-effective-target ilp32 } */
 /* { dg-lto-options {{-fPIC -r -nostdlib -flto} {-fPIC -r -nostdlib -O2
-flto}} } */

 void * HeapAlloc(void*,unsigned int,unsigned long);

Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Mike Stump]

On Jan 13, 2012, at 4:33 AM, Georg-Johann Lay wrote:
> This test case is obviously written for 32-bit platforms, thus added
> "dg-require-effective-target ilp32" to ensure that the pointer mess won't lead
> to FAILs because of
> 
> warning: cast to pointer from integer of different size
> 
> and
> 
> warning: cast from pointer to integer of different size
> 
> Ok to apply?

Ok.

> 	* gcc.dg/lto/20091013-1_0.c: Add dg-require-effective-target ilp32.

Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Rainer Orth]

Mike Stump <mikestump@comcast.net> writes:

> On Jan 13, 2012, at 4:33 AM, Georg-Johann Lay wrote:
>> This test case is obviously written for 32-bit platforms, thus added
>> "dg-require-effective-target ilp32" to ensure that the pointer mess won't lead
>> to FAILs because of
>> 
>> warning: cast to pointer from integer of different size
>> 
>> and
>> 
>> warning: cast from pointer to integer of different size
>> 
>> Ok to apply?
>
> Ok.
>
>> 	* gcc.dg/lto/20091013-1_0.c: Add dg-require-effective-target ilp32.

I wonder if the fix is right, though: the testcase currenly passes for
64-bit multilibs, but won't run there any longer if the patch is
applied.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Mike Stump]

On Jan 13, 2012, at 9:23 AM, Rainer Orth wrote:
> Mike Stump <mikestump@comcast.net> writes:
> 
>> On Jan 13, 2012, at 4:33 AM, Georg-Johann Lay wrote:
>>> This test case is obviously written for 32-bit platforms, thus added
>>> "dg-require-effective-target ilp32" to ensure that the pointer mess won't lead
>>> to FAILs because of
>>> 
>>> warning: cast to pointer from integer of different size
>>> 
>>> and
>>> 
>>> warning: cast from pointer to integer of different size
>>> 
>>> Ok to apply?
>> 
>> Ok.
>> 
>>> 	* gcc.dg/lto/20091013-1_0.c: Add dg-require-effective-target ilp32.
> 
> I wonder if the fix is right, though: the testcase currenly passes for
> 64-bit multilibs, but won't run there any longer if the patch is
> applied.

I rely upon words like obviously written for 32-bit platforms a little too much apparently, sorry.  I agree, let's find some other solution.  I've starred at the code, and is was not clear to me why you thought it was obviously for 32-bit platforms.

So, I'd propose the -Wno- option silence the warning...  Seem reasonable to people, if so, Ok.

Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Georg-Johann Lay]

Mike Stump wrote:
> On Jan 13, 2012, at 9:23 AM, Rainer Orth wrote:
>> Mike Stump <mikestump@comcast.net> writes:
>>
>>> On Jan 13, 2012, at 4:33 AM, Georg-Johann Lay wrote:
>>>> This test case is obviously written for 32-bit platforms, thus added
>>>> "dg-require-effective-target ilp32" to ensure that the pointer mess won't lead
>>>> to FAILs because of
>>>>
>>>> warning: cast to pointer from integer of different size
>>>>
>>>> and
>>>>
>>>> warning: cast from pointer to integer of different size
>>>>
>>>> Ok to apply?
>>> Ok.
>>>
>>>> 	* gcc.dg/lto/20091013-1_0.c: Add dg-require-effective-target ilp32.
>> I wonder if the fix is right, though: the testcase currenly passes for
>> 64-bit multilibs, but won't run there any longer if the patch is
>> applied.
> 
> I rely upon words like obviously written for 32-bit platforms a little too much apparently, sorry.  I agree, let's find some other solution.  I've starred at the code, and is was not clear to me why you thought it was obviously for 32-bit platforms.
> 
> So, I'd propose the -Wno- option silence the warning...  Seem reasonable to people, if so, Ok.

The ilp32 is the closes match:

The source casts pointer to int, int to pointer, long to int, uses 32-bit
initializers for int, assumes size_t is unsigned long any maybe others.

Test cases like these are a permanent annoyance. There are hundreds of test
cases that are not properly written or reviewed or gated by dg-require-foo.

I wonder why such test cases go into the test suite in the first place if the
come with such bunch of implications.

Most of the test cases run on *all* targets that GCC claims to support.

Johann

Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Jakub Jelinek]

On Fri, Jan 13, 2012 at 07:40:59PM +0100, Georg-Johann Lay wrote:
> The ilp32 is the closes match:
> 
> The source casts pointer to int, int to pointer, long to int, uses 32-bit
> initializers for int, assumes size_t is unsigned long any maybe others.

No.  The source is just fine for any target where sizeof (long) == sizeof (void *).
So both ilp32 and lp64.

> I wonder why such test cases go into the test suite in the first place if the
> come with such bunch of implications.

Because they test actual problems that were reported and usually fixed with
the same commit, to avoid regressing in the future?

	Jakub

Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Georg-Johann Lay]

Jakub Jelinek wrote:
> On Fri, Jan 13, 2012 at 07:40:59PM +0100, Georg-Johann Lay wrote:
>> The ilp32 is the closes match:
>>
>> The source casts pointer to int, int to pointer, long to int, uses 32-bit
>> initializers for int, assumes size_t is unsigned long any maybe others.
> 
> No.  The source is just fine for any target where sizeof (long) == sizeof (void *).
> So both ilp32 and lp64.
> 
>> I wonder why such test cases go into the test suite in the first place if the
>> come with such bunch of implications.
> 
> Because they test actual problems that were reported and usually fixed with
> the same commit, to avoid regressing in the future?
> 
> 	Jakub

So it's fine to dump any code to the test suite no matter on what platform it
might break or work?

I.e. it is sufficient that it runs on at least one platform that complies to
the C standard (for C test case)?

Are there really no policies for test case that they behave reasonably?

If, for example, there were hundreds of testcases in the test suites that break
if sizeof(void) != 2 adding to the fun working with your platform: What would
you think?

Johann

Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Jakub Jelinek]

On Fri, Jan 13, 2012 at 08:25:39PM +0100, Georg-Johann Lay wrote:
> So it's fine to dump any code to the test suite no matter on what platform it
> might break or work?

It is wrong to knowingly commit testcases that break on such platforms,
but you really shouldn't expect every committer to test all testsuite
additions on all targets.

	Jakub

Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Georg-Johann Lay]

Jakub Jelinek wrote:
> On Fri, Jan 13, 2012 at 08:25:39PM +0100, Georg-Johann Lay wrote:
>> So it's fine to dump any code to the test suite no matter on what platform it
>> might break or work?
> 
> It is wrong to knowingly commit testcases that break on such platforms,
> but you really shouldn't expect every committer to test all testsuite
> additions on all targets.
> 
> 	Jakub

I really don't expect anyone to run test suites on any platform or target
supported by GCC. Such an approach is completely useless because of the amount
of time and system resources and I never proposed that.

But it appears that many of the test that are exposed to all targets are just
dropped at the testsuite without even looking at them with respect to platform
requirements.

The most prominent of these requirements is sizeof(int) >= 4 because the most
PRs are reported for such platforms.

Still I think that patches that are applied to test suite should experience
some review. Anything else will just increase annoyance and frustration of
developers that work for platforms that are not in the center of bolide hardware.

For many test cases it's already sufficient to cut down constants so that they
fit into 16 bits, and if actually 32-bit variable is needed GCC provides magic
like __INT32_TYPE__.

Johann

Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Mike Stump]

On Jan 13, 2012, at 11:25 AM, Georg-Johann Lay wrote:
> So it's fine to dump any code to the test suite no matter on what platform it
> might break or work?

No.  It isn't fine, but it does happen.  Some people spend a lot of time, trying to get the testcases minimal, portable and correct.  Other people, can't even be bothered to test the testcase to ensure to actually even tested the failure.  :-(

The best way forward IMHO, is to not just fix it, but to provide feedback to those that author such testcases, to see if you can sensitize people to some of the issues.  Some issues, like, ints are 16 bits, you're going to loose some or most people on most of the time.  Other issues, should be easier.  Timely feedback I think is the key though, best would be automated testing that can provide feedback within the hour, failing that, nightly.

Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Georg-Johann Lay]

Rainer Orth schrieb:
> Mike Stump <mikestump@comcast.net> writes:
> 
> 
>>On Jan 13, 2012, at 4:33 AM, Georg-Johann Lay wrote:
>>
>>>This test case is obviously written for 32-bit platforms, thus added
>>>"dg-require-effective-target ilp32" to ensure that the pointer mess won't lead
>>>to FAILs because of
>>>
>>>warning: cast to pointer from integer of different size
>>>
>>>and
>>>
>>>warning: cast from pointer to integer of different size
>>>
>>>Ok to apply?
>>
>>Ok.
>>
>>>	* gcc.dg/lto/20091013-1_0.c: Add dg-require-effective-target ilp32.
> 
> 
> I wonder if the fix is right, though: the testcase currenly passes for
> 64-bit multilibs, but won't run there any longer if the patch is
> applied.
> 
> 	Rainer

What about int32plus instead of ilp32?

That's still not exactly what the test case needs but closer than ilp32.

Johann

Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Pedro Alves]

On 01/13/2012 08:21 PM, Georg-Johann Lay wrote:

> I really don't expect anyone to run test suites on any platform or target
> supported by GCC. Such an approach is completely useless because of the amount
> of time and system resources and I never proposed that.
> 
> But it appears that many of the test that are exposed to all targets are just
> dropped at the testsuite without even looking at them with respect to platform
> requirements.
> 
> The most prominent of these requirements is sizeof(int) >= 4 because the most
> PRs are reported for such platforms.
> 
> Still I think that patches that are applied to test suite should experience
> some review. Anything else will just increase annoyance and frustration of
> developers that work for platforms that are not in the center of bolide hardware.
> 
> For many test cases it's already sufficient to cut down constants so that they
> fit into 16 bits, and if actually 32-bit variable is needed GCC provides magic
> ike __INT32_TYPE__.

I suggest listing these common testcase pitfalls / best practices somewhere, like
for example, on a gcc wiki page.  It's then easier to point people at the issues.

-- 
Pedro Alves

Re: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Georg-Johann Lay]

Jakub Jelinek wrote:
> On Fri, Jan 13, 2012 at 07:40:59PM +0100, Georg-Johann Lay wrote:
>> The ilp32 is the closes match:
>>
>> The source casts pointer to int, int to pointer, long to int, uses 32-bit
>> initializers for int, assumes size_t is unsigned long any maybe others.
> 
> No.  The source is just fine for any target where sizeof (long) == sizeof (void *).
> So both ilp32 and lp64.

Now just factored out avr. There is no dg-requite to filter out long!=void*, or
did I miss something?

Ok to apply?

Johann

	* gcc.dg/lto/20091013-1_1.c: xfail for avr.
	* gcc.dg/lto/20091013-1_2.c: xfail for avr.



Index: gcc.dg/lto/20091013-1_1.c
===================================================================
--- gcc.dg/lto/20091013-1_1.c   (revision 183472)
+++ gcc.dg/lto/20091013-1_1.c   (working copy)
@@ -1,3 +1,4 @@
+/* { dg-xfail-if "cast to pointer of different size" { "avr-*-*" } { "*" } {
"" } } */
 typedef struct HDC__ { int unused; } *HDC;
 typedef struct HFONT__ { int unused; } *HFONT;

Index: gcc.dg/lto/20091013-1_2.c
===================================================================
--- gcc.dg/lto/20091013-1_2.c   (revision 183472)
+++ gcc.dg/lto/20091013-1_2.c   (working copy)
@@ -1,3 +1,4 @@
+/* { dg-xfail-if "cast to pointer of different size" { "avr-*-*" } { "*" } {
"" } } */
 typedef struct HDC__ { int unused; } *HDC;
 typedef struct HFONT__ { int unused; } *HFONT;

Ping #1: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Georg-Johann Lay]

http://gcc.gnu.org/ml/gcc-patches/2012-01/msg01502.html

Georg-Johann Lay wrote:

> Jakub Jelinek wrote:
>> On Fri, Jan 13, 2012 at 07:40:59PM +0100, Georg-Johann Lay wrote:
>>> The ilp32 is the closes match:
>>>
>>> The source casts pointer to int, int to pointer, long to int, uses 32-bit
>>> initializers for int, assumes size_t is unsigned long any maybe others.
>> No.  The source is just fine for any target where sizeof (long) == sizeof (void *).
>> So both ilp32 and lp64.
> 
> Now just factored out avr. There is no dg-requite to filter out long!=void*, or
> did I miss something?
> 
> Ok to apply?
> 
> Johann
> 
> 	* gcc.dg/lto/20091013-1_1.c: xfail for avr.
> 	* gcc.dg/lto/20091013-1_2.c: xfail for avr.
> 
> 
> 
> Index: gcc.dg/lto/20091013-1_1.c
> ===================================================================
> --- gcc.dg/lto/20091013-1_1.c   (revision 183472)
> +++ gcc.dg/lto/20091013-1_1.c   (working copy)
> @@ -1,3 +1,4 @@
> +/* { dg-xfail-if "cast to pointer of different size" { "avr-*-*" } { "*" } {
> "" } } */
>  typedef struct HDC__ { int unused; } *HDC;
>  typedef struct HFONT__ { int unused; } *HFONT;
> 
> Index: gcc.dg/lto/20091013-1_2.c
> ===================================================================
> --- gcc.dg/lto/20091013-1_2.c   (revision 183472)
> +++ gcc.dg/lto/20091013-1_2.c   (working copy)
> @@ -1,3 +1,4 @@
> +/* { dg-xfail-if "cast to pointer of different size" { "avr-*-*" } { "*" } {
> "" } } */
>  typedef struct HDC__ { int unused; } *HDC;
>  typedef struct HFONT__ { int unused; } *HFONT;
> 
> 

Re: Ping #1: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Jakub Jelinek]

On Wed, Feb 01, 2012 at 02:00:53PM +0100, Georg-Johann Lay wrote:
> > 	* gcc.dg/lto/20091013-1_1.c: xfail for avr.
> > 	* gcc.dg/lto/20091013-1_2.c: xfail for avr.

If it is just because of the warnings, can't you
add
-Wno-int-to-pointer-cast -Wno-pointer-to-int-cast
to dg-options?  Also, not sure how effective is dg-xfail-if in the secondary
source files, the *_0.c is where all the dg directives are for it.

	Jakub

Re: Ping #1: [Patch,testsuite]: Fix testcase that bangs long and int against void*

[Georg-Johann Lay]

Jakub Jelinek wrote:
> On Wed, Feb 01, 2012 at 02:00:53PM +0100, Georg-Johann Lay wrote:
>>> 	* gcc.dg/lto/20091013-1_1.c: xfail for avr.
>>> 	* gcc.dg/lto/20091013-1_2.c: xfail for avr.
> 
> If it is just because of the warnings, can't you
> add
> -Wno-int-to-pointer-cast -Wno-pointer-to-int-cast
> to dg-options?

dg-additional-options triggers
WARNING: lto.exp does not support dg-additional-options

> Also, not sure how effective is dg-xfail-if in the secondary
> source files, the *_0.c is where all the dg directives are for it.

The FAILs return if dg-xfail-if is removed from 1.c/2.c and added to 0.c

dg-xfail-if only works if added in each and every individual source that
triggers a warning.

Johann

Re: Ping #1: [Committed,testsuite]: Fix testcase that bangs long and int against void*

[Georg-Johann Lay]

Georg-Johann Lay wrote:
> Jakub Jelinek wrote:
>> On Wed, Feb 01, 2012 at 02:00:53PM +0100, Georg-Johann Lay wrote:
>>>> 	* gcc.dg/lto/20091013-1_1.c: xfail for avr.
>>>> 	* gcc.dg/lto/20091013-1_2.c: xfail for avr.
>> If it is just because of the warnings, can't you
>> add
>> -Wno-int-to-pointer-cast -Wno-pointer-to-int-cast
>> to dg-options?
> 
> dg-additional-options triggers
> WARNING: lto.exp does not support dg-additional-options
> 
>> Also, not sure how effective is dg-xfail-if in the secondary
>> source files, the *_0.c is where all the dg directives are for it.
> 
> The FAILs return if dg-xfail-if is removed from 1.c/2.c and added to 0.c
> 
> dg-xfail-if only works if added in each and every individual source that
> triggers a warning.

I went ahead and installed the patch. It only disables for avr to avoid
disturbing other targets:

http://gcc.gnu.org/viewcvs?view=revision&revision=184276

Johann