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

Message ID 201504062339.t36NdMHg006262@ignucius.se.axis.com
State Superseded
Headers

Commit Message

Hans-Peter Nilsson April 6, 2015, 11:39 p.m. UTC
  (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.


brgds, H-P
  

Comments

Mike Frysinger April 7, 2015, 2:58 a.m. UTC | #1
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
  
Mike Frysinger April 12, 2015, 8:21 a.m. UTC | #2
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
  

Patch

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.