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

From: "Jan Beulich" <JBeulich@suse.com>
To: Daniel Kiper <daniel.kiper@oracle.com>
Cc: Juergen Gross <JGross@suse.com>,
	stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com,
	cardoe@cardoe.com, pgnet.dev@gmail.com, ning.sun@intel.com,
	david.vrabel@citrix.com, xen-devel@lists.xenproject.org,
	qiaowei.ren@intel.com, richard.l.maliszewski@intel.com,
	gang.wei@intel.com, fu.wei@linaro.org
Subject: Re: [PATCH v3 07/16] x86/boot: create *.lnk files with linker script
Date: Tue, 24 May 2016 03:05:06 -0600	[thread overview]
Message-ID: <5744356202000078000EE2A4@prv-mh.provo.novell.com> (raw)
In-Reply-To: <1460723596-13261-8-git-send-email-daniel.kiper@oracle.com>

>>> On 15.04.16 at 14:33, <daniel.kiper@oracle.com> 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@oracle.com>
> + *
> + * 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
.rodata but not also .rodata.* here.

> +  }
> +
> +  /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
reason for the linker to create an otherwise empty .got.plt in the
first place. And discarding it without being sure it is empty is not
that good an idea anyway.

Jan

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