RFC: Document unexpected behaviour of --fatal-warnings

  Hi Guys,

  It was recently pointed out to me that the bfd linker's
  --fatal-warnings option can behave in an unexpected manner.  For
  example:

    $ ld.bfd -z bad-option --fatal-warnings -e 0/dev/null
    ld.bfd: warning: -z bad-option ignored
    $ echo $?
    0

  ie the warning about the ignored option is not being treated as an
  error.  This happens because the --fatal-warnings option only takes
  affect after it has been processed, and we process the options in a
  linear order.  So the following works:

    $ ld.bfd --fatal-warnings -z bad-option -e 0 /dev/null
    ld.bfd: warning: -z bad-option ignored
    $ echo $?
    1

  This behaviour differs from gold and lld, both of which honour
  --fatal-warnings no matter where it occurs on the command line.

  So we could fix the linker, create a two pass argument scan and the
  problem would be solved.  But a) I am lazy and b) we already have a
  precedent for options on the command line only affecting options that
  come after it.  (I am thinking of the -L option here, although there
  are probably several others).  So instead I am considering documenting
  the current behaviour as expected.  (See the patch below).

  What do people think ?

Cheers
  Nick
  

On 23.01.2024 13:28, Nick Clifton wrote:
> Hi Guys,
> 
>   It was recently pointed out to me that the bfd linker's
>   --fatal-warnings option can behave in an unexpected manner.  For
>   example:
> 
>     $ ld.bfd -z bad-option --fatal-warnings -e 0/dev/null
>     ld.bfd: warning: -z bad-option ignored
>     $ echo $?
>     0
> 
>   ie the warning about the ignored option is not being treated as an
>   error.  This happens because the --fatal-warnings option only takes
>   affect after it has been processed, and we process the options in a
>   linear order.  So the following works:
> 
>     $ ld.bfd --fatal-warnings -z bad-option -e 0 /dev/null
>     ld.bfd: warning: -z bad-option ignored
>     $ echo $?
>     1
> 
>   This behaviour differs from gold and lld, both of which honour
>   --fatal-warnings no matter where it occurs on the command line.
> 
>   So we could fix the linker, create a two pass argument scan and the
>   problem would be solved.  But a) I am lazy and b) we already have a
>   precedent for options on the command line only affecting options that
>   come after it.  (I am thinking of the -L option here, although there
>   are probably several others).  So instead I am considering documenting
>   the current behaviour as expected.  (See the patch below).
> 
>   What do people think ?

I'd be fine either way, and I agree documenting is cheaper and sufficient.

Jan
  

On Tue, Jan 23, 2024 at 4:39 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 23.01.2024 13:28, Nick Clifton wrote:
> > Hi Guys,
> >
> >   It was recently pointed out to me that the bfd linker's
> >   --fatal-warnings option can behave in an unexpected manner.  For
> >   example:
> >
> >     $ ld.bfd -z bad-option --fatal-warnings -e 0/dev/null
> >     ld.bfd: warning: -z bad-option ignored
> >     $ echo $?
> >     0
> >
> >   ie the warning about the ignored option is not being treated as an
> >   error.  This happens because the --fatal-warnings option only takes
> >   affect after it has been processed, and we process the options in a
> >   linear order.  So the following works:
> >
> >     $ ld.bfd --fatal-warnings -z bad-option -e 0 /dev/null
> >     ld.bfd: warning: -z bad-option ignored
> >     $ echo $?
> >     1
> >
> >   This behaviour differs from gold and lld, both of which honour
> >   --fatal-warnings no matter where it occurs on the command line.
> >
> >   So we could fix the linker, create a two pass argument scan and the
> >   problem would be solved.  But a) I am lazy and b) we already have a
> >   precedent for options on the command line only affecting options that
> >   come after it.  (I am thinking of the -L option here, although there
> >   are probably several others).  So instead I am considering documenting
> >   the current behaviour as expected.  (See the patch below).
> >
> >   What do people think ?
>
> I'd be fine either way, and I agree documenting is cheaper and sufficient.
>
> Jan

We should give a warning or an error if --fatal-warnings isn't the first
command-line option.
  

On Tue, Jan 23, 2024 at 4:50 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Jan 23, 2024 at 4:39 AM Jan Beulich <jbeulich@suse.com> wrote:
> >
> > On 23.01.2024 13:28, Nick Clifton wrote:
> > > Hi Guys,
> > >
> > >   It was recently pointed out to me that the bfd linker's
> > >   --fatal-warnings option can behave in an unexpected manner.  For
> > >   example:
> > >
> > >     $ ld.bfd -z bad-option --fatal-warnings -e 0/dev/null
> > >     ld.bfd: warning: -z bad-option ignored
> > >     $ echo $?
> > >     0
> > >
> > >   ie the warning about the ignored option is not being treated as an
> > >   error.  This happens because the --fatal-warnings option only takes
> > >   affect after it has been processed, and we process the options in a
> > >   linear order.  So the following works:
> > >
> > >     $ ld.bfd --fatal-warnings -z bad-option -e 0 /dev/null
> > >     ld.bfd: warning: -z bad-option ignored
> > >     $ echo $?
> > >     1
> > >
> > >   This behaviour differs from gold and lld, both of which honour
> > >   --fatal-warnings no matter where it occurs on the command line.
> > >
> > >   So we could fix the linker, create a two pass argument scan and the
> > >   problem would be solved.  But a) I am lazy and b) we already have a
> > >   precedent for options on the command line only affecting options that
> > >   come after it.  (I am thinking of the -L option here, although there
> > >   are probably several others).  So instead I am considering documenting
> > >   the current behaviour as expected.  (See the patch below).
> > >
> > >   What do people think ?
> >
> > I'd be fine either way, and I agree documenting is cheaper and sufficient.
> >
> > Jan
>
> We should give a warning or an error if --fatal-warnings isn't the first
> command-line option.
>

I ran into a similar issue with --text-section-ordering-file.  My patch
issues an error if --text-section-ordering-file is placed after -T/--script.

@@ -670,6 +673,7 @@ parse_args (unsigned argc, char **argv)
     dynamic_list
   } opt_dynamic_list = dynamic_list_unset;
   struct bfd_elf_dynamic_list *export_list = NULL;
+  bool seen_linker_script = false;

   shortopts = (char *) xmalloc (OPTION_COUNT * 3 + 2);
   longopts = (struct option *)
@@ -1394,6 +1398,12 @@ parse_args (unsigned argc, char **argv)
      einfo (_("%F%P: invalid section sorting option: %s\n"),
     optarg);
    break;
+ case OPTION_TEXT_SECTION_ORDERING_FILE:
+   if (seen_linker_script)
+     einfo (_("%F%P: --text-section-ordering-file must be placed"
+      " before -T/--script\n"));
+   config.text_section_ordering_file = optarg;
+   break;
  case OPTION_STATS:
    config.stats = true;
    break;
@@ -1410,6 +1420,7 @@ parse_args (unsigned argc, char **argv)
    ++trace_files;
    break;
  case 'T':
+   seen_linker_script = true;
    previous_script_handle = saved_script_handle;
    ldfile_open_script_file (optarg);
    parser_input = input_script;
  

On Tue, Jan 23, 2024 at 12:28:17PM +0000, Nick Clifton wrote:
> Hi Guys,
> 
>   It was recently pointed out to me that the bfd linker's
>   --fatal-warnings option can behave in an unexpected manner.  For
>   example:
> 
>     $ ld.bfd -z bad-option --fatal-warnings -e 0/dev/null
>     ld.bfd: warning: -z bad-option ignored
>     $ echo $?
>     0
> 
>   ie the warning about the ignored option is not being treated as an
>   error.  This happens because the --fatal-warnings option only takes
>   affect after it has been processed, and we process the options in a
>   linear order.  So the following works:
> 
>     $ ld.bfd --fatal-warnings -z bad-option -e 0 /dev/null
>     ld.bfd: warning: -z bad-option ignored
>     $ echo $?
>     1
> 
>   This behaviour differs from gold and lld, both of which honour
>   --fatal-warnings no matter where it occurs on the command line.
> 
>   So we could fix the linker, create a two pass argument scan and the
>   problem would be solved.  But a) I am lazy and b) we already have a

Hey Nick,

In case you're worried about doing two passes, you could also record the fact
that a warning has been raised in a boolean somewhere, and choke on it if
--fatal-warnings was specified.

Anyway, thanks for fighting the lazyness and making the effort of looking into
this ;-)
  

Nick Clifton <nickc@redhat.com> writes:

> Hi Guys,

Hi Nick,

>
>   It was recently pointed out to me that the bfd linker's
>   --fatal-warnings option can behave in an unexpected manner.  For
>   example:
>
>     $ ld.bfd -z bad-option --fatal-warnings -e 0/dev/null
>     ld.bfd: warning: -z bad-option ignored
>     $ echo $?
>     0
>
>   ie the warning about the ignored option is not being treated as an
>   error.  This happens because the --fatal-warnings option only takes
>   affect after it has been processed, and we process the options in a
>   linear order.  So the following works:
>
>     $ ld.bfd --fatal-warnings -z bad-option -e 0 /dev/null
>     ld.bfd: warning: -z bad-option ignored
>     $ echo $?
>     1
>
>   This behaviour differs from gold and lld, both of which honour
>   --fatal-warnings no matter where it occurs on the command line.
>
>   So we could fix the linker, create a two pass argument scan and the
>   problem would be solved.  But a) I am lazy and b) we already have a
>   precedent for options on the command line only affecting options that
>   come after it.  (I am thinking of the -L option here, although there
>   are probably several others).  So instead I am considering documenting
>   the current behaviour as expected.  (See the patch below).
>
>   What do people think ?

I'm OK with it either way. We're really used to this kind of linker
behaviour so it's not a problem to just document it. I've not hit
this as a problem at all.

Then if someone wants to improve it later on, so be it.

(We could maybe change the docs to say "may not affect" if we want
leeway to change it later.)

>
> Cheers
>   Nick
>

thanks,
sam
  

On Wed, Jan 24, 2024 at 7:15 AM Sam James <sam@gentoo.org> wrote:
>
>
> Nick Clifton <nickc@redhat.com> writes:
>
> > Hi Guys,
>
> Hi Nick,
>
> >
> >   It was recently pointed out to me that the bfd linker's
> >   --fatal-warnings option can behave in an unexpected manner.  For
> >   example:
> >
> >     $ ld.bfd -z bad-option --fatal-warnings -e 0/dev/null
> >     ld.bfd: warning: -z bad-option ignored
> >     $ echo $?
> >     0
> >
> >   ie the warning about the ignored option is not being treated as an
> >   error.  This happens because the --fatal-warnings option only takes
> >   affect after it has been processed, and we process the options in a
> >   linear order.  So the following works:
> >
> >     $ ld.bfd --fatal-warnings -z bad-option -e 0 /dev/null
> >     ld.bfd: warning: -z bad-option ignored
> >     $ echo $?
> >     1
> >
> >   This behaviour differs from gold and lld, both of which honour
> >   --fatal-warnings no matter where it occurs on the command line.
> >
> >   So we could fix the linker, create a two pass argument scan and the
> >   problem would be solved.  But a) I am lazy and b) we already have a
> >   precedent for options on the command line only affecting options that
> >   come after it.  (I am thinking of the -L option here, although there
> >   are probably several others).  So instead I am considering documenting
> >   the current behaviour as expected.  (See the patch below).
> >
> >   What do people think ?
>
> I'm OK with it either way. We're really used to this kind of linker
> behaviour so it's not a problem to just document it. I've not hit
> this as a problem at all.
>
> Then if someone wants to improve it later on, so be it.
>
> (We could maybe change the docs to say "may not affect" if we want
> leeway to change it later.)

I opened:

https://sourceware.org/bugzilla/show_bug.cgi?id=31289

and will submit a fix this week.
  

On Wed, Jan 24, 2024 at 8:06 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Wed, Jan 24, 2024 at 7:15 AM Sam James <sam@gentoo.org> wrote:
> >
> >
> > Nick Clifton <nickc@redhat.com> writes:
> >
> > > Hi Guys,
> >
> > Hi Nick,
> >
> > >
> > >   It was recently pointed out to me that the bfd linker's
> > >   --fatal-warnings option can behave in an unexpected manner.  For
> > >   example:
> > >
> > >     $ ld.bfd -z bad-option --fatal-warnings -e 0/dev/null
> > >     ld.bfd: warning: -z bad-option ignored
> > >     $ echo $?
> > >     0
> > >
> > >   ie the warning about the ignored option is not being treated as an
> > >   error.  This happens because the --fatal-warnings option only takes
> > >   affect after it has been processed, and we process the options in a
> > >   linear order.  So the following works:
> > >
> > >     $ ld.bfd --fatal-warnings -z bad-option -e 0 /dev/null
> > >     ld.bfd: warning: -z bad-option ignored
> > >     $ echo $?
> > >     1
> > >
> > >   This behaviour differs from gold and lld, both of which honour
> > >   --fatal-warnings no matter where it occurs on the command line.
> > >
> > >   So we could fix the linker, create a two pass argument scan and the
> > >   problem would be solved.  But a) I am lazy and b) we already have a
> > >   precedent for options on the command line only affecting options that
> > >   come after it.  (I am thinking of the -L option here, although there
> > >   are probably several others).  So instead I am considering documenting
> > >   the current behaviour as expected.  (See the patch below).
> > >
> > >   What do people think ?
> >
> > I'm OK with it either way. We're really used to this kind of linker
> > behaviour so it's not a problem to just document it. I've not hit
> > this as a problem at all.
> >
> > Then if someone wants to improve it later on, so be it.
> >
> > (We could maybe change the docs to say "may not affect" if we want
> > leeway to change it later.)
>
> I opened:
>
> https://sourceware.org/bugzilla/show_bug.cgi?id=31289
>
> and will submit a fix this week.
>

A patch is posted at:

https://sourceware.org/pipermail/binutils/2024-January/132106.html