[RFC-v2] Support for const char and strings in stabs reader

["Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : Tuesday, March 23, 2010 7:53 PM
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFC] Support for const char and strings in stabs reader
> 
> > 2010-03-16  Pierre Muller  <muller@ics.u-strasbg.fr>
> >
> > 	* stabsread.c (define_symbol): Add support for char
> > 	and string constants.
> 
> As a general comment, the formatting of the section supporting strings
> is not consistent. It looks like it's because you have a mixture of
> spaces and tabs...
 Strange because I now use the script recently given on gdb mailing list
to avoid such problems...
 
> > +	case 's':
> > +	  {
> > +            struct type *range_type;
> > +	    char quote = *p++;
> > +            char *startp = p;
> > +            gdb_byte *string_value;
> 
> Can you add an empty line after your local variable declarations?
 Should be corrected below.
 
> > +            if (quote != '\'' && quote != '"')
> > +              {
> > +                SYMBOL_CLASS (sym) = LOC_CONST;
> > +                SYMBOL_TYPE (sym) = error_type (&p, objfile);
> > +                SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
> > +                add_symbol_to_list (sym, &file_symbols);
> > +                return sym;
> > +              }
> > +            /* Find matching quote, rejecting escaped quotes.  */
> > +            while (*p && *p != quote)
> > +              {
> > +                if (*p == '\\')
> > +                  p++;
> > +                if (*p)
> > +                  p++;
> > +              }
> 
> What if you don't find the matching quote?  I think we should reject
> the stabs the same way you do when you don't find an opening quote.

This now also creates an error type symbol.
 
> > +            *p = '\0';
> 
> Outch. I see that you restore the stabs later on, but I am still
> concerned by this, particularly if any of the functions you use end up
> raising an exception...

  OK, to avoid your concern, I used alloca and copied
byte per byte the different chars, which also allowed me to
correctly discard escaped quotes.
 
 
> One question I asked myself, and got confused over, is: What happens
> with escaped quotes. For instance, let's imagine that we have the
> following string: 'hello \' there'.  I think that the string value
> that you want to record is <<hello ' there>>, not <<hello \' there>>.
> In other word, the symbol value should not contain the backslash,
> right?
  You are right that my first patch did not handle this,
the new one below should handle it correctly.


Thanks for all those comments,


  Here is a new version of this patch.

  I had troubles with the testsuite,
and finally had to resort to use of extended
regular expression to be able to parse
the stabs string correctly for the stabs to stabx
conversion in xcoff.sed.
  This version works correctly (without failure)
for aout.sed script that is used by cygwin environment,
but I don't know if it is correct for the other modifications that 
I made.

Pierre



projecttype:gdb
revision:HEAD
email:muller@ics.u-strasbg.fr

2010-03-24  Pierre Muller  <muller@ics.u-strasbg.fr>

	* stabsread.c (define_symbol): Add support for char
	and string constants.

testsuite ChangeLog entry:

	* gdb.stabs/aout.sed: Convert all backslash to double backslash
	within one line, unless it is followed by a double quote.
	* gdb.stabs/hppa.sed: Idem.
	* gdb.stabs/weird.def: Add char and String constants
	* gdb.stabs/weird.exp: Check for correct parsing of 
	chhar and string constants.
	* gdb.stabs/xcoff.sed: Ignore escaped quote quotes
	in .stabs to .stabx substitution.
	

Index: src/gdb/stabsread.c
===================================================================
RCS file: /cvs/src/src/gdb/stabsread.c,v
retrieving revision 1.123
diff -u -p -r1.123 stabsread.c
--- src/gdb/stabsread.c	8 Jan 2010 08:55:16 -0000	1.123
+++ src/gdb/stabsread.c	24 Mar 2010 22:38:04 -0000
@@ -793,6 +793,74 @@ define_symbol (CORE_ADDR valu, char *str
 	    SYMBOL_CLASS (sym) = LOC_CONST;
 	  }
 	  break;
+
+	case 'c':
+	  {
+	    SYMBOL_TYPE (sym) = objfile_type (objfile)->builtin_char;
+	    SYMBOL_VALUE (sym) = atoi (p);
+	    SYMBOL_CLASS (sym) = LOC_CONST;
+	  }
+	  break;
+
+	case 's':
+	  {
+            struct type *range_type;
+	    int ind = 0;
+	    char quote = *p++;
+            char *startp = p;
+	    gdb_byte *string_local = (gdb_byte *) alloca (strlen (p));
+	    gdb_byte *string_value;
+
+            if (quote != '\'' && quote != '"')
+              {
+                SYMBOL_CLASS (sym) = LOC_CONST;
+                SYMBOL_TYPE (sym) = error_type (&p, objfile);
+                SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
+                add_symbol_to_list (sym, &file_symbols);
+                return sym;
+              }
+            /* Find matching quote, rejecting escaped quotes.  */
+            while (*p && *p != quote)
+              {
+                if (*p == '\\' && p[1] == quote)
+		  {
+		    string_local[ind] = (gdb_byte) quote;
+		    ind++;
+                    p += 2;
+		  }
+		else if (*p) 
+                  {
+		    string_local[ind] = (gdb_byte) (*p);
+		    ind++;
+		    p++;
+		  }
+              }
+	    if (*p != quote)
+	       {
+                SYMBOL_CLASS (sym) = LOC_CONST;
+                SYMBOL_TYPE (sym) = error_type (&p, objfile);
+                SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
+                add_symbol_to_list (sym, &file_symbols);
+                return sym;
+	      }
+
+	    /* NULL terminate the string.  */
+	    string_local[ind] = 0;
+            range_type = create_range_type (NULL,
+					    objfile_type
(objfile)->builtin_int,
+					    0, ind);
+ 	    SYMBOL_TYPE (sym) = create_array_type (NULL,
+				  objfile_type (objfile)->builtin_char,
+				  range_type);
+	    string_value = obstack_alloc (&objfile->objfile_obstack, ind +
1);
+	    memcpy (string_value, string_local, ind + 1);
+            p++;
+
+	    SYMBOL_VALUE_BYTES (sym) = string_value;
+	    SYMBOL_CLASS (sym) = LOC_CONST_BYTES;
+	  }
+	  break;
+
 	case 'e':
 	  /* SYMBOL:c=eTYPE,INTVALUE for a constant symbol whose value
 	     can be represented as integral.
Index: src/gdb/testsuite/gdb.stabs/aout.sed
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.stabs/aout.sed,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 aout.sed
--- src/gdb/testsuite/gdb.stabs/aout.sed	16 Apr 1999 01:34:36 -0000
1.1.1.1
+++ src/gdb/testsuite/gdb.stabs/aout.sed	24 Mar 2010 22:38:04 -0000
@@ -9,7 +9,7 @@ Label0:
 s/N_LSYM/0x80/
 s/N_GSYM/0x20/
 s/# Replace a single backslash with a doubled backslash//
-/\.stabs/s/\\/\\\\/
+/\.stabs/s/\\\([^"]\)/\\\\\1/g
 s/\.begin_common\(.*\)/.stabs \1,0xe2,0,0,0/
 s/\.end_common\(.*\)/.stabs \1,0xe4,0,0,0/
 s/\.align_it/.align 2/
Index: src/gdb/testsuite/gdb.stabs/hppa.sed
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.stabs/hppa.sed,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 hppa.sed
--- src/gdb/testsuite/gdb.stabs/hppa.sed	16 Apr 1999 01:34:36 -0000
1.1.1.1
+++ src/gdb/testsuite/gdb.stabs/hppa.sed	24 Mar 2010 22:38:04 -0000
@@ -9,7 +9,7 @@ Label0:
 s/N_LSYM/0x80/
 s/N_GSYM/0x20/
 s/# Replace a single backslash with a doubled backslash//
-/\.stabs/s/\\/\\\\/
+/\.stabs/s/\\/\\\\/g
 s/# Only labels should be at the beginning of a line, assembler
directives//
 s/# and instructions should start somewhere after column zero.//
 /^\./s/^\./	./
Index: src/gdb/testsuite/gdb.stabs/weird.def
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.stabs/weird.def,v
retrieving revision 1.3
diff -u -p -r1.3 weird.def
--- src/gdb/testsuite/gdb.stabs/weird.def	8 Jan 2010 08:55:16 -0000
1.3
+++ src/gdb/testsuite/gdb.stabs/weird.def	24 Mar 2010 22:38:04 -0000
@@ -286,6 +286,20 @@ attr69:
 # Test constant with the type embedded.  
 .stabs "const70:c=e190=bs2;0;16;,70", N_LSYM,0,0, 0
 
+# Test char constant
+.stabs "constchar:c=c97", N_LSYM,0,0, 0
+
+# Test string constant
+.stabs "constString1:c=s'Single quote String1'", N_LSYM,0,0, 0
+# Using double quotes requires an escaping, as the stabs string
+# is a double quote delimited string.
+.stabs "constString2:c=s\"Double quote String2\"", N_LSYM,0,0, 0
+# Escaping single quote with is easy
+.stabs "constString3:c=s'String3 with embedded quote \' in the middle'",
N_LSYM,0,0, 0
+# Escaping double quotes is less clear...
+.stabs "constString4:c=s\"String4 with embedded quote \\" in the middle\"",
N_LSYM,0,0, 0
+
+
 .stabs "attr38:G338=@&
!#$%&'()*+,-./0123456789:<=>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]^_`abcdefghijklmno
pqrstuvwxyz{|}~;1",N_GSYM,0,0, 0
 
 # Unrecognized negative type number.  
Index: src/gdb/testsuite/gdb.stabs/weird.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.stabs/weird.exp,v
retrieving revision 1.18
diff -u -p -r1.18 weird.exp
--- src/gdb/testsuite/gdb.stabs/weird.exp	1 Jan 2010 07:32:06 -0000
1.18
+++ src/gdb/testsuite/gdb.stabs/weird.exp	24 Mar 2010 22:38:04 -0000
@@ -164,6 +164,12 @@ proc do_tests {} {
 
 	gdb_test "p sizeof (const70)" " = 2" "'e' constant with embedded
type"
 
+	gdb_test "p constchar" " = 97 'a'" "char constant"
+        gdb_test "p constString1" " = \"Single quote String1\"" "String
constant 1"
+        gdb_test "p constString2" " = \"Double quote String2\"" "String
constant 2"
+
+        gdb_test "p constString3" " = \"String3 with embedded quote ' in
the middle\"" "String constant 3"
+        gdb_test "p constString4" { = "String4 with embedded quote \\" in
the middle"} "String constant 4"
 	gdb_test "p bad_neg0" " = \{field0 = 42, field2 =.*field3 = 45\}" "p
bad_neg0"
 
 	gdb_test "ptype inttype" "type = (unsigned int|inttype)" "ptype on
inttype"
@@ -251,6 +257,8 @@ proc print_weird_var { var } {
 
 global target_os
 set sedscript ${srcdir}/${subdir}/aout.sed
+set sedoptions ""
+
 switch -glob ${target_triplet} {
     "hppa*-*-*" {
 	set sedscript ${srcdir}/${subdir}/hppa.sed
@@ -260,15 +268,18 @@ switch -glob ${target_triplet} {
     }
     "powerpc-*-aix*" {
 	set sedscript ${srcdir}/${subdir}/xcoff.sed
+	set sedoptions "-r"
     }
     "rs6000-*-aix*" {
 	set sedscript ${srcdir}/${subdir}/xcoff.sed
+	set sedoptions "-r"
     }
     "*-*-aout" {
 	set sedscript ${srcdir}/${subdir}/aout.sed
     }
     "*-*-xcoff" {
 	set sedscript ${srcdir}/${subdir}/xcoff.sed
+	set sedoptions "-r"
     }
     "alpha-*-*" {
 	set sedscript ${srcdir}/${subdir}/ecoff.sed
@@ -276,7 +287,7 @@ switch -glob ${target_triplet} {
 }
 
 # Hope this is a Unix box.
-set exec_output [remote_exec build "sed" "-f ${sedscript}"
"${srcdir}/${subdir}/weird.def" "${srcfile}"]
+set exec_output [remote_exec build "sed" "${sedoptions} -f ${sedscript}"
"${srcdir}/${subdir}/weird.def" "${srcfile}"]
 if { [lindex $exec_output 0] != 0 } {
     perror "Couldn't make test case. $exec_output"
     return -1
@@ -287,7 +298,7 @@ if  { [gdb_compile "${srcfile}" "${binfi
      return -1
 }
 
-remote_file build delete ${srcfile}
+# remote_file build delete ${srcfile}
 
 # Start with a fresh gdb
 gdb_exit
Index: src/gdb/testsuite/gdb.stabs/xcoff.sed
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.stabs/xcoff.sed,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 xcoff.sed
--- src/gdb/testsuite/gdb.stabs/xcoff.sed	16 Apr 1999 01:34:36 -0000
1.1.1.1
+++ src/gdb/testsuite/gdb.stabs/xcoff.sed	24 Mar 2010 22:38:04 -0000
@@ -4,7 +4,7 @@
 1i\
 	.csect .data[RW]
 # .stabs string,type,0,0,value  ->  .stabx string,value,type,0
-s/^[ 	]*\.stabs[ 	]*\("[^"]*"\),[ 	]*\([^,]*\),[ 	]*0,0,[
]*\(.*\)$/.stabx \1,\3,\2,0/
+s/^[ 	]*\.stabs[ 	]*("(\"|[^"])*"),[ 	]*([^,]*),[ 	]*0,0,[
]*(.*)$/.stabx \1,\4,\3,0/
 s/N_GSYM/128/
 # This needs to be C_DECL, which is used for types, not C_LSYM, which is
 # ignored on the initial scan.