Re: [U-Boot] [PATCH v3] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch
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 gabebl...@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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH v3] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch
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 gabebl...@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 000..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
Re: [U-Boot] [PATCH v3] x86: Wrap small helper functions from libgcc to avoid an ABI mismatch
On Thu, Nov 17, 2011 at 1:01 AM, Gabe Black gabebl...@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 gabebl...@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 000..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 + *
