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

[Ian Lance Taylor]

Jakub Jelinek  writes:

> 2017-10-18  Jakub Jelinek  
>
>   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.

This is OK.

Thanks.

Ian

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

[Jakub Jelinek]

On Wed, Oct 18, 2017 at 10:17:49AM +0200, Richard Biener wrote:
> Works for me but as said, why's the linker even caring about
> .note.GNU-stack in objects that do not contain executable code?

The linker simply collects the notes from all *.o files.  One case is
when none of the objects have the notes, then it is considered to be
the legacy state, another when then there is at least one object with that
note, then any object without the note or with X note forces the result
to be RWE, otherwise if all objects have non-X note the result is RW.
The linker doesn't have information what all kinds of sections could contain
trampolines.

If I were to design that stuff now, I'd probably do it differently, but
it behaves this way for 14 years already...

> When we link an object with just .rodata and no proper note will
> the final executable also have an executable stack?

If any other object in the link contains the note, yes.

Jakub

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

[Jakub Jelinek]

On Wed, Oct 18, 2017 at 10:15:21AM +0200, Richard Biener wrote:
> On Wed, 18 Oct 2017, 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.
> 
> Uh.  But those objects don't even have a .text section...
> 
> > 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.
> > 
> > Tested on x86_64-linux, ok for trunk?
> 
> Can't we simply always append a .note.GNU-stack to indicate a

No.  .note.GNU-stack is only added on some targets, on other targets it
isn't, so if the linker will see the note on some target where it isn't
normally seen, it will force different behavior than it used to have
normally upon all the other objects.

> non-executable stack on debug objects?  Or as you say clear
> SHF_EXECINSTR (whatever that means)?

See the second patch I've posted which does that.

Jakub

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

[Richard Biener]

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  
> 
>   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.0 +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.0 +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_EXECINSTR0x0004  /* Executable section.  */
>  #define SHF_EXCLUDE  0x8000  /* 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 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

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

[Richard Biener]

On Wed, 18 Oct 2017, Jakub Jelinek wrote:

> Hi!
> 
> 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.

Uh.  But those objects don't even have a .text section...

> 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.
> 
> Tested on x86_64-linux, ok for trunk?

Can't we simply always append a .note.GNU-stack to indicate a
non-executable stack on debug objects?  Or as you say clear
SHF_EXECINSTR (whatever that means)?

That would also be more portable to non-GNU targets?

After all the note doesn't really tell anything about the debug part
but about the fat part (or the pointless .text section we always emit
in slim objects).

Richard.

> 2017-10-18  Jakub Jelinek  
> 
>   PR lto/82598
>   * simple-object.c (handle_lto_debug_sections): Copy over also
>   .note.GNU-stack section with unchanged name.
> 
> --- libiberty/simple-object.c.jj  2017-09-01 09:27:00.0 +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;
>  }
>  
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)