[Bug libffi/60073] New: [4.9 regression] 64-bit libffi.call/cls_double_va.c FAILs on Solaris/SPARC

[Bug libffi/60073] New: [4.9 regression] 64-bit libffi.call/cls_double_va.c FAILs on Solaris/SPARC

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60073

            Bug ID: 60073
           Summary: [4.9 regression] 64-bit libffi.call/cls_double_va.c
                    FAILs on Solaris/SPARC
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libffi
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ro at gcc dot gnu.org
                CC: amodra at gcc dot gnu.org, ebotcazou at gcc dot gnu.org,
                    green at gcc dot gnu.org
              Host: sparc*-sun-solaris2.*
            Target: sparc*-sun-solaris2.*
             Build: sparc*-sun-solaris2.*

Since ca. 20131121, the 64-bit libffi.call/cls_double_va.c test FAILs on
Solaris/SPARC:

FAIL: libffi.call/cls_double_va.c -O0 -W -Wall output pattern test, is 7.0
FAIL: libffi.call/cls_double_va.c -O2 output pattern test, is 7.0
FAIL: libffi.call/cls_double_va.c -O3 output pattern test, is 7.0
FAIL: libffi.call/cls_double_va.c -Os output pattern test, is 7.0
FAIL: libffi.call/cls_double_va.c -O2 -fomit-frame-pointer output pattern test,
is 7.0

E.g.

FAIL: libffi.call/cls_double_va.c -O0 -W -Wall output pattern test, is 7.0
res: 4
0.0
res: 4
, should match 7.0?
res: 4?
7.0?
res: 4

This is due to

2013-11-18  Alan Modra  <amodra@gmail.com>

    * src/powerpc/ppc_closure.S: Don't bl .Luint128.

    * src/powerpc/ffitarget.h: Import from upstream.
    * src/powerpc/ffi.c: Likewise.
    * src/powerpc/linux64.S: Likewise.
    * src/powerpc/linux64_closure.S: Likewise.
    * doc/libffi.texi: Likewise.
    * testsuite/libffi.call/cls_double_va.c: Likewise.
    * testsuite/libffi.call/cls_longdouble_va.c: Likewise.

Reverting the cls_double.ca change lets the testcase pass.  Strangely, the
cls_longdouble_va.c test still passes even with the patch, so this could
indicate
a bug in the libffi sparcv9 code.

  Rainer

[Bug libffi/60073] [4.9 regression] 64-bit libffi.call/cls_double_va.c FAILs on Solaris/SPARC

[ro at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60073

Rainer Orth <ro at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.9.0

[Bug libffi/60073] [4.9 regression] 64-bit libffi.call/cls_double_va.c FAILs on Solaris/SPARC

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60073

--- Comment #1 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> Reverting the cls_double.ca change lets the testcase pass.  Strangely, the
> cls_longdouble_va.c test still passes even with the patch, so this could
> indicate a bug in the libffi sparcv9 code.

Possibly, but modifying a testcase without explanation ("Import from upstream"
doesn't say anything about what has changed) is problematic.

[Bug libffi/60073] [4.9 regression] 64-bit libffi.call/cls_double_va.c FAILs on Solaris/SPARC

[amodra at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60073

Alan Modra <amodra at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amodra at gmail dot com

--- Comment #2 from Alan Modra <amodra at gmail dot com> ---
This is almost certainly a bug in sparc libffi.  The explanation for the
testcase fix went to the upstream libffi list..

"This also fixes two faulty tests that used a non-variadic function
cast to call a variadic function, and spuriously reasoned that this is
somehow necessary for static functions."

[Bug libffi/60073] [4.9 regression] 64-bit libffi.call/cls_double_va.c FAILs after recent modification

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60073

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|sparc*-sun-solaris2.*       |sparc*-*-*
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2014-02-05
               Host|sparc*-sun-solaris2.*       |sparc*-*-*
            Summary|[4.9 regression] 64-bit     |[4.9 regression] 64-bit
                   |libffi.call/cls_double_va.c |libffi.call/cls_double_va.c
                   |FAILs on Solaris/SPARC      |FAILs after recent
                   |                            |modification
     Ever confirmed|0                           |1
              Build|sparc*-sun-solaris2.*       |sparc*-*-*

--- Comment #3 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
I can reproduce, probably not specific to Solaris.

[Bug libffi/60073] [4.9 regression] 64-bit libffi.call/cls_double_va.c FAILs after recent modification

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60073

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
                 CC|ebotcazou at gcc dot gnu.org       |
           Assignee|unassigned at gcc dot gnu.org      |ebotcazou at gcc dot gnu.org

--- Comment #4 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Investigating.

[Bug libffi/60073] [4.9 regression] 64-bit libffi.call/cls_double_va.c FAILs after recent modification

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60073

--- Comment #5 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Btw, in the introduction section of the documentation, there is:

"Compilers for high level languages generate code that follow certain
conventions.  These conventions are necessary, in part, for separate
compilation to work.  One such convention is the "calling convention".
The calling convention is a set of assumptions made by the compiler
about where function arguments will be found on entry to a function.  A
calling convention also specifies where the return value for a function
is found.  The calling convention is also sometimes called the "ABI" or
"Application Binary Interface"."

The last sentence is slightly inaccurate, an ABI is much more than a calling
convention, it also specifies the layout of code and data among other things.
"The calling convention is part of what is called the..." would be more
correct.
And I think this should be "high-level languages" in the first sentence.

[Bug libffi/60073] [4.9 regression] 64-bit libffi.call/cls_double_va.c FAILs after recent modification

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60073

--- Comment #6 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
It's a bad interaction between closures and variadic functions (and the other
test cls_longdouble_va.c passes by accident), which will require the SPARC-V9
code to define FFI_TARGET_SPECIFIC_VARIADIC.  I'm a bit surprised that only ARM
and PowerPC define it at the moment.

[Bug libffi/60073] [4.9 regression] 64-bit libffi.call/cls_double_va.c FAILs after recent modification

[amodra at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60073

--- Comment #7 from Alan Modra <amodra at gmail dot com> ---
The variadic support in libfii is fairly recent.  And yes, it might have been
there earlier if the testcases actually tested what they were suppose to test!

Note that powerpc64-gcc passes fp arguments corresponding to the ellipsis part
of variadic functions in both the parameter save area and fp registers,
contrary to the ABI which says they just go in the parameter save area if the
function has a prototype.  I think this was in part to satisfy libffi, where
without variadic support you don't know whether a function call is normal or
variadic and thus the powerpc libffi support had to pull fp values from fp
registers.  So we broke gcc (a little) to satisfy libffi..

[Bug libffi/60073] [4.9 regression] 64-bit libffi.call/cls_double_va.c FAILs after recent modification

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60073

--- Comment #9 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Author: ebotcazou
Date: Thu Feb 13 16:18:13 2014
New Revision: 207763

URL: http://gcc.gnu.org/viewcvs?rev=207763&root=gcc&view=rev
Log:
    PR libffi/60073
    * src/sparc/ffitarget.h (FFI_TARGET_SPECIFIC_VARIADIC): Define.
    (FFI_EXTRA_CIF_FIELDS): Likewise.
    (FFI_NATIVE_RAW_API): Move around.
    * src/sparc/ffi.c (ffi_prep_cif_machdep_core): New function from...
    (ffi_prep_cif_machdep): ...here.  Call ffi_prep_cif_machdep_core.
    (ffi_prep_cif_machdep_var): New function.
    (ffi_closure_sparc_inner_v9): Do not pass anonymous FP arguments in
    FP registers.
    * doc/libffi.texi (Introduction): Fix inaccuracy.

Modified:
    trunk/libffi/ChangeLog
    trunk/libffi/doc/libffi.texi
    trunk/libffi/src/sparc/ffi.c
    trunk/libffi/src/sparc/ffitarget.h

[Bug libffi/60073] [4.9 regression] 64-bit libffi.call/cls_double_va.c FAILs after recent modification

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60073

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

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

--- Comment #10 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Thanks for reporting the problem.

[Bug libffi/60073] [4.9 regression] 64-bit libffi.call/cls_double_va.c FAILs after recent modification

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60073

--- Comment #11 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Author: ebotcazou
Date: Mon Feb 17 12:00:04 2014
New Revision: 207822

URL: http://gcc.gnu.org/viewcvs?rev=207822&root=gcc&view=rev
Log:
    PR libffi/60073
    * src/sparc/v8.S: Assemble only if !SPARC64.
    * src/sparc/v9.S: Remove obsolete comment.
    * src/sparc/ffitarget.h (enum ffi_abi): Add FFI_COMPAT_V9.
    (V8_ABI_P): New macro.
    (V9_ABI_P): Likewise.
    (FFI_EXTRA_CIF_FIELDS): Define only if SPARC64.
    * src/sparc/ffi.c (ffi_prep_args_v8): Compile only if !SPARC64.
    (ffi_prep_args_v9): Compile only if SPARC64.
    (ffi_prep_cif_machdep_core): Use V9_ABI_P predicate.
    (ffi_prep_cif_machdep): Guard access to nfixedargs field.
    (ffi_prep_cif_machdep_var): Likewise.
    (ffi_v9_layout_struct): Compile only if SPARC64.
    (ffi_call): Deal with FFI_V8PLUS and FFI_COMPAT_V9 and fix warnings.
    (ffi_prep_closure_loc): Use V9_ABI_P and V8_ABI_P predicates.
    (ffi_closure_sparc_inner_v8): Compile only if !SPARC64.
    (ffi_closure_sparc_inner_v9): Compile only if SPARC64.  Guard access
    to nfixedargs field.

Modified:
    trunk/libffi/ChangeLog
    trunk/libffi/src/sparc/ffi.c
    trunk/libffi/src/sparc/ffitarget.h
    trunk/libffi/src/sparc/v8.S
    trunk/libffi/src/sparc/v9.S