[PATCH 02/16] gdb: tests: add support for testing FLAT toolchains

[PATCH 02/16] gdb: tests: add support for testing FLAT toolchains

[Mike Frysinger]

From: Jie Zhang <jie.zhang@analog.com>

FLAT toolchains output a FLAT binary for the named output and create
another file with a .gdb suffix that is used for debugging.  So when
testing a FLAT toolchain and we need to load up a file, use the .gdb.

Signed-off-by: Jie Zhang <jie.zhang@analog.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
2010-03-18  Jie Zhang  <jie@codesourcery.com>

	* gdb.base/cursal.exp: Append ".gdb" to binfile when use_binfmt_flat
	is set in target_info.
	* gdb.base/dump.exp: Declare new symfile variable and use it.
	* gdb.mi/mi-basics.exp: Likewise.
	* lib/gdbserver-support.exp (gdbserver_reconnect): Check for
	$host_exec.gdb when use_binfmt_flat is set.

 gdb/testsuite/gdb.base/cursal.exp       |    7 ++++++-
 gdb/testsuite/gdb.base/dump.exp         |    9 ++++++++-
 gdb/testsuite/gdb.mi/mi-basics.exp      |   14 +++++++++++---
 gdb/testsuite/lib/gdbserver-support.exp |   11 +++++++++++
 4 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/gdb/testsuite/gdb.base/cursal.exp b/gdb/testsuite/gdb.base/cursal.exp
index 69f864a..649a844 100644
--- a/gdb/testsuite/gdb.base/cursal.exp
+++ b/gdb/testsuite/gdb.base/cursal.exp
@@ -32,7 +32,12 @@ if {[gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug
 gdb_exit
 gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
-gdb_file_cmd ${binfile}
+if [target_info exists use_binfmt_flat] {
+    gdb_file_cmd ${binfile}.gdb
+} else {
+    gdb_file_cmd ${binfile}
+}
+
 gdb_test "set listsize 1" \
     ".*"
 
diff --git a/gdb/testsuite/gdb.base/dump.exp b/gdb/testsuite/gdb.base/dump.exp
index bc992dc..3820fd4 100644
--- a/gdb/testsuite/gdb.base/dump.exp
+++ b/gdb/testsuite/gdb.base/dump.exp
@@ -27,6 +27,13 @@ set testfile "dump"
 
 set srcfile  ${testfile}.c
 set binfile  ${objdir}/${subdir}/${testfile}
+
+if [target_info exists use_binfmt_flat] {
+    set symfile "${binfile}.gdb"
+} else {
+    set symfile ${binfile}
+}
+
 set options  {debug}
 
 set is64bitonly "no"
@@ -208,7 +215,7 @@ make_dump_file \
 
 gdb_exit
 gdb_start
-gdb_file_cmd ${binfile}
+gdb_file_cmd ${symfile}
 
 # Now fix the endianness at the correct state.
 
diff --git a/gdb/testsuite/gdb.mi/mi-basics.exp b/gdb/testsuite/gdb.mi/mi-basics.exp
index dcf4dd4..a0054b8 100644
--- a/gdb/testsuite/gdb.mi/mi-basics.exp
+++ b/gdb/testsuite/gdb.mi/mi-basics.exp
@@ -37,6 +37,13 @@ if [mi_gdb_start separate-inferior-tty] {
 set testfile "basics"
 set srcfile ${testfile}.c
 set binfile ${objdir}/${subdir}/mi-basics
+
+if [target_info exists use_binfmt_flat] {
+    set symfile "${binfile}.gdb"
+} else {
+    set symfile ${binfile}
+}
+
 set escapedobjdir [string_to_regexp ${objdir}]
 set envirodir [string_to_regexp ${objdir}/${subdir}]
 
@@ -83,6 +90,7 @@ proc test_mi_interpreter_selection {} {
 proc test_exec_and_symbol_mi_operatons {} {
     global mi_gdb_prompt
     global binfile
+    global symfile
 
     # Load symbols and specify executable on a single operation
     # Tests:
@@ -90,7 +98,7 @@ proc test_exec_and_symbol_mi_operatons {} {
 
     # Can't use mi_gdb_test as if this doesn't work,
     #  we must give up on the whole test file
-    send_gdb "-file-exec-and-symbols ${binfile}\n"
+    send_gdb "-file-exec-and-symbols ${symfile}\n"
     gdb_expect {
            -re "\[\r\n\]*\\\^done\r\n$mi_gdb_prompt$" \
                { pass "file-exec-and-symbols operation" }
@@ -114,11 +122,11 @@ proc test_exec_and_symbol_mi_operatons {} {
 #            "\\\^done" \
 #            "file-clear operation"
 
-    mi_gdb_test "-file-exec-file ${binfile}" \
+    mi_gdb_test "-file-exec-file ${symfile}" \
              "\\\^done" \
              "file-exec-file operation"
 
-    mi_gdb_test "-file-symbol-file ${binfile}" \
+    mi_gdb_test "-file-symbol-file ${symfile}" \
              "\\\^done" \
              "file-symbol-file operation"
 
diff --git a/gdb/testsuite/lib/gdbserver-support.exp b/gdb/testsuite/lib/gdbserver-support.exp
index 5d53dfa..a32a2e5 100644
--- a/gdb/testsuite/lib/gdbserver-support.exp
+++ b/gdb/testsuite/lib/gdbserver-support.exp
@@ -315,6 +315,17 @@ proc gdbserver_reconnect { } {
     global gdbserver_protocol
     global gdbserver_gdbport
 
+    if { $host_exec != "" } {
+	if [target_info exists use_binfmt_flat] {
+	    if [gdb_file_cmd "$host_exec.gdb"] {
+		return -1
+	    }
+	} else {
+	    if [gdb_file_cmd $host_exec] {
+		return -1
+	    }
+	}
+    }
     return [gdb_target_cmd $gdbserver_protocol $gdbserver_gdbport]
 }
 
-- 
1.7.0.2

Re: [PATCH 02/16] gdb: tests: add support for testing FLAT toolchains

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 52 bytes --]

ugh, sorry, ignore the "02/16" in the subject
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 02/16] gdb: tests: add support for testing FLAT toolchains

[Daniel Jacobowitz]

On Thu, Mar 18, 2010 at 03:10:40AM -0400, Mike Frysinger wrote:
> From: Jie Zhang <jie.zhang@analog.com>
> 
> FLAT toolchains output a FLAT binary for the named output and create
> another file with a .gdb suffix that is used for debugging.  So when
> testing a FLAT toolchain and we need to load up a file, use the .gdb.

FWIW, in our board files, we found it simpler to swap the two.  So
we have a gdb_compile which generates an elf file as foo, and a flat
file as foo.flt.  Nothing in the GDB testsuite except for the board
file itself needs to access the FLT file.

That's not the only way to do it, we can accomodate both.  But I'm not
thrilled with having binfmt_flat references scattered around random
tests.  In lib/ maybe, but not everywhere that uses gdb_file_cmd...

Or in the main definition of gdb_compile?

-- 
Daniel Jacobowitz
CodeSourcery

Re: [PATCH 02/16] gdb: tests: add support for testing FLAT toolchains

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1554 bytes --]

On Thursday 18 March 2010 09:38:22 Daniel Jacobowitz wrote:
> On Thu, Mar 18, 2010 at 03:10:40AM -0400, Mike Frysinger wrote:
> > FLAT toolchains output a FLAT binary for the named output and create
> > another file with a .gdb suffix that is used for debugging.  So when
> > testing a FLAT toolchain and we need to load up a file, use the .gdb.
> 
> FWIW, in our board files, we found it simpler to swap the two.  So
> we have a gdb_compile which generates an elf file as foo, and a flat
> file as foo.flt.  Nothing in the GDB testsuite except for the board
> file itself needs to access the FLT file.

hmm, that is one way of going about the issue, but personally i feel that this 
kind of logic should be in the test suite.  imo, the dejagnu framework already 
puts too much demand on the end user to simply run `make check`.  plus, since 
this isnt specific to an arch and FLAT isnt an uncommon target, doing it once 
in common code seems like it'd be easier for people to collaborate on.

> That's not the only way to do it, we can accomodate both.  But I'm not
> thrilled with having binfmt_flat references scattered around random
> tests.  In lib/ maybe, but not everywhere that uses gdb_file_cmd...
> 
> Or in the main definition of gdb_compile?

i agree the current patch isnt terrible suave, but i'm not terribly familiar 
with the gdb testing framework.  so i thought i'd post the patch to garner 
feedback on a better way and then attack the problem based on that.

i'll give gdb_compile a try ... thanks !
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH/RFC] gdb: tests: add support for testing FLAT toolchains

[Mike Frysinger]

From: Jie Zhang <jie@codesourcery.com>

FLAT toolchains output a FLAT binary for the named output and create
another file with a .gdb suffix that is used for debugging.  So when
testing a FLAT toolchain and we need to load up a file, use the .gdb.

This restarts an old thread and implements things as suggested by
Daniel Jacobowitz:
http://sourceware.org/ml/gdb-patches/2010-03/msg00648.html

Pretty sure this works (I'll double check), but I want to make sure
this is going in the direction people are OK with.

Signed-off-by: Jie Zhang <jie@codesourcery.com>
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
 gdb/testsuite/lib/gdb.exp |   22 +++++++++++++++++++++-
 1 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index dc16cfa..091b271 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -2091,6 +2091,16 @@ proc current_target_name { } {
     return $answer
 }
 
+proc gdb_get_exec_name { binfile } {
+    # FLAT toolchains have to load the .flt file to the board
+    if { [target_info exists use_binfmt_flat]
+	 && [file exists "${binfile}.flt"] } {
+	return "${binfile}.flt"
+    } else {
+	return ${binfile}
+    }
+}
+
 set gdb_wrapper_initialized 0
 set gdb_wrapper_target ""
 
@@ -2286,6 +2296,16 @@ proc gdb_compile {source dest type options} {
 	    clone_output "gdb compile failed, $result"
 	}
     }
+
+    # FLAT toolchains output a bFLT file with the default name and create
+    # a secondary ELF file with a .gdb suffix.  GDB needs the ELF file in
+    # order to do debugging, so flip-flop things.
+    if { [target_info exists use_binfmt_flat]
+	 && [file exists "${dest}.gdb"] } {
+	file rename -force "${dest}" "${dest}.flt"
+	file rename -force "${dest}.gdb" "${dest}"
+    }
+
     return $result;
 }
 
@@ -2762,7 +2782,7 @@ proc gdb_touch_execfile { binfile } {
 proc gdb_download { filename } {
     global cleanfiles
 
-    set destname [remote_download target $filename]
+    set destname [remote_download target [gdb_get_exec_name $filename]]
     lappend cleanfiles $destname
     return $destname
 }
-- 
1.7.6

Re: [PATCH/RFC] gdb: tests: add support for testing FLAT toolchains

[Pedro Alves]

On Friday 01 July 2011 01:24:41, Mike Frysinger wrote:
> From: Jie Zhang <jie@codesourcery.com>
> 
> FLAT toolchains output a FLAT binary for the named output and create
> another file with a .gdb suffix that is used for debugging.  So when
> testing a FLAT toolchain and we need to load up a file, use the .gdb.

Sounds a lot like the recently added
lib/gdb.exp:exec_target_file/exec_symbol_file hooks?  We added them
to solve the exact same problem.  We then have this in our board file
for uclinux/flat toolchains:

       proc exec_target_file { binfile } {
           return ${binfile}.flt
       }

-- 
Pedro Alves

Re: [PATCH/RFC] gdb: tests: add support for testing FLAT toolchains

[Mike Frysinger]

On Fri, Jul 1, 2011 at 05:16, Pedro Alves wrote:
> On Friday 01 July 2011 01:24:41, Mike Frysinger wrote:
>> From: Jie Zhang <jie@codesourcery.com>
>>
>> FLAT toolchains output a FLAT binary for the named output and create
>> another file with a .gdb suffix that is used for debugging.  So when
>> testing a FLAT toolchain and we need to load up a file, use the .gdb.
>
> Sounds a lot like the recently added
> lib/gdb.exp:exec_target_file/exec_symbol_file hooks?  We added them
> to solve the exact same problem.  We then have this in our board file
> for uclinux/flat toolchains:
>
>       proc exec_target_file { binfile } {
>           return ${binfile}.flt
>       }

probably ... this code does date back years ago, and when i poked
things again one year ago, it still needed to be done.  i'll take a
look at what you refer to now though.
-mike

Re: [PATCH/RFC] gdb: tests: add support for testing FLAT toolchains

[Pedro Alves]

On Friday 01 July 2011 15:28:17, Mike Frysinger wrote:
> On Fri, Jul 1, 2011 at 05:16, Pedro Alves wrote:
> > On Friday 01 July 2011 01:24:41, Mike Frysinger wrote:
> >> From: Jie Zhang <jie@codesourcery.com>
> >>
> >> FLAT toolchains output a FLAT binary for the named output and create
> >> another file with a .gdb suffix that is used for debugging.  So when
> >> testing a FLAT toolchain and we need to load up a file, use the .gdb.
> >
> > Sounds a lot like the recently added
> > lib/gdb.exp:exec_target_file/exec_symbol_file hooks?  We added them
> > to solve the exact same problem.  We then have this in our board file
> > for uclinux/flat toolchains:
> >
> >       proc exec_target_file { binfile } {
> >           return ${binfile}.flt
> >       }
> 
> probably ... this code does date back years ago, and when i poked
> things again one year ago, it still needed to be done.  i'll take a
> look at what you refer to now though.

I took another look at how we do things.  Those hooks I pointed at
are only part of the story, and are not enough for what you
need --- we _also_ override gdb_compile in our board files, calling
the existing version, and then doing the renaming very similarly to
what your patch does.

(What I've wondered before was if it was possible to pass enough
switches to the tools in order to have them output the files
with the names we need in the first place, in order to avoid the
renaming.)

I'm not super keen on having these target and environment pecularities
straight in the core testsuite code.  I'd rather we had enough
hooks to make it easy for a board file or a config file to do what
it needs to.  E.g., symbian toolchains also have separate binary
and symbol files (the reason shlib_target_file/shlib_symbol_file
were added a while ago, uppon which exec_target_file/exec_symbol_file
were later modelled on), and the extension is not .flt then,
of course.  Maybe we could do something like 
gdb/testsuite/config/gdbserver.exp/gdb/testsuite/lib/gdbserver-support.exp
instead?  That is, move your code to new gdb/testsuite/config/flt.exp
gdb/testsuite/lib/flt-support.exp files that do what you
have, but by overriding what needs overriding (gdb_compile
and the exec_*_file hooks mainly)?  WDYT?

-- 
Pedro Alves

Re: [PATCH/RFC] gdb: tests: add support for testing FLAT toolchains

[Mike Frysinger]

On Fri, Jul 1, 2011 at 11:16, Pedro Alves wrote:
> On Friday 01 July 2011 15:28:17, Mike Frysinger wrote:
>> On Fri, Jul 1, 2011 at 05:16, Pedro Alves wrote:
>> > On Friday 01 July 2011 01:24:41, Mike Frysinger wrote:
>> >> From: Jie Zhang <jie@codesourcery.com>
>> >>
>> >> FLAT toolchains output a FLAT binary for the named output and create
>> >> another file with a .gdb suffix that is used for debugging.  So when
>> >> testing a FLAT toolchain and we need to load up a file, use the .gdb.
>> >
>> > Sounds a lot like the recently added
>> > lib/gdb.exp:exec_target_file/exec_symbol_file hooks?  We added them
>> > to solve the exact same problem.  We then have this in our board file
>> > for uclinux/flat toolchains:
>> >
>> >       proc exec_target_file { binfile } {
>> >           return ${binfile}.flt
>> >       }
>>
>> probably ... this code does date back years ago, and when i poked
>> things again one year ago, it still needed to be done.  i'll take a
>> look at what you refer to now though.
>
> I took another look at how we do things.  Those hooks I pointed at
> are only part of the story, and are not enough for what you
> need --- we _also_ override gdb_compile in our board files, calling
> the existing version, and then doing the renaming very similarly to
> what your patch does.
>
> (What I've wondered before was if it was possible to pass enough
> switches to the tools in order to have them output the files
> with the names we need in the first place, in order to avoid the
> renaming.)

it is possible if the link is done twice, but that isnt better.
bfin-uclinux-gcc ... -o foo -no-elf2flt
bfin-uclinux-gcc ... -o foo.flt
(of course, you would have an extra foo.flt.gdb)

> I'm not super keen on having these target and environment pecularities
> straight in the core testsuite code.  I'd rather we had enough
> hooks to make it easy for a board file or a config file to do what
> it needs to.  E.g., symbian toolchains also have separate binary
> and symbol files (the reason shlib_target_file/shlib_symbol_file
> were added a while ago, uppon which exec_target_file/exec_symbol_file
> were later modelled on), and the extension is not .flt then,
> of course.  Maybe we could do something like
> gdb/testsuite/config/gdbserver.exp/gdb/testsuite/lib/gdbserver-support.exp
> instead?  That is, move your code to new gdb/testsuite/config/flt.exp
> gdb/testsuite/lib/flt-support.exp files that do what you
> have, but by overriding what needs overriding (gdb_compile
> and the exec_*_file hooks mainly)?  WDYT?

i dont need the logic to be in the core gdb testsuite, but i think it
does have to be in the gdb testsuite somewhere.  i dont think making
all board porters do the same thing over and over again is the right
path.  after all, this is going to be the same issue across all
toolchains that target this binary format.  which is what we just
found out since i doubt the code you have is for Blackfin processors
:).

it seems like the shlib/exec hooks you mention should be sufficient.
do you know why in your setup you still need to bang on gdb_compile ?
-mike

Re: [PATCH/RFC] gdb: tests: add support for testing FLAT toolchains

[Pedro Alves]

On Friday 01 July 2011 16:55:02, Mike Frysinger wrote:

> i dont need the logic to be in the core gdb testsuite, but i think it
> does have to be in the gdb testsuite somewhere.  i dont think making
> all board porters do the same thing over and over again is the right
> path.  after all, this is going to be the same issue across all
> toolchains that target this binary format.  which is what we just
> found out since i doubt the code you have is for Blackfin processors
> :).

:-)  Yes, agreed.

> it seems like the shlib/exec hooks you mention should be sufficient.
> do you know why in your setup you still need to bang on gdb_compile ?

Hmm.  The gdb_compile overriding was added much earlier on our 
board files than the new hooks.  The hooks were added recently,
to fix the gdb.base/reread.exp test on flt targets.  It's sole
purpose is to the the same renaming dance you have.
That is, IIUC, if we don't do the gdb_compile overriding + .flt
renaming, as is, tests will still try to load the FLAT file, instead
of the .gdb file.  exec_symbol_file/exec_target_file are only used
in gdb_rename_execfile/gdb_touch_execfile presently.  :-/
Maybe there's some central places, like gdb_load, where we
should be using exec_symbol_file on?  Not sure what else (if
anything) would break; some tests themselves might need some
output adjustment.

-- 
Pedro Alves