RFC: Document unexpected behaviour of --fatal-warnings

[H.J. Lu]

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;


-- 
H.J.