[SH] gdb.arch/gdb1291.exp, gdb.arch/gdb1431.exp

[SH] gdb.arch/gdb1291.exp, gdb.arch/gdb1431.exp

[Thomas Schwinge]

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

Hi!

For sh-linux-gnu, both gdb.arch/gdb1291.exp and gdb.arch/gdb1431.exp
currently fail to build as follows:

    Running [...]/gdb.arch/gdb1291.exp ...
    Executing on host: sh-linux-gnu-gcc [...]/gdb.arch/gdb1291.s [...] -o [...]/gdb.arch/gdb1291    (timeout = 3600)
    [...]/usr/lib/crt1.o: In function `L_main':
    (.text+0x1c): undefined reference to `main'
    /tmp/cciSZQqk.o: In function `_main':
    gdb1291.c:(.text+0x2c): undefined reference to `_printf'
    collect2: error: ld returned 1 exit status

Exactly the same for gdb.arch/gdb1431.exp: their source files
(gdb.arch/gdb1291.s, gdb.arch/gdb1431.s) are identical, and I first
propose to get rid of one of them:

gdb/testsuite/

	* gdb.arch/gdb1431.s: Remove file.
	* gdb.arch/gdb1431.exp (srcfile): Refer to gdb1291.s.

Index: gdb.arch/gdb1291.s
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.arch/gdb1291.s,v
retrieving revision 1.5
diff -u -p -r1.5 gdb1291.s
--- gdb.arch/gdb1291.s	4 Jan 2012 08:17:44 -0000	1.5
+++ gdb.arch/gdb1291.s	23 Mar 2012 15:32:32 -0000
@@ -18,6 +18,8 @@
 !
 ! This file is part of the gdb testsuite.
 !
+! It is shared between gdb1291.exp and gdb1431.exp.
+!
 ! It was generated using "sh-elf-gcc -S gdb1291.c", using the following
 ! source file:
 !
Index: gdb.arch/gdb1431.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.arch/gdb1431.exp,v
retrieving revision 1.12
diff -u -p -r1.12 gdb1431.exp
--- gdb.arch/gdb1431.exp	16 Jan 2012 16:21:44 -0000	1.12
+++ gdb.arch/gdb1431.exp	23 Mar 2012 15:28:11 -0000
@@ -32,7 +32,8 @@ if ![istarget "sh-*-*"] then {
 }
 
 set testfile "gdb1431"
-set srcfile ${testfile}.s
+# Use gdb1291.s instead of duplicating it.
+set srcfile "gdb1291.s"
 set binfile ${objdir}/${subdir}/${testfile}
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable ""] != "" } {
     untested gdb1431.exp


Next, for the linking error, the problem is that global symbols (such as
printf) are specified with a leading underscore, which is correct for
sh-elf, but not for sh-linux-gnu.  Relying on GCC's
__USER_LABEL_PREFIX__, I propose the following:

gdb/testsuite/

	* gdb.arch/gdb1291.s: Rename file to...
	* gdb.arch/gdb1291.S: ... this.
	(CONCAT1, CONCAT2, SYM): New macros.
	(_main, _printf, _sub1, _sub2): Replace with SYM(main), etc.
	* gdb.arch/gdb1291.exp (srcfile): Update for the changed filename.
	* gdb.arch/gdb1431.exp (srcfile): Likewise.

--- gdb.arch/gdb1291.s	2012-03-23 16:32:22.160825674 +0100
+++ gdb.arch/gdb1291.S	2012-03-23 12:46:10.730852730 +0100
@@ -20,8 +20,8 @@
 !
 ! It is shared between gdb1291.exp and gdb1431.exp.
 !
-! It was generated using "sh-elf-gcc -S gdb1291.c", using the following
-! source file:
+! It has originally been generated using "sh-elf-gcc -S gdb1291.c", using the
+! following source file:
 !
 !	#include <stdio.h>
 !	
@@ -46,6 +46,11 @@
 ! We use a pregenerated assembly file as the test input to avoid possible
 ! problems with future versions of gcc generating different code.
 
+
+#define CONCAT1(a, b) CONCAT2(a, b)
+#define CONCAT2(a, b) a ## b
+#define SYM(x) CONCAT1 (__USER_LABEL_PREFIX__, x)
+
 	.file	"gdb1291.c"
 	.text
 	.section	.rodata
@@ -54,9 +59,9 @@
 	.string	"hello world\n"
 	.text
 	.align 1
-	.global	_main
-	.type	_main, @function
-_main:
+	.global	SYM(main)
+	.type	SYM(main), @function
+SYM(main):
 	mov.l	r14,@-r15
 	sts.l	pr,@-r15
 	mov	r15,r14
@@ -81,16 +86,16 @@
 .L2:
 	.long	.LC0
 .L3:
-	.long	_printf
+	.long	SYM(printf)
 .L4:
-	.long	_sub1
+	.long	SYM(sub1)
 .L5:
-	.long	_sub2
-	.size	_main, .-_main
+	.long	SYM(sub2)
+	.size	SYM(main), . - SYM(main)
 	.align 1
-	.global	_sub1
-	.type	_sub1, @function
-_sub1:
+	.global	SYM(sub1)
+	.type	SYM(sub1), @function
+SYM(sub1):
 	mov.l	r14,@-r15
 	sts.l	pr,@-r15
 	add	#-128,r15
@@ -106,11 +111,11 @@
 	.align 1
 .L8:
 	.short	256
-	.size	_sub1, .-_sub1
+	.size	SYM(sub1), . - SYM(sub1)
 	.align 1
-	.global	_sub2
-	.type	_sub2, @function
-_sub2:
+	.global	SYM(sub2)
+	.type	SYM(sub2), @function
+SYM(sub2):
 	mov.l	r14,@-r15
 	sts.l	pr,@-r15
 	mov.w	.L11,r1
@@ -126,5 +131,5 @@
 	.align 1
 .L11:
 	.short	260
-	.size	_sub2, .-_sub2
+	.size	SYM(sub2), . - SYM(sub2)
 	.ident	"GCC: (GNU) 3.5.0 20040204 (experimental)"
Index: gdb.arch/gdb1291.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.arch/gdb1291.exp,v
retrieving revision 1.12
diff -u -p -r1.12 gdb1291.exp
--- gdb.arch/gdb1291.exp	16 Jan 2012 16:21:44 -0000	1.12
+++ gdb.arch/gdb1291.exp	23 Mar 2012 15:44:37 -0000
@@ -30,7 +30,7 @@ if ![istarget "sh-*-*"] then {
 }
 
 set testfile "gdb1291"
-set srcfile ${testfile}.s
+set srcfile ${testfile}.S
 set binfile ${objdir}/${subdir}/${testfile}
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable ""] != "" } {
     untested gdb1291.exp
--- gdb.arch/gdb1431.exp_	2012-03-23 16:31:15.551606197 +0100
+++ gdb.arch/gdb1431.exp	2012-03-23 16:15:04.511666412 +0100
@@ -32,8 +32,8 @@
 }
 
 set testfile "gdb1431"
-# Use gdb1291.s instead of duplicating it.
-set srcfile "gdb1291.s"
+# Use gdb1291.S instead of duplicating it.
+set srcfile "gdb1291.S"
 set binfile ${objdir}/${subdir}/${testfile}
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable ""] != "" } {
     untested gdb1431.exp


And then we additionally need the following bit for cases where gdbserver
is involved:

gdb/testsuite/

	* gdb.arch/gdb1431.exp <advance sub1>: Only expect to hear the
	debuggee's friendly greeting if we're not deaf.

Index: gdb.arch/gdb1431.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.arch/gdb1431.exp,v
retrieving revision 1.12
diff -u -p -r1.12 gdb1431.exp
--- gdb.arch/gdb1431.exp	16 Jan 2012 16:21:44 -0000	1.12
+++ gdb.arch/gdb1431.exp	23 Mar 2012 15:28:11 -0000
@@ -52,6 +53,11 @@ if ![runto_main] then {
     gdb_suppress_tests
 }
 
-gdb_test "advance sub1" "hello world\r\n$hex in sub1 \\(\\)" "get to sub1"
+set expect "$hex in sub1 \\(\\)"
+if { ![target_info exists gdb,noinferiorio] } then {
+    # Accept the debuggee's friendly greeting.
+    set expect "hello world\r\n$expect"
+}
+gdb_test "advance sub1" "$expect" "get to sub1"
 gdb_test "advance sub2" "$hex in main \\(\\)" "advance returns from sub1 frame"
 gdb_test "advance sub2" "$hex in sub2 \\(\\)" "get to sub2"


End result for both sh-linux-gnu and sh-elf:

    Running [...]/gdb.arch/gdb1291.exp ...
    PASS: gdb.arch/gdb1291.exp: set breakpoint
    PASS: gdb.arch/gdb1291.exp: get to sub1
    PASS: gdb.arch/gdb1291.exp: backtrace with local variable less than or equal to 256 bytes
    PASS: gdb.arch/gdb1291.exp: set breakpoint
    PASS: gdb.arch/gdb1291.exp: get to sub2
    PASS: gdb.arch/gdb1291.exp: backtrace with local variable larger than 256 bytes
    Running [...]/gdb.arch/gdb1431.exp ...
    PASS: gdb.arch/gdb1431.exp: get to sub1
    PASS: gdb.arch/gdb1431.exp: advance returns from sub1 frame
    PASS: gdb.arch/gdb1431.exp: get to sub2


OK to commit?


Grüße,
 Thomas

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

Re: [SH] gdb.arch/gdb1291.exp, gdb.arch/gdb1431.exp

[Kevin Buettner]

On Fri, 23 Mar 2012 16:46:32 +0100
Thomas Schwinge <thomas@codesourcery.com> wrote:

> Exactly the same for gdb.arch/gdb1431.exp: their source files
> (gdb.arch/gdb1291.s, gdb.arch/gdb1431.s) are identical, and I first
> propose to get rid of one of them:
> 
> gdb/testsuite/
> 
> 	* gdb.arch/gdb1431.s: Remove file.
> 	* gdb.arch/gdb1431.exp (srcfile): Refer to gdb1291.s.

I don't know enough about the tests in question to say whether
this is a good approach or not.  Is there a chance that someone
will want to tweak the code for one of the test cases without
also having to tweak the other?

Kevin

Re: [SH] gdb.arch/gdb1291.exp, gdb.arch/gdb1431.exp

[Thomas Schwinge]

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

Hi!

On Tue, 27 Mar 2012 21:52:13 -0700, Kevin Buettner <kevinb@redhat.com> wrote:
> On Fri, 23 Mar 2012 16:46:32 +0100
> Thomas Schwinge <thomas@codesourcery.com> wrote:
> 
> > Exactly the same for gdb.arch/gdb1431.exp: their source files
> > (gdb.arch/gdb1291.s, gdb.arch/gdb1431.s) are identical, and I first
> > propose to get rid of one of them:
> > 
> > gdb/testsuite/
> > 
> > 	* gdb.arch/gdb1431.s: Remove file.
> > 	* gdb.arch/gdb1431.exp (srcfile): Refer to gdb1291.s.
> 
> I don't know enough about the tests in question to say whether
> this is a good approach or not.

Apparently nobody else does, too.  ;-)

> Is there a chance that someone
> will want to tweak the code for one of the test cases without
> also having to tweak the other?

No, given what they're testing, I don't expect this would ever happen.


Grüße,
 Thomas

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

Re: [SH] gdb.arch/gdb1291.exp, gdb.arch/gdb1431.exp

[Thomas Schwinge]

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

Hi!

On Fri, 23 Mar 2012 16:46:32 +0100, I wrote:
> gdb/testsuite/
> 
> 	* gdb.arch/gdb1431.s: Remove file.
> 	* gdb.arch/gdb1431.exp (srcfile): Refer to gdb1291.s.

> gdb/testsuite/
> 
> 	* gdb.arch/gdb1291.s: Rename file to...
> 	* gdb.arch/gdb1291.S: ... this.
> 	(CONCAT1, CONCAT2, SYM): New macros.
> 	(_main, _printf, _sub1, _sub2): Replace with SYM(main), etc.
> 	* gdb.arch/gdb1291.exp (srcfile): Update for the changed filename.
> 	* gdb.arch/gdb1431.exp (srcfile): Likewise.

> gdb/testsuite/
> 
> 	* gdb.arch/gdb1431.exp <advance sub1>: Only expect to hear the
> 	debuggee's friendly greeting if we're not deaf.

> OK to commit?


Grüße,
 Thomas

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

Re: [SH] gdb.arch/gdb1291.exp, gdb.arch/gdb1431.exp

[Thomas Schwinge]

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

Hi!

On Mon, 16 Apr 2012 09:50:00 +0200, I wrote:
> On Fri, 23 Mar 2012 16:46:32 +0100, I wrote:
> > gdb/testsuite/
> > 
> > 	* gdb.arch/gdb1431.s: Remove file.
> > 	* gdb.arch/gdb1431.exp (srcfile): Refer to gdb1291.s.
> 
> > gdb/testsuite/
> > 
> > 	* gdb.arch/gdb1291.s: Rename file to...
> > 	* gdb.arch/gdb1291.S: ... this.
> > 	(CONCAT1, CONCAT2, SYM): New macros.
> > 	(_main, _printf, _sub1, _sub2): Replace with SYM(main), etc.
> > 	* gdb.arch/gdb1291.exp (srcfile): Update for the changed filename.
> > 	* gdb.arch/gdb1431.exp (srcfile): Likewise.
> 
> > gdb/testsuite/
> > 
> > 	* gdb.arch/gdb1431.exp <advance sub1>: Only expect to hear the
> > 	debuggee's friendly greeting if we're not deaf.
> 
> > OK to commit?


Grüße,
 Thomas

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

Re: [SH] gdb.arch/gdb1291.exp, gdb.arch/gdb1431.exp

[Thomas Schwinge]

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

Hi!

On Wed, 09 May 2012 10:26:49 +0800, I wrote:
> On Mon, 16 Apr 2012 09:50:00 +0200, I wrote:
> > On Fri, 23 Mar 2012 16:46:32 +0100, I wrote:
> > > gdb/testsuite/
> > > 
> > > 	* gdb.arch/gdb1431.s: Remove file.
> > > 	* gdb.arch/gdb1431.exp (srcfile): Refer to gdb1291.s.
> > 
> > > gdb/testsuite/
> > > 
> > > 	* gdb.arch/gdb1291.s: Rename file to...
> > > 	* gdb.arch/gdb1291.S: ... this.
> > > 	(CONCAT1, CONCAT2, SYM): New macros.
> > > 	(_main, _printf, _sub1, _sub2): Replace with SYM(main), etc.
> > > 	* gdb.arch/gdb1291.exp (srcfile): Update for the changed filename.
> > > 	* gdb.arch/gdb1431.exp (srcfile): Likewise.
> > 
> > > gdb/testsuite/
> > > 
> > > 	* gdb.arch/gdb1431.exp <advance sub1>: Only expect to hear the
> > > 	debuggee's friendly greeting if we're not deaf.
> > 
> > > OK to commit?

If nobody is interested in commenting, I might as well commit this?


Grüße,
 Thomas

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

Re: [SH] gdb.arch/gdb1291.exp, gdb.arch/gdb1431.exp

[Joel Brobecker]

Really sorry for the delay...

> Exactly the same for gdb.arch/gdb1431.exp: their source files
> (gdb.arch/gdb1291.s, gdb.arch/gdb1431.s) are identical, and I first
> propose to get rid of one of them:
> 
> gdb/testsuite/
> 
> 	* gdb.arch/gdb1431.s: Remove file.
> 	* gdb.arch/gdb1431.exp (srcfile): Refer to gdb1291.s.

No one objected to this, it seems. I verified that you still
generate one executable per testcase, which is what we want.
So no objection from me.

> Next, for the linking error, the problem is that global symbols (such as
> printf) are specified with a leading underscore, which is correct for
> sh-elf, but not for sh-linux-gnu.  Relying on GCC's
> __USER_LABEL_PREFIX__, I propose the following:

I would rather not rely on GCC-isms for this. You should take a look
at how we deal with this in other testcases.  See
gdb_target_symbol_prefix_flags in testsuite/lib/gdb.exp, and
how it is used in some of the Windows targets.


-- 
Joel