[RFA] Testsuite centralize -DSYMBOL_PREFIX uses

[RFA] Testsuite centralize -DSYMBOL_PREFIX uses

[Pierre Muller]

  Currently, several sources in gdb.arch and gdb.reverse use: 
set additional_flags "additional_flags=-DSYMBOL_PREFIX=\"_\""
depending on the target, but the list of targets included 
varies from one file to another, which is pointless.

  I thus thought that this should instead be centralized into gdb.exp file.

  This patch simply add a new expect procedure named
gdb_target_symbol_prefix_flags that returns
"additional_flags=-DSYMBOL_PREFIX=\"_\""
if the target is known to used an underscore as a prefix for symbols.

  The list is far from complete, but for now, it is only
used in gdb.arch and gdb.reverse directories and exclusively for i386
architecture.
  There was a TODO on several places about finding a way to 
figure out automatically if the target needs this, but I am not sure
there is an easy way to do this. Thus I simply left that TODO
in the comment of the new procedure.


This is patch OK?

Pierre Muller

2011-05-20  Pierre Muller  <muller@ics.u-strasbg.fr>

	Centralize -DSYMBOL_PREFIX=\"_\" additional flags in gdb.exp.
	* lib/gdb.exp (gdb_target_symbol_prefix_flags): New procedure.
	* gdb.arch/i386-bp_permanent.exp: Use
gdb_target_symbol_prefix_flags.
	* gdb.arch/i386-gnu-cfi.exp: Likewise.
	* gdb.arch/i386-permbkpt.exp: Likewise.
	* gdb.arch/i386-prologue.exp: Likewise.
	* gdb.arch/i386-size-overlap.exp: Likewise.
	* gdb.arch/i386-size.exp: Likewise.
	* gdb.arch/i386-unwind.exp: Likewise.
	* gdb.reverse/i386-precsave.exp: Likewise.
	* gdb.reverse/i386-reverse.exp: Likewise.
	* gdb.reverse/i386-sse-reverse.exp: Likewise.
	* gdb.reverse/i387-env-reverse.exp: Likewise.
	* gdb.reverse/i387-stack-reverse.exp: Likewise.

Index: src/gdb/testsuite/lib/gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.176
diff -u -p -r1.176 gdb.exp
--- src/gdb/testsuite/lib/gdb.exp	20 May 2011 14:37:20 -0000
1.176
+++ src/gdb/testsuite/lib/gdb.exp	20 May 2011 15:44:32 -0000
@@ -3658,3 +3658,22 @@ proc core_find {binfile {deletefiles {}}
     }
     return $destcore
 }
+
+# gdb_target_symbol_prefix_flags returns a string that can be added
+# to gdb_compile options to define SYMBOL_PREFIX macro value
+# symbol_prefix_flags returns a string that can be added
+# for targets that use underscore as symbol prefix.
+# The list of targets is incomplete and should be enhanced as
+# reports about missing targets come in.
+# TODO: find out automatically if the target needs this.
+
+proc gdb_target_symbol_prefix_flags {} {
+if { [istarget "*-*-cygwin*"] || [istarget "i?86-*-mingw*"]
+     || [istarget "*-*-msdosdjgpp*"] || [istarget "*-*-go32*"]
+     || [istarget "arm*-*-wince*"] || [istarget "arm*-*-mingwce*"]} {
+	return "additional_flags=-DSYMBOL_PREFIX=\"_\""
+    } else {
+	return ""
+    }
+}
+
Index: src/gdb/testsuite/gdb.arch/i386-bp_permanent.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.arch/i386-bp_permanent.exp,v
retrieving revision 1.7
diff -u -p -r1.7 i386-bp_permanent.exp
--- src/gdb/testsuite/gdb.arch/i386-bp_permanent.exp	16 May 2011 13:56:40
-0000	1.7
+++ src/gdb/testsuite/gdb.arch/i386-bp_permanent.exp	20 May 2011 15:44:31
-0000
@@ -32,11 +32,7 @@ set srcfile i386-prologue.c
 set binfile ${objdir}/${subdir}/${testfile}
 
 # some targets have leading underscores on assembly symbols.
-# TODO: detect this automatically
-set additional_flags ""
-if [istarget "i?86-*-cygwin*"] then {
-    set additional_flags "additional_flags=-DSYMBOL_PREFIX=\"_\""
-}
+set additional_flags [gdb_target_symbol_prefix_flags]
 
 # Don't use "debug", so that we don't have line information for the
assembly
 # fragments.
Index: src/gdb/testsuite/gdb.arch/i386-gnu-cfi.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.arch/i386-gnu-cfi.exp,v
retrieving revision 1.9
diff -u -p -r1.9 i386-gnu-cfi.exp
--- src/gdb/testsuite/gdb.arch/i386-gnu-cfi.exp	1 Jan 2011 15:33:40 -0000
1.9
+++ src/gdb/testsuite/gdb.arch/i386-gnu-cfi.exp	20 May 2011 15:44:31 -0000
@@ -38,11 +38,7 @@ set srcfileasm ${testfile}-asm.S
 set binfile ${objdir}/${subdir}/${testfile}
 
 # some targets have leading underscores on assembly symbols.
-# TODO: detect this automatically
-set additional_flags ""
-if { [istarget "*-*-cygwin*"] || [istarget "*-*-mingw*"] } then {
-  set additional_flags "additional_flags=-DSYMBOL_PREFIX=\"_\""
-}
+set additional_flags [gdb_target_symbol_prefix_flags]
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfilec}
${srcdir}/${subdir}/${srcfileasm}" "${binfile}" executable [list debug
$additional_flags]] != "" } {
     untested i386-gnu-cfi.exp
Index: src/gdb/testsuite/gdb.arch/i386-permbkpt.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.arch/i386-permbkpt.exp,v
retrieving revision 1.3
diff -u -p -r1.3 i386-permbkpt.exp
--- src/gdb/testsuite/gdb.arch/i386-permbkpt.exp	1 Jan 2011 15:33:40
-0000	1.3
+++ src/gdb/testsuite/gdb.arch/i386-permbkpt.exp	20 May 2011 15:44:31
-0000
@@ -31,12 +31,8 @@ set testfile "i386-permbkpt"
 set srcfile ${testfile}.S
 set binfile ${objdir}/${subdir}/${testfile}
 
-# Some targets have leading underscores on assembly symbols.
-# TODO: detect this automatically
-set additional_flags ""
-if { [istarget "*-*-cygwin*"] || [istarget "*-*-mingw*"] } then {
-    set additional_flags "additional_flags=-DSYMBOL_PREFIX=_"
-}
+# some targets have leading underscores on assembly symbols.
+set additional_flags [gdb_target_symbol_prefix_flags]
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
[list debug $additional_flags]] != "" } {
     untested i386-permbkpt.exp
Index: src/gdb/testsuite/gdb.arch/i386-prologue.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.arch/i386-prologue.exp,v
retrieving revision 1.24
diff -u -p -r1.24 i386-prologue.exp
--- src/gdb/testsuite/gdb.arch/i386-prologue.exp	1 Jan 2011 15:33:40
-0000	1.24
+++ src/gdb/testsuite/gdb.arch/i386-prologue.exp	20 May 2011 15:44:31
-0000
@@ -36,11 +36,7 @@ set srcfile ${testfile}.c
 set binfile ${objdir}/${subdir}/${testfile}
 
 # some targets have leading underscores on assembly symbols.
-# TODO: detect this automatically
-set additional_flags ""
-if { [istarget "*-*-cygwin*"] || [istarget "*-*-mingw*"] } then {
-  set additional_flags "additional_flags=-DSYMBOL_PREFIX=\"_\""
-}   
+set additional_flags [gdb_target_symbol_prefix_flags]
 
 # Don't use "debug", so that we don't have line information for the
assembly
 # fragments.
Index: src/gdb/testsuite/gdb.arch/i386-size-overlap.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.arch/i386-size-overlap.exp,v
retrieving revision 1.8
diff -u -p -r1.8 i386-size-overlap.exp
--- src/gdb/testsuite/gdb.arch/i386-size-overlap.exp	16 May 2011 13:56:40
-0000	1.8
+++ src/gdb/testsuite/gdb.arch/i386-size-overlap.exp	20 May 2011 15:44:31
-0000
@@ -31,11 +31,7 @@ set srcfile ${testfile}.c
 set binfile ${objdir}/${subdir}/${testfile}
 
 # some targets have leading underscores on assembly symbols.
-# TODO: detect this automatically
-set additional_flags ""
-if [istarget "i?86-*-cygwin*"] then {
-  set additional_flags "additional_flags=-DSYMBOL_PREFIX=\"_\""
-}
+set additional_flags [gdb_target_symbol_prefix_flags]
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
 	  executable [list debug $additional_flags]] != "" } {
Index: src/gdb/testsuite/gdb.arch/i386-size.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.arch/i386-size.exp,v
retrieving revision 1.9
diff -u -p -r1.9 i386-size.exp
--- src/gdb/testsuite/gdb.arch/i386-size.exp	1 Jan 2011 15:33:40 -0000
1.9
+++ src/gdb/testsuite/gdb.arch/i386-size.exp	20 May 2011 15:44:31 -0000
@@ -34,11 +34,7 @@ set srcfile ${testfile}.c
 set binfile ${objdir}/${subdir}/${testfile}
 
 # some targets have leading underscores on assembly symbols.
-# TODO: detect this automatically
-set additional_flags ""
-if [istarget "i?86-*-cygwin*"] then {
-  set additional_flags "additional_flags=-DSYMBOL_PREFIX=\"_\""
-}
+set additional_flags [gdb_target_symbol_prefix_flags]
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" \
 	  executable [list debug $additional_flags]] != "" } {
Index: src/gdb/testsuite/gdb.arch/i386-unwind.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.arch/i386-unwind.exp,v
retrieving revision 1.14
diff -u -p -r1.14 i386-unwind.exp
--- src/gdb/testsuite/gdb.arch/i386-unwind.exp	1 Jan 2011 15:33:40 -0000
1.14
+++ src/gdb/testsuite/gdb.arch/i386-unwind.exp	20 May 2011 15:44:31 -0000
@@ -36,11 +36,7 @@ set srcfile ${testfile}.c
 set binfile ${objdir}/${subdir}/${testfile}
 
 # some targets have leading underscores on assembly symbols.
-# TODO: detect this automatically
-set additional_flags ""
-if { [istarget "*-*-cygwin*"] || [istarget "*-*-mingw*"] } then {
-  set additional_flags "additional_flags=-DSYMBOL_PREFIX=\"_\""
-}
+set additional_flags [gdb_target_symbol_prefix_flags]
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
[list debug $additional_flags]] != "" } {
     untested i386-unwind.exp
Index: src/gdb/testsuite/gdb.reverse/i386-precsave.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.reverse/i386-precsave.exp,v
retrieving revision 1.6
diff -u -p -r1.6 i386-precsave.exp
--- src/gdb/testsuite/gdb.reverse/i386-precsave.exp	16 May 2011 13:56:41
-0000	1.6
+++ src/gdb/testsuite/gdb.reverse/i386-precsave.exp	20 May 2011 15:44:31
-0000
@@ -39,11 +39,7 @@ set srcfile i386-reverse.c
 set binfile ${objdir}/${subdir}/${testfile}
 
 # some targets have leading underscores on assembly symbols.
-# TODO: detect this automatically
-set additional_flags ""
-if [istarget "i?86-*-cygwin*"] then {
-  set additional_flags "additional_flags=-DSYMBOL_PREFIX=\"_\""
-}
+set additional_flags [gdb_target_symbol_prefix_flags]
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
[list debug $additional_flags]] != "" } {
     untested ${testfile}.exp
Index: src/gdb/testsuite/gdb.reverse/i386-reverse.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.reverse/i386-reverse.exp,v
retrieving revision 1.5
diff -u -p -r1.5 i386-reverse.exp
--- src/gdb/testsuite/gdb.reverse/i386-reverse.exp	1 Jan 2011 15:33:49
-0000	1.5
+++ src/gdb/testsuite/gdb.reverse/i386-reverse.exp	20 May 2011 15:44:31
-0000
@@ -38,11 +38,7 @@ set srcfile ${testfile}.c
 set binfile ${objdir}/${subdir}/${testfile}
 
 # some targets have leading underscores on assembly symbols.
-# TODO: detect this automatically
-set additional_flags ""
-if [istarget "i?86-*-cygwin*"] then {
-  set additional_flags "additional_flags=-DSYMBOL_PREFIX=\"_\""
-}
+set additional_flags [gdb_target_symbol_prefix_flags]
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
[list debug $additional_flags]] != "" } {
     untested i386-reverse
Index: src/gdb/testsuite/gdb.reverse/i386-sse-reverse.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.reverse/i386-sse-reverse.exp,v
retrieving revision 1.4
diff -u -p -r1.4 i386-sse-reverse.exp
--- src/gdb/testsuite/gdb.reverse/i386-sse-reverse.exp	1 Jan 2011 15:33:49
-0000	1.4
+++ src/gdb/testsuite/gdb.reverse/i386-sse-reverse.exp	20 May 2011 15:44:31
-0000
@@ -38,11 +38,7 @@ set srcfile ${testfile}.c
 set binfile ${objdir}/${subdir}/${testfile}
 
 # some targets have leading underscores on assembly symbols.
-# TODO: detect this automatically
-set additional_flags ""
-if [istarget "i?86-*-cygwin*"] then {
-  set additional_flags "additional_flags=-DSYMBOL_PREFIX=\"_\""
-}
+set additional_flags [gdb_target_symbol_prefix_flags]
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
[list debug $additional_flags]] != "" } {
     untested i386-sse-reverse
Index: src/gdb/testsuite/gdb.reverse/i387-env-reverse.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.reverse/i387-env-reverse.exp,v
retrieving revision 1.6
diff -u -p -r1.6 i387-env-reverse.exp
--- src/gdb/testsuite/gdb.reverse/i387-env-reverse.exp	16 May 2011 13:56:41
-0000	1.6
+++ src/gdb/testsuite/gdb.reverse/i387-env-reverse.exp	20 May 2011 15:44:31
-0000
@@ -30,11 +30,7 @@ set srcfile ${testfile}.c
 set binfile ${objdir}/${subdir}/${testfile}
 
 # some targets have leading underscores on assembly symbols.
-# TODO: detect this automatically
-set additional_flags ""
-if [istarget "i?86-*-cygwin*"] then {
-  set additional_flags "additional_flags=-DSYMBOL_PREFIX=\"_\""
-}
+set additional_flags [gdb_target_symbol_prefix_flags]
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
[list debug $additional_flags]] != "" } {
     untested ${testfile}.exp
Index: src/gdb/testsuite/gdb.reverse/i387-stack-reverse.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.reverse/i387-stack-reverse.exp,v
retrieving revision 1.6
diff -u -p -r1.6 i387-stack-reverse.exp
--- src/gdb/testsuite/gdb.reverse/i387-stack-reverse.exp	16 May 2011
13:56:41 -0000	1.6
+++ src/gdb/testsuite/gdb.reverse/i387-stack-reverse.exp	20 May 2011
15:44:31 -0000
@@ -30,11 +30,7 @@ set srcfile ${testfile}.c
 set binfile ${objdir}/${subdir}/${testfile}
 
 # some targets have leading underscores on assembly symbols.
-# TODO: detect this automatically
-set additional_flags ""
-if [istarget "i?86-*-cygwin*"] then {
-  set additional_flags "additional_flags=-DSYMBOL_PREFIX=\"_\""
-}
+set additional_flags [gdb_target_symbol_prefix_flags]
 
 if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable
[list debug $additional_flags]] != "" } {
     untested ${testfile}.exp

Re: [RFA] Testsuite centralize -DSYMBOL_PREFIX uses

[Pedro Alves]

On Tuesday 24 May 2011 08:48:38, Pierre Muller wrote:

> +# gdb_target_symbol_prefix_flags returns a string that can be added
> +# to gdb_compile options to define SYMBOL_PREFIX macro value
> +# symbol_prefix_flags returns a string that can be added
> +# for targets that use underscore as symbol prefix.

> +# The list of targets is incomplete and should be enhanced as
> +# reports about missing targets come in.

This sentence is useless, IMO.

> +# TODO: find out automatically if the target needs this.
> +
> +proc gdb_target_symbol_prefix_flags {} {
> +if { [istarget "*-*-cygwin*"] || [istarget "i?86-*-mingw*"]

Missing indentation.

> +     || [istarget "*-*-msdosdjgpp*"] || [istarget "*-*-go32*"]
> +     || [istarget "arm*-*-wince*"] || [istarget "arm*-*-mingwce*"]} {
> +	return "additional_flags=-DSYMBOL_PREFIX=\"_\""

ARM Windows CE is not underscored.  Is DJGPP/GO32?  bfd/config.bfd
appears to indicate it isn't.  None of the places you touched listed
wince or djgpp, afaics.

> +    } else {
> +	return ""
> +    }
> +}
> +

-- 
Pedro Alves

RE: [RFA] Testsuite centralize -DSYMBOL_PREFIX uses

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : mardi 24 mai 2011 10:51
> À : gdb-patches@sourceware.org
> Cc : Pierre Muller
> Objet : Re: [RFA] Testsuite centralize -DSYMBOL_PREFIX uses
> 
> On Tuesday 24 May 2011 08:48:38, Pierre Muller wrote:
> 
> > +# gdb_target_symbol_prefix_flags returns a string that can be added
> > +# to gdb_compile options to define SYMBOL_PREFIX macro value
> > +# symbol_prefix_flags returns a string that can be added
> > +# for targets that use underscore as symbol prefix.
> 
> > +# The list of targets is incomplete and should be enhanced as
> > +# reports about missing targets come in.
> 
> This sentence is useless, IMO.
Removed, 
> > +# TODO: find out automatically if the target needs this.
> > +
> > +proc gdb_target_symbol_prefix_flags {} {
> > +if { [istarget "*-*-cygwin*"] || [istarget "i?86-*-mingw*"]
> 
> Missing indentation.
 This should be 4 spaces and 1 tab for double indent, is that right?
 
> > +     || [istarget "*-*-msdosdjgpp*"] || [istarget "*-*-go32*"]
> > +     || [istarget "arm*-*-wince*"] || [istarget "arm*-*-mingwce*"]} {
> > +	return "additional_flags=-DSYMBOL_PREFIX=\"_\""
> 
> ARM Windows CE is not underscored.  Is DJGPP/GO32?  bfd/config.bfd
> appears to indicate it isn't.  None of the places you touched listed
> wince or djgpp, afaics.

  I did a search:
grep TARGET_UNDERSCORE in bfd directory,
$ grep  "TARGET_UNDERSCORE[[:space:]]*'_'" *.[hc]
coff-arm.c:#define TARGET_UNDERSCORE '_'
coff-go32.c:#define TARGET_UNDERSCORE   '_'
coff-stgo32.c:#define TARGET_UNDERSCORE '_'
pe-i386.c:#define TARGET_UNDERSCORE     '_'
pe-sh.c:#define TARGET_UNDERSCORE '_'
pe-x86_64.c:#define TARGET_UNDERSCORE   '_'
pei-i386.c:#define TARGET_UNDERSCORE    '_'
pei-ia64.c:#define TARGET_UNDERSCORE '_'
pei-sh.c:#define TARGET_UNDERSCORE '_'
pei-x86_64.c:#define TARGET_UNDERSCORE  '_'

I knew that there was a change in win64 x86_64 target
leading underscore was default for a moment, but now is
only used if USE_MINGW64_LEADING_UNDERSCORES

 The coff-arm.c lead to my confusion about arm windows CE
pe-arm-wince.c and pei-arm-wince.c
both use
#define TARGET_UNDERSCORE 0
so that coff-arm.c doesn't change its value 
(the :#define TARGET_UNDERSCORE '_'
in guarded inside a #ifndef TARGET_UNDERSCORE)

  I remember that djgpp/go32 targets are using the prefix
as I used DJGPP/go32 quite a lot ...
(dgjpp/go32 was not listed in any test because
using expect for djgpp is quite tricky and only works partially
in windows environment with a special target board that I wrote a while
ago...)

Below is an update lib/gdb.exp change,
does this look better?

Pierre



$ cvs diff -u -p gdb.exp
Index: gdb.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/lib/gdb.exp,v
retrieving revision 1.176
diff -u -p -r1.176 gdb.exp
--- gdb.exp     20 May 2011 14:37:20 -0000      1.176
+++ gdb.exp     24 May 2011 11:39:40 -0000
@@ -3658,3 +3658,19 @@ proc core_find {binfile {deletefiles {}}
     }
     return $destcore
 }
+
+# gdb_target_symbol_prefix_flags returns a string that can be added
+# to gdb_compile options to define SYMBOL_PREFIX macro value
+# symbol_prefix_flags returns a string that can be added
+# for targets that use underscore as symbol prefix.
+# TODO: find out automatically if the target needs this.
+
+proc gdb_target_symbol_prefix_flags {} {
+    if { [istarget "*-*-cygwin*"] || [istarget "i?86-*-mingw*"]
+        || [istarget "*-*-msdosdjgpp*"] || [istarget "*-*-go32*"] } {
+       return "additional_flags=-DSYMBOL_PREFIX=\"_\""
+    } else {
+       return ""
+    }
+}
+

Re: [RFA] Testsuite centralize -DSYMBOL_PREFIX uses

[Pedro Alves]

On Tuesday 24 May 2011 12:42:53, Pierre Muller wrote:
> Below is an update lib/gdb.exp change,
> does this look better?

Yes, thanks.  This version is okay.

-- 
Pedro Alves

RE: [RFA] Testsuite centralize -DSYMBOL_PREFIX uses

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : mardi 24 mai 2011 13:55
> À : gdb-patches@sourceware.org
> Cc : Pierre Muller
> Objet : Re: [RFA] Testsuite centralize -DSYMBOL_PREFIX uses
> 
> On Tuesday 24 May 2011 12:42:53, Pierre Muller wrote:
> > Below is an update lib/gdb.exp change,
> > does this look better?
> 
> Yes, thanks.  This version is okay.

 Thanks for the fast comments and approval,

patch committed,

Pierre