[v2,11/12] Fix the ld flags not be applied to tst-execstack-mod.so

Message ID 5d131e1e95cc613c64df3a91dbce86bd2c3fb1b0.1531801545.git.zong@andestech.com
State New, archived
Headers

Commit Message

Zong Li July 17, 2018, 5:07 a.m. UTC
  Fix the ld flags for applying the 'execstack' option correctly.
---
 elf/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Joseph Myers July 17, 2018, 10:11 p.m. UTC | #1
On Tue, 17 Jul 2018, Zong Li wrote:

> Fix the ld flags for applying the 'execstack' option correctly.

Could you give a more detailed explanation?  Presumably it's of the form 
that a particular file was intended to be linked with particular options, 
but that's ineffective because of the incorrect Makefile variable name.

Please also include GNU-style ChangeLog entries with patch submissions.

As with other such changes, I think this is not logically part of the new 
port and so should not be included in the patch series.  However, it's the 
sort of safe bug fix that should be appropriate during the freeze, given a 
suitable commit message and ChangeLog entry.
  
Zong Li July 18, 2018, 5:19 a.m. UTC | #2
Joseph Myers <joseph@codesourcery.com> 於 2018年7月18日 週三 上午6:12寫道:
>
> On Tue, 17 Jul 2018, Zong Li wrote:
>
> > Fix the ld flags for applying the 'execstack' option correctly.
>
> Could you give a more detailed explanation?  Presumably it's of the form
> that a particular file was intended to be linked with particular options,
> but that's ineffective because of the incorrect Makefile variable name.
>
> Please also include GNU-style ChangeLog entries with patch submissions.

OK, I will add the ChangeLog on next version.

> As with other such changes, I think this is not logically part of the new
> port and so should not be included in the patch series.  However, it's the
> sort of safe bug fix that should be appropriate during the freeze, given a
> suitable commit message and ChangeLog entry.
>

OK, I will separate this patch. I think there is another patch
'[PATCH v2 12/12] Change URL of gcc's tarball' should be separate and
be included during the freeze, because it cannot obtain the new
version of gcc from ftp.


> --
> Joseph S. Myers
> joseph@codesourcery.com
  
Zong Li July 18, 2018, 9:03 a.m. UTC | #3
Zong Li <zongbox@gmail.com> 於 2018年7月18日 週三 下午1:19寫道:
>
> Joseph Myers <joseph@codesourcery.com> 於 2018年7月18日 週三 上午6:12寫道:
> >
> > On Tue, 17 Jul 2018, Zong Li wrote:
> >
> > > Fix the ld flags for applying the 'execstack' option correctly.
> >
> > Could you give a more detailed explanation?  Presumably it's of the form
> > that a particular file was intended to be linked with particular options,
> > but that's ineffective because of the incorrect Makefile variable name.
> >
> > Please also include GNU-style ChangeLog entries with patch submissions.
>
> OK, I will add the ChangeLog on next version.
>

For the test cases elf/tst-execstack, elf/tst-execstack-needed and
nptl/tst-execstack,
they test the stack execution privilege through the shared object
tst-execstack-mod.so.

In tst-execstack-mod.so, it run the nested function on stack to ensure
stack can be executed.

Normally, this shared object will be fed the linker option
'-Wl,-z,execstack' to open the stack
execution privilege, but the incorrect Makefile variable name causes
the option not be
applied to this shared object.

On other architectures, they still can pass this case because the
compiler generate the
GNU_STACK segment and give the execution privilege if there is nested function,
but the RISC-V gcc not implement this mechanism.

This gcc issue had been resolved on 7/14.
https://gcc.gnu.org/ml/gcc-patches/2018-07/msg00755.html
So we can pass this case on RISC-V port without this modification in Makefile.

But the incorrect variable name is still there, I think we can correct it.

> > As with other such changes, I think this is not logically part of the new
> > port and so should not be included in the patch series.  However, it's the
> > sort of safe bug fix that should be appropriate during the freeze, given a
> > suitable commit message and ChangeLog entry.
> >
>
> OK, I will separate this patch. I think there is another patch
> '[PATCH v2 12/12] Change URL of gcc's tarball' should be separate and
> be included during the freeze, because it cannot obtain the new
> version of gcc from ftp.
>
>
> > --
> > Joseph S. Myers
> > joseph@codesourcery.com
  

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 41cc368..15483a9 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