[patch] ldfile.c: Fix the search path ordering for a linker script.

[patch] ldfile.c: Fix the search path ordering for a linker script.

[Kazu Hirata]

Hi,

Attached is a patch to fix the search path ordering for a linker
script.

  http://sourceware.org/ml/binutils-cvs/2008-08/msg00062.html

seems to have changed the search path ordering for a linker script in
a way that contradicts what the ld documentation says.

Here is what happens:

Consider:

fido-none-elf-ld \
    -L /scratch/kazu/f/mylib \
    -o empty.elf empty.o -T kazu.ld

where /scratch/kazu/f/mylib/kazu.ld is a completely bogus linker
script and should trigger a syntax error if it is read.  (I've made it
bogus so I can easily tell if the linker actually uses it.)

/scratch/kazu/f/install/fido-none-elf/bin/../lib/kazu.ld is a
legitimate linker script.

When I run the above linker command, the linker does not issue a
syntax error about the bogus linker script even though the linker is
supposed to read /scratch/kazu/f/mylib/kazu.ld.  Instead, the linker
reads /scratch/kazu/f/install/fido-none-elf/bin/../lib/kazu.ld and
happily produces empty.elf.

Here is what happens inside the linker:

Before the linker encounters -T kazu.ld, the linker maintains the
search path list containing nothing but:

  /scratch/kazu/f/mylib

When the linker encounters -T kazu.ld, the linker, namely
ldfile_find_command_file, adds
/scratch/kazu/f/install/fido-none-elf/bin/../lib to the beginning of
the search path list.  The resulting list looks like:

  /scratch/kazu/f/install/fido-none-elf/bin/../lib
  /scratch/kazu/f/mylib

ldfile_find_command_file then traverses the search path list (in the
above order) and finds a linker script at:

  /scratch/kazu/f/install/fido-none-elf/bin/../lib/kazu.ld

which is the legitimate one, so we don't get any syntax error.

Here is what ld.texinfo says:

-T scriptfile
--script=scriptfile
  Use scriptfile as the linker script.  This script replaces ld's
  default linker script (rather than adding to it), so commandfile
  must specify everything necessary to describe the output file.  See
  Scripts.  If scriptfile does not exist in the current directory, ld
  looks for it in the directories specified by any preceding `-L'
  options.  Multiple `-T' options accumulate.

Since the current directory does not contain kazu.ld in this case, the
linker should look for a linker script in -L directories first, not
the default directory obtained from the program name.

The patch fixes the problem by appending the default directory to the
search path list instead of prepending.  Since a subsequent -L option
may add more search paths, we temporarily add the default directory to
the search path list and removes it from the list when we are done
with a traversal.

Tested on fido-none-elf.  OK to apply?

Kazu Hirata

2009-04-02  Kazu Hirata  <kazu@codesourcery.com>

	* ldfile.c (ldfile_find_command_file): Append the path obtained
	from the program name to the search path instead of prepending.

Index: ld/ldfile.c
===================================================================
RCS file: /cvs/src/src/ld/ldfile.c,v
retrieving revision 1.47
diff -u -d -p -r1.47 ldfile.c
--- ld/ldfile.c	9 Aug 2008 10:15:38 -0000	1.47
+++ ld/ldfile.c	2 Apr 2009 01:18:51 -0000
@@ -569,14 +569,14 @@ ldfile_find_command_file (const char *na
 	  ldfile_add_library_path (script_dir, TRUE);
 	  search_tail_ptr = save_tail_ptr;
 	}
-      if (!script_search)
-	script_search = search_head;
-      else
-	script_search->next = search_head;
     }
 
+  /* Temporarily append script_search to the path list so that the
+     paths specified with -L will be searched first.  */
+  *search_tail_ptr = script_search;
+
   /* Try now prefixes.  */
-  for (search = script_search; search != NULL; search = search->next)
+  for (search = search_head; search != NULL; search = search->next)
     {
 
       buffer = concat (search->name, slash, name, (const char *) NULL);
@@ -586,6 +586,9 @@ ldfile_find_command_file (const char *na
 	break;
     }
 
+  /* Restore the original path list.  */
+  *search_tail_ptr = NULL;
+
   return result;
 }
 

Re: [patch] ldfile.c: Fix the search path ordering for a linker script.

[Alan Modra]

On Wed, Apr 01, 2009 at 07:50:40PM -0700, Kazu Hirata wrote:
>   http://sourceware.org/ml/binutils-cvs/2008-08/msg00062.html
> 
> seems to have changed the search path ordering for a linker script in
> a way that contradicts what the ld documentation says.

There was a reason why I chose that particular search order.  Consider
the most common case, where someone invokes ld without giving a -T
or -dT option.  If your copy of ld supports built-in scripts then for
the default target you'll always use one of the built-in scripts.  If
ld doesn't support built-in scripts, then ld reads the script from
ldscripts/, which is usually installed in $prefix/lib/.  Your change
could make ld choose some ldscripts/ other than the one installed
along with the ld binary.  For instance, if I build and install a new
version of ld using a prefix of /usr/local, then I always want to use
scripts in /usr/local/lib/ldscripts/.  I wouldn't want to pick up the
old system scripts in /usr/lib/ldscripts/ if -L /usr/lib happened to
be passed to ld!

-- 
Alan Modra
Australia Development Lab, IBM

Re: [patch] ldfile.c: Fix the search path ordering for a linker script.

[Daniel Jacobowitz]

On Thu, Apr 02, 2009 at 03:19:27PM +1030, Alan Modra wrote:
> On Wed, Apr 01, 2009 at 07:50:40PM -0700, Kazu Hirata wrote:
> >   http://sourceware.org/ml/binutils-cvs/2008-08/msg00062.html
> > 
> > seems to have changed the search path ordering for a linker script in
> > a way that contradicts what the ld documentation says.
> 
> There was a reason why I chose that particular search order.  Consider
> the most common case, where someone invokes ld without giving a -T
> or -dT option.  If your copy of ld supports built-in scripts then for
> the default target you'll always use one of the built-in scripts.  If
> ld doesn't support built-in scripts, then ld reads the script from
> ldscripts/, which is usually installed in $prefix/lib/.  Your change
> could make ld choose some ldscripts/ other than the one installed
> along with the ld binary.  For instance, if I build and install a new
> version of ld using a prefix of /usr/local, then I always want to use
> scripts in /usr/local/lib/ldscripts/.  I wouldn't want to pick up the
> old system scripts in /usr/lib/ldscripts/ if -L /usr/lib happened to
> be passed to ld!

Actually, I think that would be a reasonable thing to do... anyway,
can we treat the default linker scripts specially in this regard?  For
people with a customized linker script, having a copy of a system-wide
installed script is quite common.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] ldfile.c: Fix the search path ordering for a linker script.

[Kazu Hirata]

Hi Daniel and Alan,

>> There was a reason why I chose that particular search order.  Consider
>> the most common case, where someone invokes ld without giving a -T
>> or -dT option.  If your copy of ld supports built-in scripts then for
>> the default target you'll always use one of the built-in scripts.  If
>> ld doesn't support built-in scripts, then ld reads the script from
>> ldscripts/, which is usually installed in $prefix/lib/.  Your change
>> could make ld choose some ldscripts/ other than the one installed
>> along with the ld binary.  For instance, if I build and install a new
>> version of ld using a prefix of /usr/local, then I always want to use
>> scripts in /usr/local/lib/ldscripts/.  I wouldn't want to pick up the
>> old system scripts in /usr/lib/ldscripts/ if -L /usr/lib happened to
>> be passed to ld!
> 
> Actually, I think that would be a reasonable thing to do... anyway,
> can we treat the default linker scripts specially in this regard?  For
> people with a customized linker script, having a copy of a system-wide
> installed script is quite common.

Do you mean something like this?

If -T is explicitly specified anywhere in the command line:
   we search:
     current directory
     search -L directories
     what find_scripts_dir finds
else
   we search
     current directory
     what find_scripts_dir finds
     search -L directories

In any event, I'd like consistency between the documentation and 
implementation.  If we are leaving the implementation as is, we should 
document that the user cannot have a linker script that's named identically 
to one of the system-wide scripts.

Kazu Hirata

Re: [patch] ldfile.c: Fix the search path ordering for a linker script.

[Alan Modra]

On Thu, Apr 02, 2009 at 01:42:26PM -0400, Kazu Hirata wrote:
> If -T is explicitly specified anywhere in the command line:
>   we search:
>     current directory
>     search -L directories
>     what find_scripts_dir finds
> else
>   we search
>     current directory
>     what find_scripts_dir finds
>     search -L directories

I would be happy with your current patch if you changed the existing
call to ldfile_open_command_file after ldemul_get_script in ldmain.c
to call a new function that only searched find_scripts_dir directory.

-- 
Alan Modra
Australia Development Lab, IBM