* [U-Boot] [PATCH] [x86] Wrap small helper functions from libgcc to avoid an ABI mismatch @ 2011-11-08 9:27 Gabe Black 2011-11-08 13:33 ` Mike Frysinger 2011-11-08 22:34 ` [U-Boot] [PATCH v2] x86: " Gabe Black 0 siblings, 2 replies; 36+ messages in thread From: Gabe Black @ 2011-11-08 9:27 UTC (permalink / raw) To: u-boot When gcc compiles some 64 bit operations on a 32 bit machine, it generates calls to small functions instead of instructions which do the job directly. Those functions are defined in libgcc and transparently provide whatever functionality was necessary. Unfortunately, u-boot can be built with a non-standard ABI when libgcc isn't. When the two are linked together, very confusing bugs can crop up, for instance seemingly normal integer division or modulus getting the wrong answer or even raising a spurious divide by zero exception. This change barrows (steals) a technique and some code from coreboot which solves this problem by creating wrappers which translate the calling convention when calling the functions in libgcc. Unfortunately that means that these instructions which had already been turned into functions have even more overhead, but more importantly it makes them work properly. To find all of the functions that needed wrapping, u-boot was compiled without linking in libgcc. All the symbols the linker complained were undefined were presumed to be the symbols that are needed from libgcc. These were a subset of the symbols covered by the coreboot code, so it was used unmodified. Signed-off-by: Gabe Black <gabeblack@chromium.org> --- arch/x86/config.mk | 3 +++ arch/x86/lib/Makefile | 1 + arch/x86/lib/gcc.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 0 deletions(-) create mode 100644 arch/x86/lib/gcc.c diff --git a/arch/x86/config.mk b/arch/x86/config.mk index fe9083f..c38ac70 100644 --- a/arch/x86/config.mk +++ b/arch/x86/config.mk @@ -41,3 +41,6 @@ PLATFORM_RELFLAGS += -ffunction-sections -fvisibility=hidden PLATFORM_LDFLAGS += --emit-relocs -Bsymbolic -Bsymbolic-functions LDFLAGS_FINAL += --gc-sections -pie +LDFLAGS_FINAL += --wrap=__divdi3 --wrap=__udivdi3 +LDFLAGS_FINAL += --wrap=__moddi3 --wrap=__umoddi3 +LDSCRIPT := $(SRCTREE)/$(CPUDIR)/u-boot.lds diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile index 71e94f7..210dbbe 100644 --- a/arch/x86/lib/Makefile +++ b/arch/x86/lib/Makefile @@ -32,6 +32,7 @@ SOBJS-y += realmode_switch.o COBJS-y += bios_setup.o COBJS-y += board.o COBJS-y += bootm.o +COBJS-y += gcc.o COBJS-y += interrupts.o COBJS-$(CONFIG_SYS_PCAT_INTERRUPTS) += pcat_interrupts.o COBJS-$(CONFIG_SYS_GENERIC_TIMER) += pcat_timer.o diff --git a/arch/x86/lib/gcc.c b/arch/x86/lib/gcc.c new file mode 100644 index 0000000..b11ea5f --- /dev/null +++ b/arch/x86/lib/gcc.c @@ -0,0 +1,38 @@ +/* + * 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 or later 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 + */ + +#ifdef __GNUC__ + +/* + * 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. See gcc bug PR41055 for more information. + */ +#define WRAP_LIBGCC_CALL(type, name) \ + type __real_##name(type a, type b) __attribute__((regparm(0))); \ + type __wrap_##name(type a, type b); \ + 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) + +#endif -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [U-Boot] [PATCH] [x86] Wrap small helper functions from libgcc to avoid an ABI mismatch 2011-11-08 9:27 [U-Boot] [PATCH] [x86] Wrap small helper functions from libgcc to avoid an ABI mismatch Gabe Black @ 2011-11-08 13:33 ` Mike Frysinger 2011-11-08 22:27 ` Gabe Black 2011-11-08 22:34 ` [U-Boot] [PATCH v2] x86: " Gabe Black 1 sibling, 1 reply; 36+ messages in thread From: Mike Frysinger @ 2011-11-08 13:33 UTC (permalink / raw) To: u-boot On Tuesday 08 November 2011 04:27:44 Gabe Black wrote: > When gcc compiles some 64 bit operations on a 32 bit machine, it generates > calls to small functions instead of instructions which do the job directly. > Those functions are defined in libgcc and transparently provide whatever > functionality was necessary. Unfortunately, u-boot can be built with a > non-standard ABI when libgcc isn't. When the two are linked together, very > confusing bugs can crop up, for instance seemingly normal integer division > or modulus getting the wrong answer or even raising a spurious divide by > zero exception. might be good to explicitly mention that this is due to u-boot using -mregparm > --- a/arch/x86/config.mk > +++ b/arch/x86/config.mk > > +LDSCRIPT := $(SRCTREE)/$(CPUDIR)/u-boot.lds looks like some old code sneaked in. bad rebase ? -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20111108/ce4175e8/attachment.pgp ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [PATCH] [x86] Wrap small helper functions from libgcc to avoid an ABI mismatch 2011-11-08 13:33 ` Mike Frysinger @ 2011-11-08 22:27 ` Gabe Black 0 siblings, 0 replies; 36+ messages in thread From: Gabe Black @ 2011-11-08 22:27 UTC (permalink / raw) To: u-boot On Tue, Nov 8, 2011 at 5:33 AM, Mike Frysinger <vapier@gentoo.org> wrote: > On Tuesday 08 November 2011 04:27:44 Gabe Black wrote: > > When gcc compiles some 64 bit operations on a 32 bit machine, it > generates > > calls to small functions instead of instructions which do the job > directly. > > Those functions are defined in libgcc and transparently provide whatever > > functionality was necessary. Unfortunately, u-boot can be built with a > > non-standard ABI when libgcc isn't. When the two are linked together, > very > > confusing bugs can crop up, for instance seemingly normal integer > division > > or modulus getting the wrong answer or even raising a spurious divide by > > zero exception. > > might be good to explicitly mention that this is due to u-boot using > -mregparm > > > --- a/arch/x86/config.mk > > +++ b/arch/x86/config.mk > > > > +LDSCRIPT := $(SRCTREE)/$(CPUDIR)/u-boot.lds > > looks like some old code sneaked in. bad rebase ? > Yep, that's right. The original change doesn't have that line so it must have slipped in during a rebase. Gabe ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [PATCH v2] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch 2011-11-08 9:27 [U-Boot] [PATCH] [x86] Wrap small helper functions from libgcc to avoid an ABI mismatch Gabe Black 2011-11-08 13:33 ` Mike Frysinger @ 2011-11-08 22:34 ` Gabe Black 2011-11-08 23:14 ` Graeme Russ ` (2 more replies) 1 sibling, 3 replies; 36+ messages in thread From: Gabe Black @ 2011-11-08 22:34 UTC (permalink / raw) To: u-boot When gcc compiles some 64 bit operations on a 32 bit machine, it generates calls to small functions instead of instructions which do the job directly. Those functions are defined in libgcc and transparently provide whatever functionality was necessary. Unfortunately, u-boot can be built with a non-standard ABI when libgcc isn't. More specifically, u-boot uses -mregparm. When the u-boot and libgcc are linked together, very confusing bugs can crop up, for instance seemingly normal integer division or modulus getting the wrong answer or even raising a spurious divide by zero exception. This change barrows (steals) a technique and some code from coreboot which solves this problem by creating wrappers which translate the calling convention when calling the functions in libgcc. Unfortunately that means that these instructions which had already been turned into functions have even more overhead, but more importantly it makes them work properly. To find all of the functions that needed wrapping, u-boot was compiled without linking in libgcc. All the symbols the linker complained were undefined were presumed to be the symbols that are needed from libgcc. These were a subset of the symbols covered by the coreboot code, so it was used unmodified. Signed-off-by: Gabe Black <gabeblack@chromium.org> --- Changes in v2: - Change the [x86] tag to x86: - Mention -mregparm in the commit message. - Get rid of a stray line which snuck in during a rebase. arch/x86/config.mk | 2 ++ arch/x86/lib/Makefile | 1 + arch/x86/lib/gcc.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 0 deletions(-) create mode 100644 arch/x86/lib/gcc.c diff --git a/arch/x86/config.mk b/arch/x86/config.mk index fe9083f..995fa1e 100644 --- a/arch/x86/config.mk +++ b/arch/x86/config.mk @@ -41,3 +41,5 @@ PLATFORM_RELFLAGS += -ffunction-sections -fvisibility=hidden PLATFORM_LDFLAGS += --emit-relocs -Bsymbolic -Bsymbolic-functions LDFLAGS_FINAL += --gc-sections -pie +LDFLAGS_FINAL += --wrap=__divdi3 --wrap=__udivdi3 +LDFLAGS_FINAL += --wrap=__moddi3 --wrap=__umoddi3 diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile index 71e94f7..210dbbe 100644 --- a/arch/x86/lib/Makefile +++ b/arch/x86/lib/Makefile @@ -32,6 +32,7 @@ SOBJS-y += realmode_switch.o COBJS-y += bios_setup.o COBJS-y += board.o COBJS-y += bootm.o +COBJS-y += gcc.o COBJS-y += interrupts.o COBJS-$(CONFIG_SYS_PCAT_INTERRUPTS) += pcat_interrupts.o COBJS-$(CONFIG_SYS_GENERIC_TIMER) += pcat_timer.o diff --git a/arch/x86/lib/gcc.c b/arch/x86/lib/gcc.c new file mode 100644 index 0000000..b11ea5f --- /dev/null +++ b/arch/x86/lib/gcc.c @@ -0,0 +1,38 @@ +/* + * 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 or later 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 + */ + +#ifdef __GNUC__ + +/* + * 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. See gcc bug PR41055 for more information. + */ +#define WRAP_LIBGCC_CALL(type, name) \ + type __real_##name(type a, type b) __attribute__((regparm(0))); \ + type __wrap_##name(type a, type b); \ + 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) + +#endif -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [U-Boot] [PATCH v2] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch 2011-11-08 22:34 ` [U-Boot] [PATCH v2] x86: " Gabe Black @ 2011-11-08 23:14 ` Graeme Russ 2011-11-09 4:49 ` Mike Frysinger 2011-11-09 3:05 ` [U-Boot] [PATCH v2] " Graeme Russ 2011-11-09 10:32 ` [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage Graeme Russ 2 siblings, 1 reply; 36+ messages in thread From: Graeme Russ @ 2011-11-08 23:14 UTC (permalink / raw) To: u-boot Hi Gabe, On Wed, Nov 9, 2011 at 9:34 AM, Gabe Black <gabeblack@chromium.org> wrote: > When gcc compiles some 64 bit operations on a 32 bit machine, it generates > calls to small functions instead of instructions which do the job directly. > Those functions are defined in libgcc and transparently provide whatever > functionality was necessary. Unfortunately, u-boot can be built with a > non-standard ABI when libgcc isn't. More specifically, u-boot uses > -mregparm. When the u-boot and libgcc are linked together, very confusing > bugs can crop up, for instance seemingly normal integer division or modulus > getting the wrong answer or even raising a spurious divide by zero > exception. Hmmm, very interesting. I would think this would apply to _all_ libgcc functions, but somehow the issue is being avoided. I haven't had a good look at what libgcc functions are called by U-Boot - have you? > This change barrows (steals) a technique and some code from coreboot which s/barrows/borrows/ > solves this problem by creating wrappers which translate the calling > convention when calling the functions in libgcc. Unfortunately that means that > these instructions which had already been turned into functions have even more > overhead, but more importantly it makes them work properly. > > To find all of the functions that needed wrapping, u-boot was compiled without > linking in libgcc. All the symbols the linker complained were undefined were > presumed to be the symbols that are needed from libgcc. These were a subset of > the symbols covered by the coreboot code, so it was used unmodified. Ah, yes you have - Can you give me a list? I think to be pragmatic we should either wrap the all or go down the private libgcc path like ARM has done. Private lib functions would elliminate the overhead, but is this really such a problem anyway? Regards, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [PATCH v2] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch 2011-11-08 23:14 ` Graeme Russ @ 2011-11-09 4:49 ` Mike Frysinger 2011-11-09 3:57 ` Graeme Russ 0 siblings, 1 reply; 36+ messages in thread From: Mike Frysinger @ 2011-11-09 4:49 UTC (permalink / raw) To: u-boot On Tuesday 08 November 2011 18:14:46 Graeme Russ wrote: > Ah, yes you have - Can you give me a list? I think to be pragmatic we > should either wrap the all or go down the private libgcc path like ARM has > done. Private lib functions would elliminate the overhead, but is this > really such a problem anyway? it then becomes a sync issue ... updates to gcc's libgcc aren't reflected in u- boot automatically ... -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20111108/8392dba8/attachment.pgp ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [PATCH v2] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch 2011-11-09 4:49 ` Mike Frysinger @ 2011-11-09 3:57 ` Graeme Russ 2011-11-09 5:34 ` Mike Frysinger 0 siblings, 1 reply; 36+ messages in thread From: Graeme Russ @ 2011-11-09 3:57 UTC (permalink / raw) To: u-boot On Wed, Nov 9, 2011 at 3:49 PM, Mike Frysinger <vapier@gentoo.org> wrote: > On Tuesday 08 November 2011 18:14:46 Graeme Russ wrote: >> Ah, yes you have - Can you give me a list? I think to be pragmatic we >> should either wrap the all or go down the private libgcc path like ARM has >> done. Private lib functions would elliminate the overhead, but is this >> really such a problem anyway? > > it then becomes a sync issue ... updates to gcc's libgcc aren't reflected in u- > boot automatically ... Are those updates needed? We already have a fair chunk of libc which is not automatically sync'd and going by what is in arch/arm/lib, there are very few libgcc functions (far less than libc) and each of those are relatively trivial and unlikely to require updating. Also, I already know of issues compiling U-Boot on an x64 OS because of the 32/64 bit incompatibility of libgcc. I never encountered this because I only have a 32-bit build machine So the handful of libgcc functions are starting to become a disproportionate headache Regards, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [PATCH v2] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch 2011-11-09 3:57 ` Graeme Russ @ 2011-11-09 5:34 ` Mike Frysinger 2011-11-17 9:01 ` [U-Boot] [PATCH v3] " Gabe Black 0 siblings, 1 reply; 36+ messages in thread From: Mike Frysinger @ 2011-11-09 5:34 UTC (permalink / raw) To: u-boot On Tuesday 08 November 2011 22:57:31 Graeme Russ wrote: > On Wed, Nov 9, 2011 at 3:49 PM, Mike Frysinger wrote: > > On Tuesday 08 November 2011 18:14:46 Graeme Russ wrote: > >> Ah, yes you have - Can you give me a list? I think to be pragmatic we > >> should either wrap the all or go down the private libgcc path like ARM > >> has done. Private lib functions would elliminate the overhead, but is > >> this really such a problem anyway? > > > > it then becomes a sync issue ... updates to gcc's libgcc aren't reflected > > in u- boot automatically ... > > Are those updates needed? you mean bug/math fixes and optimizations ? i would think so. > We already have a fair chunk of libc which is not automatically sync'd we have very very little of glibc ... really only a few optimized string/memory funcs, and those aren't glibc specific. the only other C lib type stuff is taken from Linux, or zlib, or dlmalloc, or other locations that aren't possible to link against. libgcc is a bit unique in this regard. > and going by what is in arch/arm/lib, > there are very few libgcc functions (far less than libc) and each of > those are relatively trivial and unlikely to require updating. it depends on the architecture. if the core ISA doesn't change, then the math funcs won't change much. i'm not terribly familiar with the gcc x86 internals to say how much we actually rely on libgcc.a considering most of the fun stuff is real hardware insns. unlike the normal embedded risc arches which need to implement more complicated funcs with many insns. > Also, I already know of issues compiling U-Boot on an x64 OS because > of the 32/64 bit incompatibility of libgcc. I never encountered this > because I only have a 32-bit build machine yes, this would be an issue, although most people on x86-64 have multilib toolchains, so it'd work anyways. maybe the x86 config.mk should be automatically adding -m32 when available. > So the handful of libgcc functions are starting to become a > disproportionate headache seems fairly low to me, but i'm not the x86 maintainer. i'm not seeing these funcs in Linux' arch/x86/ tree though ... maybe there's a better solution ? -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20111109/21da441d/attachment.pgp ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [PATCH v3] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch 2011-11-09 5:34 ` Mike Frysinger @ 2011-11-17 9:01 ` Gabe Black 2011-11-17 9:13 ` Gabe Black 2011-11-30 11:03 ` Graeme Russ 0 siblings, 2 replies; 36+ messages in thread From: Gabe Black @ 2011-11-17 9:01 UTC (permalink / raw) To: u-boot When gcc compiles some 64 bit operations on a 32 bit machine, it generates calls to small functions instead of instructions which do the job directly. Those functions are defined in libgcc and transparently provide whatever functionality was necessary. Unfortunately, u-boot can be built with a non-standard ABI when libgcc isn't. More specifically, u-boot uses -mregparm. When the u-boot and libgcc are linked together, very confusing bugs can crop up, for instance seemingly normal integer division or modulus getting the wrong answer or even raising a spurious divide by zero exception. This change borrows (steals) a technique and some code from coreboot which solves this problem by creating wrappers which translate the calling convention when calling the functions in libgcc. Unfortunately that means that these instructions which had already been turned into functions have even more overhead, but more importantly it makes them work properly. To find all of the functions that needed wrapping, u-boot was compiled without linking in libgcc. All the symbols the linker complained were undefined were presumed to be the symbols that are needed from libgcc. These were a subset of the symbols covered by the coreboot code, so it was used unmodified. To prevent symbols which are provided by libgcc but not currently wrapped (or even known about) from being silently linked against by code generated by libgcc, a new copy of libgcc is created where all the symbols are prefixed with __normal_. Without being purposefully wrapped, these symbols will cause linker errors instead of silently introducing very subtle, confusing bugs. Another approach would be to whitelist symbols from libgcc and strip out all the others. The problem with this approach is that it requires the white listed symbols to be specified three times, once for objcopy, once so the linker inserts the wrapped, and once to generate the wrapper itself, while this implementation needs it to be listed only twice. There isn't much tangible difference in what each approach produces, so this one was preferred. Signed-off-by: Gabe Black <gabeblack@chromium.org> --- Changes in v2: - Change the [x86] tag to x86: - Mention -mregparm in the commit message. - Get rid of a stray line which snuck in during a rebase. Changes in v3: - Prevent symbols from libgcc which aren't wrapped from getting silently picked up by the linker. arch/x86/config.mk | 7 +++++++ arch/x86/lib/Makefile | 6 ++++++ arch/x86/lib/gcc.c | 38 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 0 deletions(-) create mode 100644 arch/x86/lib/gcc.c diff --git a/arch/x86/config.mk b/arch/x86/config.mk index fe9083f..23cacff 100644 --- a/arch/x86/config.mk +++ b/arch/x86/config.mk @@ -41,3 +41,10 @@ PLATFORM_RELFLAGS += -ffunction-sections -fvisibility=hidden PLATFORM_LDFLAGS += --emit-relocs -Bsymbolic -Bsymbolic-functions LDFLAGS_FINAL += --gc-sections -pie +LDFLAGS_FINAL += --wrap=__divdi3 --wrap=__udivdi3 +LDFLAGS_FINAL += --wrap=__moddi3 --wrap=__umoddi3 + +NORMAL_LIBGCC = $(shell $(CC) $(CFLAGS) -print-libgcc-file-name) +PREFIXED_LIBGCC = $(OBJTREE)/arch/$(ARCH)/lib/$(shell basename $(NORMAL_LIBGCC)) + +export USE_PRIVATE_LIBGCC=$(shell dirname $(PREFIXED_LIBGCC)) diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile index eb5fa10..16db73f 100644 --- a/arch/x86/lib/Makefile +++ b/arch/x86/lib/Makefile @@ -32,6 +32,7 @@ SOBJS-$(CONFIG_SYS_X86_REALMODE) += realmode_switch.o COBJS-$(CONFIG_SYS_PC_BIOS) += bios_setup.o COBJS-y += board.o COBJS-y += bootm.o +COBJS-y += gcc.o COBJS-y += interrupts.o COBJS-$(CONFIG_SYS_PCAT_INTERRUPTS) += pcat_interrupts.o COBJS-$(CONFIG_SYS_GENERIC_TIMER) += pcat_timer.o @@ -49,6 +50,11 @@ OBJS := $(addprefix $(obj),$(SOBJS-y) $(COBJS-y)) $(LIB): $(obj).depend $(OBJS) $(call cmd_link_o_target, $(OBJS)) +$(PREFIXED_LIBGCC): $(NORMAL_LIBGCC) + $(OBJCOPY) $< $@ --prefix-symbols=__normal_ + +$(LIB): $(PREFIXED_LIBGCC) + ######################################################################### # defines $(obj).depend target diff --git a/arch/x86/lib/gcc.c b/arch/x86/lib/gcc.c new file mode 100644 index 0000000..4043431 --- /dev/null +++ b/arch/x86/lib/gcc.c @@ -0,0 +1,38 @@ +/* + * 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 or later 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 + */ + +#ifdef __GNUC__ + +/* + * 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. See gcc bug PR41055 for more information. + */ +#define WRAP_LIBGCC_CALL(type, name) \ + type __normal_##name(type a, type b) __attribute__((regparm(0))); \ + type __wrap_##name(type a, type b); \ + type __wrap_##name(type a, type b) { return __normal_##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) + +#endif -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [U-Boot] [PATCH v3] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch 2011-11-17 9:01 ` [U-Boot] [PATCH v3] " Gabe Black @ 2011-11-17 9:13 ` Gabe Black 2011-11-30 11:03 ` Graeme Russ 1 sibling, 0 replies; 36+ messages in thread From: Gabe Black @ 2011-11-17 9:13 UTC (permalink / raw) To: u-boot On Thu, Nov 17, 2011 at 1:01 AM, Gabe Black <gabeblack@chromium.org> wrote: > When gcc compiles some 64 bit operations on a 32 bit machine, it generates > calls to small functions instead of instructions which do the job directly. > Those functions are defined in libgcc and transparently provide whatever > functionality was necessary. Unfortunately, u-boot can be built with a > non-standard ABI when libgcc isn't. More specifically, u-boot uses > -mregparm. When the u-boot and libgcc are linked together, very confusing > bugs can crop up, for instance seemingly normal integer division or modulus > getting the wrong answer or even raising a spurious divide by zero > exception. > > This change borrows (steals) a technique and some code from coreboot which > solves this problem by creating wrappers which translate the calling > convention when calling the functions in libgcc. Unfortunately that means > that these instructions which had already been turned into functions have > even more overhead, but more importantly it makes them work properly. > > To find all of the functions that needed wrapping, u-boot was compiled > without linking in libgcc. All the symbols the linker complained were > undefined were presumed to be the symbols that are needed from libgcc. > These were a subset of the symbols covered by the coreboot code, so it was > used unmodified. > > To prevent symbols which are provided by libgcc but not currently wrapped > (or even known about) from being silently linked against by code generated > by libgcc, a new copy of libgcc is created where all the symbols are > prefixed with __normal_. Without being purposefully wrapped, these symbols > will cause linker errors instead of silently introducing very subtle, > confusing bugs. > > Another approach would be to whitelist symbols from libgcc and strip out > all the others. The problem with this approach is that it requires the > white listed symbols to be specified three times, once for objcopy, once so > the linker inserts the wrapped, and once to generate the wrapper itself, > while this implementation needs it to be listed only twice. There isn't > much tangible difference in what each approach produces, so this one was > preferred. > > Signed-off-by: Gabe Black <gabeblack@chromium.org> > --- > Changes in v2: > - Change the [x86] tag to x86: > - Mention -mregparm in the commit message. > - Get rid of a stray line which snuck in during a rebase. > > Changes in v3: > - Prevent symbols from libgcc which aren't wrapped from getting silently > picked up by the linker. > > arch/x86/config.mk | 7 +++++++ > arch/x86/lib/Makefile | 6 ++++++ > arch/x86/lib/gcc.c | 38 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 51 insertions(+), 0 deletions(-) > create mode 100644 arch/x86/lib/gcc.c > > diff --git a/arch/x86/config.mk b/arch/x86/config.mk > index fe9083f..23cacff 100644 > --- a/arch/x86/config.mk > +++ b/arch/x86/config.mk > @@ -41,3 +41,10 @@ PLATFORM_RELFLAGS += -ffunction-sections > -fvisibility=hidden > PLATFORM_LDFLAGS += --emit-relocs -Bsymbolic -Bsymbolic-functions > > LDFLAGS_FINAL += --gc-sections -pie > +LDFLAGS_FINAL += --wrap=__divdi3 --wrap=__udivdi3 > +LDFLAGS_FINAL += --wrap=__moddi3 --wrap=__umoddi3 > + > +NORMAL_LIBGCC = $(shell $(CC) $(CFLAGS) -print-libgcc-file-name) > +PREFIXED_LIBGCC = $(OBJTREE)/arch/$(ARCH)/lib/$(shell basename > $(NORMAL_LIBGCC)) > + > +export USE_PRIVATE_LIBGCC=$(shell dirname $(PREFIXED_LIBGCC)) > diff --git a/arch/x86/lib/Makefile b/arch/x86/lib/Makefile > index eb5fa10..16db73f 100644 > --- a/arch/x86/lib/Makefile > +++ b/arch/x86/lib/Makefile > @@ -32,6 +32,7 @@ SOBJS-$(CONFIG_SYS_X86_REALMODE) += > realmode_switch.o > COBJS-$(CONFIG_SYS_PC_BIOS) += bios_setup.o > COBJS-y += board.o > COBJS-y += bootm.o > +COBJS-y += gcc.o > COBJS-y += interrupts.o > COBJS-$(CONFIG_SYS_PCAT_INTERRUPTS) += pcat_interrupts.o > COBJS-$(CONFIG_SYS_GENERIC_TIMER) += pcat_timer.o > @@ -49,6 +50,11 @@ OBJS := $(addprefix $(obj),$(SOBJS-y) $(COBJS-y)) > $(LIB): $(obj).depend $(OBJS) > $(call cmd_link_o_target, $(OBJS)) > > +$(PREFIXED_LIBGCC): $(NORMAL_LIBGCC) > + $(OBJCOPY) $< $@ --prefix-symbols=__normal_ > + > +$(LIB): $(PREFIXED_LIBGCC) > + > ######################################################################### > > # defines $(obj).depend target > diff --git a/arch/x86/lib/gcc.c b/arch/x86/lib/gcc.c > new file mode 100644 > index 0000000..4043431 > --- /dev/null > +++ b/arch/x86/lib/gcc.c > @@ -0,0 +1,38 @@ > +/* > + * 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 or later 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 > + */ > + > +#ifdef __GNUC__ > + > +/* > + * 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. See gcc bug PR41055 for more information. > + */ > +#define WRAP_LIBGCC_CALL(type, name) \ > + type __normal_##name(type a, type b) __attribute__((regparm(0))); \ > + type __wrap_##name(type a, type b); \ > + type __wrap_##name(type a, type b) { return __normal_##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) > + > +#endif > -- > 1.7.3.1 > > The patch I just sent out is not 100% checkpatch clean (1 error, 1 warning), but the error is because it misidentifies the macro in gcc.c as a collection of statements instead of function prototypes/definitions and insists they're wrapped in a do {} while loop. That wouldn't be possible, so I'm sending this out as is. There may also be small bits of review feedback I haven't implemented (the name of gcc.c?) because I wasn't able to find the email the feedback was in. Please send them again and I'll spin another version as necessary. This has been tested in our tree for our build target and upstream for the eNET target, both in our environment. It wouldn't be a bad idea to test it again in a different environment to be sure I'm not just getting lucky somehow. Gabe ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [PATCH v3] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch 2011-11-17 9:01 ` [U-Boot] [PATCH v3] " Gabe Black 2011-11-17 9:13 ` Gabe Black @ 2011-11-30 11:03 ` Graeme Russ 1 sibling, 0 replies; 36+ messages in thread From: Graeme Russ @ 2011-11-30 11:03 UTC (permalink / raw) To: u-boot Hi Gabe, On 17/11/11 20:01, Gabe Black wrote: > When gcc compiles some 64 bit operations on a 32 bit machine, it generates > calls to small functions instead of instructions which do the job directly. > Those functions are defined in libgcc and transparently provide whatever > functionality was necessary. Unfortunately, u-boot can be built with a > non-standard ABI when libgcc isn't. More specifically, u-boot uses > -mregparm. When the u-boot and libgcc are linked together, very confusing > bugs can crop up, for instance seemingly normal integer division or modulus > getting the wrong answer or even raising a spurious divide by zero > exception. > > This change borrows (steals) a technique and some code from coreboot which > solves this problem by creating wrappers which translate the calling > convention when calling the functions in libgcc. Unfortunately that means > that these instructions which had already been turned into functions have > even more overhead, but more importantly it makes them work properly. > > To find all of the functions that needed wrapping, u-boot was compiled > without linking in libgcc. All the symbols the linker complained were > undefined were presumed to be the symbols that are needed from libgcc. > These were a subset of the symbols covered by the coreboot code, so it was > used unmodified. > > To prevent symbols which are provided by libgcc but not currently wrapped > (or even known about) from being silently linked against by code generated > by libgcc, a new copy of libgcc is created where all the symbols are > prefixed with __normal_. Without being purposefully wrapped, these symbols > will cause linker errors instead of silently introducing very subtle, > confusing bugs. > > Another approach would be to whitelist symbols from libgcc and strip out > all the others. The problem with this approach is that it requires the > white listed symbols to be specified three times, once for objcopy, once so > the linker inserts the wrapped, and once to generate the wrapper itself, > while this implementation needs it to be listed only twice. There isn't > much tangible difference in what each approach produces, so this one was > preferred. > > Signed-off-by: Gabe Black <gabeblack@chromium.org> > --- > Changes in v2: > - Change the [x86] tag to x86: > - Mention -mregparm in the commit message. > - Get rid of a stray line which snuck in during a rebase. > > Changes in v3: > - Prevent symbols from libgcc which aren't wrapped from getting silently > picked up by the linker. > > arch/x86/config.mk | 7 +++++++ > arch/x86/lib/Makefile | 6 ++++++ > arch/x86/lib/gcc.c | 38 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 51 insertions(+), 0 deletions(-) > create mode 100644 arch/x86/lib/gcc.c Applied to u-boot-x86/master Thanks, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [PATCH v2] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch 2011-11-08 22:34 ` [U-Boot] [PATCH v2] x86: " Gabe Black 2011-11-08 23:14 ` Graeme Russ @ 2011-11-09 3:05 ` Graeme Russ 2011-11-09 10:32 ` [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage Graeme Russ 2 siblings, 0 replies; 36+ messages in thread From: Graeme Russ @ 2011-11-09 3:05 UTC (permalink / raw) To: u-boot Hi Gabe, On Wed, Nov 9, 2011 at 9:34 AM, Gabe Black <gabeblack@chromium.org> wrote: > When gcc compiles some 64 bit operations on a 32 bit machine, it generates > calls to small functions instead of instructions which do the job directly. > Those functions are defined in libgcc and transparently provide whatever > functionality was necessary. Unfortunately, u-boot can be built with a > non-standard ABI when libgcc isn't. More specifically, u-boot uses > -mregparm. When the u-boot and libgcc are linked together, very confusing > bugs can crop up, for instance seemingly normal integer division or modulus > getting the wrong answer or even raising a spurious divide by zero > exception. > > This change barrows (steals) a technique and some code from coreboot which > solves this problem by creating wrappers which translate the calling > convention when calling the functions in libgcc. Unfortunately that means that > these instructions which had already been turned into functions have even more > overhead, but more importantly it makes them work properly. > > To find all of the functions that needed wrapping, u-boot was compiled without > linking in libgcc. All the symbols the linker complained were undefined were > presumed to be the symbols that are needed from libgcc. These were a subset of > the symbols covered by the coreboot code, so it was used unmodified. > > Signed-off-by: Gabe Black <gabeblack@chromium.org> > --- > Changes in v2: > - Change the [x86] tag to x86: > - Mention -mregparm in the commit message. > - Get rid of a stray line which snuck in during a rebase. As mentioned in a reply to 'Import the glibc memset implementa?tion', I think I would prefer to either: - Investigate if regparm usage can be dropped so U-Boot is ABI compliant - If not, use USE_PRIVATE_LIBGCC and implement all required libgcc functions in U-Boot Either way, I don't want to have the possibility that someone uses another libgcc function, forgets to put a wrapper around it, and encounter 'weird' bugs Regards, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-08 22:34 ` [U-Boot] [PATCH v2] x86: " Gabe Black 2011-11-08 23:14 ` Graeme Russ 2011-11-09 3:05 ` [U-Boot] [PATCH v2] " Graeme Russ @ 2011-11-09 10:32 ` Graeme Russ 2011-11-09 17:12 ` Mike Frysinger 2011-11-16 23:00 ` Graeme Russ 2 siblings, 2 replies; 36+ messages in thread From: Graeme Russ @ 2011-11-09 10:32 UTC (permalink / raw) To: u-boot Hi Gabe, Can you please try this patch - If it solves your libgcc problem, I will add it to the misc cleanup patch Thanks, Graeme --- arch/x86/config.mk | 3 --- arch/x86/cpu/interrupts.c | 2 +- arch/x86/cpu/start.S | 5 ++--- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/arch/x86/config.mk b/arch/x86/config.mk index fe9083f..ec5f707 100644 --- a/arch/x86/config.mk +++ b/arch/x86/config.mk @@ -23,10 +23,7 @@ CONFIG_STANDALONE_LOAD_ADDR ?= 0x40000 -PLATFORM_CPPFLAGS += -fno-strict-aliasing PLATFORM_CPPFLAGS += -Wstrict-prototypes -PLATFORM_CPPFLAGS += -mregparm=3 -PLATFORM_CPPFLAGS += -fomit-frame-pointer PF_CPPFLAGS_X86 := $(call cc-option, -ffreestanding) \ $(call cc-option, -fno-toplevel-reorder, \ $(call cc-option, -fno-unit-at-a-time)) \ diff --git a/arch/x86/cpu/interrupts.c b/arch/x86/cpu/interrupts.c index e0958eb..a15d70a 100644 --- a/arch/x86/cpu/interrupts.c +++ b/arch/x86/cpu/interrupts.c @@ -249,7 +249,7 @@ int disable_interrupts(void) } /* IRQ Low-Level Service Routine */ -void irq_llsr(struct irq_regs *regs) +void __attribute__ ((regparm(1))) irq_llsr(struct irq_regs *regs) { /* * For detailed description of each exception, refer to: diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S index f87633b..119ca2d 100644 --- a/arch/x86/cpu/start.S +++ b/arch/x86/cpu/start.S @@ -84,9 +84,8 @@ car_init_ret: */ movl $CONFIG_SYS_INIT_SP_ADDR, %esp - /* Set parameter to board_init_f() to boot flags */ - xorl %eax, %eax - movw %bx, %ax + /* Set parameter to board_init_f() - Unused dummy value */ + pushl $0 /* Enter, U-boot! */ call board_init_f -- 1.7.5.2.317.g391b14 ^ permalink raw reply related [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-09 10:32 ` [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage Graeme Russ @ 2011-11-09 17:12 ` Mike Frysinger 2011-11-09 21:42 ` Graeme Russ 2011-11-16 23:00 ` Graeme Russ 1 sibling, 1 reply; 36+ messages in thread From: Mike Frysinger @ 2011-11-09 17:12 UTC (permalink / raw) To: u-boot On Wednesday 09 November 2011 05:32:59 Graeme Russ wrote: > --- a/arch/x86/config.mk > +++ b/arch/x86/config.mk > > -PLATFORM_CPPFLAGS += -mregparm=3 > -PLATFORM_CPPFLAGS += -fomit-frame-pointer this sounds like you're throwing the baby out with the bath water. doesn't this severely affect the generated code ? -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20111109/e693844b/attachment.pgp ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-09 17:12 ` Mike Frysinger @ 2011-11-09 21:42 ` Graeme Russ 2011-11-10 4:13 ` Mike Frysinger 0 siblings, 1 reply; 36+ messages in thread From: Graeme Russ @ 2011-11-09 21:42 UTC (permalink / raw) To: u-boot Hi Mike, On Thu, Nov 10, 2011 at 4:12 AM, Mike Frysinger <vapier@gentoo.org> wrote: > On Wednesday 09 November 2011 05:32:59 Graeme Russ wrote: >> --- a/arch/x86/config.mk >> +++ b/arch/x86/config.mk >> >> -PLATFORM_CPPFLAGS += -mregparm=3 >> -PLATFORM_CPPFLAGS += -fomit-frame-pointer > > this sounds like you're throwing the baby out with the bath water. ?doesn't > this severely affect the generated code ? No - omit-frame-pointer is enabled by -O2 and also: http://gcc.gnu.org/gcc-4.6/changes.html "The default setting (when not optimizing for size) for 32-bit GNU/Linux and Darwin x86 targets has been changed to -fomit-frame-pointer. The default can be reverted to -fno-omit-frame-pointer by configuring GCC with the --enable-frame-pointer configure option." So the flag is a do-nothing Regards, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-09 21:42 ` Graeme Russ @ 2011-11-10 4:13 ` Mike Frysinger 2011-11-10 4:22 ` Graeme Russ 0 siblings, 1 reply; 36+ messages in thread From: Mike Frysinger @ 2011-11-10 4:13 UTC (permalink / raw) To: u-boot On Wednesday 09 November 2011 16:42:51 Graeme Russ wrote: > On Thu, Nov 10, 2011 at 4:12 AM, Mike Frysinger wrote: > > On Wednesday 09 November 2011 05:32:59 Graeme Russ wrote: > >> --- a/arch/x86/config.mk > >> +++ b/arch/x86/config.mk > >> > >> -PLATFORM_CPPFLAGS += -mregparm=3 > >> -PLATFORM_CPPFLAGS += -fomit-frame-pointer > > > > this sounds like you're throwing the baby out with the bath water. > > doesn't this severely affect the generated code ? > > No - omit-frame-pointer is enabled by -O2 and also: > > http://gcc.gnu.org/gcc-4.6/changes.html > > "The default setting (when not optimizing for size) for 32-bit > GNU/Linux and Darwin x86 targets has been changed to > -fomit-frame-pointer. The default can be reverted to > -fno-omit-frame-pointer by configuring GCC with the > --enable-frame-pointer configure option." > > So the flag is a do-nothing ok, except: - we build u-boot with -Os and not -O2 - i'd say most people aren't using gcc-4.6 i was referring also to throwing away -mregparm=3 ... -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20111109/2aae86ce/attachment.pgp ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-10 4:13 ` Mike Frysinger @ 2011-11-10 4:22 ` Graeme Russ 2011-11-10 5:10 ` Graeme Russ 2011-11-10 17:15 ` Mike Frysinger 0 siblings, 2 replies; 36+ messages in thread From: Graeme Russ @ 2011-11-10 4:22 UTC (permalink / raw) To: u-boot Hi Mike, On Thu, Nov 10, 2011 at 3:13 PM, Mike Frysinger <vapier@gentoo.org> wrote: > On Wednesday 09 November 2011 16:42:51 Graeme Russ wrote: >> On Thu, Nov 10, 2011 at 4:12 AM, Mike Frysinger wrote: >> > On Wednesday 09 November 2011 05:32:59 Graeme Russ wrote: >> >> --- a/arch/x86/config.mk >> >> +++ b/arch/x86/config.mk >> >> >> >> -PLATFORM_CPPFLAGS += -mregparm=3 >> >> -PLATFORM_CPPFLAGS += -fomit-frame-pointer >> > >> > this sounds like you're throwing the baby out with the bath water. >> > ?doesn't this severely affect the generated code ? >> >> No - omit-frame-pointer is enabled by -O2 and also: >> >> http://gcc.gnu.org/gcc-4.6/changes.html >> >> "The default setting (when not optimizing for size) for 32-bit >> GNU/Linux and Darwin x86 targets has been changed to >> -fomit-frame-pointer. The default can be reverted to >> -fno-omit-frame-pointer by configuring GCC with the >> --enable-frame-pointer configure option." >> >> So the flag is a do-nothing > > ok, except: > ?- we build u-boot with -Os and not -O2 > ?- i'd say most people aren't using gcc-4.6 Ah, OK then I'll leave it in - It was only a cruft reduction plan anyway :) > i was referring also to throwing away -mregparm=3 ... Yes, it does effect the code - It makes it ABI compliant like everyone else (except ARM) :) I expect a code size increase (have not measured it yet) As I've stated, I really do not want arbitrary wrapper functions where it is not obvious that they need to be updated if new code uses previously unused (and unwrapped) libgcc functions (in particular if there are new libgcc functions in the future which we can't wrap todday anyway) Option a) is to remove regparm=3 Option b) is to use private libgcc Option c) is to use wrappers If this patch works, I'll look at the code impact and we can discuss which option we take :) Regards, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-10 4:22 ` Graeme Russ @ 2011-11-10 5:10 ` Graeme Russ 2011-11-10 17:15 ` Mike Frysinger 1 sibling, 0 replies; 36+ messages in thread From: Graeme Russ @ 2011-11-10 5:10 UTC (permalink / raw) To: u-boot Hi Mike, On Thu, Nov 10, 2011 at 3:22 PM, Graeme Russ <graeme.russ@gmail.com> wrote: > Hi Mike, > > On Thu, Nov 10, 2011 at 3:13 PM, Mike Frysinger <vapier@gentoo.org> wrote: >> On Wednesday 09 November 2011 16:42:51 Graeme Russ wrote: >>> On Thu, Nov 10, 2011 at 4:12 AM, Mike Frysinger wrote: >>> > On Wednesday 09 November 2011 05:32:59 Graeme Russ wrote: >>> >> --- a/arch/x86/config.mk >>> >> +++ b/arch/x86/config.mk >>> >> >>> >> -PLATFORM_CPPFLAGS += -mregparm=3 >>> >> -PLATFORM_CPPFLAGS += -fomit-frame-pointer >>> > >>> > this sounds like you're throwing the baby out with the bath water. >>> > ?doesn't this severely affect the generated code ? >>> >>> No - omit-frame-pointer is enabled by -O2 and also: >>> >>> http://gcc.gnu.org/gcc-4.6/changes.html >>> >>> "The default setting (when not optimizing for size) for 32-bit >>> GNU/Linux and Darwin x86 targets has been changed to >>> -fomit-frame-pointer. The default can be reverted to >>> -fno-omit-frame-pointer by configuring GCC with the >>> --enable-frame-pointer configure option." >>> >>> So the flag is a do-nothing >> >> ok, except: >> ?- we build u-boot with -Os and not -O2 >> ?- i'd say most people aren't using gcc-4.6 http://gcc.gnu.org/onlinedocs/gcc-4.6.2/gcc/Optimize-Options.html#Optimize-Options -fomit-frame-pointer ... Enabled at levels -O, -O2, -O3, -Os Thats goes back to at least gcc 4.3.x But I'll keep it in anyway, just in case someone wants to remove -Os (for testing/debugging etc) Regards, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-10 4:22 ` Graeme Russ 2011-11-10 5:10 ` Graeme Russ @ 2011-11-10 17:15 ` Mike Frysinger 2011-11-10 22:53 ` Graeme Russ 1 sibling, 1 reply; 36+ messages in thread From: Mike Frysinger @ 2011-11-10 17:15 UTC (permalink / raw) To: u-boot On Wednesday 09 November 2011 23:22:34 Graeme Russ wrote: > On Thu, Nov 10, 2011 at 3:13 PM, Mike Frysinger wrote: > > i was referring also to throwing away -mregparm=3 ... > > Yes, it does effect the code - It makes it ABI compliant like everyone > else (except ARM) :) I expect a code size increase (have not measured > it yet) ABI compliance only matters at the boundaries. since u-boot is largely self- contained, we shouldn't be afraid to break internal ABI. > As I've stated, I really do not want arbitrary wrapper functions where > it is not obvious that they need to be updated if new code uses > previously unused (and unwrapped) libgcc functions (in particular if > there are new libgcc functions in the future which we can't wrap > todday anyway) > > Option a) is to remove regparm=3 > Option b) is to use private libgcc > Option c) is to use wrappers > > If this patch works, I'll look at the code impact and we can discuss > which option we take :) for the record, i'm not against a private libgcc. it just seems to me that the wrapper approach proposed by Gabe has the best pro/con ratio. -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20111110/9683c94d/attachment.pgp ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-10 17:15 ` Mike Frysinger @ 2011-11-10 22:53 ` Graeme Russ 2011-11-11 0:23 ` Mike Frysinger 0 siblings, 1 reply; 36+ messages in thread From: Graeme Russ @ 2011-11-10 22:53 UTC (permalink / raw) To: u-boot Hi Mike, On Fri, Nov 11, 2011 at 4:15 AM, Mike Frysinger <vapier@gentoo.org> wrote: > On Wednesday 09 November 2011 23:22:34 Graeme Russ wrote: >> On Thu, Nov 10, 2011 at 3:13 PM, Mike Frysinger wrote: >> > i was referring also to throwing away -mregparm=3 ... >> >> Yes, it does effect the code - It makes it ABI compliant like everyone >> else (except ARM) :) I expect a code size increase (have not measured >> it yet) > > ABI compliance only matters at the boundaries. since u-boot is largely self- > contained, we shouldn't be afraid to break internal ABI. And I'm not afraid to do so >> As I've stated, I really do not want arbitrary wrapper functions where >> it is not obvious that they need to be updated if new code uses >> previously unused (and unwrapped) libgcc functions (in particular if >> there are new libgcc functions in the future which we can't wrap >> todday anyway) >> >> Option a) is to remove regparm=3 >> Option b) is to use private libgcc >> Option c) is to use wrappers >> >> If this patch works, I'll look at the code impact and we can discuss >> which option we take :) > > for the record, i'm not against a private libgcc. it just seems to me that > the wrapper approach proposed by Gabe has the best pro/con ratio. I'm sorry, but I must respectfully disagree... The biggest con with wrappers is that the proposed patch only wraps four functions. arch/arm/lib/ has private libgcc implementations for eight libgcc functions - I can only assume they are used somewhere. The kicker is that if anyone uses a libgcc function which is not one of the four already wrapped, and they do not realise this and fail to wrap them themselves, no warning will be given by the compiler or linker. Now that unwrapped function may be in a rarely executed code path (as evidenced by the fact that this bug has been dormant for a year now). So you could have in-the-wild version of U-Boot which start exhibiting strange behaviour and nobody knows why Another big(ish) con, for me, is that we already have a mechanism in place to resolve this (USE_PRIVATE_LIBGCC) - I don't see any benefit to add a second to the mix The final (trivially small) con is the overhead added to these calls Now if we use USE_PRIVATE_LIBGCC, unimplemented libgcc functions will result in link errors, so using an unimplemented libgcc will be obvious at build time - It may lead to a posting on the mailing list, but at least we won't have latent libgcc related bugs in-the-wild. Also, We use an existing mechanism and it is in keeping with --no-builtin. libgcc is really just a library of functions that are too large to implement as inline functions internally by gcc anyway: "GCC provides a low-level runtime library, libgcc.a or libgcc_s.so.1 on some platforms. GCC generates calls to routines in this library automatically, whenever it needs to perform some operation that is too complicated to emit inline code for." Now the downside has been raised regarding keeping the private libgcc functions in-sync with mainline libgcc - We are only talking about a handful of functions. There are no floating point or 'special' functions, so the list is wholly restricted to: http://gcc.gnu.org/onlinedocs/gccint/Integer-library-routines.html I haven't looked at them, but I doubt they are very big (going by the ARM implementations) and I doubt they change very often (probably not in years) The more I think about this, the more I feel to just use USE_PRIVATE_LIBGCC Regards, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-10 22:53 ` Graeme Russ @ 2011-11-11 0:23 ` Mike Frysinger 2011-11-11 1:23 ` Graeme Russ 0 siblings, 1 reply; 36+ messages in thread From: Mike Frysinger @ 2011-11-11 0:23 UTC (permalink / raw) To: u-boot On Thursday 10 November 2011 17:53:06 Graeme Russ wrote: > The biggest con with wrappers is that the proposed patch only wraps four > functions. arch/arm/lib/ has private libgcc implementations for eight > libgcc functions - I can only assume they are used somewhere. i don't think you can look across arches like that. arm provides a lot more libgcc funcs because it, like most RISC/embedded cpus, do not provide dedicated math insns in the ISA. or the number of insns is so large, that creating a dedicated library func and emitting a function call makes more sense than emitting them inline. x86 is a fairly "fat" ISA in that most math operations can be accomplished in single or a few insns, and is certainly better than emitting func calls to an external library. in fact, building the current eNET board (the only x86 board) shows that it doesn't use *any* calls from libgcc: make PLATFORM_LIBGCC= eNET -j4 > The kicker > is that if anyone uses a libgcc function which is not one of the four > already wrapped, and they do not realise this and fail to wrap them > themselves, no warning will be given by the compiler or linker. Now that > unwrapped function may be in a rarely executed code path (as evidenced by > the fact that this bug has been dormant for a year now). So you could have > in-the-wild version of U-Boot which start exhibiting strange behaviour and > nobody knows why yes, but the better question is whether those funcs should be called in the first place > The final (trivially small) con is the overhead added to these calls this con is insignificant when weighed against the alternatives: not using regparm anywhere. further, these funcs are rarely used, so you're talking about adding a minor amount of overhead to one or two call sites. > Now if we use USE_PRIVATE_LIBGCC, unimplemented libgcc functions will > result in link errors, so using an unimplemented libgcc will be obvious at > build time - It may lead to a posting on the mailing list, but at least we > won't have latent libgcc related bugs in-the-wild. perhaps x86 should be setting PLATFORM_LIBGCC to nothing all the time. the funcs Gabe wants to wrap are 64bit operations. u-boot should not be doing 64- bit operations. that's why we have do_div(). -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20111110/1ed27e65/attachment.pgp ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-11 0:23 ` Mike Frysinger @ 2011-11-11 1:23 ` Graeme Russ 2011-11-11 1:40 ` Mike Frysinger 0 siblings, 1 reply; 36+ messages in thread From: Graeme Russ @ 2011-11-11 1:23 UTC (permalink / raw) To: u-boot Hi Mike, On Fri, Nov 11, 2011 at 11:23 AM, Mike Frysinger <vapier@gentoo.org> wrote: > On Thursday 10 November 2011 17:53:06 Graeme Russ wrote: >> The biggest con with wrappers is that the proposed patch only wraps four >> functions. arch/arm/lib/ has private libgcc implementations for eight >> libgcc functions - I can only assume they are used somewhere. > > i don't think you can look across arches like that. arm provides a lot more > libgcc funcs because it, like most RISC/embedded cpus, do not provide > dedicated math insns in the ISA. or the number of insns is so large, that > creating a dedicated library func and emitting a function call makes more > sense than emitting them inline. x86 is a fairly "fat" ISA in that most math > operations can be accomplished in single or a few insns, and is certainly > better than emitting func calls to an external library. > > in fact, building the current eNET board (the only x86 board) shows that it > doesn't use *any* calls from libgcc: > make PLATFORM_LIBGCC= eNET -j4 Which supports my point - The issue was latent because there were no call sites > >> The kicker >> is that if anyone uses a libgcc function which is not one of the four >> already wrapped, and they do not realise this and fail to wrap them >> themselves, no warning will be given by the compiler or linker. Now that >> unwrapped function may be in a rarely executed code path (as evidenced by >> the fact that this bug has been dormant for a year now). So you could have >> in-the-wild version of U-Boot which start exhibiting strange behaviour and >> nobody knows why > > yes, but the better question is whether those funcs should be called in the > first place But we can't control that - especially if the code is not submitted to mainline. Then get people hitting the ML asking for help with non mainline code because they used a function they did not know they should not have used and they are seeing some random weird crash >> The final (trivially small) con is the overhead added to these calls > > this con is insignificant when weighed against the alternatives: not using > regparm anywhere. further, these funcs are rarely used, so you're talking > about adding a minor amount of overhead to one or two call sites. 100% agree >> Now if we use USE_PRIVATE_LIBGCC, unimplemented libgcc functions will >> result in link errors, so using an unimplemented libgcc will be obvious at >> build time - It may lead to a posting on the mailing list, but at least we >> won't have latent libgcc related bugs in-the-wild. > > perhaps x86 should be setting PLATFORM_LIBGCC to nothing all the time. the > funcs Gabe wants to wrap are 64bit operations. u-boot should not be doing 64- > bit operations. that's why we have do_div(). Remember that there was a lot of discussion regarding the timer API that pointed towards using 64-bit counters for all arches. We cannot take it as gospel that 64-bit operations will never be used in U-Boot. Some people may have a need for this as part of hardware init. Regards, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-11 1:23 ` Graeme Russ @ 2011-11-11 1:40 ` Mike Frysinger 2011-11-11 1:51 ` Graeme Russ 0 siblings, 1 reply; 36+ messages in thread From: Mike Frysinger @ 2011-11-11 1:40 UTC (permalink / raw) To: u-boot On Thursday 10 November 2011 20:23:49 Graeme Russ wrote: > On Fri, Nov 11, 2011 at 11:23 AM, Mike Frysinger wrote: > > On Thursday 10 November 2011 17:53:06 Graeme Russ wrote: > >> Now if we use USE_PRIVATE_LIBGCC, unimplemented libgcc functions will > >> result in link errors, so using an unimplemented libgcc will be obvious > >> at build time - It may lead to a posting on the mailing list, but at > >> least we won't have latent libgcc related bugs in-the-wild. > > > > perhaps x86 should be setting PLATFORM_LIBGCC to nothing all the time. > > the funcs Gabe wants to wrap are 64bit operations. u-boot should not be > > doing 64- bit operations. that's why we have do_div(). > > Remember that there was a lot of discussion regarding the timer API that > pointed towards using 64-bit counters for all arches. We cannot take it > as gospel that 64-bit operations will never be used in U-Boot. Some people > may have a need for this as part of hardware init. Linux has no problem doing 64bit timers without 64bit mul/div. i don't see how u-boot could possibly be more special than Linux. -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20111110/4ac00f1d/attachment.pgp ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-11 1:40 ` Mike Frysinger @ 2011-11-11 1:51 ` Graeme Russ 2011-11-11 1:55 ` Mike Frysinger 0 siblings, 1 reply; 36+ messages in thread From: Graeme Russ @ 2011-11-11 1:51 UTC (permalink / raw) To: u-boot Hi Mike, On Fri, Nov 11, 2011 at 12:40 PM, Mike Frysinger <vapier@gentoo.org> wrote: > On Thursday 10 November 2011 20:23:49 Graeme Russ wrote: >> On Fri, Nov 11, 2011 at 11:23 AM, Mike Frysinger wrote: >> > On Thursday 10 November 2011 17:53:06 Graeme Russ wrote: >> >> Now if we use USE_PRIVATE_LIBGCC, unimplemented libgcc functions will >> >> result in link errors, so using an unimplemented libgcc will be obvious >> >> at build time - It may lead to a posting on the mailing list, but at >> >> least we won't have latent libgcc related bugs in-the-wild. >> > >> > perhaps x86 should be setting PLATFORM_LIBGCC to nothing all the time. >> > the funcs Gabe wants to wrap are 64bit operations. u-boot should not be >> > doing 64- bit operations. that's why we have do_div(). >> >> Remember that there was a lot of discussion regarding the timer API that >> pointed towards using 64-bit counters for all arches. We cannot take it >> as gospel that 64-bit operations will never be used in U-Boot. Some people >> may have a need for this as part of hardware init. > > Linux has no problem doing 64bit timers without 64bit mul/div. i don't see > how u-boot could possibly be more special than Linux. A few questions (I am unfamiliar with the Linux build environment): a) Does Linux link to libgcc b) Does Linux use regparm c) If a & b are both yes, does Linux use wrappers for libgcc functions Regards, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-11 1:51 ` Graeme Russ @ 2011-11-11 1:55 ` Mike Frysinger 2011-11-11 1:59 ` Graeme Russ 2011-11-11 19:59 ` Scott Wood 0 siblings, 2 replies; 36+ messages in thread From: Mike Frysinger @ 2011-11-11 1:55 UTC (permalink / raw) To: u-boot On Thursday 10 November 2011 20:51:47 Graeme Russ wrote: > A few questions (I am unfamiliar with the Linux build environment): > > a) Does Linux link to libgcc no Linux port uses libgcc. they've always done the equivalent of PRIVATE_LIBGCC. but in the case of x86, i can't see them providing any libgcc funcs. so i don't think u-boot should either. > b) Does Linux use regparm yes, it uses -mregparm=3 -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20111110/e373047f/attachment.pgp ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-11 1:55 ` Mike Frysinger @ 2011-11-11 1:59 ` Graeme Russ 2011-11-11 2:10 ` Gabe Black 2011-11-11 2:44 ` Mike Frysinger 2011-11-11 19:59 ` Scott Wood 1 sibling, 2 replies; 36+ messages in thread From: Graeme Russ @ 2011-11-11 1:59 UTC (permalink / raw) To: u-boot Hi Mike, On Fri, Nov 11, 2011 at 12:55 PM, Mike Frysinger <vapier@gentoo.org> wrote: > On Thursday 10 November 2011 20:51:47 Graeme Russ wrote: >> A few questions (I am unfamiliar with the Linux build environment): >> >> a) Does Linux link to libgcc > > no Linux port uses libgcc. they've always done the equivalent of > PRIVATE_LIBGCC. but in the case of x86, i can't see them providing any libgcc > funcs. so i don't think u-boot should either. > >> b) Does Linux use regparm > > yes, it uses -mregparm=3 Well I think we have an answer - use PRIVATE_LIBGCC but do not implement any libgcc functions and treat link errors as coding errors. If for some bizarre reason we need to really, truly, honestly use a 64-bit libgcc function, we'll port it over then Regards, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-11 1:59 ` Graeme Russ @ 2011-11-11 2:10 ` Gabe Black 2011-11-11 2:22 ` Graeme Russ 2011-11-11 2:44 ` Mike Frysinger 1 sibling, 1 reply; 36+ messages in thread From: Gabe Black @ 2011-11-11 2:10 UTC (permalink / raw) To: u-boot On Thu, Nov 10, 2011 at 5:59 PM, Graeme Russ <graeme.russ@gmail.com> wrote: > Hi Mike, > > On Fri, Nov 11, 2011 at 12:55 PM, Mike Frysinger <vapier@gentoo.org> > wrote: > > On Thursday 10 November 2011 20:51:47 Graeme Russ wrote: > >> A few questions (I am unfamiliar with the Linux build environment): > >> > >> a) Does Linux link to libgcc > > > > no Linux port uses libgcc. they've always done the equivalent of > > PRIVATE_LIBGCC. but in the case of x86, i can't see them providing any > libgcc > > funcs. so i don't think u-boot should either. > > > >> b) Does Linux use regparm > > > > yes, it uses -mregparm=3 > > Well I think we have an answer - use PRIVATE_LIBGCC but do not implement > any libgcc functions and treat link errors as coding errors. If for some > bizarre reason we need to really, truly, honestly use a 64-bit libgcc > function, we'll port it over then > > Regards, > > Graeme > I haven't checked in on this thread in a little while, but I wanted to point out some things. First, I was originally planning to measure the performance difference with regparm turned off. I realized that would be quite annoying to actually do, though, since we have a number of extra libraries linked into u-boot and they would all have to be recompiled with different options. Then the things they link with would have to be recompiled, etc. Even if upstream U-Boot drops regparm, we may need to keep it just for that reason. Second, I think I have a solution that preserves regparm, keeps libgcc and gcc in sync, and also stops unwrapped functions slipping into u-boot. We can use this command: objcopy /path/to/your/libgcc/libgcc.a /somewhere/in/the/u-boot/build/libgcc.a --prefix-symbols=__real to create a libgcc that has all of its symbols prefixed with the string __real. Then *all* symbols are prepped for wrapping, regardless of if we use them now or even know about them. Only the symbols we've explicitly wrapped will be available. Note that I haven't actually implemented this yet, but it seems to me like it captures the positive aspects of all the alternatives. Gabe ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-11 2:10 ` Gabe Black @ 2011-11-11 2:22 ` Graeme Russ 2011-11-11 2:41 ` Gabe Black 0 siblings, 1 reply; 36+ messages in thread From: Graeme Russ @ 2011-11-11 2:22 UTC (permalink / raw) To: u-boot Hi Gabe, On Fri, Nov 11, 2011 at 1:10 PM, Gabe Black <gabeblack@google.com> wrote: > On Thu, Nov 10, 2011 at 5:59 PM, Graeme Russ <graeme.russ@gmail.com> wrote: >> >> Hi Mike, >> >> On Fri, Nov 11, 2011 at 12:55 PM, Mike Frysinger <vapier@gentoo.org> >> wrote: >> > On Thursday 10 November 2011 20:51:47 Graeme Russ wrote: >> >> A few questions (I am unfamiliar with the Linux build environment): >> >> >> >> a) Does Linux link to libgcc >> > >> > no Linux port uses libgcc. they've always done the equivalent of >> > PRIVATE_LIBGCC. but in the case of x86, i can't see them providing any >> > libgcc >> > funcs. so i don't think u-boot should either. >> > >> >> b) Does Linux use regparm >> > >> > yes, it uses -mregparm=3 >> >> Well I think we have an answer - use PRIVATE_LIBGCC but do not implement >> any libgcc functions and treat link errors as coding errors. If for some >> bizarre reason we need to really, truly, honestly use a 64-bit libgcc >> function, we'll port it over then >> >> Regards, >> >> Graeme > > I haven't checked in on this thread in a little while, but I wanted to point > out some things. First, I was originally planning to measure the performance > difference with regparm turned off. I realized that would be quite annoying > to actually do, though, since we have a number of extra libraries linked > into u-boot and they would all have to be recompiled with different options. I assume they are all GPL :) > Then the things they link with would have to be recompiled, etc. Even if > upstream U-Boot drops regparm, we may need to keep it just for that reason. Hmm, that raises an interresing point - by using a non-standard ABI we can run afoul of external libraries. Which raises the question - what external libraries do you need to link to? > Second, I think I have a solution that preserves regparm, keeps libgcc and > gcc in sync, and also stops unwrapped functions slipping into u-boot. We can > use this command: > objcopy /path/to/your/libgcc/libgcc.a > /somewhere/in/the/u-boot/build/libgcc.a --prefix-symbols=__real > to create a libgcc that has all of its symbols prefixed with the string > __real. Then *all* symbols are prepped for wrapping, regardless of if we use > them now or even know about them. Only the symbols we've explicitly wrapped > will be available. Note that I haven't actually implemented this yet, but it > seems to me like it captures the positive aspects of all the alternatives. Sorry, I find that rather un-aesthetic - We are only looking at a trivial handful of functions, and to copy and rename ALL the symbols in libgcc.a is rather overkill The other thing I like about using PRIVATE_LIBGCC is that on 64-bit build platforms, we don't need to worry about installing 32-bit libraries - All we need to do is add the -m32 option to tell gcc and ld to generate 32-bit code Regards, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-11 2:22 ` Graeme Russ @ 2011-11-11 2:41 ` Gabe Black 2011-11-11 4:49 ` Graeme Russ 0 siblings, 1 reply; 36+ messages in thread From: Gabe Black @ 2011-11-11 2:41 UTC (permalink / raw) To: u-boot On Thu, Nov 10, 2011 at 6:22 PM, Graeme Russ <graeme.russ@gmail.com> wrote: > Hi Gabe, > > On Fri, Nov 11, 2011 at 1:10 PM, Gabe Black <gabeblack@google.com> wrote: > > On Thu, Nov 10, 2011 at 5:59 PM, Graeme Russ <graeme.russ@gmail.com> > wrote: > >> > >> Hi Mike, > >> > >> On Fri, Nov 11, 2011 at 12:55 PM, Mike Frysinger <vapier@gentoo.org> > >> wrote: > >> > On Thursday 10 November 2011 20:51:47 Graeme Russ wrote: > >> >> A few questions (I am unfamiliar with the Linux build environment): > >> >> > >> >> a) Does Linux link to libgcc > >> > > >> > no Linux port uses libgcc. they've always done the equivalent of > >> > PRIVATE_LIBGCC. but in the case of x86, i can't see them providing > any > >> > libgcc > >> > funcs. so i don't think u-boot should either. > >> > > >> >> b) Does Linux use regparm > >> > > >> > yes, it uses -mregparm=3 > >> > >> Well I think we have an answer - use PRIVATE_LIBGCC but do not implement > >> any libgcc functions and treat link errors as coding errors. If for some > >> bizarre reason we need to really, truly, honestly use a 64-bit libgcc > >> function, we'll port it over then > >> > >> Regards, > >> > >> Graeme > > > > I haven't checked in on this thread in a little while, but I wanted to > point > > out some things. First, I was originally planning to measure the > performance > > difference with regparm turned off. I realized that would be quite > annoying > > to actually do, though, since we have a number of extra libraries linked > > into u-boot and they would all have to be recompiled with different > options. > > I assume they are all GPL :) > > > Then the things they link with would have to be recompiled, etc. Even if > > upstream U-Boot drops regparm, we may need to keep it just for that > reason. > > Hmm, that raises an interresing point - by using a non-standard ABI we can > run afoul of external libraries. Which raises the question - what external > libraries do you need to link to? > You have it reversed. By using a standard ABI we run afoul of external libraries. We implement our verified boot infrastructure as a library most notably, and it links in a few other libraries. > > > Second, I think I have a solution that preserves regparm, keeps libgcc > and > > gcc in sync, and also stops unwrapped functions slipping into u-boot. We > can > > use this command: > > objcopy /path/to/your/libgcc/libgcc.a > > /somewhere/in/the/u-boot/build/libgcc.a --prefix-symbols=__real > > to create a libgcc that has all of its symbols prefixed with the string > > __real. Then *all* symbols are prepped for wrapping, regardless of if we > use > > them now or even know about them. Only the symbols we've explicitly > wrapped > > will be available. Note that I haven't actually implemented this yet, > but it > > seems to me like it captures the positive aspects of all the > alternatives. > > Sorry, I find that rather un-aesthetic - We are only looking at a trivial > handful of functions, and to copy and rename ALL the symbols in libgcc.a > is rather overkill > > The other thing I like about using PRIVATE_LIBGCC is that on 64-bit build > platforms, we don't need to worry about installing 32-bit libraries - All > we need to do is add the -m32 option to tell gcc and ld to generate 32-bit > code This argument really doesn't make any sense to me. The number of symbols involved doesn't make any difference at all. You could imagine we are only copying a few if you want and there would be no visible, hence aesthetic, difference. Reimplementing libgcc, along with the inevitable bugs that go along with new code, breaking builds when new functions are added silently by new versions of gcc, etc., seems much less appealing in basically every respect. Gabe ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-11 2:41 ` Gabe Black @ 2011-11-11 4:49 ` Graeme Russ 2011-11-11 5:04 ` Mike Frysinger 0 siblings, 1 reply; 36+ messages in thread From: Graeme Russ @ 2011-11-11 4:49 UTC (permalink / raw) To: u-boot Hi Gabe, On Fri, Nov 11, 2011 at 1:41 PM, Gabe Black <gabeblack@google.com> wrote: > > > On Thu, Nov 10, 2011 at 6:22 PM, Graeme Russ <graeme.russ@gmail.com> wrote: >> >> Hi Gabe, >> >> On Fri, Nov 11, 2011 at 1:10 PM, Gabe Black <gabeblack@google.com> wrote: >> > On Thu, Nov 10, 2011 at 5:59 PM, Graeme Russ <graeme.russ@gmail.com> >> > wrote: >> >> >> >> Hi Mike, >> >> >> >> On Fri, Nov 11, 2011 at 12:55 PM, Mike Frysinger <vapier@gentoo.org> >> >> wrote: >> >> > On Thursday 10 November 2011 20:51:47 Graeme Russ wrote: [snip] >> Hmm, that raises an interresing point - by using a non-standard ABI we can >> run afoul of external libraries. Which raises the question - what external >> libraries do you need to link to? > > > You have it reversed. By using a standard ABI we run afoul of external > libraries. We implement our verified boot infrastructure as a library most > notably, and it links in a few other libraries. Ah, so your libraries are regparm(3) Still, life is going to get interesting for anyone linking in libraries that are not the saem ABI as U-Boot (as we have seen with libgcc) >> > Second, I think I have a solution that preserves regparm, keeps libgcc >> > and >> > gcc in sync, and also stops unwrapped functions slipping into u-boot. We >> > can >> > use this command: >> > objcopy /path/to/your/libgcc/libgcc.a >> > /somewhere/in/the/u-boot/build/libgcc.a --prefix-symbols=__real >> > to create a libgcc that has all of its symbols prefixed with the string >> > __real. Then *all* symbols are prepped for wrapping, regardless of if we >> > use >> > them now or even know about them. Only the symbols we've explicitly >> > wrapped >> > will be available. Note that I haven't actually implemented this yet, >> > but it >> > seems to me like it captures the positive aspects of all the >> > alternatives. >> >> Sorry, I find that rather un-aesthetic - We are only looking at a trivial >> handful of functions, and to copy and rename ALL the symbols in libgcc.a >> is rather overkill >> >> The other thing I like about using PRIVATE_LIBGCC is that on 64-bit build >> platforms, we don't need to worry about installing 32-bit libraries - All >> we need to do is add the -m32 option to tell gcc and ld to generate 32-bit >> code > > This argument really doesn't make any sense to me. The number of symbols > involved doesn't make any difference at all. You could imagine we are only > copying a few if you want and there would be no visible, hence aesthetic, > difference. Reimplementing libgcc, along with the inevitable bugs that go > along with new code, breaking builds when new functions are added silently > by new versions of gcc, etc., seems much less appealing in basically every > respect. Remember, U-Boot uses --no-builtin, so apart from the libgcc functions, there are no gcc functions included. And we are not 'reimplementing libgcc', we are either 'reimplementing a select few functions' or 'not using any libgcc functions in U-Boot at all'. So a build will only break when code is added that uses a function never before used - There should not be any build breakage due to 'functions added silently by newer versions of gcc' I'm assuming that it is the 'verified boot infrastructure' libraries that are introducing the calls to libgcc? If that is the case, then it is starting to get very messy - Why should U-Boot have to deal with this? Regards, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-11 4:49 ` Graeme Russ @ 2011-11-11 5:04 ` Mike Frysinger 2011-11-11 5:16 ` Graeme Russ 0 siblings, 1 reply; 36+ messages in thread From: Mike Frysinger @ 2011-11-11 5:04 UTC (permalink / raw) To: u-boot On Thursday 10 November 2011 23:49:07 Graeme Russ wrote: > Remember, U-Boot uses --no-builtin, so apart from the libgcc functions, > there are no gcc functions included. i don't think that's generally how gcc builtin's work. for the vast majority, they're of the "optimize away with simple insns when possible" variety. so if you do something like: char c[4]; memset(c, 0, sizeof(c)); gcc will optimize that into a single 32bit load rather than calling memcpy(). but because we use -fno-builtins, gcc will make sure to call memcpy(). i can't think of any calls off the top of my head which would result in invoking a func in libgcc.a. -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20111111/12662c0a/attachment.pgp ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-11 5:04 ` Mike Frysinger @ 2011-11-11 5:16 ` Graeme Russ 2011-11-11 16:24 ` Mike Frysinger 0 siblings, 1 reply; 36+ messages in thread From: Graeme Russ @ 2011-11-11 5:16 UTC (permalink / raw) To: u-boot Hi Mike, On Fri, Nov 11, 2011 at 4:04 PM, Mike Frysinger <vapier@gentoo.org> wrote: > On Thursday 10 November 2011 23:49:07 Graeme Russ wrote: >> Remember, U-Boot uses --no-builtin, so apart from the libgcc functions, >> there are no gcc functions included. > > i don't think that's generally how gcc builtin's work. for the vast majority, > they're of the "optimize away with simple insns when possible" variety. so if > you do something like: > char c[4]; > memset(c, 0, sizeof(c)); > gcc will optimize that into a single 32bit load rather than calling memcpy(). > but because we use -fno-builtins, gcc will make sure to call memcpy(). List of builtin functions not in libgcc: http://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html > i can't think of any calls off the top of my head which would result in > invoking a func in libgcc.a. Any function listed here: http://gcc.gnu.org/onlinedocs/gccint/Libgcc.html But we can discount any float/double routines, exception handling and split stack which leaves just: http://gcc.gnu.org/onlinedocs/gccint/Integer-library-routines.html Regards, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-11 5:16 ` Graeme Russ @ 2011-11-11 16:24 ` Mike Frysinger 0 siblings, 0 replies; 36+ messages in thread From: Mike Frysinger @ 2011-11-11 16:24 UTC (permalink / raw) To: u-boot On Friday 11 November 2011 00:16:47 Graeme Russ wrote: > On Fri, Nov 11, 2011 at 4:04 PM, Mike Frysinger wrote: > > i can't think of any calls off the top of my head which would result in > > invoking a func in libgcc.a. > > Any function listed here: > > http://gcc.gnu.org/onlinedocs/gccint/Libgcc.html > > But we can discount any float/double routines, exception handling and > split stack which leaves just: > > http://gcc.gnu.org/onlinedocs/gccint/Integer-library-routines.html yes, and pretty much all of those are emitted implicitly due to math operations (64bit divs/mults/etc...), or explicitly when the user does __builtin_xxx() (and use of __builtin_xxx is not affected by -fno-builtins). none of those that i can see would come via a C library call that gcc would implicitly rewrite. -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20111111/594c08d2/attachment.pgp ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-11 1:59 ` Graeme Russ 2011-11-11 2:10 ` Gabe Black @ 2011-11-11 2:44 ` Mike Frysinger 1 sibling, 0 replies; 36+ messages in thread From: Mike Frysinger @ 2011-11-11 2:44 UTC (permalink / raw) To: u-boot On Thursday 10 November 2011 20:59:46 Graeme Russ wrote: > Well I think we have an answer - use PRIVATE_LIBGCC but do not implement > any libgcc functions and treat link errors as coding errors. If for some > bizarre reason we need to really, truly, honestly use a 64-bit libgcc > function, we'll port it over then that's not how USE_PRIVATE_LIBGCC works in u-boot though. i'd suggest: echo "PLATFORM_LIBGCC :=" >> arch/x86/config.mk -mike -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 836 bytes Desc: This is a digitally signed message part. Url : http://lists.denx.de/pipermail/u-boot/attachments/20111110/249155d7/attachment.pgp ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-11 1:55 ` Mike Frysinger 2011-11-11 1:59 ` Graeme Russ @ 2011-11-11 19:59 ` Scott Wood 1 sibling, 0 replies; 36+ messages in thread From: Scott Wood @ 2011-11-11 19:59 UTC (permalink / raw) To: u-boot On 11/10/2011 07:55 PM, Mike Frysinger wrote: > On Thursday 10 November 2011 20:51:47 Graeme Russ wrote: >> A few questions (I am unfamiliar with the Linux build environment): >> >> a) Does Linux link to libgcc > > no Linux port uses libgcc. they've always done the equivalent of > PRIVATE_LIBGCC. but in the case of x86, i can't see them providing any libgcc > funcs. so i don't think u-boot should either. Some of the less common architectures (openrisc, h8300, cris, m32r, tile, xtensa) appear to use libgcc. unicore32 appears to pull selected objects out of both libgcc and libc. -Scott ^ permalink raw reply [flat|nested] 36+ messages in thread
* [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage 2011-11-09 10:32 ` [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage Graeme Russ 2011-11-09 17:12 ` Mike Frysinger @ 2011-11-16 23:00 ` Graeme Russ 1 sibling, 0 replies; 36+ messages in thread From: Graeme Russ @ 2011-11-16 23:00 UTC (permalink / raw) To: u-boot Hi Gabe, On Wed, Nov 9, 2011 at 9:32 PM, Graeme Russ <graeme.russ@gmail.com> wrote: > Hi Gabe, > > Can you please try this patch - If it solves your libgcc problem, I will > add it to the misc cleanup patch > > Thanks, > > Graeme > --- > ?arch/x86/config.mk ? ? ? ?| ? ?3 --- > ?arch/x86/cpu/interrupts.c | ? ?2 +- > ?arch/x86/cpu/start.S ? ? ?| ? ?5 ++--- > ?3 files changed, 3 insertions(+), 7 deletions(-) I think we all agree this is not the way to go Regards, Graeme ^ permalink raw reply [flat|nested] 36+ messages in thread
end of thread, other threads:[~2011-11-30 11:03 UTC | newest] Thread overview: 36+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-11-08 9:27 [U-Boot] [PATCH] [x86] Wrap small helper functions from libgcc to avoid an ABI mismatch Gabe Black 2011-11-08 13:33 ` Mike Frysinger 2011-11-08 22:27 ` Gabe Black 2011-11-08 22:34 ` [U-Boot] [PATCH v2] x86: " Gabe Black 2011-11-08 23:14 ` Graeme Russ 2011-11-09 4:49 ` Mike Frysinger 2011-11-09 3:57 ` Graeme Russ 2011-11-09 5:34 ` Mike Frysinger 2011-11-17 9:01 ` [U-Boot] [PATCH v3] " Gabe Black 2011-11-17 9:13 ` Gabe Black 2011-11-30 11:03 ` Graeme Russ 2011-11-09 3:05 ` [U-Boot] [PATCH v2] " Graeme Russ 2011-11-09 10:32 ` [U-Boot] [RFC] x86: Do no use reparm as it break libgcc linkage Graeme Russ 2011-11-09 17:12 ` Mike Frysinger 2011-11-09 21:42 ` Graeme Russ 2011-11-10 4:13 ` Mike Frysinger 2011-11-10 4:22 ` Graeme Russ 2011-11-10 5:10 ` Graeme Russ 2011-11-10 17:15 ` Mike Frysinger 2011-11-10 22:53 ` Graeme Russ 2011-11-11 0:23 ` Mike Frysinger 2011-11-11 1:23 ` Graeme Russ 2011-11-11 1:40 ` Mike Frysinger 2011-11-11 1:51 ` Graeme Russ 2011-11-11 1:55 ` Mike Frysinger 2011-11-11 1:59 ` Graeme Russ 2011-11-11 2:10 ` Gabe Black 2011-11-11 2:22 ` Graeme Russ 2011-11-11 2:41 ` Gabe Black 2011-11-11 4:49 ` Graeme Russ 2011-11-11 5:04 ` Mike Frysinger 2011-11-11 5:16 ` Graeme Russ 2011-11-11 16:24 ` Mike Frysinger 2011-11-11 2:44 ` Mike Frysinger 2011-11-11 19:59 ` Scott Wood 2011-11-16 23:00 ` Graeme Russ
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox