ld: Improve --fatal-warnings for unknown command-line options
Checks
Commit Message
There are 2 problems with --fatal-warnings for ignored command-line
options:
1. --fatal-warnings doesn't trigger an error for an unknown command-line
option when --fatal-warnings is the last command-line option.
2. When --fatal-warnings triggers an error for an unknown command-line
option, the message says that the unknown command-line option is ignored.
This patch queues unknown command-line option warnings and outputs queued
command-line option warnings after all command-line options have been
processed so that --fatal-warnings can work for unknown command-line
options regardless of the order of --fatal-warnings.
When --fatal-warnings is used, the linker message is changed from
ld: warning: -z bad-option ignored
to
ld: error: unsupported option: -z bad-option
The above also applies to "-z dynamic-undefined-weak" when the known
"-z dynamic-undefined-weak" option is ignored.
PR ld/31289
* ldelf.c (ldelf_after_parse): Use queue_unknown_cmdline_warning
to warn the ignored -z dynamic-undefined-weak option.
* ldmain.c (main): Call output_unknown_cmdline_warnings after
calling ldemul_after_parse.
* ldmisc.c (CMDLINE_WARNING_SIZE): New.
(cmdline_warning_list): Likewise.
(cmdline_warning_head): Likewise.
(cmdline_warning_tail): Likewise.
(queue_unknown_cmdline_warning): Likewise.
(output_unknown_cmdline_warnings): Likewise.
* ldmisc.h (queue_unknown_cmdline_warning): Likewise.
(output_unknown_cmdline_warnings): Likewise.
* emultempl/elf.em (gld${EMULATION_NAME}_handle_option): Use
queue_unknown_cmdline_warning to warn unknown -z option.
* testsuite/ld-elf/pr31289-1a.d: New file.
* testsuite/ld-elf/pr31289-1b.d: Likewise.
* testsuite/ld-elf/pr31289-2a.d: Likewise.
* testsuite/ld-elf/pr31289-2b.d: Likewise.
* testsuite/ld-elf/pr31289-3a.d: Likewise.
* testsuite/ld-elf/pr31289-3b.d: Likewise.
* testsuite/ld-elf/pr31289-4a.d: Likewise.
* testsuite/ld-elf/pr31289-4b.d: Likewise.
---
ld/emultempl/elf.em | 2 +-
ld/ldelf.c | 2 +-
ld/ldmain.c | 2 +
ld/ldmisc.c | 75 ++++++++++++++++++++++++++++++++
ld/ldmisc.h | 2 +
ld/testsuite/ld-elf/pr31289-1a.d | 5 +++
ld/testsuite/ld-elf/pr31289-1b.d | 5 +++
ld/testsuite/ld-elf/pr31289-2a.d | 5 +++
ld/testsuite/ld-elf/pr31289-2b.d | 5 +++
ld/testsuite/ld-elf/pr31289-3a.d | 5 +++
ld/testsuite/ld-elf/pr31289-3b.d | 5 +++
ld/testsuite/ld-elf/pr31289-4a.d | 5 +++
ld/testsuite/ld-elf/pr31289-4b.d | 5 +++
13 files changed, 121 insertions(+), 2 deletions(-)
create mode 100644 ld/testsuite/ld-elf/pr31289-1a.d
create mode 100644 ld/testsuite/ld-elf/pr31289-1b.d
create mode 100644 ld/testsuite/ld-elf/pr31289-2a.d
create mode 100644 ld/testsuite/ld-elf/pr31289-2b.d
create mode 100644 ld/testsuite/ld-elf/pr31289-3a.d
create mode 100644 ld/testsuite/ld-elf/pr31289-3b.d
create mode 100644 ld/testsuite/ld-elf/pr31289-4a.d
create mode 100644 ld/testsuite/ld-elf/pr31289-4b.d
Comments
On Wed, Jan 24, 2024 at 2:51 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> There are 2 problems with --fatal-warnings for ignored command-line
> options:
>
> 1. --fatal-warnings doesn't trigger an error for an unknown command-line
> option when --fatal-warnings is the last command-line option.
> 2. When --fatal-warnings triggers an error for an unknown command-line
> option, the message says that the unknown command-line option is ignored.
>
> This patch queues unknown command-line option warnings and outputs queued
> command-line option warnings after all command-line options have been
> processed so that --fatal-warnings can work for unknown command-line
> options regardless of the order of --fatal-warnings.
>
> When --fatal-warnings is used, the linker message is changed from
>
> ld: warning: -z bad-option ignored
>
> to
>
> ld: error: unsupported option: -z bad-option
>
> The above also applies to "-z dynamic-undefined-weak" when the known
> "-z dynamic-undefined-weak" option is ignored.
>
> PR ld/31289
> * ldelf.c (ldelf_after_parse): Use queue_unknown_cmdline_warning
> to warn the ignored -z dynamic-undefined-weak option.
> * ldmain.c (main): Call output_unknown_cmdline_warnings after
> calling ldemul_after_parse.
> * ldmisc.c (CMDLINE_WARNING_SIZE): New.
> (cmdline_warning_list): Likewise.
> (cmdline_warning_head): Likewise.
> (cmdline_warning_tail): Likewise.
> (queue_unknown_cmdline_warning): Likewise.
> (output_unknown_cmdline_warnings): Likewise.
> * ldmisc.h (queue_unknown_cmdline_warning): Likewise.
> (output_unknown_cmdline_warnings): Likewise.
> * emultempl/elf.em (gld${EMULATION_NAME}_handle_option): Use
> queue_unknown_cmdline_warning to warn unknown -z option.
> * testsuite/ld-elf/pr31289-1a.d: New file.
> * testsuite/ld-elf/pr31289-1b.d: Likewise.
> * testsuite/ld-elf/pr31289-2a.d: Likewise.
> * testsuite/ld-elf/pr31289-2b.d: Likewise.
> * testsuite/ld-elf/pr31289-3a.d: Likewise.
> * testsuite/ld-elf/pr31289-3b.d: Likewise.
> * testsuite/ld-elf/pr31289-4a.d: Likewise.
> * testsuite/ld-elf/pr31289-4b.d: Likewise.
> ---
> ld/emultempl/elf.em | 2 +-
> ld/ldelf.c | 2 +-
> ld/ldmain.c | 2 +
> ld/ldmisc.c | 75 ++++++++++++++++++++++++++++++++
> ld/ldmisc.h | 2 +
> ld/testsuite/ld-elf/pr31289-1a.d | 5 +++
> ld/testsuite/ld-elf/pr31289-1b.d | 5 +++
> ld/testsuite/ld-elf/pr31289-2a.d | 5 +++
> ld/testsuite/ld-elf/pr31289-2b.d | 5 +++
> ld/testsuite/ld-elf/pr31289-3a.d | 5 +++
> ld/testsuite/ld-elf/pr31289-3b.d | 5 +++
> ld/testsuite/ld-elf/pr31289-4a.d | 5 +++
> ld/testsuite/ld-elf/pr31289-4b.d | 5 +++
> 13 files changed, 121 insertions(+), 2 deletions(-)
> create mode 100644 ld/testsuite/ld-elf/pr31289-1a.d
> create mode 100644 ld/testsuite/ld-elf/pr31289-1b.d
> create mode 100644 ld/testsuite/ld-elf/pr31289-2a.d
> create mode 100644 ld/testsuite/ld-elf/pr31289-2b.d
> create mode 100644 ld/testsuite/ld-elf/pr31289-3a.d
> create mode 100644 ld/testsuite/ld-elf/pr31289-3b.d
> create mode 100644 ld/testsuite/ld-elf/pr31289-4a.d
> create mode 100644 ld/testsuite/ld-elf/pr31289-4b.d
Thanks for the patch. For newer tests, I wonder whether a descriptive
short name (in this case, fatal-warnings-*[ab].s) would be more
suitable than PR<number>.
A descriptive name helps future readers group related tests together.
A PR number is useful as well, but it can be noted down as a comment.
For most other warnings, with --fatal-warnings, "warning: " is still
emitted. `ld: error: unsupported option: -z bad-option` is a nice
improvement:)
On 25.01.2024 08:58, Fangrui Song wrote:
> On Wed, Jan 24, 2024 at 2:51 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>> There are 2 problems with --fatal-warnings for ignored command-line
>> options:
>>
>> 1. --fatal-warnings doesn't trigger an error for an unknown command-line
>> option when --fatal-warnings is the last command-line option.
>> 2. When --fatal-warnings triggers an error for an unknown command-line
>> option, the message says that the unknown command-line option is ignored.
>>
>> This patch queues unknown command-line option warnings and outputs queued
>> command-line option warnings after all command-line options have been
>> processed so that --fatal-warnings can work for unknown command-line
>> options regardless of the order of --fatal-warnings.
>>
>> When --fatal-warnings is used, the linker message is changed from
>>
>> ld: warning: -z bad-option ignored
>>
>> to
>>
>> ld: error: unsupported option: -z bad-option
>>
>> The above also applies to "-z dynamic-undefined-weak" when the known
>> "-z dynamic-undefined-weak" option is ignored.
>>
>> PR ld/31289
>> * ldelf.c (ldelf_after_parse): Use queue_unknown_cmdline_warning
>> to warn the ignored -z dynamic-undefined-weak option.
>> * ldmain.c (main): Call output_unknown_cmdline_warnings after
>> calling ldemul_after_parse.
>> * ldmisc.c (CMDLINE_WARNING_SIZE): New.
>> (cmdline_warning_list): Likewise.
>> (cmdline_warning_head): Likewise.
>> (cmdline_warning_tail): Likewise.
>> (queue_unknown_cmdline_warning): Likewise.
>> (output_unknown_cmdline_warnings): Likewise.
>> * ldmisc.h (queue_unknown_cmdline_warning): Likewise.
>> (output_unknown_cmdline_warnings): Likewise.
>> * emultempl/elf.em (gld${EMULATION_NAME}_handle_option): Use
>> queue_unknown_cmdline_warning to warn unknown -z option.
>> * testsuite/ld-elf/pr31289-1a.d: New file.
>> * testsuite/ld-elf/pr31289-1b.d: Likewise.
>> * testsuite/ld-elf/pr31289-2a.d: Likewise.
>> * testsuite/ld-elf/pr31289-2b.d: Likewise.
>> * testsuite/ld-elf/pr31289-3a.d: Likewise.
>> * testsuite/ld-elf/pr31289-3b.d: Likewise.
>> * testsuite/ld-elf/pr31289-4a.d: Likewise.
>> * testsuite/ld-elf/pr31289-4b.d: Likewise.
>> ---
>> ld/emultempl/elf.em | 2 +-
>> ld/ldelf.c | 2 +-
>> ld/ldmain.c | 2 +
>> ld/ldmisc.c | 75 ++++++++++++++++++++++++++++++++
>> ld/ldmisc.h | 2 +
>> ld/testsuite/ld-elf/pr31289-1a.d | 5 +++
>> ld/testsuite/ld-elf/pr31289-1b.d | 5 +++
>> ld/testsuite/ld-elf/pr31289-2a.d | 5 +++
>> ld/testsuite/ld-elf/pr31289-2b.d | 5 +++
>> ld/testsuite/ld-elf/pr31289-3a.d | 5 +++
>> ld/testsuite/ld-elf/pr31289-3b.d | 5 +++
>> ld/testsuite/ld-elf/pr31289-4a.d | 5 +++
>> ld/testsuite/ld-elf/pr31289-4b.d | 5 +++
>> 13 files changed, 121 insertions(+), 2 deletions(-)
>> create mode 100644 ld/testsuite/ld-elf/pr31289-1a.d
>> create mode 100644 ld/testsuite/ld-elf/pr31289-1b.d
>> create mode 100644 ld/testsuite/ld-elf/pr31289-2a.d
>> create mode 100644 ld/testsuite/ld-elf/pr31289-2b.d
>> create mode 100644 ld/testsuite/ld-elf/pr31289-3a.d
>> create mode 100644 ld/testsuite/ld-elf/pr31289-3b.d
>> create mode 100644 ld/testsuite/ld-elf/pr31289-4a.d
>> create mode 100644 ld/testsuite/ld-elf/pr31289-4b.d
>
> Thanks for the patch. For newer tests, I wonder whether a descriptive
> short name (in this case, fatal-warnings-*[ab].s) would be more
> suitable than PR<number>.
> A descriptive name helps future readers group related tests together.
> A PR number is useful as well, but it can be noted down as a comment.
+1
Jan
Hi H.J.
Thanks for taking this problem on. When I qrote my RFC I had
not expected anyone to care that much...
>>> This patch queues unknown command-line option warnings and outputs queued
>>> command-line option warnings after all command-line options have been
>>> processed so that --fatal-warnings can work for unknown command-line
>>> options regardless of the order of --fatal-warnings.
>>>
>>> When --fatal-warnings is used, the linker message is changed from
>>>
>>> ld: warning: -z bad-option ignored
>>>
>>> to
>>>
>>> ld: error: unsupported option: -z bad-option
>>>
>>> The above also applies to "-z dynamic-undefined-weak" when the known
>>> "-z dynamic-undefined-weak" option is ignored.
Patch approved - with the change suggested by Fangrui:
>> Thanks for the patch. For newer tests, I wonder whether a descriptive
>> short name (in this case, fatal-warnings-*[ab].s) would be more
>> suitable than PR<number>.
Cheers
Nick
On Thu, Jan 25, 2024 at 3:32 AM Nick Clifton <nickc@redhat.com> wrote:
>
> Hi H.J.
>
> Thanks for taking this problem on. When I qrote my RFC I had
> not expected anyone to care that much...
>
> >>> This patch queues unknown command-line option warnings and outputs queued
> >>> command-line option warnings after all command-line options have been
> >>> processed so that --fatal-warnings can work for unknown command-line
> >>> options regardless of the order of --fatal-warnings.
> >>>
> >>> When --fatal-warnings is used, the linker message is changed from
> >>>
> >>> ld: warning: -z bad-option ignored
> >>>
> >>> to
> >>>
> >>> ld: error: unsupported option: -z bad-option
> >>>
> >>> The above also applies to "-z dynamic-undefined-weak" when the known
> >>> "-z dynamic-undefined-weak" option is ignored.
>
> Patch approved - with the change suggested by Fangrui:
Here is the patch I am checking. I renamed pr31289-*.d to
fatal-warnings-*.d.
> >> Thanks for the patch. For newer tests, I wonder whether a descriptive
> >> short name (in this case, fatal-warnings-*[ab].s) would be more
> >> suitable than PR<number>.
>
> Cheers
> Nick
>
>
>
Thanks.
On Thu, Jan 25, 2024 at 5:41 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Jan 25, 2024 at 3:32 AM Nick Clifton <nickc@redhat.com> wrote:
> >
> > Hi H.J.
> >
> > Thanks for taking this problem on. When I qrote my RFC I had
> > not expected anyone to care that much...
> >
> > >>> This patch queues unknown command-line option warnings and outputs queued
> > >>> command-line option warnings after all command-line options have been
> > >>> processed so that --fatal-warnings can work for unknown command-line
> > >>> options regardless of the order of --fatal-warnings.
> > >>>
> > >>> When --fatal-warnings is used, the linker message is changed from
> > >>>
> > >>> ld: warning: -z bad-option ignored
> > >>>
> > >>> to
> > >>>
> > >>> ld: error: unsupported option: -z bad-option
> > >>>
> > >>> The above also applies to "-z dynamic-undefined-weak" when the known
> > >>> "-z dynamic-undefined-weak" option is ignored.
> >
> > Patch approved - with the change suggested by Fangrui:
>
> Here is the patch I am checking. I renamed pr31289-*.d to
> fatal-warnings-*.d.
>
Need this patch:
https://sourceware.org/pipermail/binutils/2024-January/132126.html
to make ld to report
$ ld -z bad-option
reports
ld: warning: -z bad-option ignored
ld: no input files
instead of
ld: no input files
On Thu, Jan 25, 2024 at 7:45 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Jan 25, 2024 at 5:41 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > On Thu, Jan 25, 2024 at 3:32 AM Nick Clifton <nickc@redhat.com> wrote:
> > >
> > > Hi H.J.
> > >
> > > Thanks for taking this problem on. When I qrote my RFC I had
> > > not expected anyone to care that much...
> > >
> > > >>> This patch queues unknown command-line option warnings and outputs queued
> > > >>> command-line option warnings after all command-line options have been
> > > >>> processed so that --fatal-warnings can work for unknown command-line
> > > >>> options regardless of the order of --fatal-warnings.
> > > >>>
> > > >>> When --fatal-warnings is used, the linker message is changed from
> > > >>>
> > > >>> ld: warning: -z bad-option ignored
> > > >>>
> > > >>> to
> > > >>>
> > > >>> ld: error: unsupported option: -z bad-option
> > > >>>
> > > >>> The above also applies to "-z dynamic-undefined-weak" when the known
> > > >>> "-z dynamic-undefined-weak" option is ignored.
> > >
> > > Patch approved - with the change suggested by Fangrui:
> >
> > Here is the patch I am checking. I renamed pr31289-*.d to
> > fatal-warnings-*.d.
> >
>
> Need this patch:
>
> https://sourceware.org/pipermail/binutils/2024-January/132126.html
>
> to make ld to report
>
> $ ld -z bad-option
>
> reports
>
> ld: warning: -z bad-option ignored
> ld: no input files
>
> instead of
>
> ld: no input files
>
Another patch is needed:
https://sourceware.org/pipermail/binutils/2024-January/132130.html
to xfail PR ld/31289 tests for targets which don't use standard ELF
emulation since the -z option is unrecognized.
Hi H.J.
I am applying the attached patch to fix a small problem with the
new fatal warnings 1a and 1b tests: they were failing on targets
which do not support the -pie command line option (eg RX and H8300).
Since -pie is not needed for the tests to work and other architectures
do not mind if it is absent, the patch just removes the option from
the command line.
Cheers
Nick
On Fri, Jan 26, 2024 at 2:24 AM Nick Clifton <nickc@redhat.com> wrote:
>
> Hi H.J.
>
> I am applying the attached patch to fix a small problem with the
> new fatal warnings 1a and 1b tests: they were failing on targets
> which do not support the -pie command line option (eg RX and H8300).
>
> Since -pie is not needed for the tests to work and other architectures
> do not mind if it is absent, the patch just removes the option from
> the command line.
>
> Cheers
> Nick
Thanks.
@@ -873,7 +873,7 @@ fi
fragment <<EOF
else
- einfo (_("%P: warning: -z %s ignored\n"), optarg);
+ queue_unknown_cmdline_warning ("-z %s", optarg);
break;
EOF
@@ -74,7 +74,7 @@ ldelf_after_parse (void)
&& link_info.nointerp)
{
if (link_info.dynamic_undefined_weak > 0)
- einfo (_("%P: warning: -z dynamic-undefined-weak ignored\n"));
+ queue_unknown_cmdline_warning ("-z dynamic-undefined-weak");
link_info.dynamic_undefined_weak = 0;
}
@@ -479,6 +479,8 @@ main (int argc, char **argv)
ldemul_after_parse ();
+ output_unknown_cmdline_warnings ();
+
if (config.map_filename)
{
if (strcmp (config.map_filename, "-") == 0)
@@ -620,6 +620,81 @@ einfo (const char *fmt, ...)
fflush (stderr);
}
+/* The buffer size for each command-line option warning. */
+#define CMDLINE_WARNING_SIZE 256
+
+/* A linked list of command-line option warnings. */
+
+struct cmdline_warning_list
+{
+ struct cmdline_warning_list *next;
+ char *warning;
+};
+
+/* The head of the linked list of command-line option warnings. */
+static struct cmdline_warning_list *cmdline_warning_head = NULL;
+
+/* The tail of the linked list of command-line option warnings. */
+static struct cmdline_warning_list **cmdline_warning_tail
+ = &cmdline_warning_head;
+
+/* Queue an unknown command-line option warning. */
+
+void
+queue_unknown_cmdline_warning (const char *fmt, ...)
+{
+ va_list arg;
+ struct cmdline_warning_list *warning_ptr
+ = xmalloc (sizeof (*warning_ptr));
+ warning_ptr->warning = xmalloc (CMDLINE_WARNING_SIZE);
+ warning_ptr->next = NULL;
+ int written;
+
+ va_start (arg, fmt);
+ written = vsnprintf (warning_ptr->warning, CMDLINE_WARNING_SIZE, fmt,
+ arg);
+ if (written < 0 || written >= CMDLINE_WARNING_SIZE)
+ {
+ /* If vsnprintf fails or truncates, output the warning directly. */
+ fflush (stdout);
+ va_start (arg, fmt);
+ vfinfo (stderr, fmt, arg, true);
+ fflush (stderr);
+ }
+ else
+ {
+ *cmdline_warning_tail = warning_ptr;
+ cmdline_warning_tail = &warning_ptr->next;
+ }
+ va_end (arg);
+}
+
+/* Output queued unknown command-line option warnings. */
+
+void
+output_unknown_cmdline_warnings (void)
+{
+ struct cmdline_warning_list *list = cmdline_warning_head;
+ struct cmdline_warning_list *next;
+ if (list == NULL)
+ return;
+
+ fflush (stdout);
+
+ for (; list != NULL; list = next)
+ {
+ next = list->next;
+ if (config.fatal_warnings)
+ einfo (_("%P: error: unsupported option: %s\n"), list->warning);
+ else
+ einfo (_("%P: warning: %s ignored\n"), list->warning);
+ free (list->warning);
+ free (list);
+ }
+
+ fflush (stderr);
+}
+
void
info_assert (const char *file, unsigned int line)
{
@@ -27,6 +27,8 @@ extern void minfo (const char *, ...);
extern void info_msg (const char *, ...);
extern void lfinfo (FILE *, const char *, ...);
extern void info_assert (const char *, unsigned int);
+extern void queue_unknown_cmdline_warning (const char *, ...);
+extern void output_unknown_cmdline_warnings (void);
#define ASSERT(x) \
do { if (!(x)) info_assert(__FILE__,__LINE__); } while (0)
new file mode 100644
@@ -0,0 +1,5 @@
+#source: pr22269.s
+#ld: -pie --no-dynamic-linker --fatal-warnings -z dynamic-undefined-weak
+#readelf: -r -x .data.rel.ro
+#error: unsupported option: -z dynamic-undefined-weak
+#target: *-*-linux* *-*-gnu* *-*-nacl* arm*-*-uclinuxfdpiceabi
new file mode 100644
@@ -0,0 +1,5 @@
+#source: pr22269.s
+#ld: -pie --no-dynamic-linker -z dynamic-undefined-weak --fatal-warnings
+#readelf: -r -x .data.rel.ro
+#error: unsupported option: -z dynamic-undefined-weak
+#target: *-*-linux* *-*-gnu* *-*-nacl* arm*-*-uclinuxfdpiceabi
new file mode 100644
@@ -0,0 +1,5 @@
+#source: start.s
+#ld: -z bad-option1 -z bad-option2
+#warning: -z bad-option1 ignored
+#xfail: [is_generic]
+# generic linker targets don't support -z options.
new file mode 100644
@@ -0,0 +1,5 @@
+#source: start.s
+#ld: -z bad-option1 -z bad-option2
+#warning: -z bad-option2 ignored
+#xfail: [is_generic]
+# generic linker targets don't support -z options.
new file mode 100644
@@ -0,0 +1,5 @@
+#source: start.s
+#ld: --fatal-warnings -z bad-option1 -z bad-option2
+#error: unsupported option: -z bad-option1
+#xfail: [is_generic]
+# generic linker targets don't support -z options.
new file mode 100644
@@ -0,0 +1,5 @@
+#source: start.s
+#ld: --fatal-warnings -z bad-option1 -z bad-option2
+#error: unsupported option: -z bad-option2
+#xfail: [is_generic]
+# generic linker targets don't support -z options.
new file mode 100644
@@ -0,0 +1,5 @@
+#source: start.s
+#ld: -z bad-option1 -z bad-option2 --fatal-warnings
+#error: unsupported option: -z bad-option1
+#xfail: [is_generic]
+# generic linker targets don't support -z options.
new file mode 100644
@@ -0,0 +1,5 @@
+#source: start.s
+#ld: -z bad-option1 -z bad-option2 --fatal-warnings
+#error: unsupported option: -z bad-option2
+#xfail: [is_generic]
+# generic linker targets don't support -z options.