Messages in this thread Patch in this message | | | Date | Fri, 5 Jun 2009 22:07:01 +0200 | From | Sam Ravnborg <> | Subject | Re: [PATCH] x86: clean up vdso-layout.lds.S |
| |
> > diff --git a/arch/x86/vdso/Makefile b/arch/x86/vdso/Makefile > > index 16a9020..8c7f06a 100644 > > --- a/arch/x86/vdso/Makefile > > +++ b/arch/x86/vdso/Makefile > > @@ -23,7 +23,8 @@ $(obj)/vdso.o: $(obj)/vdso.so > > > > targets += vdso.so vdso.so.dbg vdso.lds $(vobjs-y) > > > > -export CPPFLAGS_vdso.lds += -P -C > > +vdso-cppflags = -P -C > > +export CPPFLAGS_vdso.lds += -m64 $(vdso-cppflags)
I am wondering why we need -P -C here - but we do not need it for lds.S files? Seems like something we could let go.
> > > > VDSO_LDFLAGS_vdso.lds = -m elf_x86_64 -Wl,-soname=linux-vdso.so.1 \ > > -Wl,-z,max-page-size=4096 -Wl,-z,common-page-size=4096 > > @@ -68,7 +69,7 @@ vdso32.so-$(VDSO32-y) += sysenter > > > > vdso32-images = $(vdso32.so-y:%=vdso32-%.so) > > > > -CPPFLAGS_vdso32.lds = $(CPPFLAGS_vdso.lds) > > +CPPFLAGS_vdso32.lds = -m32 $(vdso-cppflags) > > VDSO_LDFLAGS_vdso32.lds = -m elf_i386 -Wl,-soname=linux-gate.so.1 > > > > # This makes sure the $(obj) subdirectory exists even though vdso32/ > > diff --git a/arch/x86/vdso/vdso-layout.lds.S b/arch/x86/vdso/vdso-layout.lds.S > > index 634a2cf..1f4b215 100644 > > --- a/arch/x86/vdso/vdso-layout.lds.S > > +++ b/arch/x86/vdso/vdso-layout.lds.S > > @@ -22,16 +22,15 @@ SECTIONS > > .eh_frame : { KEEP (*(.eh_frame)) } :text > > > > .dynamic : { *(.dynamic) } :text :dynamic > > + .got : { *(.got.plt) *(.got) } :text
The style we try to introduce for .lds files in arch/$ARCH/kernel/vmlinux.lds.S is much more C-like. The above would have been: .got : { *(.got.plt) *(.got) } :text
Please use this all over so we have a consistent style in linker scripts.
> > .data : { > > - *(.data*) > > - *(.sdata*) > > - *(.got.plt) *(.got) > > - *(.gnu.linkonce.d.*) > > - *(.bss*) > > - *(.dynbss*) > > - *(.gnu.linkonce.b.*) > > + *(.data .data.* .gnu.linkonce.d.*) > > + *(.bss .bss.* .gnu.linkonce.b.*) > > + *(COMMON) > > } Where did *(.sdata*) go? Why do we need *(.data .data.*) rather than *(.data*)? *(.dynbss*)?
In your changelog you say: "discard sections which are not useful to user-space" - but as you do not list which one it is hard to tell what you removed on purpose and what you removed by accident.
> > > > .altinstructions : { *(.altinstructions) } > > @@ -43,9 +42,49 @@ SECTIONS > > */ > > . = ALIGN(0x100);
What is 0x100?
> > > > - .text : { *(.text*) } :text =0x90909090 > > + .text : { > > + *(.text .text.* .gnu.linkonce.t.*) > > + } :text =0x90909090 > > + > > + /* We would need a more sophisticated dynamic linker for the > > + * vDSO to make the following sections work. Put them into > > + * a special section and raise a link-time error if they get > > + * used. > > + */ > > + .broken : { > > + /* Code in the Procedure Linkage Table will segfault */ > > + *(.plt) > > + > > + /* Relocation will not be done, so any pointers will > > + * still point to the prelinked address, which is wrong > > + */ > > + *(.data.rel.ro*) > > + *(.gnu.linkonce.d.rel.ro.*) > > + > > + /* Initialization/termination won't work this way */ > > + *(.init) *(.fini) > > + *(.preinit_array) *(.init_array*) > > + *(.fini_array*) > > + > > + /* Thread-local data cannot be defined like this */ > > + *(.tdata .tdata.* .gnu.linkonce.td.*) > > + *(.tbss .tbss.* .gnu.linkonce.tb.*) > > + *(.tcommon) > > + } > > + > > + /* These sections are not useful */ > > + /DISCARD/ : { > > + *(.gnu.warning.*) > > + *(.note.GNU-stack) > > + } > > } > > > > +ASSERT(!SIZEOF(.broken), "VDSO contains sections that don't work properly"); Can you give any better hints where to look and what to look for? > > + > > +/* Check that GOT has only the three entries defined by the ABI */ > > +ASSERT(SIZEOF(.got) == 3*__SIZEOF_POINTER__, > > + "Found extra GOT entries. Check your use of external vars."); Can you give any better hints where to look and what to look for?
Sam
| |