public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] ld/testsuite: consistently add board_ldflags when linking with GCC
@ 2022-09-30 14:05 Clément Chigot
  2022-10-03 17:10 ` Palmer Dabbelt
  2022-10-13 12:05 ` Clément Chigot
  0 siblings, 2 replies; 5+ messages in thread
From: Clément Chigot @ 2022-09-30 14:05 UTC (permalink / raw)
  To: binutils; +Cc: Clément Chigot

Currently, the functions checking if the compiler is available or if a
feature is available add both board_cflags and board_ldflags.
However, functions running the tests only retrieve board_cflags. This
can lead to unexpected errors when mandaratory flags are defined in
board_ldflags and not board_cflags.

ld/ChangeLog:

	* testsuite/ld-unique/unique.exp: Add board_ldflags when
	linking with GCC.
	* testsuite/lib/ld-lib.exp: Likewise.
---
 ld/testsuite/ld-unique/unique.exp |  8 +++++++-
 ld/testsuite/lib/ld-lib.exp       | 22 ++++++++++++++++++----
 2 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/ld/testsuite/ld-unique/unique.exp b/ld/testsuite/ld-unique/unique.exp
index f3d5a5a6b7d..ab24eef50c3 100644
--- a/ld/testsuite/ld-unique/unique.exp
+++ b/ld/testsuite/ld-unique/unique.exp
@@ -122,8 +122,14 @@ if [board_info [target_info name] exists cflags] {
   set board_cflags ""
 }
 
+if [board_info [target_info name] exists ldflags] {
+    set board_ldflags " [board_info [target_info name] ldflags]"
+} else {
+    set board_ldflags ""
+}
+
 # Create executable containing unique symbol.
-if ![ld_link "$CC_FOR_TARGET $NOPIE_LDFLAGS $board_cflags" "tmpdir/unique_prog" "tmpdir/unique.o"] {
+if ![ld_link "$CC_FOR_TARGET $NOPIE_LDFLAGS $board_cflags $board_ldflags" "tmpdir/unique_prog" "tmpdir/unique.o"] {
     fail "Could not link a unique executable"
     set fails [expr $fails + 1]
 }
diff --git a/ld/testsuite/lib/ld-lib.exp b/ld/testsuite/lib/ld-lib.exp
index ec27388a72e..2cd840c0169 100644
--- a/ld/testsuite/lib/ld-lib.exp
+++ b/ld/testsuite/lib/ld-lib.exp
@@ -690,6 +690,7 @@ proc run_ld_link_exec_tests { ldtests args } {
     global errcnt
     global exec_output
     global board_cflags
+    global board_ldflags
     global STATIC_LDFLAGS
 
     # When using GCC as the linker driver, we need to specify board cflags when
@@ -702,6 +703,12 @@ proc run_ld_link_exec_tests { ldtests args } {
 	set board_cflags ""
     }
 
+    if [board_info [target_info name] exists ldflags] {
+	set board_ldflags " [board_info [target_info name] ldflags]"
+    } else {
+	set board_ldflags ""
+    }
+
     foreach testitem $ldtests {
 	set testname [lindex $testitem 0]
 	set ld_options [lindex $testitem 1]
@@ -777,11 +784,11 @@ proc run_ld_link_exec_tests { ldtests args } {
 	    continue;
 	} else {
 	    if { [string match "" $STATIC_LDFLAGS] \
-		 && [regexp -- ".* \[-\]+static .*" " $board_cflags $ld_options $objfiles $ld_after "] } {
+		 && [regexp -- ".* \[-\]+static .*" " $board_cflags $board_ldflags $ld_options $objfiles $ld_after "] } {
 		untested $testname
 		continue
 	    }
-	    if ![$link_proc $link_cmd $binfile "$board_cflags -L$srcdir/$subdir $ld_options $objfiles $ld_after"] {
+	    if ![$link_proc $link_cmd $binfile "$board_cflags $board_ldflags -L$srcdir/$subdir $ld_options $objfiles $ld_after"] {
 		set failed 1
 	    }
 	}
@@ -858,6 +865,7 @@ proc run_cc_link_tests { ldtests } {
     global ar
     global exec_output
     global board_cflags
+    global board_ldflags
     global STATIC_LDFLAGS
 
     if [board_info [target_info name] exists cflags] {
@@ -866,6 +874,12 @@ proc run_cc_link_tests { ldtests } {
 	set board_cflags ""
     }
 
+    if [board_info [target_info name] exists ldflags] {
+	set board_ldflags " [board_info [target_info name] ldflags]"
+    } else {
+	set board_ldflags ""
+    }
+
     foreach testitem $ldtests {
 	set testname [lindex $testitem 0]
 	set ldflags [lindex $testitem 1]
@@ -968,11 +982,11 @@ proc run_cc_link_tests { ldtests } {
 	    }
 	} else {
 	    if { [string match "" $STATIC_LDFLAGS] \
-		 && [regexp -- ".* \[-\]+static .*" " $board_cflags $ldflags $objfiles "] } {
+		 && [regexp -- ".* \[-\]+static .*" " $board_cflags $board_ldflags $ldflags $objfiles "] } {
 		untested $testname
 		continue
 	    }
-	    ld_link $cc_cmd $binfile "$board_cflags -L$srcdir/$subdir $ldflags $objfiles"
+	    ld_link $cc_cmd $binfile "$board_cflags $board_ldflags -L$srcdir/$subdir $ldflags $objfiles"
 	    set ld_output "$exec_output"
 
 	    if { $check_ld(source) == "regexp" } then {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ld/testsuite: consistently add board_ldflags when linking with GCC
  2022-09-30 14:05 [PATCH] ld/testsuite: consistently add board_ldflags when linking with GCC Clément Chigot
@ 2022-10-03 17:10 ` Palmer Dabbelt
  2022-10-04  7:36   ` Clément Chigot
  2022-10-13 12:05 ` Clément Chigot
  1 sibling, 1 reply; 5+ messages in thread
From: Palmer Dabbelt @ 2022-10-03 17:10 UTC (permalink / raw)
  To: binutils, chigot

On Fri, 30 Sep 2022 07:05:03 PDT (-0700), binutils@sourceware.org wrote:
> Currently, the functions checking if the compiler is available or if a
> feature is available add both board_cflags and board_ldflags.
> However, functions running the tests only retrieve board_cflags. This
> can lead to unexpected errors when mandaratory flags are defined in
> board_ldflags and not board_cflags.
>
> ld/ChangeLog:
>
> 	* testsuite/ld-unique/unique.exp: Add board_ldflags when
> 	linking with GCC.
> 	* testsuite/lib/ld-lib.exp: Likewise.
> ---
>  ld/testsuite/ld-unique/unique.exp |  8 +++++++-
>  ld/testsuite/lib/ld-lib.exp       | 22 ++++++++++++++++++----
>  2 files changed, 25 insertions(+), 5 deletions(-)

Sorry if I'm misunderstanding what's going on here, but with this 
applied I'm still getting the tests skipped when I just run "check-ld".  

Running /home/palmer/life/binutils-gdb/ld/testsuite/ld-undefined/undefined.exp ...
UNTESTED: undefined
UNTESTED: undefined function
UNTESTED: undefined line

                === ld Summary ===

# of expected passes            797
# of expected failures          8
# of untested testcases         26
# of unsupported tests          175
./ld-new 2.39.50.20220930

I always kind of end up fumbling my way around these test suite 
infrastructure things though, so sorry if I'm just lost...

>
> diff --git a/ld/testsuite/ld-unique/unique.exp b/ld/testsuite/ld-unique/unique.exp
> index f3d5a5a6b7d..ab24eef50c3 100644
> --- a/ld/testsuite/ld-unique/unique.exp
> +++ b/ld/testsuite/ld-unique/unique.exp
> @@ -122,8 +122,14 @@ if [board_info [target_info name] exists cflags] {
>    set board_cflags ""
>  }
>
> +if [board_info [target_info name] exists ldflags] {
> +    set board_ldflags " [board_info [target_info name] ldflags]"
> +} else {
> +    set board_ldflags ""
> +}
> +
>  # Create executable containing unique symbol.
> -if ![ld_link "$CC_FOR_TARGET $NOPIE_LDFLAGS $board_cflags" "tmpdir/unique_prog" "tmpdir/unique.o"] {
> +if ![ld_link "$CC_FOR_TARGET $NOPIE_LDFLAGS $board_cflags $board_ldflags" "tmpdir/unique_prog" "tmpdir/unique.o"] {
>      fail "Could not link a unique executable"
>      set fails [expr $fails + 1]
>  }
> diff --git a/ld/testsuite/lib/ld-lib.exp b/ld/testsuite/lib/ld-lib.exp
> index ec27388a72e..2cd840c0169 100644
> --- a/ld/testsuite/lib/ld-lib.exp
> +++ b/ld/testsuite/lib/ld-lib.exp
> @@ -690,6 +690,7 @@ proc run_ld_link_exec_tests { ldtests args } {
>      global errcnt
>      global exec_output
>      global board_cflags
> +    global board_ldflags
>      global STATIC_LDFLAGS
>
>      # When using GCC as the linker driver, we need to specify board cflags when
> @@ -702,6 +703,12 @@ proc run_ld_link_exec_tests { ldtests args } {
>  	set board_cflags ""
>      }
>
> +    if [board_info [target_info name] exists ldflags] {
> +	set board_ldflags " [board_info [target_info name] ldflags]"
> +    } else {
> +	set board_ldflags ""
> +    }
> +
>      foreach testitem $ldtests {
>  	set testname [lindex $testitem 0]
>  	set ld_options [lindex $testitem 1]
> @@ -777,11 +784,11 @@ proc run_ld_link_exec_tests { ldtests args } {
>  	    continue;
>  	} else {
>  	    if { [string match "" $STATIC_LDFLAGS] \
> -		 && [regexp -- ".* \[-\]+static .*" " $board_cflags $ld_options $objfiles $ld_after "] } {
> +		 && [regexp -- ".* \[-\]+static .*" " $board_cflags $board_ldflags $ld_options $objfiles $ld_after "] } {
>  		untested $testname
>  		continue
>  	    }
> -	    if ![$link_proc $link_cmd $binfile "$board_cflags -L$srcdir/$subdir $ld_options $objfiles $ld_after"] {
> +	    if ![$link_proc $link_cmd $binfile "$board_cflags $board_ldflags -L$srcdir/$subdir $ld_options $objfiles $ld_after"] {
>  		set failed 1
>  	    }
>  	}
> @@ -858,6 +865,7 @@ proc run_cc_link_tests { ldtests } {
>      global ar
>      global exec_output
>      global board_cflags
> +    global board_ldflags
>      global STATIC_LDFLAGS
>
>      if [board_info [target_info name] exists cflags] {
> @@ -866,6 +874,12 @@ proc run_cc_link_tests { ldtests } {
>  	set board_cflags ""
>      }
>
> +    if [board_info [target_info name] exists ldflags] {
> +	set board_ldflags " [board_info [target_info name] ldflags]"
> +    } else {
> +	set board_ldflags ""
> +    }
> +
>      foreach testitem $ldtests {
>  	set testname [lindex $testitem 0]
>  	set ldflags [lindex $testitem 1]
> @@ -968,11 +982,11 @@ proc run_cc_link_tests { ldtests } {
>  	    }
>  	} else {
>  	    if { [string match "" $STATIC_LDFLAGS] \
> -		 && [regexp -- ".* \[-\]+static .*" " $board_cflags $ldflags $objfiles "] } {
> +		 && [regexp -- ".* \[-\]+static .*" " $board_cflags $board_ldflags $ldflags $objfiles "] } {
>  		untested $testname
>  		continue
>  	    }
> -	    ld_link $cc_cmd $binfile "$board_cflags -L$srcdir/$subdir $ldflags $objfiles"
> +	    ld_link $cc_cmd $binfile "$board_cflags $board_ldflags -L$srcdir/$subdir $ldflags $objfiles"
>  	    set ld_output "$exec_output"
>
>  	    if { $check_ld(source) == "regexp" } then {

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ld/testsuite: consistently add board_ldflags when linking with GCC
  2022-10-03 17:10 ` Palmer Dabbelt
@ 2022-10-04  7:36   ` Clément Chigot
  0 siblings, 0 replies; 5+ messages in thread
From: Clément Chigot @ 2022-10-04  7:36 UTC (permalink / raw)
  To: Palmer Dabbelt; +Cc: binutils

On Mon, Oct 3, 2022 at 7:10 PM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> On Fri, 30 Sep 2022 07:05:03 PDT (-0700), binutils@sourceware.org wrote:
> > Currently, the functions checking if the compiler is available or if a
> > feature is available add both board_cflags and board_ldflags.
> > However, functions running the tests only retrieve board_cflags. This
> > can lead to unexpected errors when mandaratory flags are defined in
> > board_ldflags and not board_cflags.
> >
> > ld/ChangeLog:
> >
> >       * testsuite/ld-unique/unique.exp: Add board_ldflags when
> >       linking with GCC.
> >       * testsuite/lib/ld-lib.exp: Likewise.
> > ---
> >  ld/testsuite/ld-unique/unique.exp |  8 +++++++-
> >  ld/testsuite/lib/ld-lib.exp       | 22 ++++++++++++++++++----
> >  2 files changed, 25 insertions(+), 5 deletions(-)
>
> Sorry if I'm misunderstanding what's going on here, but with this
> applied I'm still getting the tests skipped when I just run "check-ld".
>
> Running /home/palmer/life/binutils-gdb/ld/testsuite/ld-undefined/undefined.exp ...
> UNTESTED: undefined
> UNTESTED: undefined function
> UNTESTED: undefined line
>
>                 === ld Summary ===
>
> # of expected passes            797
> # of expected failures          8
> # of untested testcases         26
> # of unsupported tests          175
> ./ld-new 2.39.50.20220930
>
> I always kind of end up fumbling my way around these test suite
> infrastructure things though, so sorry if I'm just lost...

Haha don't worry, I spent quite some time trying to understand what
was going on.

AFAIU, there are several ways to pass flags to the DejaGNU driver.
The easiest is through environment variables or .exp files. For the ld
testsuite, they will be detected and applied correctly within
config/default.exp.
However, for more complex targets (like cross),  you can pass a
"board" to DejaGNU with --target_board flag [1]. This allows deeper
configuration like setting up a "simulator" (we are using qemu to run
our Risc-V executables for exemple).
My patch aims to improve supports when LDFLAGS are passed through the
board as follow:
  | set_board_info ldflags  "${LDFLAGS}"
  | set_currtarget_info ldflags  "${LDFLAGS}"

Note that I don't know the exact difference between the two. Sometimes
the first one is enough, sometimes the second is required.
If someone has better knowledge than me on that I would be glad.

[1] https://www.gnu.org/software/dejagnu/manual/Adding-a-new-board.html

Thanks,
Clément

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ld/testsuite: consistently add board_ldflags when linking with GCC
  2022-09-30 14:05 [PATCH] ld/testsuite: consistently add board_ldflags when linking with GCC Clément Chigot
  2022-10-03 17:10 ` Palmer Dabbelt
@ 2022-10-13 12:05 ` Clément Chigot
  2022-10-17 10:52   ` Nick Clifton
  1 sibling, 1 reply; 5+ messages in thread
From: Clément Chigot @ 2022-10-13 12:05 UTC (permalink / raw)
  To: binutils

Gentle ping

Thanks,
Clément

On Fri, Sep 30, 2022 at 4:05 PM Clément Chigot <chigot@adacore.com> wrote:
>
> Currently, the functions checking if the compiler is available or if a
> feature is available add both board_cflags and board_ldflags.
> However, functions running the tests only retrieve board_cflags. This
> can lead to unexpected errors when mandaratory flags are defined in
> board_ldflags and not board_cflags.
>
> ld/ChangeLog:
>
>         * testsuite/ld-unique/unique.exp: Add board_ldflags when
>         linking with GCC.
>         * testsuite/lib/ld-lib.exp: Likewise.
> ---
>  ld/testsuite/ld-unique/unique.exp |  8 +++++++-
>  ld/testsuite/lib/ld-lib.exp       | 22 ++++++++++++++++++----
>  2 files changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/ld/testsuite/ld-unique/unique.exp b/ld/testsuite/ld-unique/unique.exp
> index f3d5a5a6b7d..ab24eef50c3 100644
> --- a/ld/testsuite/ld-unique/unique.exp
> +++ b/ld/testsuite/ld-unique/unique.exp
> @@ -122,8 +122,14 @@ if [board_info [target_info name] exists cflags] {
>    set board_cflags ""
>  }
>
> +if [board_info [target_info name] exists ldflags] {
> +    set board_ldflags " [board_info [target_info name] ldflags]"
> +} else {
> +    set board_ldflags ""
> +}
> +
>  # Create executable containing unique symbol.
> -if ![ld_link "$CC_FOR_TARGET $NOPIE_LDFLAGS $board_cflags" "tmpdir/unique_prog" "tmpdir/unique.o"] {
> +if ![ld_link "$CC_FOR_TARGET $NOPIE_LDFLAGS $board_cflags $board_ldflags" "tmpdir/unique_prog" "tmpdir/unique.o"] {
>      fail "Could not link a unique executable"
>      set fails [expr $fails + 1]
>  }
> diff --git a/ld/testsuite/lib/ld-lib.exp b/ld/testsuite/lib/ld-lib.exp
> index ec27388a72e..2cd840c0169 100644
> --- a/ld/testsuite/lib/ld-lib.exp
> +++ b/ld/testsuite/lib/ld-lib.exp
> @@ -690,6 +690,7 @@ proc run_ld_link_exec_tests { ldtests args } {
>      global errcnt
>      global exec_output
>      global board_cflags
> +    global board_ldflags
>      global STATIC_LDFLAGS
>
>      # When using GCC as the linker driver, we need to specify board cflags when
> @@ -702,6 +703,12 @@ proc run_ld_link_exec_tests { ldtests args } {
>         set board_cflags ""
>      }
>
> +    if [board_info [target_info name] exists ldflags] {
> +       set board_ldflags " [board_info [target_info name] ldflags]"
> +    } else {
> +       set board_ldflags ""
> +    }
> +
>      foreach testitem $ldtests {
>         set testname [lindex $testitem 0]
>         set ld_options [lindex $testitem 1]
> @@ -777,11 +784,11 @@ proc run_ld_link_exec_tests { ldtests args } {
>             continue;
>         } else {
>             if { [string match "" $STATIC_LDFLAGS] \
> -                && [regexp -- ".* \[-\]+static .*" " $board_cflags $ld_options $objfiles $ld_after "] } {
> +                && [regexp -- ".* \[-\]+static .*" " $board_cflags $board_ldflags $ld_options $objfiles $ld_after "] } {
>                 untested $testname
>                 continue
>             }
> -           if ![$link_proc $link_cmd $binfile "$board_cflags -L$srcdir/$subdir $ld_options $objfiles $ld_after"] {
> +           if ![$link_proc $link_cmd $binfile "$board_cflags $board_ldflags -L$srcdir/$subdir $ld_options $objfiles $ld_after"] {
>                 set failed 1
>             }
>         }
> @@ -858,6 +865,7 @@ proc run_cc_link_tests { ldtests } {
>      global ar
>      global exec_output
>      global board_cflags
> +    global board_ldflags
>      global STATIC_LDFLAGS
>
>      if [board_info [target_info name] exists cflags] {
> @@ -866,6 +874,12 @@ proc run_cc_link_tests { ldtests } {
>         set board_cflags ""
>      }
>
> +    if [board_info [target_info name] exists ldflags] {
> +       set board_ldflags " [board_info [target_info name] ldflags]"
> +    } else {
> +       set board_ldflags ""
> +    }
> +
>      foreach testitem $ldtests {
>         set testname [lindex $testitem 0]
>         set ldflags [lindex $testitem 1]
> @@ -968,11 +982,11 @@ proc run_cc_link_tests { ldtests } {
>             }
>         } else {
>             if { [string match "" $STATIC_LDFLAGS] \
> -                && [regexp -- ".* \[-\]+static .*" " $board_cflags $ldflags $objfiles "] } {
> +                && [regexp -- ".* \[-\]+static .*" " $board_cflags $board_ldflags $ldflags $objfiles "] } {
>                 untested $testname
>                 continue
>             }
> -           ld_link $cc_cmd $binfile "$board_cflags -L$srcdir/$subdir $ldflags $objfiles"
> +           ld_link $cc_cmd $binfile "$board_cflags $board_ldflags -L$srcdir/$subdir $ldflags $objfiles"
>             set ld_output "$exec_output"
>
>             if { $check_ld(source) == "regexp" } then {
> --
> 2.25.1
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] ld/testsuite: consistently add board_ldflags when linking with GCC
  2022-10-13 12:05 ` Clément Chigot
@ 2022-10-17 10:52   ` Nick Clifton
  0 siblings, 0 replies; 5+ messages in thread
From: Nick Clifton @ 2022-10-17 10:52 UTC (permalink / raw)
  To: Clément Chigot, binutils

Hi Clément,

> Gentle ping

*embarrassed apology*

>> ld/ChangeLog:
>>
>>          * testsuite/ld-unique/unique.exp: Add board_ldflags when
>>          linking with GCC.
>>          * testsuite/lib/ld-lib.exp: Likewise.

Approved - please apply.

Cheers
   Nick


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-10-17 10:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-30 14:05 [PATCH] ld/testsuite: consistently add board_ldflags when linking with GCC Clément Chigot
2022-10-03 17:10 ` Palmer Dabbelt
2022-10-04  7:36   ` Clément Chigot
2022-10-13 12:05 ` Clément Chigot
2022-10-17 10:52   ` Nick Clifton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).