Re: [PATCH] Fix PT_GNU_STACK on LTO compiled binaries with debug info (PR lto/82598, take 2)

[Richard Biener <rguenther at suse dot de>]

On Wed, 18 Oct 2017, Jakub Jelinek wrote:

> On Wed, Oct 18, 2017 at 09:24:04AM +0200, Jakub Jelinek wrote:
> > When creating lto debugobj, we copy over just the debug sections,
> > and from the lack of .note.GNU-stack section then on various targets
> > the linker implies RWE PT_GNU_STACK segment header.
> > 
> > Fixed by copying over also the .note.GNU-stack section if present.
> > It is not 100% perfect solution if .note.GNU-stack in the original
> > indicates executable stack, in the debugobj we really don't need
> > it, so could get away with clearing the SHF_EXECINSTR bit, but in reality
> > it shouldn't be that bad, if the source had trampolines, then likely the
> > LTO objects will have them too.
> 
> This updated version does that clearing.
> 
> Tested with
> cat pr82598-1.c
> extern void bar (void (*) (void));
> 
> int
> main ()
> {
>   volatile int i = 0;
>   auto void foo (void) { i++; }
>   bar (foo);
>   return 0;
> }
> cat pr82598-2.c
> void
> bar (void (*fn) (void))
> {
>   fn ();
>   fn ();
> }
> ./xgcc -B ./ -g -O3 -flto -ffat-lto-objects -fuse-linker-plugin -o pr82598{-1,-1.c,-2.c} -save-temps -v 2>&1 | less
> readelf -Wl pr82598-1 | grep STACK
> and readelf -WS on the debugobj objects mentioned in the -v output.
> While LTO inlines it, it keeps the trampoline in, so the second
> part of the patch doesn't change anything right now, but guess it would
> at least if the function with the trampoline is optimized away.

Works for me but as said, why's the linker even caring about
.note.GNU-stack in objects that do not contain executable code?
When we link an object with just .rodata and no proper note will
the final executable also have an executable stack?

Richard.

> 2017-10-18  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR lto/82598
> 	* simple-object.c (handle_lto_debug_sections): Copy over also
> 	.note.GNU-stack section with unchanged name.
> 	* simple-object-elf.c (SHF_EXECINSTR): Define.
> 	(simple_object_elf_copy_lto_debug_section): Drop SHF_EXECINSTR bit
> 	on .note.GNU-stack section.
> 
> --- libiberty/simple-object.c.jj	2017-09-01 09:27:00.000000000 +0200
> +++ libiberty/simple-object.c	2017-10-18 09:15:51.088756028 +0200
> @@ -273,6 +273,9 @@ handle_lto_debug_sections (const char **
>        *name = *name + sizeof (".gnu.lto_") - 1;
>        return 1;
>      }
> +  /* Copy over .note.GNU-stack section under the same name if present.  */
> +  else if (strcmp (*name, ".note.GNU-stack") == 0)
> +    return 1;
>    return 0;
>  }
>  
> --- libiberty/simple-object-elf.c.jj	2017-09-15 18:09:31.000000000 +0200
> +++ libiberty/simple-object-elf.c	2017-10-18 09:45:47.421383452 +0200
> @@ -196,6 +196,7 @@ typedef struct {
>  
>  /* Values for sh_flags field.  */
>  
> +#define SHF_EXECINSTR	0x00000004	/* Executable section.  */
>  #define SHF_EXCLUDE	0x80000000	/* Link editor is to exclude this
>  					   section from executable and
>  					   shared library that it builds
> @@ -1403,7 +1404,14 @@ simple_object_elf_copy_lto_debug_section
>        flags = ELF_FETCH_FIELD (type_functions, ei_class, Shdr,
>  			       shdr, sh_flags, Elf_Addr);
>        if (ret == 0)
> -	flags &= ~SHF_EXCLUDE;
> +	{
> +	  /* The debugobj doesn't contain any code, thus no trampolines.
> +	     Even when the original object needs trampolines, debugobj
> +	     doesn't.  */
> +	  if (strcmp (name, ".note.GNU-stack") == 0)
> +	    flags &= ~SHF_EXECINSTR;
> +	  flags &= ~SHF_EXCLUDE;
> +	}
>        else if (ret == -1)
>  	flags = SHF_EXCLUDE;
>        ELF_SET_FIELD (type_functions, ei_class, Shdr,
> 
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)