[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1244532418.5199.24.camel@nathan.suse.cz>
Date: Tue, 09 Jun 2009 09:26:58 +0200
From: Petr Tesarik <ptesarik@...e.cz>
To: Sam Ravnborg <sam@...nborg.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>,
"H. Peter Anvin" <hpa@...or.com>, Andi Kleen <andi@...stfloor.org>,
Roland McGrath <roland@...hat.com>
Subject: Re: [PATCH] x86: clean up vdso-layout.lds.S
Sam Ravnborg píše v Pá 05. 06. 2009 v 22:07 +0200:
> > > 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.
Frankly, I don't know, but it's been there for ages, and I don't see
what you mean, anyway. Doesn't the top-level Makefile contain this line,
for example:
export CPPFLAGS_vmlinux.lds += -P -C -U$(ARCH)
> > > 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.
OK, so should I first post a patch which doesn't change anything but
adjusts the coding style of vdso-layout.lds.S?
> > > .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?
Nothing changed, really. This section is never produced for i386 or
x86_64. The line might have been a remnant of the (long abandoned) idea
that x86_64 would have .sdata and .data, but it then turned to be
rather .data and .ldata.
> Why do we need *(.data .data.*) rather than *(.data*)?
> *(.dynbss*)?
I don't have any strong opinion here, but the former is exactly what the
default linker script has.
> 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.
All those which end up in the section ".broken". I'll make a better
wording next time.
> > >
> > > .altinstructions : { *(.altinstructions) }
> > > @@ -43,9 +42,49 @@ SECTIONS
> > > */
> > > . = ALIGN(0x100);
>
> What is 0x100?
Um. No idea. Roland, you added this line in commit
f6b46ebf904f69a73907a5e6b1ed2228e3f03d9e. Can you shed some light on
this magic constant?
> > > - .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?
What would you expect? The linker script language is quite limited in
its capabilities... Best I could do is split the ".broken" section into
several sections and move the descriptions from the individual comments
above here. If this muckle of empty ".broken.*" sections gets correctly
discarded and triggers no bug in binutils, I can probably do it.
> > > +
> > > +/* 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?
What would you consider a better hint? If there are any entries in the
GOT, this means that the vDSO tries to access an external function or
variable. But since we don't have a real in-kernel linker for the vDSO,
these references will remain unresolved (and most likely cause a
segmentation fault at run-time).
Cheers,
Petr Tesarik
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists