ld: Append LDFLAGS to flags variable in default_ld_link

Message ID ae-BMT4p0QeM4CKL@mx3210.local
State New
Headers
Series ld: Append LDFLAGS to flags variable in default_ld_link |

Commit Message

John David Anglin April 27, 2026, 3:30 p.m. UTC
  ld: Append LDFLAGS to flags variable in default_ld_link

The following change is needed to fix errors trying to load the
milli.a archive for the hppa*64*-*-hpux* target when the host
system isn't hpux.

We need to create a dummy milli.a archive for this target and
append the directory of this archive to LDFLAGS.

This assumes the testsuite takes the LDFLAGS environment variable
into account.  However, some tests in the ld testsuite use the
low-level `ld-link' procedure and it fails to take LDFLAGS into
account.

This change modifies the `default_ld_link' procedure to append
$LDFLAGS to the flags variable and fix the above issue.

This change potentially affects the `alpha*-*-*vms*' target but
currently no tests run for it make direct use of `ld-link'.

Suggested-by: Maciej W. Rozycki <macro@orcam.me.uk>

2026-04-27  John David Anglin  <danglin@gcc.gnu.org>

ld/ChangeLog:

	* testsuite/lib/ld-lib.exp (default_ld_link): Append
	LDFLAGS to `flags' variable.  Also use the append command
	to append the board flags.
  

Comments

Maciej W. Rozycki April 27, 2026, 11:34 p.m. UTC | #1
On Mon, 27 Apr 2026, John David Anglin wrote:

> ld: Append LDFLAGS to flags variable in default_ld_link
> 
> The following change is needed to fix errors trying to load the
> milli.a archive for the hppa*64*-*-hpux* target when the host
> system isn't hpux.
> 
> We need to create a dummy milli.a archive for this target and
> append the directory of this archive to LDFLAGS.
> 
> This assumes the testsuite takes the LDFLAGS environment variable
> into account.  However, some tests in the ld testsuite use the
> low-level `ld-link' procedure and it fails to take LDFLAGS into
> account.
> 
> This change modifies the `default_ld_link' procedure to append
> $LDFLAGS to the flags variable and fix the above issue.
> 
> This change potentially affects the `alpha*-*-*vms*' target but
> currently no tests run for it make direct use of `ld-link'.

 Technically this is a fix to the testsuite/lib/ld-lib.exp part of commit 
740341b9be65 ("Provide dummy libraries for alpha-vms"), so as I previously 
suggested can you please combine your update with the removal of $LDFLAGS 
propagation from `run_ld_link_tests' (and now that I've double-checked 
said commit, also other places such as `section_check') and mention it in 
the description that this is so that $LDFLAGS is taken into account with 
all linker invocations rather than the chosen ones only?  Or do you have 
reasons you'd rather not to?

 Alan: do you agree that this approach makes more sense?  We have numerous 
`ld_link' invocations across the test framework, a few of which only get 
LDFLAGS passed and all would otherwise have to be updated and any new ones 
arranged accordingly.  While one can argue this is an incompatible change, 
because `default_ld_link' can be overridden and therefore people's setups 
out there will have to be updated accordingly, it seems to me in line with 
its sibling handlers such as `default_ld_assemble' or `default_ld_compile' 
which use analogous variables such as $ASFLAGS or $CFLAGS_FOR_TARGET, so 
that will be consistent with what we already have anyway.

 NB `ld_link' rather than `ld-link' please.

  Maciej
  
Alan Modra April 29, 2026, 2:18 a.m. UTC | #2
On Mon, Apr 27, 2026 at 11:30:57AM -0400, John David Anglin wrote:
> ld/ChangeLog:
> 
> 	* testsuite/lib/ld-lib.exp (default_ld_link): Append
> 	LDFLAGS to `flags' variable.  Also use the append command
> 	to append the board flags.

OK, thanks.
  
John David Anglin April 29, 2026, 3:57 p.m. UTC | #3
On 2026-04-28 10:18 p.m., Alan Modra wrote:
> On Mon, Apr 27, 2026 at 11:30:57AM -0400, John David Anglin wrote:
>> ld/ChangeLog:
>>
>> 	* testsuite/lib/ld-lib.exp (default_ld_link): Append
>> 	LDFLAGS to `flags' variable.  Also use the append command
>> 	to append the board flags.
> 
> OK, thanks.

After Maciej's last comments, I had updated the change to remove references
to LDFLAGS from `run_ld_link_tests'.  But after inspecting the following commit
https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=740341b9be657772538f9cf0b563c49798f47b3c
it seemed that LDFLAGS was intentionally not added to the flags variable in default_ld_link.
In that change, checks.exp and a number of other .exp files were updated to pass LDFLAGS
to ld.  Maybe it would be safer to update the .exp files that don't currently pass
LDFLAGS to ld as that seems to be the current practice?

Dave
  
Alan Modra April 30, 2026, 10:55 a.m. UTC | #4
On Wed, Apr 29, 2026 at 11:57:36AM -0400, John David Anglin wrote:
> On 2026-04-28 10:18 p.m., Alan Modra wrote:
> > On Mon, Apr 27, 2026 at 11:30:57AM -0400, John David Anglin wrote:
> >> ld/ChangeLog:
> >>
> >> 	* testsuite/lib/ld-lib.exp (default_ld_link): Append
> >> 	LDFLAGS to `flags' variable.  Also use the append command
> >> 	to append the board flags.
> > 
> > OK, thanks.
> 
> After Maciej's last comments, I had updated the change to remove references
> to LDFLAGS from `run_ld_link_tests'.  But after inspecting the following commit
> https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=740341b9be657772538f9cf0b563c49798f47b3c
> it seemed that LDFLAGS was intentionally not added to the flags variable in default_ld_link.
> In that change, checks.exp and a number of other .exp files were updated to pass LDFLAGS
> to ld.  Maybe it would be safer to update the .exp files that don't currently pass
> LDFLAGS to ld as that seems to be the current practice?

That was likely why I did it that way, but if you need all occurrences
of ld_link to use LDFLAGS then I'm happy for you to put it in
default_ld_link, and ideally remove the then unnecessary places I
added LDFLAGS.

> 
> Dave
> -- 
> John David Anglin  dave.anglin@bell.net
  

Patch

diff --git a/ld/testsuite/lib/ld-lib.exp b/ld/testsuite/lib/ld-lib.exp
index d37d33cd96c..5da81424cb5 100644
--- a/ld/testsuite/lib/ld-lib.exp
+++ b/ld/testsuite/lib/ld-lib.exp
@@ -231,11 +231,13 @@  proc get_board_flags {} {
 proc default_ld_link { ld target objects } {
     global host_triplet
     global exec_output
+    global LDFLAGS
 
     set flags ""
     if [is_endian_output_format $objects] then {
 	set flags [big_or_little_endian]
     }
+    append flags " $LDFLAGS"
 
     # When using GCC as the linker driver, we need to specify board cflags when
     # linking because cflags may contain linker options.  For example when
@@ -245,7 +247,7 @@  proc default_ld_link { ld target objects } {
     if {[string match "*cc*" $gccexe] ||
 	[string match "*++*" $gccexe] ||
 	[string match "clang*" $gccexe]} then {
-	set flags "$flags [get_board_flags]"
+	append flags " [get_board_flags]"
     }
 
     remote_file host delete $target