This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Incorrect code for __builtin_frame_address(0)
On Sat, 4 Jun 2005, Richard Sandiford wrote:
> > Though for the HEAD, using "frame_pointer_rtx" as the
> > default in place of "hard_frame_pointer_rtx" might be a reasonable
> > alternative. Other platforms that implement -fomit-frame-pointer may need
> > a similar fix -- currently only s390.h defines INITIAL_FRAME_ADDRESS_RTX.
>
> FWIW, this was what I argued for when the s390 folks introduced
> INITIAL_FRAME_ADDRESS_RTX. I still think it makes more sense
> than overriding it with frame_pointer_rtx on a target-by-target
> basis.
Well, I agree, but I don't have a strong preference as long as the
problem gets fixed.
> That said, if the decision is to stick with the current default,
> the MIPS patch is definitely OK. I'll hold off applying it
> until I hear what others think about the alpha and i386 bits.
OK and as an aid to determine which targets are broken and which are not,
I have prepared a simple test case. It fails correctly without the fix
and passes once it is applied. I don't think it's possible to validate
the value of __builtin_frame_address(0) in a portable way, so the test
only checks if it's reasonable.
gcc/:
2005-06-06 Maciej W. Rozycki <macro@linux-mips.org>
* config/alpha/alpha.h (INITIAL_FRAME_ADDRESS_RTX): New macro.
* config/i386/i386.h (INITIAL_FRAME_ADDRESS_RTX): Likewise.
* config/mips/mips.h (INITIAL_FRAME_ADDRESS_RTX): Likewise.
gcc/testsuite/:
2005-06-06 Maciej W. Rozycki <macro@linux-mips.org>
* gcc.c-torture/execute/frame-address.c: New test.
Please consider,
Maciej
gcc-4.0.0-frame_address.patch
diff -up --recursive --new-file gcc-4.0.0.macro/gcc/config/alpha/alpha.h gcc-4.0.0/gcc/config/alpha/alpha.h
--- gcc-4.0.0.macro/gcc/config/alpha/alpha.h 2005-04-17 06:39:08.000000000 +0000
+++ gcc-4.0.0/gcc/config/alpha/alpha.h 2005-06-03 00:25:43.000000000 +0000
@@ -936,6 +936,10 @@ extern int alpha_memory_latency;
#define FIRST_PARM_OFFSET(FNDECL) 0
+/* Defining this macro makes __builtin_frame_address(0) work
+ with -fomit-frame-pointer. */
+#define INITIAL_FRAME_ADDRESS_RTX frame_pointer_rtx
+
/* Definitions for register eliminations.
We have two registers that can be eliminated on the Alpha. First, the
diff -up --recursive --new-file gcc-4.0.0.macro/gcc/config/i386/i386.h gcc-4.0.0/gcc/config/i386/i386.h
--- gcc-4.0.0.macro/gcc/config/i386/i386.h 2005-04-05 22:59:21.000000000 +0000
+++ gcc-4.0.0/gcc/config/i386/i386.h 2005-06-03 00:26:10.000000000 +0000
@@ -1655,6 +1655,10 @@ enum reg_class
/* Offset of first parameter from the argument pointer register value. */
#define FIRST_PARM_OFFSET(FNDECL) 0
+/* Defining this macro makes __builtin_frame_address(0) work
+ with -fomit-frame-pointer. */
+#define INITIAL_FRAME_ADDRESS_RTX frame_pointer_rtx
+
/* Define this macro if functions should assume that stack space has been
allocated for arguments even when their values are passed in registers.
diff -up --recursive --new-file gcc-4.0.0.macro/gcc/config/mips/mips.h gcc-4.0.0/gcc/config/mips/mips.h
--- gcc-4.0.0.macro/gcc/config/mips/mips.h 2005-04-15 07:00:18.000000000 +0000
+++ gcc-4.0.0/gcc/config/mips/mips.h 2005-06-03 00:26:34.000000000 +0000
@@ -2149,6 +2149,10 @@ extern enum reg_class mips_char_to_class
/* The argument pointer always points to the first argument. */
#define FIRST_PARM_OFFSET(FNDECL) 0
+/* Defining this macro makes __builtin_frame_address(0) work
+ with -fomit-frame-pointer. */
+#define INITIAL_FRAME_ADDRESS_RTX frame_pointer_rtx
+
/* o32 and o64 reserve stack space for all argument registers. */
#define REG_PARM_STACK_SPACE(FNDECL) \
(TARGET_OLDABI \
diff -up --recursive --new-file gcc-4.0.0.macro/gcc/testsuite/gcc.c-torture/execute/frame-address.c gcc-4.0.0/gcc/testsuite/gcc.c-torture/execute/frame-address.c
--- gcc-4.0.0.macro/gcc/testsuite/gcc.c-torture/execute/frame-address.c 1970-01-01 00:00:00.000000000 +0000
+++ gcc-4.0.0/gcc/testsuite/gcc.c-torture/execute/frame-address.c 2005-06-06 15:54:29.000000000 +0000
@@ -0,0 +1,42 @@
+int check_fa_work (const char *, const char *) __attribute__((noinline));
+int check_fa_mid (const char *) __attribute__((noinline));
+int check_fa (char *) __attribute__((noinline));
+int how_much (void) __attribute__((noinline));
+
+int check_fa_work (const char *c, const char *f)
+{
+ const char d = 0;
+
+ if (c >= &d)
+ return c >= f && f >= &d;
+ else
+ return c <= f && f <= &d;
+}
+
+int check_fa_mid (const char *c)
+{
+ const char *f = __builtin_frame_address (0);
+
+ return check_fa_work (c, f);
+}
+
+int check_fa (char *unused)
+{
+ const char c = 0;
+
+ return check_fa_mid (&c);
+}
+
+int how_much (void)
+{
+ return 8;
+}
+
+int main (void)
+{
+ char *unused = __builtin_alloca (how_much ());
+
+ if (!check_fa(unused))
+ abort();
+ return 0;
+}