[PATCH] libgcc regparm workaround

[Stefan Reinauer]

programs that are compiled with non-default regparm values are
miscompiled if they use libgcc.
This patch works around the problem for coreboot.

* drop libgcc from coreboot_apc.o, not needed.
* wrap libgcc calls into regparm(0) variants so that coreboot can be compiled
  with other regparm values

Signed-off-by: Stefan Reinauer

Index: src/config/Config.lb
===================================================================
--- src/config/Config.lb	(revision 4675)
+++ src/config/Config.lb	(working copy)
@@ -47,7 +47,7 @@

makerule coreboot_ram.o
    depends	"src/arch/$(CONFIG_ARCH)/lib/c_start.o $(DRIVER) coreboot.a $(LIBGCC_FILE_NAME)" 
-	action	"$(CC) $(DISTRO_LFLAGS) -nostdlib -r -o $@ src/arch/$(CONFIG_ARCH)/lib/c_start.o $(DRIVER) -Wl,-( coreboot.a $(LIBGCC_FILE_NAME) -Wl,-)"
+	action	"$(CC) $(DISTRO_LFLAGS) -nostdlib -r -o $@ src/arch/$(CONFIG_ARCH)/lib/c_start.o $(DRIVER) -Wl,--wrap,__divdi3 -Wl,--wrap,__udivdi3 -Wl,--wrap,__moddi3 -Wl,--wrap,__umoddi3 -Wl,-( coreboot.a $(LIBGCC_FILE_NAME) -Wl,-)"
 end

makerule coreboot_ram
@@ -89,7 +89,7 @@
    end

makerule coreboot_apc.o
-		depends "src/arch/$(CONFIG_ARCH)/lib/c_start.o coreboot_apc.a $(LIBGCC_FILE_NAME)"
+		depends "src/arch/$(CONFIG_ARCH)/lib/c_start.o coreboot_apc.a"
         	action  "$(CC) $(DISTRO_LFLAGS) -nostdlib -r -o $@ $^"
    end

Index: src/lib/Makefile.inc
===================================================================
--- src/lib/Makefile.inc	(revision 4675)
+++ src/lib/Makefile.inc	(working copy)
@@ -12,6 +12,7 @@
 obj-y += cbfs.o
 obj-y += lzma.o
 #obj-y += lzmadecode.o
+obj-y += gcc.o

initobj-y += uart8250.o
 initobj-y += memset.o
Index: src/lib/Config.lb
===================================================================
--- src/lib/Config.lb	(revision 4675)
+++ src/lib/Config.lb	(working copy)
@@ -17,6 +17,8 @@
 object fallback_boot.o
 object compute_ip_checksum.o
 object version.o
+object gcc.o
+
 # Force version.o to recompile every time
 makedefine .PHONY : version.o

Index: src/lib/gcc.c
===================================================================
--- src/lib/gcc.c	(revision 0)
+++ src/lib/gcc.c	(revision 0)
@@ -0,0 +1,34 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2009 coresystems GmbH
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA, 02110-1301 USA
+ */
+
+/* GCC's libgcc handling is quite broken. While the libgcc functions
+ * are always regparm(0) the code that calls them uses whatever the
+ * compiler call specifies. Therefore we need a wrapper around those
+ * functions.
+ */
+
+#define WRAP_LIBGCC_CALL(type, name) \
+	type __real_##name(type a, type b) __attribute__((regparm(0))); \
+	type __wrap_##name(type a, type b) { return __real_##name(a, b); }
+
+WRAP_LIBGCC_CALL(long long, __divdi3)
+WRAP_LIBGCC_CALL(unsigned long long, __udivdi3)
+WRAP_LIBGCC_CALL(long long, __moddi3)
+WRAP_LIBGCC_CALL(unsigned long long, __umoddi3)
+

[Carl-Daniel Hailfinger]

On 26.09.2009 00:03, Stefan Reinauer wrote:
> programs that are compiled with non-default regparm values are
> miscompiled if they use libgcc.
> This patch works around the problem for coreboot.
> drop libgcc from coreboot_apc.o, not needed.
> wrap libgcc calls into regparm(0) variants so that coreboot can be compiled
> with other regparm values
>
> Signed-off-by: Stefan Reinauer

I'm worried about any libgcc compiled with regparm!=0, but for now this
patch improves the situation.

Acked-by: Carl-Daniel Hailfinger

Regards,
Carl-Daniel

[Segher Boessenkool]

> I'm worried about any libgcc compiled with regparm!=0, but for now  
> this
> patch improves the situation.

libgcc uses the default calling convention.  The problem is that
the (implicit) calls to the routines in libgcc are done with the
-mregparm from the command line.

Segher

[Stefan Reinauer]

Segher Boessenkool wrote:
> > I'm worried about any libgcc compiled with regparm!=0, but for now this
> > patch improves the situation.
>
> libgcc uses the default calling convention.

Is there an attribute to say "this function uses the default calling
convention no matter what CFLAGS we have right now"?

[Segher Boessenkool]

> > > I'm worried about any libgcc compiled with regparm!=0, but for  
> > > now this
> > > patch improves the situation.
> >
> > libgcc uses the default calling convention.
>
> Is there an attribute to say "this function uses the default calling
> convention no matter what CFLAGS we have right now"?

Sure, and you're already using that: the default on i386 is regparm=0.

Segher

[Carl-Daniel Hailfinger]

On 27.09.2009 16:38, Segher Boessenkool wrote:
> > > > I'm worried about any libgcc compiled with regparm!=0, but for now
> > > > this
> > > > patch improves the situation.
> > >
> > > libgcc uses the default calling convention.
> >
> > Is there an attribute to say "this function uses the default calling
> > convention no matter what CFLAGS we have right now"?
>
> Sure, and you're already using that: the default on i386 is regparm=0.

What happens to libgcc if someone compiles gcc with regparm=3? Will
libgcc still be compiled with regparm=0? (Disclaimer: I'm not a Gentoo
user and won't try that.)

Regards,
Carl-Daniel

[Segher Boessenkool]

> What happens to libgcc if someone compiles gcc with regparm=3?

Nothing.  It doesn't matter how the compiler itself is compiled.
> Will libgcc still be compiled with regparm=0?

Yes.

Segher

[Stefan Reinauer]

Carl-Daniel Hailfinger wrote:
> On 26.09.2009 00:03, Stefan Reinauer wrote:
> > programs that are compiled with non-default regparm values are
> > miscompiled if they use libgcc.
> > This patch works around the problem for coreboot.
> > drop libgcc from coreboot_apc.o, not needed.
> > wrap libgcc calls into regparm(0) variants so that coreboot can be compiled
> > with other regparm values
> >
> > Signed-off-by: Stefan Reinauer
>
> I'm worried about any libgcc compiled with regparm!=0, but for now this
> patch improves the situation.

Is there any way around this? Using stdcall or something? We could query
the compiler flags used to build gcc, but I think as long as this works
for the "normal" gcc case we should be fine. If people do use odd tool
chains, they're on their own..

Stefan

[Segher Boessenkool]

> programs that are compiled with non-default regparm values are
> miscompiled if they use libgcc.
> This patch works around the problem for coreboot.

Please mention the GCC bugreport # in the source near every workaround;
that way, you can one day get rid of it.  This is PR41055.
> makerule coreboot_ram.o
>    depends	"src/arch/$(CONFIG_ARCH)/lib/c_start.o $(DRIVER)  
> coreboot.a $(LIBGCC_FILE_NAME)"
> action	"$(CC) $(DISTRO_LFLAGS) -nostdlib -r -o $@ src/arch/$
>
> (CONFIG_ARCH)/lib/c_start.o $(DRIVER) -Wl,-( coreboot.a $ 
> (LIBGCC_FILE_NAME) -Wl,-)"
> action	"$(CC) $(DISTRO_LFLAGS) -nostdlib -r -o $@ src/arch/$
>
> (CONFIG_ARCH)/lib/c_start.o $(DRIVER) -Wl,--wrap,__divdi3 -Wl,-- 
> wrap,__udivdi3 -Wl,--wrap,__moddi3 -Wl,--wrap,__umoddi3 -Wl,-\ 
> ( coreboot.a $(LIBGCC_FILE_NAME) -Wl,-)"
>  end

You don't need the -( -) here, FWIW.  libgcc does not require symbols  
from
coreboot.a ;-)

Segher

[Stefan Reinauer]

Segher Boessenkool wrote:
> > programs that are compiled with non-default regparm values are
> > miscompiled if they use libgcc.
> > This patch works around the problem for coreboot.
>
> Please mention the GCC bugreport # in the source near every workaround;
> that way, you can one day get rid of it.  This is PR41055.
> > makerule coreboot_ram.o
> >      depends    "src/arch/$(CONFIG_ARCH)/lib/c_start.o $(DRIVER)
> > coreboot.a $(LIBGCC_FILE_NAME)"
> > action    "$(CC) $(DISTRO_LFLAGS) -nostdlib -r -o $@
> >
> > src/arch/$(CONFIG_ARCH)/lib/c_start.o $(DRIVER) -Wl,-( coreboot.a
> > $(LIBGCC_FILE_NAME) -Wl,-)"
> > action    "$(CC) $(DISTRO_LFLAGS) -nostdlib -r -o $@
> >
> > src/arch/$(CONFIG_ARCH)/lib/c_start.o $(DRIVER) -Wl,--wrap,__divdi3
> > -Wl,--wrap,__udivdi3 -Wl,--wrap,__moddi3 -Wl,--wrap,__umoddi3 -Wl,-(
> > coreboot.a $(LIBGCC_FILE_NAME) -Wl,-)"
> >  end
>
> You don't need the -( -) here, FWIW.  libgcc does not require symbols
> from
> coreboot.a ;-)

Yes, actually it does. On powerpc libgcc uses external symbols that it
does not provide.

Stefan

[Segher Boessenkool]

> > You don't need the -( -) here, FWIW.  libgcc does not require symbols
> > from
> > coreboot.a ;-)
>
> Yes, actually it does. On powerpc libgcc uses external symbols that it
> does not provide.

Do you mean abort()?  Yeah, you need to provide that yourself, on all  
targets
(nothing special about PowerPC); the same is true for memcpy(), memset 
(),
memmove(), memcmp().  FWIW, any GCC-compiled code can require those, not
only code in libgcc.

Segher

[Stefan Reinauer]

Segher Boessenkool wrote:
> > > You don't need the -( -) here, FWIW.  libgcc does not require symbols
> > > from
> > > coreboot.a ;-)
> >
> > Yes, actually it does. On powerpc libgcc uses external symbols that it
> > does not provide.
>
> Do you mean abort()?

Obviously.
> Yeah, you need to provide that yourself, on all targets (nothing
> special about PowerPC);

See, that's why the -( -) is there. :-)

I read the libgcc source code, and only powerpc's libgcc is calling
abort(). So while theoretically others may, they don't.

I agree completely with you that gcc might change its behavior on other
platforms, which is exactly why the -( -) construct is not only used for
our powerpc port.
> the same is true for memcpy(), memset(),  memmove(), memcmp().  FWIW,
> any GCC-compiled code can require those, not
> only code in libgcc.

The -( -) construct is only needed for symbols used in libgcc, so
everything is fine. At the current point no mem* functions are used in
the libgcc functions we "use".
Even if we did, the construct would work for us without changes.

Stefan

[Segher Boessenkool]

> > > > You don't need the -( -) here, FWIW.  libgcc does not require  
> > > > symbols
> > > > from
> > > > coreboot.a ;-)
> > >
> > > Yes, actually it does. On powerpc libgcc uses external symbols  
> > > that it
> > > does not provide.
> >
> > Do you mean abort()?
>
> Obviously.

Obvious to you perhaps, but not to me, that's why I asked :-)
> > Yeah, you need to provide that yourself, on all targets (nothing
> > special about PowerPC);
>
> See, that's why the -( -) is there. :-)

Yes, sure, supposedly nothing else requires the object file where  
abort()
is defined.  I see.
> I read the libgcc source code, and only powerpc's libgcc is calling
> abort(). So while theoretically others may, they don't.

I think you read wrong then: every single target uses abort().  The
difference you are seeing could be that PowerPC uses it in the  
trampoline
code?
> I agree completely with you that gcc might change its behavior on  
> other
> platforms, which is exactly why the -( -) construct is not only  
> used for
> our powerpc port.
> > the same is true for memcpy(), memset(),  memmove(), memcmp().  FWIW,
> > any GCC-compiled code can require those, not
> > only code in libgcc.
>
> The -( -) construct is only needed for symbols used in libgcc,

Yes.
> so
> everything is fine. At the current point no mem* functions are used in
> the libgcc functions we "use".
> Even if we did, the construct would work for us without changes.

Yes.

Look, it just looked to me like some legacy thing that could use some
cleaning up.  Obviously it still has value.

Segher