public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [RFC] Support for const char and strings in stabs reader
@ 2010-03-16 21:10 Pierre Muller
  2010-03-22 21:56 ` PING " Pierre Muller
  2010-03-23 18:53 ` Joel Brobecker
  0 siblings, 2 replies; 7+ messages in thread
From: Pierre Muller @ 2010-03-16 21:10 UTC (permalink / raw)
  To: gdb-patches

  Stabs reader currently only supports constants
of integer, real or enum types.

  This patch adds support for char and string types, as described in 
http://sourceware.org/gdb/current/onlinedocs/stabs/Constants.html#Constants

  I tried to implement support for both single and double
quote in the code.
 The string type is currently used for
Free Pascal compiler, so I could directly test it.

  I also tried to add some check to gdb.stabs testsuite,
but I got caught by some typical tcl expansion
problems while trying to test the second form
using double quotes...


The original lines in gdb.stabs/weird.def
# Test string constant
.stabs "constString1:c=s'String1'", N_LSYM,0,0, 0
.stabs "constString2:c=s\"String2\"", N_LSYM,0,0, 0
.stabs "constString3:c=s'String3 with embedded quote \' in the middle'",
N_LSYM,0,0, 0

become

.stabs "constString1:c=s'String1'", 0x80,0,0, 0
.stabs "constString2:c=s\\"String2\"", 0x80,0,0, 0
.stabs "constString3:c=s'String3 with embedded quote \\' in the middle'",
0x80,0,0, 0

Note the fact that only one of the two escaped double quote
is transformed into \\"..
I tried all combination, and nothing seemed to work...


Comments welcome,


Pierre Muller
Pascal language support maintainer for GDB

PS: I left the boolean support out
because currently objfile builtin_types
do not have a builtin_bool, which would be required
here.


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

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

Index: stabsread.c
===================================================================
RCS file: /cvs/src/src/gdb/stabsread.c,v
retrieving revision 1.123
diff -u -p -r1.123 stabsread.c
--- stabsread.c	8 Jan 2010 08:55:16 -0000	1.123
+++ stabsread.c	16 Mar 2010 21:07:48 -0000
@@ -793,6 +793,56 @@ 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;
+	    char quote = *p++;
+            char *startp = 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++;
+                if (*p) 
+                  p++;
+              }
+            *p = '\0';
+            range_type = create_range_type (NULL,
+                           objfile_type (objfile)->builtin_int,
+                           0, strlen(startp));
+ 	    SYMBOL_TYPE (sym) = create_array_type (NULL,
+              objfile_type (objfile)->builtin_char,
+              range_type);
+	    string_value =
+	      obstack_alloc (&objfile->objfile_obstack,
+			     strlen (startp) + 1);
+	    strcpy ((char *)string_value, startp);
+            *p = quote;
+            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: 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
--- testsuite/gdb.stabs/weird.def	8 Jan 2010 08:55:16 -0000	1.3
+++ testsuite/gdb.stabs/weird.def	16 Mar 2010 21:07:49 -0000
@@ -286,6 +286,15 @@ 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=70", N_LSYM,0,0, 0
+
+# Test string constant
+.stabs "constString1:c=s'String1'", N_LSYM,0,0, 0
+.stabs "constString2:c=s\"String2\"", N_LSYM,0,0, 0
+.stabs "constString3:c=s'String3 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: 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
--- testsuite/gdb.stabs/weird.exp	1 Jan 2010 07:32:06 -0000	1.18
+++ testsuite/gdb.stabs/weird.exp	16 Mar 2010 21:07:49 -0000
@@ -164,6 +164,11 @@ proc do_tests {} {
 
 	gdb_test "p sizeof (const70)" " = 2" "'e' constant with embedded
type"
 
+	   gdb_test "p /x constchar" " = 0x46" "char constant"
+        gdb_test "p constString1" " = \"String1\"" "String constant 1"
+        gdb_test "p constString2" " = \"String2\"" "String constant 2"
+        gdb_test "p constString3" " = \"String3 with embedded quote ' in
the middle\"" "String constant 3"
+
 	gdb_test "p bad_neg0" " = \{field0 = 42, field2 =.*field3 = 45\}" "p
bad_neg0"
 
 	gdb_test "ptype inttype" "type = (unsigned int|inttype)" "ptype on
inttype"

^ permalink raw reply	[flat|nested] 7+ messages in thread

* PING [RFC] Support for const char and strings in stabs reader
  2010-03-16 21:10 [RFC] Support for const char and strings in stabs reader Pierre Muller
@ 2010-03-22 21:56 ` Pierre Muller
  2010-03-22 22:19   ` Joel Brobecker
  2010-03-23 18:53 ` Joel Brobecker
  1 sibling, 1 reply; 7+ messages in thread
From: Pierre Muller @ 2010-03-22 21:56 UTC (permalink / raw)
  To: gdb-patches

  The official stabs reader maintainer
is Elena Zannoni, but I could find any recent 
email from her on this or on gdb list.
I wonder if se is still reading this list.

  Is there someone else able to review this patch
or should I try to contact Elena directly?

Pierre Muller

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pierre Muller
> Envoyé : Tuesday, March 16, 2010 10:11 PM
> À : gdb-patches@sourceware.org
> Objet : [RFC] Support for const char and strings in stabs reader
> 
>   Stabs reader currently only supports constants
> of integer, real or enum types.
> 
>   This patch adds support for char and string types, as described in
> http://sourceware.org/gdb/current/onlinedocs/stabs/Constants.html#Const
> ants
> 
>   I tried to implement support for both single and double
> quote in the code.
>  The string type is currently used for
> Free Pascal compiler, so I could directly test it.
> 
>   I also tried to add some check to gdb.stabs testsuite,
> but I got caught by some typical tcl expansion
> problems while trying to test the second form
> using double quotes...
> 
> 
> The original lines in gdb.stabs/weird.def
> # Test string constant
> .stabs "constString1:c=s'String1'", N_LSYM,0,0, 0
> .stabs "constString2:c=s\"String2\"", N_LSYM,0,0, 0
> .stabs "constString3:c=s'String3 with embedded quote \' in the
> middle'",
> N_LSYM,0,0, 0
> 
> become
> 
> .stabs "constString1:c=s'String1'", 0x80,0,0, 0
> .stabs "constString2:c=s\\"String2\"", 0x80,0,0, 0
> .stabs "constString3:c=s'String3 with embedded quote \\' in the
> middle'",
> 0x80,0,0, 0
> 
> Note the fact that only one of the two escaped double quote
> is transformed into \\"..
> I tried all combination, and nothing seemed to work...
> 
> 
> Comments welcome,
> 
> 
> Pierre Muller
> Pascal language support maintainer for GDB
> 
> PS: I left the boolean support out
> because currently objfile builtin_types
> do not have a builtin_bool, which would be required
> here.
> 
> 
> 2010-03-16  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	* stabsread.c (define_symbol): Add support for char
> 	and string constants.
> 
> Index: stabsread.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/stabsread.c,v
> retrieving revision 1.123
> diff -u -p -r1.123 stabsread.c
> --- stabsread.c	8 Jan 2010 08:55:16 -0000	1.123
> +++ stabsread.c	16 Mar 2010 21:07:48 -0000
> @@ -793,6 +793,56 @@ 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;
> +	    char quote = *p++;
> +            char *startp = 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++;
> +                if (*p)
> +                  p++;
> +              }
> +            *p = '\0';
> +            range_type = create_range_type (NULL,
> +                           objfile_type (objfile)->builtin_int,
> +                           0, strlen(startp));
> + 	    SYMBOL_TYPE (sym) = create_array_type (NULL,
> +              objfile_type (objfile)->builtin_char,
> +              range_type);
> +	    string_value =
> +	      obstack_alloc (&objfile->objfile_obstack,
> +			     strlen (startp) + 1);
> +	    strcpy ((char *)string_value, startp);
> +            *p = quote;
> +            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: 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
> --- testsuite/gdb.stabs/weird.def	8 Jan 2010 08:55:16 -0000	1.3
> +++ testsuite/gdb.stabs/weird.def	16 Mar 2010 21:07:49 -0000
> @@ -286,6 +286,15 @@ 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=70", N_LSYM,0,0, 0
> +
> +# Test string constant
> +.stabs "constString1:c=s'String1'", N_LSYM,0,0, 0
> +.stabs "constString2:c=s\"String2\"", N_LSYM,0,0, 0
> +.stabs "constString3:c=s'String3 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: 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
> --- testsuite/gdb.stabs/weird.exp	1 Jan 2010 07:32:06 -0000	1.18
> +++ testsuite/gdb.stabs/weird.exp	16 Mar 2010 21:07:49 -0000
> @@ -164,6 +164,11 @@ proc do_tests {} {
> 
>  	gdb_test "p sizeof (const70)" " = 2" "'e' constant with embedded
> type"
> 
> +	   gdb_test "p /x constchar" " = 0x46" "char constant"
> +        gdb_test "p constString1" " = \"String1\"" "String constant 1"
> +        gdb_test "p constString2" " = \"String2\"" "String constant 2"
> +        gdb_test "p constString3" " = \"String3 with embedded quote '
> in
> the middle\"" "String constant 3"
> +
>  	gdb_test "p bad_neg0" " = \{field0 = 42, field2 =.*field3 = 45\}"
> "p
> bad_neg0"
> 
>  	gdb_test "ptype inttype" "type = (unsigned int|inttype)" "ptype
> on
> inttype"


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: PING [RFC] Support for const char and strings in stabs reader
  2010-03-22 21:56 ` PING " Pierre Muller
@ 2010-03-22 22:19   ` Joel Brobecker
  0 siblings, 0 replies; 7+ messages in thread
From: Joel Brobecker @ 2010-03-22 22:19 UTC (permalink / raw)
  To: Pierre Muller; +Cc: gdb-patches

>   Is there someone else able to review this patch
> or should I try to contact Elena directly?

I was planning on reviewing this patch, I just haven't had time yet.
Do pester me if I don't get to it before the end of this week.

-- 
Joel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] Support for const char and strings in stabs reader
  2010-03-16 21:10 [RFC] Support for const char and strings in stabs reader Pierre Muller
  2010-03-22 21:56 ` PING " Pierre Muller
@ 2010-03-23 18:53 ` Joel Brobecker
  2010-03-24 22:55   ` [RFC-v2] " Pierre Muller
  1 sibling, 1 reply; 7+ messages in thread
From: Joel Brobecker @ 2010-03-23 18:53 UTC (permalink / raw)
  To: Pierre Muller; +Cc: gdb-patches

> 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...

> +	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?

> +            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.

> +            *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...

It looks like you do this mostly to be able to get the length of
the string itself.  But that's easy to obtain, thanks to the loop
that searches the ending quote character.

> +	    strcpy ((char *)string_value, startp);
> +            *p = quote;

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?
Does it look like your current implementation will in fact contain it?

> +            range_type = create_range_type (NULL,
> +                           objfile_type (objfile)->builtin_int,
> +                           0, strlen(startp));

The indentation is wrong, it should be:

            range_type =
              create_range_type (NULL,
                                 objfile_type (objfile)->builtin_int,
                                 0, strlen(startp));

> +	   gdb_test "p /x constchar" " = 0x46" "char constant"
> +        gdb_test "p constString1" " = \"String1\"" "String constant 1"
> +        gdb_test "p constString2" " = \"String2\"" "String constant 2"
> +        gdb_test "p constString3" " = \"String3 with embedded quote ' in
> the middle\"" "String constant 3"

Your last test seems to contradict me on the escape character, but
how does this end up working?

-- 
Joel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [RFC-v2] Support for const char and strings in stabs reader
  2010-03-23 18:53 ` Joel Brobecker
@ 2010-03-24 22:55   ` Pierre Muller
  2010-04-05 15:19     ` Joel Brobecker
  0 siblings, 1 reply; 7+ messages in thread
From: Pierre Muller @ 2010-03-24 22:55 UTC (permalink / raw)
  To: 'Joel Brobecker'; +Cc: gdb-patches

> -----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.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC-v2] Support for const char and strings in stabs reader
  2010-03-24 22:55   ` [RFC-v2] " Pierre Muller
@ 2010-04-05 15:19     ` Joel Brobecker
  2010-04-05 22:48       ` Pierre Muller
  0 siblings, 1 reply; 7+ messages in thread
From: Joel Brobecker @ 2010-04-05 15:19 UTC (permalink / raw)
  To: Pierre Muller; +Cc: gdb-patches

> > 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...

The tricks for the .vimrc file is not a silver bullet ensuring that all
lines start with tabs instead of using blocks of 8 spaces. What it only
does is use tabs instead of spaces when creating a new line. Some parts
of your patch have lines that start with a space, then a tab, then more
spaces...

When you need to see spaces and tabs, I recommend the following setting:

        :set list

I cannot help shaking my head as to how much grief this spaces vs tabs
thing is causing. It's insane, IMO, to be clinging to tabs in our
programming style. But I've fought and lost that battle before...


> 2010-03-24  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
> 	* stabsread.c (define_symbol): Add support for char
> 	and string constants.

This is OK, after you make sure to format the code properly (spaces
and tabs).

> 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.

OK, except for one tiny hunk that I suspect got in unintentionally:

> @@ -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

This change was probably not meant to go in.

-- 
Joel

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [RFC-v2] Support for const char and strings in stabs reader
  2010-04-05 15:19     ` Joel Brobecker
@ 2010-04-05 22:48       ` Pierre Muller
  0 siblings, 0 replies; 7+ messages in thread
From: Pierre Muller @ 2010-04-05 22:48 UTC (permalink / raw)
  To: 'Joel Brobecker'; +Cc: gdb-patches



> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : Monday, April 05, 2010 5:19 PM
> À : Pierre Muller
> Cc : gdb-patches@sourceware.org
> Objet : Re: [RFC-v2] Support for const char and strings in stabs reader
> 
> > > 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...
> 
> The tricks for the .vimrc file is not a silver bullet ensuring that all
> lines start with tabs instead of using blocks of 8 spaces. What it only
> does is use tabs instead of spaces when creating a new line. Some parts
> of your patch have lines that start with a space, then a tab, then more
> spaces...
> 
> When you need to see spaces and tabs, I recommend the following
> setting:
> 
>         :set list
> 
> I cannot help shaking my head as to how much grief this spaces vs tabs
> thing is causing. It's insane, IMO, to be clinging to tabs in our
> programming style. But I've fought and lost that battle before...
> 

 Thanks for that advice,
it was still a pain to convert, but at least
I see what Ido!
 
> > 2010-03-24  Pierre Muller  <muller@ics.u-strasbg.fr>
> >
> > 	* stabsread.c (define_symbol): Add support for char
> > 	and string constants.
> 
> This is OK, after you make sure to format the code properly (spaces
> and tabs).

  Done, hopefully without errors.

> > 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.
> 
> OK, except for one tiny hunk that I suspect got in unintentionally:
> 
> > @@ -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
> 
> This change was probably not meant to go in.
Nice catch, of course not, I just used it to inspect
that generated assembler file.

  Committed after applying your requests,


thanks,

Pierre




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2010-04-05 22:48 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-16 21:10 [RFC] Support for const char and strings in stabs reader Pierre Muller
2010-03-22 21:56 ` PING " Pierre Muller
2010-03-22 22:19   ` Joel Brobecker
2010-03-23 18:53 ` Joel Brobecker
2010-03-24 22:55   ` [RFC-v2] " Pierre Muller
2010-04-05 15:19     ` Joel Brobecker
2010-04-05 22:48       ` Pierre Muller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).