RFA: fix PR 2506

RFA: fix PR 2506

[Tom Tromey]

PR 2506 is about string concatenation.

Something like "x" "y" is a valid C expression.  However, gdb does not
parse it.

The simplest fix was to add a new production that handles string
concatenation.  I looked at doing this in the lexer, but that is
tricky due to macro expansion.

The memory allocation is handled the way it is in order to ensure that
strings are always freed.  (If we allocated a new string in the lexer
it would leak if the string occurred in a syntactically invalid
place.)

Built & regtested on x86-64 (compile farm).
New test case included.

Ok?

Tom

2008-08-20  Tom Tromey  <tromey@redhat.com>

	PR gdb/2506:
	* c-exp.y (string_exp): New production.
	(exp): Use it.

2008-08-20  Tom Tromey  <tromey@redhat.com>

	* gdb.base/exprs.exp (test_expr): Add test for string
	concatenation.

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index bd04dc2..be79f15 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -182,7 +182,7 @@ static int parse_number (char *, int, int, YYSTYPE *);
 %token <ssym> NAME /* BLOCKNAME defined below to give it higher precedence. */
 %token <voidval> COMPLETE
 %token <tsym> TYPENAME
-%type <sval> name
+%type <sval> name string_exp
 %type <ssym> name_not_typename
 %type <tsym> typename
 
@@ -560,7 +560,36 @@ exp	:	SIZEOF '(' type ')'	%prec UNARY
 			  write_exp_elt_opcode (OP_LONG); }
 	;
 
-exp	:	STRING
+string_exp:
+		STRING
+			{
+			  /* We copy the string here, and not in the
+			     lexer, to guarantee that we do not leak a
+			     string.  Note that we follow the
+			     NUL-termination convention of the
+			     lexer.  */
+			  $$.length = $1.length;
+			  $$.ptr = xmalloc ($1.length + 1);
+			  memcpy ($$.ptr, $1.ptr, $1.length + 1);
+			}
+
+	|	string_exp STRING
+			{
+			  /* Note that we NUL-terminate here, but just
+			     for convenience.  */
+			  struct stoken t;
+			  t.length = $1.length + $2.length;
+			  t.ptr = xmalloc (t.length + 1);
+			  memcpy (t.ptr, $1.ptr, $1.length);
+			  memcpy (t.ptr + $1.length, $2.ptr, $2.length);
+			  t.ptr[t.length] = '\0';
+			  xfree ($1.ptr);
+			  xfree ($2.ptr);
+			  $$ = t;
+			}
+		;
+
+exp	:	string_exp
 			{ /* C strings are converted into array constants with
 			     an explicit null byte added at the end.  Thus
 			     the array upper bound is the string length.
@@ -581,7 +610,9 @@ exp	:	STRING
 			  write_exp_elt_opcode (OP_ARRAY);
 			  write_exp_elt_longcst ((LONGEST) 0);
 			  write_exp_elt_longcst ((LONGEST) ($1.length));
-			  write_exp_elt_opcode (OP_ARRAY); }
+			  write_exp_elt_opcode (OP_ARRAY);
+			  xfree ($1.ptr);
+			}
 	;
 
 /* C++.  */
diff --git a/gdb/testsuite/gdb.base/exprs.exp b/gdb/testsuite/gdb.base/exprs.exp
index e25de77..ad60aad 100644
--- a/gdb/testsuite/gdb.base/exprs.exp
+++ b/gdb/testsuite/gdb.base/exprs.exp
@@ -248,3 +248,6 @@ gdb_test "print (void*) ((long long) (unsigned long) -1 + 1)" \
 if [expr ! $ok] { setup_xfail "*-*-*" }
 gdb_test "print (void*) (~((long long)(unsigned long) -1) - 1)" \
 	"warning: value truncated.*" "truncate (void*) 0xffffffff00000000 - 1"
+
+# String concatentation.
+test_expr "print \"x\" \"y\"" "\\$\[0-9\]* = \"xy\""

Re: RFA: fix PR 2506

[Pedro Alves]

Hi Tom,

Thanks for working on this.

On Thursday 21 August 2008 02:56:21, Tom Tromey wrote:

> PR 2506 is about string concatenation.

> 2008-08-20  Tom Tromey  <tromey@redhat.com>
> 
> 	PR gdb/2506:
> 	* c-exp.y (string_exp): New production.
> 	(exp): Use it.
> 
> 2008-08-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* gdb.base/exprs.exp (test_expr): Add test for string
> 	concatenation.
> 

This crashes for me:

 print "foo" "bar" "boo"

:-(

Also, these files don't use xmalloc/xfree.  Instead, one writes
malloc/free; they're sed'ed by the build system --- see the top of
c-exp.y.

-- 
Pedro Alves

Re: RFA: fix PR 2506

[Tom Tromey]

Pedro> This crashes for me:
Pedro>  print "foo" "bar" "boo"
Pedro> :-(

Thanks.

I added a new test case and fixed this problem.
I'm sending it back through the regression tester now.

Pedro> Also, these files don't use xmalloc/xfree.  Instead, one writes
Pedro> malloc/free; they're sed'ed by the build system --- see the top of
Pedro> c-exp.y.

I made this change.

Two notes here: first, the Makefile doesn't actually s/free/xfree/ --
it only handles malloc and realloc.  Funny.  This should not really
matter in practice.

Second, ada-exp.y uses xmalloc once and p-exp.y uses xfree once, in
case anybody wants to clean that up.


I'll resubmit once the testing is finished.

Tom

Re: RFA: fix PR 2506

[Tom Tromey]

Tom> I'll resubmit once the testing is finished.

This fixes the problems you reported, and adds a new test case.
Built & regression tested on x86-64 (compile farm).

Ok?

Tom

2008-10-21  Tom Tromey  <tromey@redhat.com>

	PR gdb/2506:
	* c-exp.y (string_exp): New production.
	(exp): Use it.

2008-08-20  Tom Tromey  <tromey@redhat.com>

	* gdb.base/exprs.exp (test_expr): Add test for string
	concatenation.

diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 6d16940..153e2be 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -184,7 +184,7 @@ static int parse_number (char *, int, int, YYSTYPE *);
 %token <ssym> NAME /* BLOCKNAME defined below to give it higher precedence. */
 %token <voidval> COMPLETE
 %token <tsym> TYPENAME
-%type <sval> name
+%type <sval> name string_exp
 %type <ssym> name_not_typename
 %type <tsym> typename
 
@@ -562,7 +562,34 @@ exp	:	SIZEOF '(' type ')'	%prec UNARY
 			  write_exp_elt_opcode (OP_LONG); }
 	;
 
-exp	:	STRING
+string_exp:
+		STRING
+			{
+			  /* We copy the string here, and not in the
+			     lexer, to guarantee that we do not leak a
+			     string.  Note that we follow the
+			     NUL-termination convention of the
+			     lexer.  */
+			  $$.length = $1.length;
+			  $$.ptr = malloc ($1.length + 1);
+			  memcpy ($$.ptr, $1.ptr, $1.length + 1);
+			}
+
+	|	string_exp STRING
+			{
+			  /* Note that we NUL-terminate here, but just
+			     for convenience.  */
+			  struct stoken t;
+			  t.length = $1.length + $2.length;
+			  t.ptr = malloc (t.length + 1);
+			  memcpy (t.ptr, $1.ptr, $1.length);
+			  memcpy (t.ptr + $1.length, $2.ptr, $2.length + 1);
+			  free ($1.ptr);
+			  $$ = t;
+			}
+		;
+
+exp	:	string_exp
 			{ /* C strings are converted into array constants with
 			     an explicit null byte added at the end.  Thus
 			     the array upper bound is the string length.
@@ -583,7 +610,9 @@ exp	:	STRING
 			  write_exp_elt_opcode (OP_ARRAY);
 			  write_exp_elt_longcst ((LONGEST) 0);
 			  write_exp_elt_longcst ((LONGEST) ($1.length));
-			  write_exp_elt_opcode (OP_ARRAY); }
+			  write_exp_elt_opcode (OP_ARRAY);
+			  free ($1.ptr);
+			}
 	;
 
 /* C++.  */
diff --git a/gdb/testsuite/gdb.base/exprs.exp b/gdb/testsuite/gdb.base/exprs.exp
index e25de77..484b5a4 100644
--- a/gdb/testsuite/gdb.base/exprs.exp
+++ b/gdb/testsuite/gdb.base/exprs.exp
@@ -248,3 +248,7 @@ gdb_test "print (void*) ((long long) (unsigned long) -1 + 1)" \
 if [expr ! $ok] { setup_xfail "*-*-*" }
 gdb_test "print (void*) (~((long long)(unsigned long) -1) - 1)" \
 	"warning: value truncated.*" "truncate (void*) 0xffffffff00000000 - 1"
+
+# String concatentation.
+test_expr "print \"x\" \"y\"" "\\$\[0-9\]* = \"xy\""
+test_expr "print \"x\" \"y\" \"z\"" "\\$\[0-9\]* = \"xyz\""

Re: RFA: fix PR 2506

[Pedro Alves]

On Tuesday 21 October 2008 22:13:59, Tom Tromey wrote:
> Tom> I'll resubmit once the testing is finished.
> 
> This fixes the problems you reported, and adds a new test case.
> Built & regression tested on x86-64 (compile farm).
> 
> Ok?
> 

OK.

Thanks again.

> 2008-10-21  Tom Tromey  <tromey@redhat.com>
> 
> 	PR gdb/2506:
> 	* c-exp.y (string_exp): New production.
> 	(exp): Use it.
> 
> 2008-08-20  Tom Tromey  <tromey@redhat.com>
> 
> 	* gdb.base/exprs.exp (test_expr): Add test for string
> 	concatenation.
> 

-- 
Pedro Alves