[PATCH v2] ld: Turn on --error-execstack/--error-rwx-segments

[PATCH v2] ld: Turn on --error-execstack/--error-rwx-segments

[H.J. Lu]

Changes in v2:

1. Update the expected error in testsuite/ld-elf/pr31299-1.error to
allow different error messages.

---
Since --fatal-warnings always turns a warning to an error, turn on
--error-execstack for --warn-execstack and --error-rwx-segments for
--warn-rwx-segments if --fatal-warnings is used, overriding
--no-error-execstack and --no-error-rwx-segments.

	PR ld/31299
	* lexsup.c (parse_args): Turn on --error-execstack for
	--warn-execstack and --error-rwx-segments for --warn-rwx-segments
	if --fatal-warnings is used.
	* testsuite/ld-elf/elf.exp: Run PR ld/31299 tests.
	* testsuite/ld-elf/pr31299-1.error: New file.
	* testsuite/ld-elf/pr31299-2.error: Likewise.
	* testsuite/ld-elf/pr31299-3.error: Likewise.
---
 ld/lexsup.c                         | 11 +++++++++++
 ld/testsuite/ld-elf/elf.exp         | 26 +++++++++++++++++++++++++-
 ld/testsuite/ld-elf/pr31299-1.error |  2 ++
 ld/testsuite/ld-elf/pr31299-2.error |  1 +
 ld/testsuite/ld-elf/pr31299-3.error |  1 +
 5 files changed, 40 insertions(+), 1 deletion(-)
 create mode 100644 ld/testsuite/ld-elf/pr31299-1.error
 create mode 100644 ld/testsuite/ld-elf/pr31299-2.error
 create mode 100644 ld/testsuite/ld-elf/pr31299-3.error

diff --git a/ld/lexsup.c b/ld/lexsup.c
index 099dff8ecde..787d3d02e51 100644
--- a/ld/lexsup.c
+++ b/ld/lexsup.c
@@ -1947,6 +1947,17 @@ parse_args (unsigned argc, char **argv)
       && command_line.check_section_addresses < 0)
     command_line.check_section_addresses = 0;
 
+  /* Override --no-error-execstack and --no-warn-execstack and turn on
+     --error-execstack for --warn-execstack and --error-rwx-segments for
+     --warn-rwx-segments if --fatal-warnings is used.  */
+  if (config.fatal_warnings)
+    {
+      if (link_info.warn_execstack)
+	link_info.error_execstack = 1;
+      if (!link_info.no_warn_rwx_segments)
+	link_info.warn_is_error_for_rwx_segments = 1;
+    }
+
   if (export_list)
     {
       struct bfd_elf_version_expr *head = export_list->head.list;
diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
index 685b87588e7..6ece36b7931 100644
--- a/ld/testsuite/ld-elf/elf.exp
+++ b/ld/testsuite/ld-elf/elf.exp
@@ -305,7 +305,23 @@ if {   [istarget *-*-*linux*]
 	    {nobits-1.s} \
 	     {} \
 	    "rwx-segments-3.exe"] \
-	  ]
+	[list "Ensure that an error issued when creating a segment with RWX permissions" \
+	    "-e 0 -Tnobits-1.t --warn-rwx-segments \
+	     --fatal-warnings --no-error-rwx-segments" \
+	    "" \
+	    "" \
+	    {nobits-1.s} \
+	    {{ld pr31299-2.error}} \
+	    "pr31299-2.exe"] \
+	[list "Ensure that an error issued when creating a TLS segment with execute permission" \
+	    "-e 0 -T rwx-segments-2.t --warn-rwx-segments \
+	     --fatal-warnings --no-error-rwx-segments" \
+	    "" \
+	    "" \
+	    {size-2.s} \
+	    {{ld pr31299-3.error}} \
+	    "pr31299-3.exe"] \
+    ]
 
     set LDFLAGS $curr_ldflags
 
@@ -318,6 +334,14 @@ if {   [istarget *-*-*linux*]
 	    {pr29072-b.s} \
 	    {{ld pr29072.b.warn}} \
 	    "pr29072-b.exe"] \
+	   [list "PR ld/31299 (error about absent .note.GNU-stack)" \
+	    "-e 0 -z stack-size=0x123400 --warn-execstack \
+	     --fatal-warnings --no-error-execstack" \
+	    "" \
+	    "" \
+	    {pr29072-b.s} \
+	    {{ld pr31299-1.error}} \
+	    "pr31299-1.exe"] \
 	]
     } else {
 	run_ld_link_tests [list \
diff --git a/ld/testsuite/ld-elf/pr31299-1.error b/ld/testsuite/ld-elf/pr31299-1.error
new file mode 100644
index 00000000000..eb2598df3e6
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr31299-1.error
@@ -0,0 +1,2 @@
+.*: error: .*\.o: is triggering the generation of an executable stack because it does not have a \.note\.GNU-stack section
+.*: failed to set dynamic section sizes: .*
diff --git a/ld/testsuite/ld-elf/pr31299-2.error b/ld/testsuite/ld-elf/pr31299-2.error
new file mode 100644
index 00000000000..e5b73657069
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr31299-2.error
@@ -0,0 +1 @@
+.*: error: .* has a LOAD segment with RWX permissions
diff --git a/ld/testsuite/ld-elf/pr31299-3.error b/ld/testsuite/ld-elf/pr31299-3.error
new file mode 100644
index 00000000000..b9ba368ba2c
--- /dev/null
+++ b/ld/testsuite/ld-elf/pr31299-3.error
@@ -0,0 +1 @@
+.*: error: .* has a TLS segment with execute permission
-- 
2.43.0

Re: [PATCH v2] ld: Turn on --error-execstack/--error-rwx-segments

[H.J. Lu]

On Fri, Jan 26, 2024 at 1:45 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> Changes in v2:
>
> 1. Update the expected error in testsuite/ld-elf/pr31299-1.error to
> allow different error messages.
>
> ---
> Since --fatal-warnings always turns a warning to an error, turn on
> --error-execstack for --warn-execstack and --error-rwx-segments for
> --warn-rwx-segments if --fatal-warnings is used, overriding
> --no-error-execstack and --no-error-rwx-segments.
>
>         PR ld/31299
>         * lexsup.c (parse_args): Turn on --error-execstack for
>         --warn-execstack and --error-rwx-segments for --warn-rwx-segments
>         if --fatal-warnings is used.
>         * testsuite/ld-elf/elf.exp: Run PR ld/31299 tests.
>         * testsuite/ld-elf/pr31299-1.error: New file.
>         * testsuite/ld-elf/pr31299-2.error: Likewise.
>         * testsuite/ld-elf/pr31299-3.error: Likewise.
> ---
>  ld/lexsup.c                         | 11 +++++++++++
>  ld/testsuite/ld-elf/elf.exp         | 26 +++++++++++++++++++++++++-
>  ld/testsuite/ld-elf/pr31299-1.error |  2 ++
>  ld/testsuite/ld-elf/pr31299-2.error |  1 +
>  ld/testsuite/ld-elf/pr31299-3.error |  1 +
>  5 files changed, 40 insertions(+), 1 deletion(-)
>  create mode 100644 ld/testsuite/ld-elf/pr31299-1.error
>  create mode 100644 ld/testsuite/ld-elf/pr31299-2.error
>  create mode 100644 ld/testsuite/ld-elf/pr31299-3.error
>
> diff --git a/ld/lexsup.c b/ld/lexsup.c
> index 099dff8ecde..787d3d02e51 100644
> --- a/ld/lexsup.c
> +++ b/ld/lexsup.c
> @@ -1947,6 +1947,17 @@ parse_args (unsigned argc, char **argv)
>        && command_line.check_section_addresses < 0)
>      command_line.check_section_addresses = 0;
>
> +  /* Override --no-error-execstack and --no-warn-execstack and turn on
> +     --error-execstack for --warn-execstack and --error-rwx-segments for
> +     --warn-rwx-segments if --fatal-warnings is used.  */
> +  if (config.fatal_warnings)
> +    {
> +      if (link_info.warn_execstack)
> +       link_info.error_execstack = 1;
> +      if (!link_info.no_warn_rwx_segments)
> +       link_info.warn_is_error_for_rwx_segments = 1;
> +    }
> +
>    if (export_list)
>      {
>        struct bfd_elf_version_expr *head = export_list->head.list;
> diff --git a/ld/testsuite/ld-elf/elf.exp b/ld/testsuite/ld-elf/elf.exp
> index 685b87588e7..6ece36b7931 100644
> --- a/ld/testsuite/ld-elf/elf.exp
> +++ b/ld/testsuite/ld-elf/elf.exp
> @@ -305,7 +305,23 @@ if {   [istarget *-*-*linux*]
>             {nobits-1.s} \
>              {} \
>             "rwx-segments-3.exe"] \
> -         ]
> +       [list "Ensure that an error issued when creating a segment with RWX permissions" \
> +           "-e 0 -Tnobits-1.t --warn-rwx-segments \
> +            --fatal-warnings --no-error-rwx-segments" \
> +           "" \
> +           "" \
> +           {nobits-1.s} \
> +           {{ld pr31299-2.error}} \
> +           "pr31299-2.exe"] \
> +       [list "Ensure that an error issued when creating a TLS segment with execute permission" \
> +           "-e 0 -T rwx-segments-2.t --warn-rwx-segments \
> +            --fatal-warnings --no-error-rwx-segments" \
> +           "" \
> +           "" \
> +           {size-2.s} \
> +           {{ld pr31299-3.error}} \
> +           "pr31299-3.exe"] \
> +    ]
>
>      set LDFLAGS $curr_ldflags
>
> @@ -318,6 +334,14 @@ if {   [istarget *-*-*linux*]
>             {pr29072-b.s} \
>             {{ld pr29072.b.warn}} \
>             "pr29072-b.exe"] \
> +          [list "PR ld/31299 (error about absent .note.GNU-stack)" \
> +           "-e 0 -z stack-size=0x123400 --warn-execstack \
> +            --fatal-warnings --no-error-execstack" \
> +           "" \
> +           "" \
> +           {pr29072-b.s} \
> +           {{ld pr31299-1.error}} \
> +           "pr31299-1.exe"] \
>         ]
>      } else {
>         run_ld_link_tests [list \
> diff --git a/ld/testsuite/ld-elf/pr31299-1.error b/ld/testsuite/ld-elf/pr31299-1.error
> new file mode 100644
> index 00000000000..eb2598df3e6
> --- /dev/null
> +++ b/ld/testsuite/ld-elf/pr31299-1.error
> @@ -0,0 +1,2 @@
> +.*: error: .*\.o: is triggering the generation of an executable stack because it does not have a \.note\.GNU-stack section
> +.*: failed to set dynamic section sizes: .*
> diff --git a/ld/testsuite/ld-elf/pr31299-2.error b/ld/testsuite/ld-elf/pr31299-2.error
> new file mode 100644
> index 00000000000..e5b73657069
> --- /dev/null
> +++ b/ld/testsuite/ld-elf/pr31299-2.error
> @@ -0,0 +1 @@
> +.*: error: .* has a LOAD segment with RWX permissions
> diff --git a/ld/testsuite/ld-elf/pr31299-3.error b/ld/testsuite/ld-elf/pr31299-3.error
> new file mode 100644
> index 00000000000..b9ba368ba2c
> --- /dev/null
> +++ b/ld/testsuite/ld-elf/pr31299-3.error
> @@ -0,0 +1 @@
> +.*: error: .* has a TLS segment with execute permission
> --
> 2.43.0
>

If there is no objection, I will check it tomorrow.

Thanks.

-- 
H.J.

Re: [PATCH v2] ld: Turn on --error-execstack/--error-rwx-segments

[Jan Beulich]

On 26.01.2024 22:45, H.J. Lu wrote:
> --- a/ld/lexsup.c
> +++ b/ld/lexsup.c
> @@ -1947,6 +1947,17 @@ parse_args (unsigned argc, char **argv)
>        && command_line.check_section_addresses < 0)
>      command_line.check_section_addresses = 0;
>  
> +  /* Override --no-error-execstack and --no-warn-execstack and turn on
> +     --error-execstack for --warn-execstack and --error-rwx-segments for
> +     --warn-rwx-segments if --fatal-warnings is used.  */
> +  if (config.fatal_warnings)
> +    {
> +      if (link_info.warn_execstack)
> +	link_info.error_execstack = 1;
> +      if (!link_info.no_warn_rwx_segments)
> +	link_info.warn_is_error_for_rwx_segments = 1;
> +    }
> +
>    if (export_list)
>      {
>        struct bfd_elf_version_expr *head = export_list->head.list;

If I'm not mistaken and if the comment is properly describing things,
this placement of the addition means --no-* last on the command line
wouldn't be honored anymore, when later arguments ought to override
earlier ones.

Jan

Re: [PATCH v2] ld: Turn on --error-execstack/--error-rwx-segments

[Jan Beulich]

On 28.01.2024 16:04, H.J. Lu wrote:
> If there is no objection, I will check it tomorrow.

I'm surprised here: Shouldn't a non-trivial (small != trivial) change
like this require active approval? At the very least I can only once
again mention that waiting for just a single day is too little, imo.

Jan

Re: [PATCH v2] ld: Turn on --error-execstack/--error-rwx-segments

[H.J. Lu]

On Sun, Jan 28, 2024 at 11:33 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 28.01.2024 16:04, H.J. Lu wrote:
> > If there is no objection, I will check it tomorrow.
>
> I'm surprised here: Shouldn't a non-trivial (small != trivial) change
> like this require active approval? At the very least I can only once
> again mention that waiting for just a single day is too little, imo.
>

My patch changes a "warning":

[hjl@gnu-cfl-3 ld]$ touch x.s
[hjl@gnu-cfl-3 ld]$ gcc -c x.s
[hjl@gnu-cfl-3 ld]$ ld -shared -z stack-size=0x123400 --fatal-warnings
--warn-execstack --no-error-execstack  x.o
ld: warning: x.o: missing .note.GNU-stack section implies executable stack
ld: NOTE: This behaviour is deprecated and will be removed in a future
version of the linker
[hjl@gnu-cfl-3 ld]$ echo $?
1
[hjl@gnu-cfl-3 ld]$

to an error:

[hjl@gnu-cfl-3 ld]$ ./ld-new -shared -z stack-size=0x123400
--fatal-warnings --warn-execstack --no-error-execstack  x.o
./ld-new: error: x.o: is triggering the generation of an executable
stack because it does not have a .note.GNU-stack section
./ld-new: failed to set dynamic section sizes: file format not recognized
[hjl@gnu-cfl-3 ld]$

I view it as trivial since it only corrects the incorrect linker
message.

-- 
H.J.

Re: [PATCH v2] ld: Turn on --error-execstack/--error-rwx-segments

[H.J. Lu]

On Sun, Jan 28, 2024 at 11:30 PM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 26.01.2024 22:45, H.J. Lu wrote:
> > --- a/ld/lexsup.c
> > +++ b/ld/lexsup.c
> > @@ -1947,6 +1947,17 @@ parse_args (unsigned argc, char **argv)
> >        && command_line.check_section_addresses < 0)
> >      command_line.check_section_addresses = 0;
> >
> > +  /* Override --no-error-execstack and --no-warn-execstack and turn on
> > +     --error-execstack for --warn-execstack and --error-rwx-segments for
> > +     --warn-rwx-segments if --fatal-warnings is used.  */
> > +  if (config.fatal_warnings)
> > +    {
> > +      if (link_info.warn_execstack)
> > +     link_info.error_execstack = 1;
> > +      if (!link_info.no_warn_rwx_segments)
> > +     link_info.warn_is_error_for_rwx_segments = 1;
> > +    }
> > +
> >    if (export_list)
> >      {
> >        struct bfd_elf_version_expr *head = export_list->head.list;
>
> If I'm not mistaken and if the comment is properly describing things,
> this placement of the addition means --no-* last on the command line
> wouldn't be honored anymore, when later arguments ought to override
> earlier ones.
>

Before my change, we got

[hjl@gnu-cfl-3 ld]$ touch x.s
[hjl@gnu-cfl-3 ld]$ gcc -c x.s
[hjl@gnu-cfl-3 ld]$ ld -shared -z stack-size=0x123400 --fatal-warnings
--warn-execstack --no-error-execstack  x.o
ld: warning: x.o: missing .note.GNU-stack section implies executable stack
ld: NOTE: This behaviour is deprecated and will be removed in a future
version of the linker
[hjl@gnu-cfl-3 ld]$ echo $?
1

The warning is misleading since --fatal-warnings IS position independent
and is always fatal for ANY warning.  My patch changes it to

[hjl@gnu-cfl-3 ld]$ ./ld-new -shared -z stack-size=0x123400
--fatal-warnings --warn-execstack --no-error-execstack  x.o
./ld-new: error: x.o: is triggering the generation of an executable
stack because it does not have a .note.GNU-stack section
./ld-new: failed to set dynamic section sizes: file format not recognized
[hjl@gnu-cfl-3 ld]$

Any other comments to my patch?

Thanks.

-- 
H.J.

Re: [PATCH v2] ld: Turn on --error-execstack/--error-rwx-segments

[Jan Beulich]

On 29.01.2024 14:03, H.J. Lu wrote:
> On Sun, Jan 28, 2024 at 11:30 PM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 26.01.2024 22:45, H.J. Lu wrote:
>>> --- a/ld/lexsup.c
>>> +++ b/ld/lexsup.c
>>> @@ -1947,6 +1947,17 @@ parse_args (unsigned argc, char **argv)
>>>        && command_line.check_section_addresses < 0)
>>>      command_line.check_section_addresses = 0;
>>>
>>> +  /* Override --no-error-execstack and --no-warn-execstack and turn on
>>> +     --error-execstack for --warn-execstack and --error-rwx-segments for
>>> +     --warn-rwx-segments if --fatal-warnings is used.  */
>>> +  if (config.fatal_warnings)
>>> +    {
>>> +      if (link_info.warn_execstack)
>>> +     link_info.error_execstack = 1;
>>> +      if (!link_info.no_warn_rwx_segments)
>>> +     link_info.warn_is_error_for_rwx_segments = 1;
>>> +    }
>>> +
>>>    if (export_list)
>>>      {
>>>        struct bfd_elf_version_expr *head = export_list->head.list;
>>
>> If I'm not mistaken and if the comment is properly describing things,
>> this placement of the addition means --no-* last on the command line
>> wouldn't be honored anymore, when later arguments ought to override
>> earlier ones.
>>
> 
> Before my change, we got
> 
> [hjl@gnu-cfl-3 ld]$ touch x.s
> [hjl@gnu-cfl-3 ld]$ gcc -c x.s
> [hjl@gnu-cfl-3 ld]$ ld -shared -z stack-size=0x123400 --fatal-warnings
> --warn-execstack --no-error-execstack  x.o
> ld: warning: x.o: missing .note.GNU-stack section implies executable stack
> ld: NOTE: This behaviour is deprecated and will be removed in a future
> version of the linker
> [hjl@gnu-cfl-3 ld]$ echo $?
> 1
> 
> The warning is misleading since --fatal-warnings IS position independent
> and is always fatal for ANY warning.  My patch changes it to
> 
> [hjl@gnu-cfl-3 ld]$ ./ld-new -shared -z stack-size=0x123400
> --fatal-warnings --warn-execstack --no-error-execstack  x.o
> ./ld-new: error: x.o: is triggering the generation of an executable
> stack because it does not have a .note.GNU-stack section
> ./ld-new: failed to set dynamic section sizes: file format not recognized
> [hjl@gnu-cfl-3 ld]$
> 
> Any other comments to my patch?

Well. To me, it is ambiguous what "--fatal-warnings --warn-execstack
--no-error-execstack" actually is intended to mean (and that's regardless
of --fatal-warnings position on the command line). One way of
interpreting is your way. The other is to say that --no-error-execstack
means "no error for anything related to executable stacks", i.e.
--fatal-warnings not affecting the overall result (when there are no
other warnings).

Jan

Re: [PATCH v2] ld: Turn on --error-execstack/--error-rwx-segments

[H.J. Lu]

On Mon, Jan 29, 2024 at 6:03 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 29.01.2024 14:03, H.J. Lu wrote:
> > On Sun, Jan 28, 2024 at 11:30 PM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> On 26.01.2024 22:45, H.J. Lu wrote:
> >>> --- a/ld/lexsup.c
> >>> +++ b/ld/lexsup.c
> >>> @@ -1947,6 +1947,17 @@ parse_args (unsigned argc, char **argv)
> >>>        && command_line.check_section_addresses < 0)
> >>>      command_line.check_section_addresses = 0;
> >>>
> >>> +  /* Override --no-error-execstack and --no-warn-execstack and turn on
> >>> +     --error-execstack for --warn-execstack and --error-rwx-segments for
> >>> +     --warn-rwx-segments if --fatal-warnings is used.  */
> >>> +  if (config.fatal_warnings)
> >>> +    {
> >>> +      if (link_info.warn_execstack)
> >>> +     link_info.error_execstack = 1;
> >>> +      if (!link_info.no_warn_rwx_segments)
> >>> +     link_info.warn_is_error_for_rwx_segments = 1;
> >>> +    }
> >>> +
> >>>    if (export_list)
> >>>      {
> >>>        struct bfd_elf_version_expr *head = export_list->head.list;
> >>
> >> If I'm not mistaken and if the comment is properly describing things,
> >> this placement of the addition means --no-* last on the command line
> >> wouldn't be honored anymore, when later arguments ought to override
> >> earlier ones.
> >>
> >
> > Before my change, we got
> >
> > [hjl@gnu-cfl-3 ld]$ touch x.s
> > [hjl@gnu-cfl-3 ld]$ gcc -c x.s
> > [hjl@gnu-cfl-3 ld]$ ld -shared -z stack-size=0x123400 --fatal-warnings
> > --warn-execstack --no-error-execstack  x.o
> > ld: warning: x.o: missing .note.GNU-stack section implies executable stack
> > ld: NOTE: This behaviour is deprecated and will be removed in a future
> > version of the linker
> > [hjl@gnu-cfl-3 ld]$ echo $?
> > 1
> >
> > The warning is misleading since --fatal-warnings IS position independent
> > and is always fatal for ANY warning.  My patch changes it to
> >
> > [hjl@gnu-cfl-3 ld]$ ./ld-new -shared -z stack-size=0x123400
> > --fatal-warnings --warn-execstack --no-error-execstack  x.o
> > ./ld-new: error: x.o: is triggering the generation of an executable
> > stack because it does not have a .note.GNU-stack section
> > ./ld-new: failed to set dynamic section sizes: file format not recognized
> > [hjl@gnu-cfl-3 ld]$
> >
> > Any other comments to my patch?
>
> Well. To me, it is ambiguous what "--fatal-warnings --warn-execstack
> --no-error-execstack" actually is intended to mean (and that's regardless
> of --fatal-warnings position on the command line). One way of
> interpreting is your way. The other is to say that --no-error-execstack
> means "no error for anything related to executable stacks", i.e.
> --fatal-warnings not affecting the overall result (when there are no
> other warnings).

My interpretation is what --fatal-warnings implements.


--
H.J.