| Message ID | 1470438282-4226-4-git-send-email-daniel.kiper@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 06.08.16 at 01:04, <daniel.kiper@oracle.com> wrote: > Newer GCC (e.g. gcc version 5.1.1 20150618 (Red Hat 5.1.1-4) (GCC)) does > some code optimizations by creating data sections (e.g. jump addresses > for C switch/case are calculated using data in .rodata section). This > thing is not accepted by *.lnk build recipe which requires that only .text > section lives in output. Potentially we can inhibit this GCC behavior by > using special options, e.g. -fno-tree-switch-conversion. However, this > does not guarantee that in the future new similar optimizations or anything > else which creates not accepted sections will not break our build recipes > again. I do not mention that probably this is not good idea to just disable > random optimizations. So, take over full control on *.lnk linking process > by using linker script and merge required text and data sections into one > .text section. > > Additionally, remove .got.plt section which is not used in our final code. > > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> > --- > v4 - suggestions/fixes: > - remove my name from copyright (Oracle requirement) > (suggested by Konrad Rzeszutek Wilk), > - improve comments, > (suggested by Jan Beulich), > - improve commit message > (suggested by Jan Beulich). > --- > xen/arch/x86/boot/build32.lds | 51 > +++++++++++++++++++++++++++++++++++++++++ > xen/arch/x86/boot/build32.mk | 10 +++++--- > 2 files changed, 58 insertions(+), 3 deletions(-) > create mode 100644 xen/arch/x86/boot/build32.lds > > diff --git a/xen/arch/x86/boot/build32.lds b/xen/arch/x86/boot/build32.lds > new file mode 100644 > index 0000000..b14c7d5 > --- /dev/null > +++ b/xen/arch/x86/boot/build32.lds > @@ -0,0 +1,51 @@ > +/* > + * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved. > + * > + * 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; either version 2 of the License, or > + * (at your option) any later version. > + * > + * 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, see <http://www.gnu.org/licenses/>. > + */ > + > +ENTRY(_start) > + > +SECTIONS > +{ > + /* Merge code and data into one section. */ > + .text : { > + *(.text) > + *(.text.*) > + *(.rodata) > + } *(.rodata.*) ? How about any other sections? Following the much improved (thanks!) description, we can't exclude their appearance. > + /DISCARD/ : { > + /* > + * PIC/PIE executable contains .got.plt section even > + * if it is not linked with dynamic libraries. In such > + * case it is just placeholder for _GLOBAL_OFFSET_TABLE_ > + * symbol and .PLT0. .PLT0 is filled by dynamic linker > + * and our code is not supposed to be loaded by dynamic > + * linker. So, from our point of view .PLT0 is unused. > + * This means that there is pretty good chance that > + * we can safely drop .got.plt as a whole here. Sadly > + * this is not true. _GLOBAL_OFFSET_TABLE_ is used as > + * a reference for relative addressing (and only for > + * that thing) and ld complains if we remove .got.plt > + * section here because it cannot find required symbol. > + * However, _GLOBAL_OFFSET_TABLE_ is no longer needed > + * in final output. So, drop .got.plt section during > + * conversion to plain binary format. Commonly we have a problem the other way around; here I'd like to ask that you don't break lines this early (often hardly going meaningfully beyond 60 columns). > --- a/xen/arch/x86/boot/build32.mk > +++ b/xen/arch/x86/boot/build32.mk > @@ -12,20 +12,24 @@ CFLAGS := $(filter-out -flto,$(CFLAGS)) > (od -v -t x $< | tr -s ' ' | awk 'NR > 1 {print s} {s=$$0}' | \ > sed 's/ /,0x/g' | sed 's/,0x$$//' | sed 's/^[0-9]*,/ .long /') >$@ > > +# > +# Drop .got.plt during conversion to plain binary format. > +# Please check build32.lds for more details. > +# > %.bin: %.lnk Please avoid the two blank comment lines. > - $(OBJCOPY) -O binary $< $@ > + $(OBJCOPY) -O binary -R .got.plt $< $@ > > %.lnk: %.o > $(OBJDUMP) -h $< | sed -n '/[0-9]/{s,00*,0,g;p;}' |\ > while read idx name sz rest; do \ > case "$$name" in \ > - .data|.data.*|.rodata|.rodata.*|.bss|.bss.*) \ > + .data|.data.*|.rodata.*|.bss|.bss.*) \ > test $$sz != 0 || continue; \ > echo "Error: non-empty $$name: 0x$$sz" >&2; \ > exit $$(expr $$idx + 1);; \ > esac; \ > done This logic largely contradicts the use of a linker script: I'd much rather see you include all the sections checked to be empty here, and instead add a check that .got.plt (which you now mean to drop) is of the expected size. Of course such a check would need to go into the other rule's commands. Jan
diff --git a/xen/arch/x86/boot/build32.lds b/xen/arch/x86/boot/build32.lds new file mode 100644 index 0000000..b14c7d5 --- /dev/null +++ b/xen/arch/x86/boot/build32.lds @@ -0,0 +1,51 @@ +/* + * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved. + * + * 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; either version 2 of the License, or + * (at your option) any later version. + * + * 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, see <http://www.gnu.org/licenses/>. + */ + +ENTRY(_start) + +SECTIONS +{ + /* Merge code and data into one section. */ + .text : { + *(.text) + *(.text.*) + *(.rodata) + } + + /DISCARD/ : { + /* + * PIC/PIE executable contains .got.plt section even + * if it is not linked with dynamic libraries. In such + * case it is just placeholder for _GLOBAL_OFFSET_TABLE_ + * symbol and .PLT0. .PLT0 is filled by dynamic linker + * and our code is not supposed to be loaded by dynamic + * linker. So, from our point of view .PLT0 is unused. + * This means that there is pretty good chance that + * we can safely drop .got.plt as a whole here. Sadly + * this is not true. _GLOBAL_OFFSET_TABLE_ is used as + * a reference for relative addressing (and only for + * that thing) and ld complains if we remove .got.plt + * section here because it cannot find required symbol. + * However, _GLOBAL_OFFSET_TABLE_ is no longer needed + * in final output. So, drop .got.plt section during + * conversion to plain binary format. + * + * Please check build32.mk for more details. + */ + /* *(.got.plt) */ + } +} diff --git a/xen/arch/x86/boot/build32.mk b/xen/arch/x86/boot/build32.mk index 4a7d388..eb02b4b 100644 --- a/xen/arch/x86/boot/build32.mk +++ b/xen/arch/x86/boot/build32.mk @@ -12,20 +12,24 @@ CFLAGS := $(filter-out -flto,$(CFLAGS)) (od -v -t x $< | tr -s ' ' | awk 'NR > 1 {print s} {s=$$0}' | \ sed 's/ /,0x/g' | sed 's/,0x$$//' | sed 's/^[0-9]*,/ .long /') >$@ +# +# Drop .got.plt during conversion to plain binary format. +# Please check build32.lds for more details. +# %.bin: %.lnk - $(OBJCOPY) -O binary $< $@ + $(OBJCOPY) -O binary -R .got.plt $< $@ %.lnk: %.o $(OBJDUMP) -h $< | sed -n '/[0-9]/{s,00*,0,g;p;}' |\ while read idx name sz rest; do \ case "$$name" in \ - .data|.data.*|.rodata|.rodata.*|.bss|.bss.*) \ + .data|.data.*|.rodata.*|.bss|.bss.*) \ test $$sz != 0 || continue; \ echo "Error: non-empty $$name: 0x$$sz" >&2; \ exit $$(expr $$idx + 1);; \ esac; \ done - $(LD) $(LDFLAGS_DIRECT) -N -Ttext 0 -o $@ $< + $(LD) $(LDFLAGS_DIRECT) -N -T build32.lds -o $@ $< %.o: %.c $(CC) $(CFLAGS) -c -fpic $< -o $@
Newer GCC (e.g. gcc version 5.1.1 20150618 (Red Hat 5.1.1-4) (GCC)) does some code optimizations by creating data sections (e.g. jump addresses for C switch/case are calculated using data in .rodata section). This thing is not accepted by *.lnk build recipe which requires that only .text section lives in output. Potentially we can inhibit this GCC behavior by using special options, e.g. -fno-tree-switch-conversion. However, this does not guarantee that in the future new similar optimizations or anything else which creates not accepted sections will not break our build recipes again. I do not mention that probably this is not good idea to just disable random optimizations. So, take over full control on *.lnk linking process by using linker script and merge required text and data sections into one .text section. Additionally, remove .got.plt section which is not used in our final code. Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> --- v4 - suggestions/fixes: - remove my name from copyright (Oracle requirement) (suggested by Konrad Rzeszutek Wilk), - improve comments, (suggested by Jan Beulich), - improve commit message (suggested by Jan Beulich). --- xen/arch/x86/boot/build32.lds | 51 +++++++++++++++++++++++++++++++++++++++++ xen/arch/x86/boot/build32.mk | 10 +++++--- 2 files changed, 58 insertions(+), 3 deletions(-) create mode 100644 xen/arch/x86/boot/build32.lds