Patch, sim: fix m68hc11 and iq2000 testsuites using dejagnu baseboard files

Patch, sim: fix m68hc11 and iq2000 testsuites using dejagnu baseboard files

[Hans-Peter Nilsson]

(I'll send a heads-up to the dejagnu list but I won't CC them
on this message as that list is subscriber-only, says
http://www.gnu.org/software/dejagnu/.)

All of a sudden (thanks to Mike) m68hc11 and iq2000 sims both
have test-suites, with a very welcome hello-world test.  But,
for people expecting that to work with the corresponding
supposedly-intended dejagnu baseboard, those tests fail linking.

For iq2000, iq2000-sim.exp has:
 # Special linker script needed to run C programs.
 set_board_info ldscript "-Tsim.ld"

but there's no sim.ld installed with today's binutils
which is all we require for running the sim testsuite.
(I see it's in libgloss.)

For m68hc11, there's a confusingly inconsistent comment and
setting:
 # No linker script needed.
 set_board_info ldscript "-Wl,--script,sim-valid.x"

I'm not sure where the .x suffix comes from; libgloss would
install a sim-valid.ld.  You might wonder how that gcc-specific
"-Wl,"-syntax is supposed to work with a plain linker call.  The
(IMHO) ugly truth is that default_link target.exp would strip a
"-Wl,"-prefix, but it doesn't take care to *also* replace ","
with " ", so I don't think that has ever worked with plain
linker calls; those that end up in default_link calls.

Locally eliminating ldscript probably seems ugly.  There's
precedent; see sim/testsuite/sim/mips/basic.exp.  An alternative
would be to automatically disabling it in the first call to
run_sim_test, but that seemed just too smart.

I'm going to install the following in a day or two, unless
someone has a better alternative (excluding "stop using the
broken {iq2000,m68hc11}-sim.exp" as I don't want the sim
testsuite to *require* different parameters to the other tools).

sim/testsuite/sim/m68hc11:
	* allinsn.exp: Disable baseboard ldscript variable.

sim/testsuite/sim/iq2000:
	* allinsn.exp: Disable baseboard ldscript variable.

diff --git a/sim/testsuite/sim/iq2000/allinsn.exp b/sim/testsuite/sim/iq2000/allinsn.exp
index 38eee9b..414b0f8 100644
--- a/sim/testsuite/sim/iq2000/allinsn.exp
+++ b/sim/testsuite/sim/iq2000/allinsn.exp
@@ -4,6 +4,14 @@ if [istarget iq2000-*] {
     # all machines
     set all_machs "iq2000"
 
+    # For people using the dejagnu iq2000-sim.exp baseboard, it
+    # (as of dejagnu post-1.5 2015-04-06 41070790) sets a ldscript
+    # variable requiring a script "sim.ld" which does not exist in a
+    # plain binutils installation of iq2000-elf; it's in libgloss.  So,
+    # as gross as it is, we locally unset the linker script specified by
+    # the target board.
+    unset_currtarget_info ldscript
+
     foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.s]] {
 	# If we're only testing specific files and this isn't one of them,
 	# skip it.
diff --git a/sim/testsuite/sim/m68hc11/allinsn.exp b/sim/testsuite/sim/m68hc11/allinsn.exp
index db0cbd5..88bbc34 100644
--- a/sim/testsuite/sim/m68hc11/allinsn.exp
+++ b/sim/testsuite/sim/m68hc11/allinsn.exp
@@ -4,6 +4,17 @@ if [istarget m68hc11-*] {
     # all machines
     set all_machs "m68hc11"
 
+    # For people using the dejagnu m68hc11-sim.exp baseboard, it
+    # (as of dejagnu post-1.5 2015-04-06 41070790) sets a ldscript
+    # variable requiring a script "sim-valid.x" which does not exist in a
+    # plain binutils installation of m68hc11-elf (there's a sim-valid.ld
+    # installed with libgloss), also, with a syntax which doesn't work
+    # for default_link (would require replacing "," with " " in addition
+    # to stripping the "-Wl,"-prefix).  So, as gross as it is, we keep
+    # it simple and just unset the linker script specified by the target
+    # board.
+    unset_currtarget_info ldscript
+
     foreach src [lsort [glob -nocomplain $srcdir/$subdir/*.s]] {
 	# If we're only testing specific files and this isn't one of them,
 	# skip it.

brgds, H-P

Re: Patch, sim: fix m68hc11 and iq2000 testsuites using dejagnu baseboard files

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1875 bytes --]

On 07 Apr 2015 01:39, Hans-Peter Nilsson wrote:
> All of a sudden (thanks to Mike) m68hc11 and iq2000 sims both
> have test-suites, with a very welcome hello-world test.  But,
> for people expecting that to work with the corresponding
> supposedly-intended dejagnu baseboard, those tests fail linking.
> 
> For iq2000, iq2000-sim.exp has:
>  # Special linker script needed to run C programs.
>  set_board_info ldscript "-Tsim.ld"
> 
> but there's no sim.ld installed with today's binutils
> which is all we require for running the sim testsuite.
> (I see it's in libgloss.)
> 
> For m68hc11, there's a confusingly inconsistent comment and
> setting:
>  # No linker script needed.
>  set_board_info ldscript "-Wl,--script,sim-valid.x"
> 
> I'm not sure where the .x suffix comes from; libgloss would
> install a sim-valid.ld.  You might wonder how that gcc-specific
> "-Wl,"-syntax is supposed to work with a plain linker call.  The
> (IMHO) ugly truth is that default_link target.exp would strip a
> "-Wl,"-prefix, but it doesn't take care to *also* replace ","
> with " ", so I don't think that has ever worked with plain
> linker calls; those that end up in default_link calls.
> 
> Locally eliminating ldscript probably seems ugly.  There's
> precedent; see sim/testsuite/sim/mips/basic.exp.  An alternative
> would be to automatically disabling it in the first call to
> run_sim_test, but that seemed just too smart.

odd, it's working for me.  what is different about your environment ?  i'm just 
doing `make sim-check`.

i really don't want the sim to get into the business of fighting dejagnu over 
what a sane environment looks like.  but i also don't want to start sprinkling 
this logic over all targets.  so moving the existing mips logic to run_sim_test 
in lib/sim-defs.exp seems like the least worse option to me.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Patch, sim: fix m68hc11 and iq2000 testsuites using dejagnu baseboard files

[Hans-Peter Nilsson]

> From: Mike Frysinger <vapier@gentoo.org>
> Date: Tue, 7 Apr 2015 04:58:22 +0200

> odd, it's working for me.  what is different about your environment ?  i'm just 
> doing `make sim-check`.

This is old. :)  You never specify the dejagnu sim baseboards
(defaulting to "unix.exp" the native one), while I do; see
the subject.  I.e. you never pass
RUNTESTFLAGS=--target_board=iq2000-sim etc.  Let's no argue
about the validity of doing either: the simulator is at such a
low level that its test-suite can be argued to know how to build
simple programs for itself, but it should also not have to have
a different baseboard required than the rest of a unified-tree
(aka. uberbaum) setup.  (Not that iq2000-elf nor m68hc11 appear
to have actively maintained toolchains...)

> i really don't want the sim to get into the business of fighting dejagnu over 
> what a sane environment looks like.

Too late.

>  but i also don't want to start sprinkling 
> this logic over all targets.  so moving the existing mips logic to run_sim_test 
> in lib/sim-defs.exp seems like the least worse option to me.

And that's what I referred to as the "too smart" alternative of
moving it do run_sim_test (I did have a quick look).  Better
keep it target-specific, as it's the specific target board file
that's "broken" (besides the "-Wl,"-stripping).  If those
targets change in level of activity, I expect the ldscript
business to be fixed too.

brgds, H-P

Re: Patch, sim: fix m68hc11 and iq2000 testsuites using dejagnu baseboard files

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 2231 bytes --]

On 07 Apr 2015 05:59, Hans-Peter Nilsson wrote:
> > From: Mike Frysinger <vapier@gentoo.org>
> > Date: Tue, 7 Apr 2015 04:58:22 +0200
> 
> > odd, it's working for me.  what is different about your environment ?  i'm just 
> > doing `make sim-check`.
> 
> This is old. :)  You never specify the dejagnu sim baseboards
> (defaulting to "unix.exp" the native one), while I do; see
> the subject.  I.e. you never pass
> RUNTESTFLAGS=--target_board=iq2000-sim etc.  Let's no argue
> about the validity of doing either: the simulator is at such a
> low level that its test-suite can be argued to know how to build
> simple programs for itself, but it should also not have to have
> a different baseboard required than the rest of a unified-tree
> (aka. uberbaum) setup.  (Not that iq2000-elf nor m68hc11 appear
> to have actively maintained toolchains...)

i'm not trying to say you're flow is wrong.  i'm merely asking how it differs 
from my own.

> > i really don't want the sim to get into the business of fighting dejagnu over 
> > what a sane environment looks like.
> 
> Too late.
> 
> >  but i also don't want to start sprinkling 
> > this logic over all targets.  so moving the existing mips logic to run_sim_test 
> > in lib/sim-defs.exp seems like the least worse option to me.
> 
> And that's what I referred to as the "too smart" alternative of
> moving it do run_sim_test (I did have a quick look).  Better
> keep it target-specific, as it's the specific target board file
> that's "broken" (besides the "-Wl,"-stripping).  If those
> targets change in level of activity, I expect the ldscript
> business to be fixed too.

the sim is not board specific, nor does the testsuite (generally) care about the 
C library.  it certainly is not a test ground for making sure newlib/libgloss 
itself is bug free (that's what testsuites in those projects are for).  that 
leads me to the opinion that board-specific linkage information does not make 
sense in the sim so we should simply clear it for all targets.  in fact, we want 
the sim to be board agnostic (ignoring the model support, and tests that 
exercise those models directly, but is still orthogonal to dejagnu setup).
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Patch, sim: fix m68hc11 and iq2000 testsuites using dejagnu baseboard files

[Hans-Peter Nilsson]

> From: Mike Frysinger <vapier@gentoo.org>
> Date: Sun, 12 Apr 2015 10:21:45 +0200

> On 07 Apr 2015 05:59, Hans-Peter Nilsson wrote:
> > And that's what I referred to as the "too smart" alternative of
> > moving it do run_sim_test (I did have a quick look).  Better
> > keep it target-specific, as it's the specific target board file
> > that's "broken" (besides the "-Wl,"-stripping).  If those
> > targets change in level of activity, I expect the ldscript
> > business to be fixed too.
> 
> the sim is not board specific, nor does the testsuite (generally) care about the 
> C library.  it certainly is not a test ground for making sure newlib/libgloss 
> itself is bug free (that's what testsuites in those projects are for).  that 
> leads me to the opinion that board-specific linkage information does not make 
> sense in the sim so we should simply clear it for all targets.  in fact, we want 
> the sim to be board agnostic (ignoring the model support, and tests that 
> exercise those models directly, but is still orthogonal to dejagnu setup).

Maybe, maybe not.  If a simulator needs a specific linker-script
to work with a specific linker, it should have a means to
specify that without colliding with workarounds for wrong
assumptions and ldscript settings for certain *other* targets.

Anyway, I found a "less smart" alternative than to push this
into run_sim_test; add it to the sim_init proc. (duh :)  After
checking that it indeed works, this is what I just checked in.

Though, I didn't check that a target *can* set its own ldscript
variable to override this; if the need arises, we'll have to
revisit and move this to just the targets that have broken board
files.  (N.B. this is *ldscript*; there is a separate *ldflags*.)

sim/testsuite:
	* sim-defs.exp (sim_init): Unset target ldscript here.

sim/testsuite/sim/mips:
	* basic.exp: Don't unset target ldscript here.

diff --git a/sim/testsuite/lib/sim-defs.exp b/sim/testsuite/lib/sim-defs.exp
index fb2346a..2faf5dc 100644
--- a/sim/testsuite/lib/sim-defs.exp
+++ b/sim/testsuite/lib/sim-defs.exp
@@ -12,6 +12,16 @@ proc sim_init { args } {
     global sim_path
     set sim_path [board_info target sim]
     # Need to return an empty string (copied from GAS).
+
+    # As gross as it is, we unset the linker script specified by the target
+    # board.  The simulator board file mips-sim.exp, sets ldscript to the
+    # MIPS libgloss linker scripts which include libgcc (and possibly other
+    # libraries), which the linker (used to link these tests rather than the
+    # compiler) can't necessarily find.  Similarly iq2000-sim.exp and
+    # m68hc11-sim.exp.  So, we make it a common rule to clear the slate for
+    # all simulators.
+    unset_currtarget_info ldscript
+
     return ""
 }
 
diff --git a/sim/testsuite/sim/mips/basic.exp b/sim/testsuite/sim/mips/basic.exp
index 1c78c87..ddef535 100644
--- a/sim/testsuite/sim/mips/basic.exp
+++ b/sim/testsuite/sim/mips/basic.exp
@@ -1,11 +1,5 @@
 # MIPS simulator instruction tests
 
-# As gross as it is, we unset the linker script specified by the target
-# board.  The MIPS libgloss linker scripts include libgcc (and possibly
-# other libraries), which the linker (used to link these tests rather
-# than the compiler) can't necessarily find.
-unset_currtarget_info ldscript
-
 # Do "run_sim_test TESTFILE MODELS" for each combination of the
 # mf{lo,hi} -> mult/div/mt{lo,hi} hazard described in mips.igen.
 # Insert NOPS nops after the mflo or mfhi.

brgds, H-P