[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] mkimage: avoid copying relocations for sections that won

From: Daniel Kiper
Subject: Re: [PATCH 1/2] mkimage: avoid copying relocations for sections that won't be copied.
Date: Tue, 20 Feb 2018 15:48:44 +0100
User-agent: Mutt/1.3.28i

On Wed, Jan 31, 2018 at 11:26:59AM -0500, Peter Jones wrote:
> Some versions of gcc include a plugin called "annobin", and in some
> build systems this is enabled by default.  This plugin creates special
> ELF note sections to track which ABI-breaking features are used by a
> binary, as well as a series of relocations to annotate where.
> If grub is compiled with this feature, then when grub-mkimage translates
> the binary to another file format which does not strongly associate
> relocation data with sections (i.e. when platform is *-efi), these
> relocations appear to be against the .text section rather than the
> original note section.  When the binary is loaded by the PE runtime
> loader, hilarity ensues.
> This issue is not necessarily limited to the annobin, but could arise
> any time there are relocations in sections that are not represented in
> grub-mkimage's output.
> This patch seeks to avoid this issue by only including relocations that
> refer to sections which will be included in the final binary.
> As an aside, this should also obviate the need to avoid -funwind-tables,
> -fasynchronous-unwind-tables, and any sections similar to .eh_frame in
> the future.  I've tested it on x86-64-efi with the following gcc command
> line options (as recorded by -grecord-gcc-flags), but I still need to
> test the result on some other platforms that have been problematic in
> the past (especially ARM Aarch64) before I feel comfortable making
> changes to the bits:
> GNU C11 7.2.1 20180116 (Red Hat 7.2.1-7) -mno-mmx -mno-sse -mno-sse2 
> -mno-sse3 -mno-3dnow -msoft-float -mno-stack-arg-probe -mcmodel=large 
> -mno-red-zone -m64 -mtune=generic -march=x86-64 -g3 -Os -freg-struct-return 
> -fno-stack-protector -ffreestanding -funwind-tables 
> -fasynchronous-unwind-tables -fno-strict-aliasing -fstack-clash-protection 
> -fno-ident -fplugin=annobin
> Signed-off-by: Peter Jones <address@hidden>

In general I am OK with the idea but...

> ---
>  util/grub-mkimagexx.c | 81 
> ++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 73 insertions(+), 8 deletions(-)
> diff --git a/util/grub-mkimagexx.c b/util/grub-mkimagexx.c
> index a2bb05439f0..b016b061c8c 100644
> --- a/util/grub-mkimagexx.c
> +++ b/util/grub-mkimagexx.c
> @@ -708,6 +708,13 @@ arm_get_trampoline_size (Elf_Ehdr *e,
>  }
>  #endif
> +static int
> +SUFFIX (is_kept_section) (Elf_Shdr *s, const struct 
> grub_install_image_target_desc *image_target);
> +static int
> +SUFFIX (is_kept_reloc_section) (Elf_Shdr *s, const struct 
> grub_install_image_target_desc *image_target,
> +                             Elf_Shdr *sections, Elf_Half section_entsize, 
> Elf_Half num_sections,
> +                             const char *strtab);

Ugh... Could not you create a struct and pass the pointer to it here?


PS Please CC me on the patches next time.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]