Re: [Xen-devel] [PATCH v3 07/16] x86/boot: create *.lnk files with linker script

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Tue, May 24, 2016 at 06:52:39AM -0600, Jan Beulich wrote:
> >>> On 24.05.16 at 14:28, <daniel.kiper@xxxxxxxxxx> wrote:
> > On Tue, May 24, 2016 at 03:05:06AM -0600, Jan Beulich wrote:
> >> >>> On 15.04.16 at 14:33, <daniel.kiper@xxxxxxxxxx> wrote:
> >> > --- /dev/null
> >> > +++ b/xen/arch/x86/boot/build32.lds
> >> > @@ -0,0 +1,49 @@
> >> > +/*
> >> > + * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
> >> > + *      Daniel Kiper <daniel.kiper@xxxxxxxxxx>
> >> > + *
> >> > + * 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)
> >>
> >> This (and the respective %.lnk rule change below) is not in line with
> >> the patch description. It's further suspicious that you only handle
> >
> > I am not sure what exactly do you mean by that.
>
> Quoting your commit message: "...  merge all text and data sections
> into one .text section." Contrast this to the limited set of sections
> above.
>
> >> .rodata but not also .rodata.* here.
> >
> > I did this deliberately. I just want to take only these sections which I
> > know that
> > contain required code and data. Nothing more. If in the future we find out
> > that
> > .rodata.* (or anything else) is needed then we can add it later.
> >
> >> > +  }
> >> > +
> >> > +  /DISCARD/ : {
> >> > +        /*
> >> > +         * .got.plt section is used only by dynamic linker
> >> > +         * and our output is not supposed to be loaded by
> >> > +         * dynamic linker. Additionally, it just contains
> >> > +         * .PLT0 which is referenced from nowhere. So, we
> >> > +         * can safely drop .got.plt here.
> >> > +         *
> >> > +         * Ha! This should be really discarded here. However,
> >> > +         * .got.plt section contains _GLOBAL_OFFSET_TABLE_
> >> > +         * symbol too and it is used as a reference for relative
> >> > +         * addressing (and only for that thing). Hence, ld
> >> > +         * complains if we remove that section because it
> >> > +         * cannot find _GLOBAL_OFFSET_TABLE_. So, drop .got.plt
> >> > +         * section during conversion to plain binary format.
> >> > +         * Please check build32.mk for more details.
> >> > +         */
> >> > +        /* *(.got.plt) */
> >> > +  }
> >>
> >> I'm afraid this needs more investigation: Afaik there should be no
> >
> > I am not sure what else we should look for.
>
> The reason why such an empty .got.plt gets created in the first place.
> If e.g. that turns out to be a bug in (some versions of) binutils, then
> that bug should be named here as the reason.

If PIC/PIE code is build then .got.plt exists in executable even if it
is not linked with dynamic libraries. Then 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.

> >> reason for the linker to create an otherwise empty .got.plt in the
> >
> > As I wrote above. It contains _GLOBAL_OFFSET_TABLE_ which is used
> > as a reference for relative addressing.
>
> But we don't use any such, so without being needed I don't think
> the symbol needs to be created.

R_386_GOTPC and R_386_GOTOFF relocations use address of _GLOBAL_OFFSET_TABLE_
as a reference. So, it is needed during linking phase. However, later it is
not needed.  Hence, .got.plt with _GLOBAL_OFFSET_TABLE_ and .PLT0 can safely
be dropped.

> >> first place. And discarding it without being sure it is empty is not
> >> that good an idea anyway.
> >
> > Good point! Potentially we can check is it empty, excluding
> > _GLOBAL_OFFSET_TABLE_ symbol, in build32.mk.
>
> Well, your comment above says it have .PLT0, which means it's not
> exactly empty.

Ditto. And right know I do not think that we need any safety checks here.

Daniel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel