[Bug fortran/102910] New: cf-descriptor-5-c.c fails to build

[Bug fortran/102910] New: cf-descriptor-5-c.c fails to build

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

            Bug ID: 102910
           Summary: cf-descriptor-5-c.c fails to build
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: fortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: kargl at gcc dot gnu.org
  Target Milestone: ---

Build log shows

Executing on host:
/usr/home/sgk/gcc/objx/gcc/testsuite/gfortran1/../../gfortran
-B/usr/home/sgk/gcc/objx/gcc/testsuite/gfortran1/../../
-B/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libgfortran/
/usr/home/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/c-interop/cf-descriptor-5.f90 
  -fdiagnostics-plain-output  -fdiagnostics-plain-output    -O0  -Werror -g 
/usr/home/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/c-interop/cf-descriptor-5-c.c
/usr/home/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/c-interop/dump-descriptors.c
-dumpbase "" 
-B/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libgfortran/.libs
-L/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libgfortran/.libs
-L/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libgfortran/.libs
-L/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libatomic/.libs
-B/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libquadmath/.libs
-L/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libquadmath/.libs
-L/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libquadmath/.libs  -lm 
-o ./cf-descriptor-5.exe    (timeout = 300)
spawn -ignore SIGHUP
/usr/home/sgk/gcc/objx/gcc/testsuite/gfortran1/../../gfortran
-B/usr/home/sgk/gcc/objx/gcc/testsuite/gfortran1/../../
-B/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libgfortran/
/usr/home/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/c-interop/cf-descriptor-5.f90
-fdiagnostics-plain-output -fdiagnostics-plain-output -O0 -Werror -g
/usr/home/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/c-interop/cf-descriptor-5-c.c
/usr/home/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/c-interop/dump-descriptors.c
-dumpbase 
-B/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libgfortran/.libs
-L/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libgfortran/.libs
-L/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libgfortran/.libs
-L/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libatomic/.libs
-B/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libquadmath/.libs
-L/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libquadmath/.libs
-L/usr/home/sgk/gcc/objx/x86_64-unknown-freebsd14.0/./libquadmath/.libs -lm -o
./cf-descriptor-5.exe
/usr/home/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/c-interop/cf-descriptor-5-c.c:3:10:
fatal error: alloca.h: No such file or directory
compilation terminated.
compiler exited with status 1
FAIL: gfortran.dg/c-interop/cf-descriptor-5.f90   -O0  (test for excess errors)
Excess errors:
/usr/home/sgk/gcc/gccx/gcc/testsuite/gfortran.dg/c-interop/cf-descriptor-5-c.c:3:10:
fatal error: alloca.h: No such file or directory
compilation terminated.

[Bug fortran/102910] cf-descriptor-5-c.c fails to build

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

kargl at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |x86_64-*-freebsd
   Target Milestone|---                         |12.0

[Bug fortran/102910] cf-descriptor-5-c.c fails to build

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

--- Comment #1 from kargl at gcc dot gnu.org ---
This fixes the issue.  The assumption that alloca.h is available on
all systems is likely not a good idea.  The function alloca() lives
in stdlib.h on at least FreeBSD.

diff --git a/gcc/testsuite/gfortran.dg/c-interop/cf-descriptor-5-c.c
b/gcc/testsuite/gfortran.dg/c
-interop/cf-descriptor-5-c.c
index 12464b55512..2a525c522b8 100644
--- a/gcc/testsuite/gfortran.dg/c-interop/cf-descriptor-5-c.c
+++ b/gcc/testsuite/gfortran.dg/c-interop/cf-descriptor-5-c.c
@@ -1,6 +1,8 @@
 #include <stdlib.h>
 #include <stdio.h>
+#ifndef alloca
 #include <alloca.h>
+#endif

 #include <ISO_Fortran_binding.h>
 #include "dump-descriptors.h"

[Bug testsuite/102910] cf-descriptor-5-c.c fails to build

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
          Component|fortran                     |testsuite
   Last reconfirmed|                            |2021-10-24
     Ever confirmed|0                           |1
            Version|unknown                     |12.0

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I think the following is better:

#ifndef alloca
#define alloca __builtin_alloca
#endif

Instead of the include of alloca.h.

[Bug testsuite/102910] cf-descriptor-5-c.c fails to build

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

--- Comment #3 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Sun, Oct 24, 2021 at 05:28:08AM +0000, pinskia at gcc dot gnu.org wrote:
> 
> --- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> I think the following is better:
> 
> #ifndef alloca
> #define alloca __builtin_alloca
> #endif
> 
> Instead of the include of alloca.h.
> 

I have no preference with regards to the fix.  I do
however prefer that when trivially obvious choices
are available that one is selected and committed.

[Bug testsuite/102910] cf-descriptor-5-c.c fails to build

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

kargl at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dje.gcc at gmail dot com

--- Comment #4 from kargl at gcc dot gnu.org ---
David, please, unfix your fix for this testcase.  It is causes 5 FAILs in the
testsuite.

[Bug testsuite/102910] cf-descriptor-5-c.c fails to build

[dje at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

David Edelsohn <dje at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dje at gcc dot gnu.org

--- Comment #5 from David Edelsohn <dje at gcc dot gnu.org> ---
Previously the test case was unresolved because it referenced alloca without a
declaration.

char *adata = (char *) alloca (n);

If you want to call __builtin_alloca() because this is a testcase for GCC,
which provides __builtin_alloca(), fine.  You should call it directly and not
alloca().

If you want to call alloca(), then you need logic for the different systems
that declare alloca() in different header files.

#ifdef alloca

is wrong because it makes the testcase system dependent and tests different
behavior.

[Bug testsuite/102910] cf-descriptor-5-c.c fails to build

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

--- Comment #6 from kargl at gcc dot gnu.org ---
(In reply to David Edelsohn from comment #5)
> Previously the test case was unresolved because it referenced alloca without
> a declaration.
> 
> char *adata = (char *) alloca (n);
> 
> If you want to call __builtin_alloca() because this is a testcase for GCC,
> which provides __builtin_alloca(), fine.  You should call it directly and
> not alloca().
> 
> If you want to call alloca(), then you need logic for the different systems
> that declare alloca() in different header files.
> 
> #ifdef alloca
> 
> is wrong because it makes the testcase system dependent and tests different
> behavior.

It seems you broke it with

% git blame gcc/testsuite/gfortran.dg/c-interop/cf-descriptor-5-c.c
cb17b5054118 (Sandra Loosemore  2021-06-30 20:03:27 -0700  1) #include
<stdlib.h
>
cb17b5054118 (Sandra Loosemore  2021-06-30 20:03:27 -0700  2) #include
<stdio.h>
000000000000 (Not Committed Yet 2021-10-24 13:54:29 -0700  3) #ifndef alloca
f1408388f2e2 (David Edelsohn    2021-09-30 16:43:58 -0400  4) #include
<alloca.h
>
000000000000 (Not Committed Yet 2021-10-24 13:54:29 -0700  5) #endif

Shouldn't the person who broke the testcase fix it?  Or, is it okay
for anyone to commit an unreviewed knowingly broken "fix" to paper over
a failure on whatever target they are working on?

[Bug testsuite/102910] cf-descriptor-5-c.c fails to build

[dje at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

--- Comment #7 from David Edelsohn <dje at gcc dot gnu.org> ---
Sandra checked in a large number of testcases for interoperability that were
broken from the outset on all platforms -- I saw them failing on multiple Linux
architectures, not just AIX. The testcases should have been XFAILed when
initially checked in.  If the testcases had been XFAILed, it would have been
apparent that the cf-descriptor-5-c.c testcase was not written portably for
different OSes.

My patch apparently swapped which targets produced UNRESOLVED and which
produced FAIL (now PASS).

Known failing portability testcases should not have been checked in to the GCC
repository.

alloca() is a mess.  If you have better visibility into the portability of
alloca() across platforms, you're welcome to fix it.  I suggest that having
different targets use different definitions of alloca muddles the test.  If the
intention is for the system header to substitute __builtin_alloca() for
alloca(), then it suggest would be better use that from the beginning.  The
testcase seems to intend to test C-Fortran interoperability, not alloca().

[Bug testsuite/102910] cf-descriptor-5-c.c fails to build

[burnus at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

--- Comment #8 from Tobias Burnus <burnus at gcc dot gnu.org> ---
I wonder whether something likewise (using __builtin_alloca) should also be
done in libgomp/testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c, which
currently uses
"#include <alloca.h>" + "alloca".

[Bug testsuite/102910] cf-descriptor-5-c.c fails to build

[sandra at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

--- Comment #9 from sandra at gcc dot gnu.org ---
I will rewrite this testcase not to use alloca.

This particular case was originally XFAIL'ed at runtime because the
functionality it was supposed to test (assumed-length character
interoperability) was broken before Tobias fixed the GFC/CFI descriptor
conversion.  I was not aware that it was also failing to compile on some
targets; I don't have any way to test FreeBSD or AIX targets, in particular.

[Bug testsuite/102910] cf-descriptor-5-c.c fails to build

[sgk at troutmask dot apl.washington.edu]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

--- Comment #10 from Steve Kargl <sgk at troutmask dot apl.washington.edu> ---
On Mon, Oct 25, 2021 at 05:05:26PM +0000, sandra at gcc dot gnu.org wrote:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910
> 
> --- Comment #9 from sandra at gcc dot gnu.org ---
> I will rewrite this testcase not to use alloca.

Thanks.  I wasn't sure if you were purposely using
alloca() to put something on the stack to check for 
correct CFI behavior or not.  I proposed the simplest
solution I could think that fixes the FreeBSD failure.
I just checked and OpenBSD and NetBSD also have
prototypes in stdlib.h for alloca().

[Bug testsuite/102910] cf-descriptor-5-c.c fails to build

[sandra at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

--- Comment #11 from sandra at gcc dot gnu.org ---
Patch posted: https://gcc.gnu.org/pipermail/fortran/2021-October/056807.html

[Bug testsuite/102910] cf-descriptor-5-c.c fails to build

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tobias Burnus <burnus@gcc.gnu.org>:

https://gcc.gnu.org/g:72dc270be793f159a3a038bef41542d85550b331

commit r12-4691-g72dc270be793f159a3a038bef41542d85550b331
Author: Tobias Burnus <tobias@codesourcery.com>
Date:   Mon Oct 25 20:40:13 2021 +0200

    libgomp.oacc-c-c++-common/loop-gwv-2.c: Use __builtin_alloca

    Some systems do not have <alloca.h> but provide alloca differently, e.g.
    via stdlib.h. Do it like other testcases do and use __builtin_alloca.

    libgomp/ChangeLog:

            PR testsuite/102910
            * testsuite/libgomp.oacc-c-c++-common/loop-gwv-2.c: Use
__builtin_alloca
            instead of #include <alloca.h> + alloca.

[Bug testsuite/102910] cf-descriptor-5-c.c fails to build

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

--- Comment #13 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Sandra Loosemore <sandra@gcc.gnu.org>:

https://gcc.gnu.org/g:f9190a1c6d001099d1409298ae3b684499bd1fdd

commit r12-4714-gf9190a1c6d001099d1409298ae3b684499bd1fdd
Author: Sandra Loosemore <sandra@codesourcery.com>
Date:   Mon Oct 25 11:08:28 2021 -0700

    Fix broken use of alloca in C interoperability testcase

    2021-10-25  Sandra Loosemore  <sandra@codesourcery.com>

            gcc/testsuite/

            PR testsuite/102910
            * gfortran.dg/c-interop/cf-descriptor-5-c.c: Use a static buffer
            instead of alloca.

[Bug testsuite/102910] cf-descriptor-5-c.c fails to build

[sandra at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

sandra at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #14 from sandra at gcc dot gnu.org ---
Should be fixed now.

[Bug testsuite/102910] cf-descriptor-5-c.c fails to build

[kargl at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102910

--- Comment #15 from kargl at gcc dot gnu.org ---
(In reply to sandra from comment #14)
> Should be fixed now.

Thanks you for fixing this, and the CFI issues you've been working through.