[PATCH v3] ld: Use stat to check if linker script appears multiple times

[H.J. Lu]

On Tue, Aug 12, 2025 at 7:16 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 12.08.2025 13:57, H.J. Lu wrote:
> > Use stat, instead of strcmp, to check if the same linker script file
> > appears multiple times for
> >
> > $ ld -L... -T ././/script.t -T script.t ...
> >
> > Although ././/script.t and script.t access the same file, but their
> > filenames are different.  strcmp won't work here.
> >
> > Copy gnulib/import/same-inode.h to include since the gnulib directory
> > isn't included in the binutils tarball.
> >
> > include/
> >
> >       PR ld/24576
> >       * same-inode.h: New file.  Copied from gnulib/import/same-inode.h.
> >
> > ld/
> >
> >       PR ld/24576
> >       * ldfile.c: Include "same-inode.h".
> >       (ldfile_find_command_file): Change the second argument from bool
> >       to enum script_open_style.  Check if the same linker script file
> >       appears multiple times by using stat, instead using strcmp.
> >       (ldfile_open_command_file_1): Don't check if the same linker
> >       script file appears multiple times here.
> >       * testsuite/ld-scripts/pr24576-2.d: New.
> >       * testsuite/ld-scripts/script.exp: Run pr24576-2.
> >
> > Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
>
> Looks largely okay to me, but I have one more question and a small remark:
>
> > --- a/ld/ldfile.c
> > +++ b/ld/ldfile.c
> > @@ -35,6 +35,7 @@
> >  #include "libiberty.h"
> >  #include "filenames.h"
> >  #include <fnmatch.h>
> > +#include "same-inode.h"
> >  #if BFD_SUPPORTS_PLUGINS
> >  #include "plugin.h"
> >  #endif /* BFD_SUPPORTS_PLUGINS */
> > @@ -828,19 +829,26 @@ find_scripts_dir (void)
> >
> >  static FILE *
> >  ldfile_find_command_file (const char *name,
> > -                       bool default_only,
> > +                       enum script_open_style open_how,
> >                         bool *sysrooted)
> >  {
> >    search_dirs_type *search;
> >    FILE *result = NULL;
> > -  char *path;
> > +  char *path = NULL;
> > +  const char *filename = NULL;
> > +  struct script_name_list *script;
> > +  size_t len;
> > +  struct stat sbuf1;
> >
> > -  if (!default_only)
> > +  if (open_how != script_defaultT)
> >      {
> >        /* First try raw name.  */
> >        result = try_open (name, sysrooted);
> >        if (result != NULL)
> > -     return result;
> > +     {
> > +       filename = name;
> > +       goto success;
> > +     }
> >      }
> >
> >    if (!script_search)
> > @@ -861,20 +869,45 @@ ldfile_find_command_file (const char *name,
> >    *search_tail_ptr = script_search;
> >
> >    /* Try now prefixes.  */
> > -  for (search = default_only ? script_search : search_head;
> > +  for (search = open_how == script_defaultT ? script_search : search_head;
> >         search != NULL;
> >         search = search->next)
> >      {
> >        path = concat (search->name, slash, name, (const char *) NULL);
> >        result = try_open (path, sysrooted);
> > -      free (path);
> >        if (result)
> > -     break;
> > +     {
> > +       filename = path;
> > +       break;
> > +     }
> >      }
> >
> >    /* Restore the original path list.  */
> >    *search_tail_ptr = NULL;
> >
> > +success:
>
> May I ask that labels be indented by at least one blank, for the sake of
> "diff -p" and alike?

Fixed in v4.

> > +  /* PR 24576: Catch the case where the user has accidentally included
> > +     the same linker script twice.  */
> > +  if (stat (filename, &sbuf1) == 0)
> > +    for (script = processed_scripts; script != NULL; script = script->next)
> > +      if (open_how != script_nonT || script->open_how != script_nonT)
> > +     {
> > +       struct stat sbuf2;
> > +       if (stat (script->name, &sbuf2) == 0
> > +           && SAME_INODE (sbuf1, sbuf2))
> > +         fatal (_("%P: error: linker script file '%s'"
> > +                  " appears multiple times\n"), name);
>
> Reporting "name" here may not be very helpful? Imo we want to report both
> "filename" and "script->name", which may be entirely different from one
> another (and also different from "name").
>

Changed in v4.

Thanks.

-- 
H.J.