diff mbox series

testsuite:analyzer: Fix header include for FreeBSD

Message ID 5ca0bcb7-b2cb-9d28-5e16-f0d3135dc0ab@fgznet.ch
State New
Headers show
Series testsuite:analyzer: Fix header include for FreeBSD | expand

Commit Message

Andreas Tobler May 1, 2020, 7:49 p.m. UTC
Hi all,

FreeBSD does not have the alloca.h header. Do not include it in the test 
cases which do include alloca.h.

There are two versions of this patch available, the one attached which 
uses ifdef or another one which defines alloca with __builtin_alloca.

I tested both approaches and they work on FreeBSD. I do not know which 
one you prefer.

Opinions welcome.

Ok for trunk?
TIA,
Andreas

Comments

Kamil Rytarowski May 3, 2020, 4:27 p.m. UTC | #1
On 01.05.2020 21:49, Andreas Tobler wrote:
> Hi all,
> 
> FreeBSD does not have the alloca.h header. Do not include it in the test
> cases which do include alloca.h.
> 
> There are two versions of this patch available, the one attached which
> uses ifdef or another one which defines alloca with __builtin_alloca.
> 
> I tested both approaches and they work on FreeBSD. I do not know which
> one you prefer.
> 
> Opinions welcome.
> 
> Ok for trunk?
> TIA,
> Andreas

Please include in your patch "|| defined(__NetBSD__)".
Andreas Tobler May 3, 2020, 8:31 p.m. UTC | #2
On 03.05.20 18:27, Kamil Rytarowski wrote:
> On 01.05.2020 21:49, Andreas Tobler wrote:
>> Hi all,
>>
>> FreeBSD does not have the alloca.h header. Do not include it in the test
>> cases which do include alloca.h.
>>
>> There are two versions of this patch available, the one attached which
>> uses ifdef or another one which defines alloca with __builtin_alloca.
>>
>> I tested both approaches and they work on FreeBSD. I do not know which
>> one you prefer.
>>
>> Opinions welcome.
>>
>> Ok for trunk?
>> TIA,
>> Andreas
> 
> Please include in your patch "|| defined(__NetBSD__)".

is this ok for you?

This is one reason why I'd prefer #define alloca __builtin_alloca and 
leave the include away.....

Andreas
From 70165e30fb5b4398c903b4522d6496c7da7c12fa Mon Sep 17 00:00:00 2001
From: Andreas Tobler <andreast@gcc.gnu.org>
Date: Fri, 1 May 2020 21:10:39 +0200
Subject: [PATCH] testsuite: analyzer: Fix header include for Free/NetBSD

FreeBSD and NetBSD do not have the alloca.h header.
Fix this with ifndef on alloca.h.
---
 gcc/testsuite/ChangeLog                        | 8 ++++++++
 gcc/testsuite/gcc.dg/analyzer/alloca-leak.c    | 5 ++++-
 gcc/testsuite/gcc.dg/analyzer/data-model-1.c   | 2 ++
 gcc/testsuite/gcc.dg/analyzer/malloc-1.c       | 2 ++
 gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c | 2 ++
 5 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 185f9ea725e..3f7b409bd42 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@
+2020-05-03  Andreas Tobler  <andreast@gcc.gnu.org>
+
+	* gcc.dg/analyzer/alloca-leak.c: Do not include
+	  alloca.h for Free/NetBSD.
+	* gcc.dg/analyzer/data-model-1.c: Likewise
+	* gcc.dg/analyzer/malloc-1.c: Likewise.
+	* gcc.dg/analyzer/malloc-paths-8.c: Likewise.
+
 2020-05-01  H.J. Lu  <hongjiu.lu@intel.com>
 
 	PR target/93492
diff --git a/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c b/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
index 6d9fe3431ce..631e81b8cf4 100644
--- a/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
+++ b/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
@@ -1,5 +1,8 @@
+#if defined (__FreeBSD__) || defined(__NetBSD__)
+#include <stdlib.h>
+#else
 #include <alloca.h>
-
+#endif
 void *test (void)
 {
   void *ptr = alloca (64);
diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
index 1db99133d50..f06651802a5 100644
--- a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
@@ -1,7 +1,9 @@
 #include <stdlib.h>
 #include <string.h>
 #include <stdio.h>
+#if (!defined(__FreeBSD__) && !defined(__NetBSD__))
 #include <alloca.h>
+#endif
 #include "analyzer-decls.h"
 
 struct foo
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
index 3024e546137..046e3353192 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
@@ -1,4 +1,6 @@
+#if (!defined(__FreeBSD__) && !defined(__NetBSD__))
 #include <alloca.h>
+#endif
 #include <stdlib.h>
 
 extern int foo (void);
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c b/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c
index 10b97a05402..d55d6350da8 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c
@@ -1,7 +1,9 @@
 /* { dg-additional-options "-fanalyzer-transitivity" } */
 
 #include <stddef.h>
+#if (!defined(__FreeBSD__) && !defined(__NetBSD__))
 #include <alloca.h>
+#endif
 #include <stdlib.h>
 
 extern void do_stuff (const void *);
Kamil Rytarowski May 4, 2020, 9:49 a.m. UTC | #3
On 03.05.2020 22:31, Andreas Tobler wrote:
> On 03.05.20 18:27, Kamil Rytarowski wrote:
>> On 01.05.2020 21:49, Andreas Tobler wrote:
>>> Hi all,
>>>
>>> FreeBSD does not have the alloca.h header. Do not include it in the test
>>> cases which do include alloca.h.
>>>
>>> There are two versions of this patch available, the one attached which
>>> uses ifdef or another one which defines alloca with __builtin_alloca.
>>>
>>> I tested both approaches and they work on FreeBSD. I do not know which
>>> one you prefer.
>>>
>>> Opinions welcome.
>>>
>>> Ok for trunk?
>>> TIA,
>>> Andreas
>>
>> Please include in your patch "|| defined(__NetBSD__)".
> 
> is this ok for you?
> 
> This is one reason why I'd prefer #define alloca __builtin_alloca and
> leave the include away.....
> 
> Andreas

It looks fine to me now.

Personally, I would include stdlib.h unconditionally and alloca.h on
!FreeBSD && !NetBSD.
Jakub Jelinek May 4, 2020, 10:04 a.m. UTC | #4
On Mon, May 04, 2020 at 11:49:27AM +0200, Kamil Rytarowski wrote:
> >> Please include in your patch "|| defined(__NetBSD__)".
> > 
> > is this ok for you?
> > 
> > This is one reason why I'd prefer #define alloca __builtin_alloca and
> > leave the include away.....
> > 
> > Andreas
> 
> It looks fine to me now.
> 
> Personally, I would include stdlib.h unconditionally and alloca.h on
> !FreeBSD && !NetBSD.

It just just a test.
Perhaps it could
#include <stdlib.h>
#if __has_include(<alloca.h>)
#include <alloca.h>
#endif
?

	Jakub
Kamil Rytarowski May 4, 2020, 10:10 a.m. UTC | #5
On 04.05.2020 12:04, Jakub Jelinek wrote:
> On Mon, May 04, 2020 at 11:49:27AM +0200, Kamil Rytarowski wrote:
>>>> Please include in your patch "|| defined(__NetBSD__)".
>>>
>>> is this ok for you?
>>>
>>> This is one reason why I'd prefer #define alloca __builtin_alloca and
>>> leave the include away.....
>>>
>>> Andreas
>>
>> It looks fine to me now.
>>
>> Personally, I would include stdlib.h unconditionally and alloca.h on
>> !FreeBSD && !NetBSD.
> 
> It just just a test.
> Perhaps it could
> #include <stdlib.h>
> #if __has_include(<alloca.h>)
> #include <alloca.h>
> #endif
> ?
> 
> 	Jakub
> 

This looks fine too.
Li, Pan2 via Gcc-patches May 5, 2020, 4:22 p.m. UTC | #6
On Sun, 2020-05-03 at 22:31 +0200, Andreas Tobler wrote:
> On 03.05.20 18:27, Kamil Rytarowski wrote:
> > On 01.05.2020 21:49, Andreas Tobler wrote:
> > > Hi all,
> > > 
> > > FreeBSD does not have the alloca.h header. Do not include it in the test
> > > cases which do include alloca.h.
> > > 
> > > There are two versions of this patch available, the one attached which
> > > uses ifdef or another one which defines alloca with __builtin_alloca.
> > > 
> > > I tested both approaches and they work on FreeBSD. I do not know which
> > > one you prefer.
> > > 
> > > Opinions welcome.
> > > 
> > > Ok for trunk?
> > > TIA,
> > > Andreas
> > 
> > Please include in your patch "|| defined(__NetBSD__)".
> 
> is this ok for you?
> 
> This is one reason why I'd prefer #define alloca __builtin_alloca and 
> leave the include away.....
OK for the trunk.
jeff
Andreas Tobler May 6, 2020, 7:54 p.m. UTC | #7
On 04.05.20 12:10, Kamil Rytarowski wrote:
> On 04.05.2020 12:04, Jakub Jelinek wrote:
>> On Mon, May 04, 2020 at 11:49:27AM +0200, Kamil Rytarowski wrote:
>>>>> Please include in your patch "|| defined(__NetBSD__)".
>>>>
>>>> is this ok for you?
>>>>
>>>> This is one reason why I'd prefer #define alloca __builtin_alloca and
>>>> leave the include away.....
>>>>
>>>> Andreas
>>>
>>> It looks fine to me now.
>>>
>>> Personally, I would include stdlib.h unconditionally and alloca.h on
>>> !FreeBSD && !NetBSD.
>>
>> It just just a test.
>> Perhaps it could
>> #include <stdlib.h>
>> #if __has_include(<alloca.h>)
>> #include <alloca.h>
>> #endif
>> ?
>>
>> 	Jakub
>>
> 
> This looks fine too.
> 

K, fine with me. I'll commit this solution once I finished testing. Thanks,
Andreas

+2020-05-07  Andreas Tobler  <andreast@gcc.gnu.org>
+
+       * gcc.dg/analyzer/alloca-leak.c: Include alloca.h only if
+         available.
+       * gcc.dg/analyzer/data-model-1.c: Likewise
+       * gcc.dg/analyzer/malloc-1.c: Likewise.
+       * gcc.dg/analyzer/malloc-paths-8.c: Likewise.

diff --git a/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c 
b/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
index 6d9fe3431ce..e4717a1bbb3 100644
--- a/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
+++ b/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
@@ -1,5 +1,6 @@
-#include <alloca.h>
-
+#include <stdlib.h>
+#if __has_include(<alloca.h>)
+#endif
  void *test (void)
  {
    void *ptr = alloca (64);
diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c 
b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
index 1db99133d50..32559952e34 100644
--- a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
@@ -1,7 +1,8 @@
  #include <stdlib.h>
  #include <string.h>
  #include <stdio.h>
-#include <alloca.h>
+#if __has_include(<alloca.h>)
+#endif
  #include "analyzer-decls.h"

  struct foo
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c 
b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
index 3024e546137..0f3f1fc760a 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
@@ -1,4 +1,5 @@
-#include <alloca.h>
+#if __has_include(<alloca.h>)
+#endif
  #include <stdlib.h>

  extern int foo (void);
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c 
b/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c
index 10b97a05402..35035d27ff7 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c
@@ -1,7 +1,8 @@
  /* { dg-additional-options "-fanalyzer-transitivity" } */

  #include <stddef.h>
-#include <alloca.h>
+#if __has_include(<alloca.h>)
+#endif
  #include <stdlib.h>

  extern void do_stuff (const void *);
Jakub Jelinek May 6, 2020, 8:12 p.m. UTC | #8
On Wed, May 06, 2020 at 09:54:47PM +0200, Andreas Tobler wrote:
> --- a/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
> +++ b/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
> @@ -1,5 +1,6 @@
> -#include <alloca.h>
> -
> +#include <stdlib.h>

It needs to be:
> +#if __has_include(<alloca.h>)

+#include <alloca.h>

> +#endif

__has_include doesn't include anything, just checks if the header is
available.

	Jakub
Andreas Tobler May 6, 2020, 8:25 p.m. UTC | #9
On 06.05.20 22:12, Jakub Jelinek wrote:
> On Wed, May 06, 2020 at 09:54:47PM +0200, Andreas Tobler wrote:
>> --- a/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
>> +++ b/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
>> @@ -1,5 +1,6 @@
>> -#include <alloca.h>
>> -
>> +#include <stdlib.h>
> 
> It needs to be:
>> +#if __has_include(<alloca.h>)
> 
> +#include <alloca.h>
> 
>> +#endif

Doh :(

Fixed, thanks.

> __has_include doesn't include anything, just checks if the header is
> available.

Then I just wonder why the tests passed on Linux (ubuntu-20.04)

Andreas
Kamil Rytarowski May 6, 2020, 8:27 p.m. UTC | #10
On 06.05.2020 22:25, Andreas Tobler wrote:
> On 06.05.20 22:12, Jakub Jelinek wrote:
>> On Wed, May 06, 2020 at 09:54:47PM +0200, Andreas Tobler wrote:
>>> --- a/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
>>> +++ b/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
>>> @@ -1,5 +1,6 @@
>>> -#include <alloca.h>
>>> -
>>> +#include <stdlib.h>
>>
>> It needs to be:
>>> +#if __has_include(<alloca.h>)
>>
>> +#include <alloca.h>
>>
>>> +#endif
> 
> Doh :(
> 
> Fixed, thanks.
> 
>> __has_include doesn't include anything, just checks if the header is
>> available.
> 
> Then I just wonder why the tests passed on Linux (ubuntu-20.04)
> 

alloca.h is only needed on solaris. Other system can use stdlib.h.

> Andreas
Andreas Tobler May 6, 2020, 8:33 p.m. UTC | #11
On 06.05.20 22:27, Kamil Rytarowski wrote:
> On 06.05.2020 22:25, Andreas Tobler wrote:
>> On 06.05.20 22:12, Jakub Jelinek wrote:
>>> On Wed, May 06, 2020 at 09:54:47PM +0200, Andreas Tobler wrote:
>>>> --- a/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
>>>> +++ b/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
>>>> @@ -1,5 +1,6 @@
>>>> -#include <alloca.h>
>>>> -
>>>> +#include <stdlib.h>
>>>
>>> It needs to be:
>>>> +#if __has_include(<alloca.h>)
>>>
>>> +#include <alloca.h>
>>>
>>>> +#endif
>>
>> Doh :(
>>
>> Fixed, thanks.
>>
>>> __has_include doesn't include anything, just checks if the header is
>>> available.
>>
>> Then I just wonder why the tests passed on Linux (ubuntu-20.04)
>>
> 
> alloca.h is only needed on solaris. Other system can use stdlib.h.

Ok, I see. and alloca-leak.c succeed because I included unconditionally 
the stdlib.h. Sigh, foot shot.

Thanks,
Andreas
diff mbox series

Patch

diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 949b81a4392..b67758bbc91 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,11 @@ 
+2020-05-01  Andreas Tobler  <andreast@gcc.gnu.org>
+
+	* gcc.dg/analyzer/alloca-leak.c: Do not include
+	alloca.h for FreeBSD.
+	*  gcc.dg/analyzer/data-model-1.c: Likewise
+	* gcc.dg/analyzer/malloc-1.c: Likewise.
+	* gcc.dg/analyzer/malloc-paths-8.c: Likewise.
+
 2020-05-01  Andreas Tobler  <andreast@gcc.gnu.org>
 
 	* gcc.dg/asan/pr87930.c: Enable on x86_64 FreeBSD.
diff --git a/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c b/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
index 6d9fe3431ce..ed4e4e38888 100644
--- a/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
+++ b/gcc/testsuite/gcc.dg/analyzer/alloca-leak.c
@@ -1,5 +1,8 @@ 
+#ifdef __FreeBSD__
+#include <stdlib.h>
+#else
 #include <alloca.h>
-
+#endif
 void *test (void)
 {
   void *ptr = alloca (64);
diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
index 1db99133d50..6fa35db03b5 100644
--- a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
@@ -1,7 +1,9 @@ 
 #include <stdlib.h>
 #include <string.h>
 #include <stdio.h>
+#ifndef __FreeBSD__
 #include <alloca.h>
+#endif
 #include "analyzer-decls.h"
 
 struct foo
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
index 3024e546137..94f631b2a1d 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c
@@ -1,4 +1,6 @@ 
+#ifndef __FreeBSD__
 #include <alloca.h>
+#endif
 #include <stdlib.h>
 
 extern int foo (void);
diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c b/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c
index 10b97a05402..5fa9ad3b58e 100644
--- a/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c
+++ b/gcc/testsuite/gcc.dg/analyzer/malloc-paths-8.c
@@ -1,7 +1,9 @@ 
 /* { dg-additional-options "-fanalyzer-transitivity" } */
 
 #include <stddef.h>
+#ifndef __FreeBSD__
 #include <alloca.h>
+#endif
 #include <stdlib.h>
 
 extern void do_stuff (const void *);