Message ID | 1532081682-25895-1-git-send-email-zong@andestech.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 109995 invoked by alias); 20 Jul 2018 10:15:31 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 109010 invoked by uid 89); 20 Jul 2018 10:15:06 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.7 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=loses, Hx-languages-length:1376, H*r:0800 X-HELO: ATCSQR.andestech.com From: Zong Li <zong@andestech.com> To: <joseph@codesourcery.com>, <libc-alpha@sourceware.org> CC: <zongbox@gmail.com>, Zong Li <zong@andestech.com> Subject: [PATCH] Fix the ld flags not be applied to tst-execstack-mod.so Date: Fri, 20 Jul 2018 18:14:42 +0800 Message-ID: <1532081682-25895-1-git-send-email-zong@andestech.com> MIME-Version: 1.0 Content-Type: text/plain X-DNSRBL: X-MAIL: ATCSQR.andestech.com w6KAFUS9040949 |
Commit Message
Zong Li
July 20, 2018, 10:14 a.m. UTC
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(-)
Comments
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
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