Fix the ld flags not be applied to tst-execstack-mod.so

The Makefile variable name loses the file extension (.so). It causes
the linker option not applies to the corresponding file that it's
file name matchs with the variable without LDFLAGS- prefix.

	* elf/Makefile (LDFLAGS-tst-execstack-mod): Change variable name by
	adding the file extension (.so).
---
 ChangeLog    | 5 +++++
 elf/Makefile | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

On 07/20/2018 03:14 AM, Zong Li wrote:
> The Makefile variable name loses the file extension (.so). It causes
> the linker option not applies to the corresponding file that it's
> file name matchs with the variable without LDFLAGS- prefix.

Hi,

Chiming in as I'm looking into these things myself in context of testing for ARC
port submission. Do we really need to fix this part - in this way. I'd vote to not
force the execstack through linker and rely on gcc generating this itself when it
knows it doing something for trampolines. And only if target gcc doesn't support
it (detected via configure test) should this be done.

-Vineet

> 
> 	* elf/Makefile (LDFLAGS-tst-execstack-mod): Change variable name by
> 	adding the file extension (.so).
> ---
>  ChangeLog    | 5 +++++
>  elf/Makefile | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index b45c83b..f87b32c 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2018-07-20  Zong Li  <zong@andestech.com>
> +
> +	* elf/Makefile (LDFLAGS-tst-execstack-mod): Change variable name by
> +	adding the file extension (.so).
> +
>  2018-07-20  Samuel Thibault  <samuel.thibault@ens-lyon.org>
>  
>  	* sysdeps/mach/hurd/i386/tls.h (_hurd_tls_init): Set multiple_threads
> diff --git a/elf/Makefile b/elf/Makefile
> index cd07713..ecc8ea2 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -1004,7 +1004,7 @@ $(objpfx)tst-execstack: $(libdl)
>  $(objpfx)tst-execstack.out: $(objpfx)tst-execstack-mod.so
>  CPPFLAGS-tst-execstack.c += -DUSE_PTHREADS=0
>  LDFLAGS-tst-execstack = -Wl,-z,noexecstack
> -LDFLAGS-tst-execstack-mod = -Wl,-z,execstack
> +LDFLAGS-tst-execstack-mod.so = -Wl,-z,execstack
>  
>  $(objpfx)tst-execstack-needed: $(objpfx)tst-execstack-mod.so
>  LDFLAGS-tst-execstack-needed = -Wl,-z,noexecstack
>

On Wed, 25 Jul 2018, Vineet Gupta wrote:

> Chiming in as I'm looking into these things myself in context of testing 
> for ARC port submission. Do we really need to fix this part - in this 
> way. I'd vote to not force the execstack through linker and rely on gcc 
> generating this itself when it knows it doing something for trampolines. 
> And only if target gcc doesn't support it (detected via configure test) 
> should this be done.

The whole point of this test is to verify executable stack changes on 
dlopen of a module requiring an executable stack.  Thus, for this test to 
work correctly, this module must be forced to have a marking as needing an 
executable stack.

On 07/25/2018 01:22 PM, Joseph Myers wrote:
> On Wed, 25 Jul 2018, Vineet Gupta wrote:
>
>> Chiming in as I'm looking into these things myself in context of testing 
>> for ARC port submission. Do we really need to fix this part - in this 
>> way. I'd vote to not force the execstack through linker and rely on gcc 
>> generating this itself when it knows it doing something for trampolines. 
>> And only if target gcc doesn't support it (detected via configure test) 
>> should this be done.
> The whole point of this test is to verify executable stack changes on 
> dlopen of a module requiring an executable stack.

I'm not contesting that at all. For this exact test, I recently had to fix a
kernel bug for allowing mprotect(PROT_EXEC) stack mappings on ARC.

>   Thus, for this test to 
> work correctly, this module must be forced to have a marking as needing an 
> executable stack.

But given the dso code has nested function, a well equipped gcc when generating
trampoline code would also generate the GNU_STACK segment for the dso
automatically, without need to force the same via the linker flags as is done in
the Makefile currently. Only when a port's gcc doesn't support this (ARC gcc
didn't until recently), should we need ld assist, and this could be detected using
a configure test - No ?

On Wed, 25 Jul 2018, Vineet Gupta wrote:

> But given the dso code has nested function, a well equipped gcc when generating
> trampoline code would also generate the GNU_STACK segment for the dso
> automatically, without need to force the same via the linker flags as is done in
> the Makefile currently. Only when a port's gcc doesn't support this (ARC gcc
> didn't until recently), should we need ld assist, and this could be detected using
> a configure test - No ?

The test should achieve the executable stack markings independent of 
whether nested functions require them on that architecture (for example, 
it should still have the markings on architectures with function 
descriptors).

Joseph Myers <joseph@codesourcery.com> 於 2018年7月26日 週四 上午4:56寫道：
>
> On Wed, 25 Jul 2018, Vineet Gupta wrote:
>
> > But given the dso code has nested function, a well equipped gcc when generating
> > trampoline code would also generate the GNU_STACK segment for the dso
> > automatically, without need to force the same via the linker flags as is done in
> > the Makefile currently. Only when a port's gcc doesn't support this (ARC gcc
> > didn't until recently), should we need ld assist, and this could be detected using
> > a configure test - No ?
>
> The test should achieve the executable stack markings independent of
> whether nested functions require them on that architecture (for example,
> it should still have the markings on architectures with function
> descriptors).
>
In glibc now, this option doesn't pass to linker, the module is still
not executable on stack.
I think that we need this patch to fix up it or another patch to
remove the variable in Makefile
if it is not necessary. Both are fine for me. For now, it is a little
bit strange because it present
in Makefile but has no effect.

On Thu, 26 Jul 2018, Zong Li wrote:

> In glibc now, this option doesn't pass to linker, the module is still 
> not executable on stack. I think that we need this patch to fix up it or 
> another patch to remove the variable in Makefile if it is not necessary. 
> Both are fine for me. For now, it is a little bit strange because it 
> present in Makefile but has no effect.

I think we should have the option in a properly named variable.  But the 
patch will need review and we're currently in a release freeze.

Joseph Myers <joseph@codesourcery.com> 於 2018年7月26日 週四 下午11:01寫道：
>
> On Thu, 26 Jul 2018, Zong Li wrote:
>
> > In glibc now, this option doesn't pass to linker, the module is still
> > not executable on stack. I think that we need this patch to fix up it or
> > another patch to remove the variable in Makefile if it is not necessary.
> > Both are fine for me. For now, it is a little bit strange because it
> > present in Makefile but has no effect.
>
> I think we should have the option in a properly named variable.  But the
> patch will need review and we're currently in a release freeze.
>

Hi Joseph,

We fix up the all failures of math cases on RV32 and we are going to
submit the next version of RV32 port.
If this patch is suitable, I think we need it to pass
elf/tst-execstack and elf/tst-execstack-needed cases.

On Mon, 15 Oct 2018, Zong Li wrote:

> If this patch is suitable, I think we need it to pass
> elf/tst-execstack and elf/tst-execstack-needed cases.

If you feel a patch has not been reviewed promptly, you should ping it 
weekly (outside of the freeze period), making sure to include the URL to 
the original patch posting in the ping messages.

https://sourceware.org/glibc/wiki/Contribution%20checklist#Ping_and_Keep_Pinging

Zong Li <zong@andestech.com> 於 2018年7月20日 週五 下午6:14寫道：
>
> The Makefile variable name loses the file extension (.so). It causes
> the linker option not applies to the corresponding file that it's
> file name matchs with the variable without LDFLAGS- prefix.
>
>         * elf/Makefile (LDFLAGS-tst-execstack-mod): Change variable name by
>         adding the file extension (.so).
> ---
>  ChangeLog    | 5 +++++
>  elf/Makefile | 2 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index b45c83b..f87b32c 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,8 @@
> +2018-07-20  Zong Li  <zong@andestech.com>
> +
> +       * elf/Makefile (LDFLAGS-tst-execstack-mod): Change variable name by
> +       adding the file extension (.so).
> +
>  2018-07-20  Samuel Thibault  <samuel.thibault@ens-lyon.org>
>
>         * sysdeps/mach/hurd/i386/tls.h (_hurd_tls_init): Set multiple_threads
> diff --git a/elf/Makefile b/elf/Makefile
> index cd07713..ecc8ea2 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -1004,7 +1004,7 @@ $(objpfx)tst-execstack: $(libdl)
>  $(objpfx)tst-execstack.out: $(objpfx)tst-execstack-mod.so
>  CPPFLAGS-tst-execstack.c += -DUSE_PTHREADS=0
>  LDFLAGS-tst-execstack = -Wl,-z,noexecstack
> -LDFLAGS-tst-execstack-mod = -Wl,-z,execstack
> +LDFLAGS-tst-execstack-mod.so = -Wl,-z,execstack
>
>  $(objpfx)tst-execstack-needed: $(objpfx)tst-execstack-mod.so
>  LDFLAGS-tst-execstack-needed = -Wl,-z,noexecstack
> --
> 2.7.4

ping

We leave the previous release freeze. I ping again here.

* Zong Li:

> The Makefile variable name loses the file extension (.so). It causes
> the linker option not applies to the corresponding file that it's
> file name matchs with the variable without LDFLAGS- prefix.
>
> 	* elf/Makefile (LDFLAGS-tst-execstack-mod): Change variable name by
> 	adding the file extension (.so).

This looks okay to me.  Can you commit this yourself, or do you need
someone who commits it for you?

Thanks,
Florian

Florian Weimer <fweimer@redhat.com> 於 2018年10月24日 週三 下午6:56寫道：
>
> * Zong Li:
>
> > The Makefile variable name loses the file extension (.so). It causes
> > the linker option not applies to the corresponding file that it's
> > file name matchs with the variable without LDFLAGS- prefix.
> >
> >       * elf/Makefile (LDFLAGS-tst-execstack-mod): Change variable name by
> >       adding the file extension (.so).
>
> This looks okay to me.  Can you commit this yourself, or do you need
> someone who commits it for you?
>

No, I can't commit it by myself.

* Zong Li:

> Florian Weimer <fweimer@redhat.com> 於 2018年10月24日 週三 下午6:56寫道：
>>
>> * Zong Li:
>>
>> > The Makefile variable name loses the file extension (.so). It causes
>> > the linker option not applies to the corresponding file that it's
>> > file name matchs with the variable without LDFLAGS- prefix.
>> >
>> >       * elf/Makefile (LDFLAGS-tst-execstack-mod): Change variable name by
>> >       adding the file extension (.so).
>>
>> This looks okay to me.  Can you commit this yourself, or do you need
>> someone who commits it for you?

> No, I can't commit it by myself.

I pushed it for you.  Thanks.

Florian