Bug 11151 - [3.3/3.4 regression] __builtin_return(__builtin_apply(...)) gives wrong result
Summary: [3.3/3.4 regression] __builtin_return(__builtin_apply(...)) gives wrong result
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: middle-end (show other bugs)
Version: 3.2
: P2 critical
Target Milestone: 3.4.0
Assignee: Eric Botcazou
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2003-06-11 06:59 UTC by Andrew Church
Modified: 2004-01-17 04:22 UTC (History)
1 user (show)

See Also:
Host: sparc-sun-solaris2.9
Target: sparc-sun-solaris2.9
Build: sparc-sun-solaris2.9
Known to work:
Known to fail:
Last reconfirmed: 2003-10-28 09:42:33


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Church 2003-06-11 06:59:27 UTC
__builtin_return(__builtin_apply(...)) does not return the correct value on
Solaris/SPARC systems.  Using the test code from bug 8028:

--------------- BEGIN foo.c ---------------
int foo(int n)
{
    return n;
}

int bar(int n)
{
    return n+1;
}

int quux(int n)
{
    foo(0);
    __builtin_return(__builtin_apply((void *)bar, __builtin_apply_args(), 64));
}

int main(int argc, char **argv)
{
    return quux(argc);
}
---------------- END foo.c ----------------

(compiled with "gcc -O0 foo.c -o foo") should exit with a value of argc+1, but
instead exits with 0 on my system.  I'm not a SPARC assembly expert by any
means, but the following part of the assembly generated for quux() looks suspicious:

        call    bar, 0          <-- __builtin_apply(bar,...)
        nop
        nop
        std     %o0, [%fp-64]   <-- save return value at -64(%fp)
        std     %f0, [%fp-56]
        mov     %l1, %sp
        add     %fp, -64, %o1   <-- %o1 = %fp-64
        [...]
        ld      [%o1], %i0      <-- restore return value
        ld      [%o1+4], %i1
        ld      [%o1+8], %f0
        ld      [%o1+12], %f1
        mov     %o0, %i0        <-- return value overwritten (BUG?)
        nop
        ret
        restore

The best guess I can make is that GCC is forgetting that the return value has
already been restored to %i0 and tries to copy it from %o0, which contains
garbage (it's used for other purposes in the intervening code, omitted here for
brevity).
Comment 1 Andrew Church 2003-06-11 07:56:17 UTC
The following construct seems to work around the problem for GCC 3.2, though
it's potentially very dependent on compiler version, optimization and whatnot:

int foo(...) { ... }
int bar(...) { __builtin_return(__builtin_apply(foo,...)); }

becomes

int foo(...) { ... }
int bar1(...) { __builtin_return(__builtin_apply(foo,...)); }
void bar2(...) {
    __builtin_apply(bar_tmp,...);
    asm("ld [%sp-128],%i0");  /* %i0 is the SPARC return register */
}
int (*bar)(...) = (void *)bar2;
Comment 2 Andrew Church 2003-06-11 08:01:59 UTC
Oops, the first line in bar2() should be:
    __builtin_apply(bar1 /*not bar_tmp*/,...);
Apologies for the mail flood.
Comment 3 Andrew Pinski 2003-08-11 12:24:34 UTC
Most likely the same bug as PR 8028 but I do not know that.
Comment 4 Eric Botcazou 2003-10-28 09:42:33 UTC
Confirmed with every release newer than GCC 3.2.2 (I can't test with earlier 3.x
releases), a regression from GCC 2.95.3.
Comment 5 Eric Botcazou 2003-10-28 09:43:29 UTC
I'm already in this business :-)
Comment 6 Eric Botcazou 2003-11-27 06:23:35 UTC
Partial fix: http://gcc.gnu.org/ml/gcc-patches/2003-10/msg00897.html
Comment 7 GCC Commits 2003-12-05 06:46:39 UTC
Subject: Bug 11151

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	ebotcazou@gcc.gnu.org	2003-12-05 06:46:36

Modified files:
	gcc            : ChangeLog function.h function.c rtl.h stmt.c 
	                 builtins.c 
	gcc/config/sparc: sparc.md 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/gcc.dg: builtin-return-1.c 

Log message:
	PR middle-end/11151
	* function.h (struct function): New field 'x_naked_return_label'.
	* function.c (free_after_compilation): Set it to NULL.
	(expand_function_end): Emit 'naked_return_label' if it exists.
	* rtl.h (expand_naked_return): Declare.
	* stmt.c (expand_naked_return): New function to generate a
	jump to 'naked_return_label'.
	* builtins.c (expand_builtin_return): Call expand_naked_return
	instead of expand_null_return.
	* config/sparc/sparc.md (untyped_return): Likewise.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.1920&r2=2.1921
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/function.h.diff?cvsroot=gcc&r1=1.104&r2=1.105
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/function.c.diff?cvsroot=gcc&r1=1.472&r2=1.473
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/rtl.h.diff?cvsroot=gcc&r1=1.441&r2=1.442
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/stmt.c.diff?cvsroot=gcc&r1=1.337&r2=1.338
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/builtins.c.diff?cvsroot=gcc&r1=1.264&r2=1.265
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/sparc/sparc.md.diff?cvsroot=gcc&r1=1.193&r2=1.194
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.3227&r2=1.3228
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/builtin-return-1.c.diff?cvsroot=gcc&r1=NONE&r2=1.1

Comment 8 Eric Botcazou 2003-12-05 06:59:49 UTC
See http://gcc.gnu.org/ml/gcc-patches/2003-11/msg02230.html