[committed] exec-stack warning for test which wants executable stacks

[committed] exec-stack warning for test which wants executable stacks

[Jeff Law]

[-- Attachment #1: Type: text/plain, Size: 1942 bytes --]

About a week ago many targets started failing pr94157_0.c test like this 
(bfin-elf, but many other targets are also affected):

> spawn -ignore SIGHUP /home/jlaw/test/obj/bfin-elf/obj/gcc/gcc/xgcc 
> -B/home/jlaw/test/obj/bfin-elf/obj/gcc/gcc/ c_lto_pr94157_0.o 
> -fdiagnostics-plain-output -dumpbase  -O0 -fipa-vrp -flto 
> -Wa,--noexecstack -Wa,--noexecstack -Wa,--execstack -Wa,--execstack 
> -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack 
> -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack 
> -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -msim 
> -Wl,-wrap,exit -Wl,-wrap,_exit -Wl,-wrap,main -Wl,-wrap,abort 
> -Wl,gcc_tg.o -o gcc-dg-lto-pr94157-01.exe^M
> /home/jlaw/test/obj/bfin-elf/installed/bfin-elf/bin/ld: warning: 
> /tmp/ccfJUEvZ.ltrans0.ltrans.o: requires executable stack (because the 
> .note.GNU-stack section is executable)^M
> FAIL: gcc.dg/lto/pr94157 c_lto_pr94157_0.o-c_lto_pr94157_0.o link,  
> -O0 -fipa-vrp -flto -Wa,--noexecstack -Wa,--noexecstack 
> -Wa,--execstack  -Wa,--execstack -Wa,--execstack -Wa,--execstack 
> -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack 
> -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack 
> -Wa,--execstack -Wa,--execstack

This is due to a new binutils warning.  This patch just suppresses the 
warning for the one test where we explicitly wanted an executable stack.

I'm guessing the repeated -Wa,--noexecstack options in this test are 
supposed to trigger a  buffer overflow or something similar, so I left 
those alone and just appended to the argument list.

I used -z execstack rather than --no-warn-execstack as the former is 
recognized by older versions of ld, but the latter is a new option.

The other approach would have been to prune the warning, but this seemed 
better since we'd like most tests to fail if somehow their stacks were 
executable.


Committed to the trunk.

Jeff


[-- Attachment #2: P --]
[-- Type: text/plain, Size: 1286 bytes --]

commit 6b7441a46c771aa6ecdc0c8ed96197417d036b9a
Author: Jeff Law <jeffreyalaw@gmail.com>
Date:   Sun Apr 24 13:38:14 2022 -0400

    [committed] exec-stack warning for test which wants executable stacks
    
    gcc/testsuite
            * gcc.dg/lto/pr94157_0.c: Also request executable stack from
            the linker.

diff --git a/gcc/testsuite/gcc.dg/lto/pr94157_0.c b/gcc/testsuite/gcc.dg/lto/pr94157_0.c
index a6e308b855b..a76141b1809 100644
--- a/gcc/testsuite/gcc.dg/lto/pr94157_0.c
+++ b/gcc/testsuite/gcc.dg/lto/pr94157_0.c
@@ -1,6 +1,6 @@
 /* { dg-lto-do link } */
 /* { dg-require-effective-target gas } */
-/* { dg-lto-options { { -O0 -fipa-vrp -flto -Wa,--noexecstack -Wa,--noexecstack -Wa,--execstack  -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack } } } */
+/* { dg-lto-options { { -O0 -fipa-vrp -flto -Wa,--noexecstack -Wa,--noexecstack -Wa,--execstack  -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wl,-z,execstack} } } */
 
 int main() {
 

Re: [committed] exec-stack warning for test which wants executable stacks

[Martin Liška]

On 4/24/22 19:42, Jeff Law via Gcc wrote:
> About a week ago many targets started failing pr94157_0.c test like this (bfin-elf, but many other targets are also affected):
> 
>> spawn -ignore SIGHUP /home/jlaw/test/obj/bfin-elf/obj/gcc/gcc/xgcc -B/home/jlaw/test/obj/bfin-elf/obj/gcc/gcc/ c_lto_pr94157_0.o -fdiagnostics-plain-output -dumpbase  -O0 -fipa-vrp -flto -Wa,--noexecstack -Wa,--noexecstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -msim -Wl,-wrap,exit -Wl,-wrap,_exit -Wl,-wrap,main -Wl,-wrap,abort -Wl,gcc_tg.o -o gcc-dg-lto-pr94157-01.exe^M
>> /home/jlaw/test/obj/bfin-elf/installed/bfin-elf/bin/ld: warning: /tmp/ccfJUEvZ.ltrans0.ltrans.o: requires executable stack (because the .note.GNU-stack section is executable)^M
>> FAIL: gcc.dg/lto/pr94157 c_lto_pr94157_0.o-c_lto_pr94157_0.o link,  -O0 -fipa-vrp -flto -Wa,--noexecstack -Wa,--noexecstack -Wa,--execstack  -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack -Wa,--execstack
> 
> This is due to a new binutils warning.  This patch just suppresses the warning for the one test where we explicitly wanted an executable stack.
> 
> I'm guessing the repeated -Wa,--noexecstack options in this test are supposed to trigger a  buffer overflow or something similar, so I left those alone and just appended to the argument list.

Yes.

> 
> I used -z execstack rather than --no-warn-execstack as the former is recognized by older versions of ld, but the latter is a new option.

Thanks for it.

Martin

> 
> The other approach would have been to prune the warning, but this seemed better since we'd like most tests to fail if somehow their stacks were executable.
> 
> 
> Committed to the trunk.
> 
> Jeff
> 

Re: [committed] exec-stack warning for test which wants executable stacks

[Jeff Law]

On 4/25/2022 6:56 AM, Martin Liška wrote:
>
>> I used -z execstack rather than --no-warn-execstack as the former is recognized by older versions of ld, but the latter is a new option.
> Thanks for it.
Unfortunately, I should have looked at the other failures that have 
popped up over the last week.  Essentially all the nested function tests 
are failing on some targets due to the same linker warning.

Either pruning or adding  the option to all those tests is going to be 
necessary ;(

jeff

Re: [committed] exec-stack warning for test which wants executable stacks

[Nick Clifton]

Hi Jeff,

>>> I used -z execstack rather than --no-warn-execstack as the former is recognized by older versions of ld, but the latter is a new option.
>> Thanks for it.
> Unfortunately, I should have looked at the other failures that have popped up over the last week.  Essentially all the nested function tests are failing on some targets due 
> to the same linker warning.
> 
> Either pruning or adding  the option to all those tests is going to be necessary ;(

Sorry for causing you so much hassle.  If it helps, Alan has already added
some pruning code to binutils/testsuite/lib/binutils-common.exp in the
prune_extra_warnings proc...

Cheers
   Nick

Re: [committed] exec-stack warning for test which wants executable stacks

[Jeff Law]

On 4/25/2022 8:42 AM, Nick Clifton wrote:
> Hi Jeff,
>
>>>> I used -z execstack rather than --no-warn-execstack as the former 
>>>> is recognized by older versions of ld, but the latter is a new option.
>>> Thanks for it.
>> Unfortunately, I should have looked at the other failures that have 
>> popped up over the last week.  Essentially all the nested function 
>> tests are failing on some targets due to the same linker warning.
>>
>> Either pruning or adding  the option to all those tests is going to 
>> be necessary ;(
>
> Sorry for causing you so much hassle.
No worries.  It's a very reasonable warning.  And it's only if you're 
testing with the latest binutils that it causes problems -- which is one 
of the goals of the tester -- to find this stuff when it happens, not 
6-12 months later.


>   If it helps, Alan has already added
> some pruning code to binutils/testsuite/lib/binutils-common.exp in the
> prune_extra_warnings proc...
Yea.  I saw that.  I ultimately decided against pruning as it would be 
useful to know if a test suddenly needs execstack.   That would 
generally be a sign of something gone horribly wrong.  I just (locally) 
added the magic to the rest of the affected tests and I'm testing it now.

jeff

Re: [committed] exec-stack warning for test which wants executable stacks

[Nick Clifton]

Hi Jeff,

   Just FYI - I am also looking at adding in another warning.  This time for
   when the linker creates a PT_LOAD segment which has all of the RWX flags
   set.  At the moment my testing seems to show that it only causes problems
   when a custom linker script is used that defines its own program headers
   and does not provide separate headers for code and data.

Cheers
   Nick

Re: [committed] exec-stack warning for test which wants executable stacks

[Jeff Law]

On 4/25/2022 9:26 AM, Nick Clifton wrote:
> Hi Jeff,
>
>   Just FYI - I am also looking at adding in another warning.  This 
> time for
>   when the linker creates a PT_LOAD segment which has all of the RWX 
> flags
>   set.  At the moment my testing seems to show that it only causes 
> problems
>   when a custom linker script is used that defines its own program 
> headers
>   and does not provide separate headers for code and data.
That also sounds quite reasonable.    My biggest worry would be the 
embedded targets which have their own linker scripts -- but like the 
executable stack warning, I think you should go for it and we'll deal 
with the fallout.

jeff

Re: [committed] exec-stack warning for test which wants executable stacks

[Hans-Peter Nilsson]

> From: Jeff Law via Binutils <binutils@sourceware.org>
> Date: Mon, 25 Apr 2022 17:30:59 +0200

> On 4/25/2022 9:26 AM, Nick Clifton wrote:
> > Hi Jeff,
> >
> >   Just FYI - I am also looking at adding in another warning.  This 
> > time for
> >   when the linker creates a PT_LOAD segment which has all of the RWX 
> > flags
> >   set.  At the moment my testing seems to show that it only causes 
> > problems
> >   when a custom linker script is used that defines its own program 
> > headers
> >   and does not provide separate headers for code and data.
> That also sounds quite reasonable.    My biggest worry would be the 
> embedded targets which have their own linker scripts -- but like the 
> executable stack warning, I think you should go for it and we'll deal 
> with the fallout.

(Looks like I'm first to be hit by the
--enable-warn-rwx-segments default, or the first one to
bother, lucky me.)

So, with my autotester for cris-elf not able to update
binutils on its own because of all the warnings in the gcc
test-suite (all code and data get a single LOAD header,
which naturally is RWX), let's discuss a method for a
binutils target to default turn off this
configure-time-default-on warning.  I just don't think its
reasonable to require an extra configure option for the
intended default behavior for a target.

How about the usual method, a line in the ld emulparams
file for the target?

brgds, H-P

Re: [committed] exec-stack warning for test which wants executable stacks

[Hans-Peter Nilsson]

> From: Hans-Peter Nilsson <Hans-Peter.Nilsson@axis.com>
> Date: Thu, 26 May 2022 03:17:01 +0200

Regarding setting the default for the RWX-segment warning
per-target:

> How about the usual method, a line in the ld emulparams
> file for the target?

JFTR: no extra infrastructure bits needed.  I found the
right spot, just a trivial clause in ld/configure.tgt.

brgds, H-P

Re: [committed] exec-stack warning for test which wants executable stacks

[Jeff Law]

On 4/25/2022 8:37 AM, Jeff Law wrote:
>
>
> On 4/25/2022 6:56 AM, Martin Liška wrote:
>>
>>> I used -z execstack rather than --no-warn-execstack as the former is 
>>> recognized by older versions of ld, but the latter is a new option.
>> Thanks for it.
> Unfortunately, I should have looked at the other failures that have 
> popped up over the last week.  Essentially all the nested function 
> tests are failing on some targets due to the same linker warning.
>
> Either pruning or adding  the option to all those tests is going to be 
> necessary ;(
And -z execstack is not universally accepted either.  In fact, we even 
have some elf targets that don't support it due to oversights (?) in 
their linker configurations.  But the key thing AFAICT is that -z 
execstack is an ELF specific flag (I didn't know that until just now).  
So we can't really depend on it across the board.  In fact, I wouldn't 
be surprised if AIX has regressed on the one file I adjusted earlier.

So I think that means we're stuck with the less desirable option of 
pruning the output.  I'll take care of it and reverting the change that 
added -z execstack to the one test.

jeff