[RFC] Strings and arrays without malloc

[Daniel Jacobowitz <drow at false dot org>]

This patch allows a number of expressions to work without having to
call malloc.  This version of the patch turns a few operations which
work today into errors.  My previous version allowed them, at the
expense of a few more places we had to call malloc.  I think this is
the right compromise, but I'd like a second opinion.

Here's some things which call malloc today, and do not with the patch
applied - yes, even the sizeof and ptype ones:

  print "abc"
  print sizeof ("abc")
  ptype "abc"
  print $cvar = "abc"
  print "abc"[1]
  print {4, 5, 6}

Here's some expressions which used to call malloc and still do; these
are operations which involve pointers, so we need to call malloc to
get a valid pointer for them.

  print *"abc"
  print "abc" + 1
  print &"abc"
  print strcmp ("abc", "def")
  print &{4, 5, 6}

Here's some expressions which used to call malloc and will now produce
an error about "value not located in memory":

  print &{4, 5, 6}[1]
  ptype &{4, 5, 6}[1]

Those last are because values carry additional information in GDB.
"{4, 5, 6}[1]" is still "5", but it used to be lval_memory with an
address in a region allocated by malloc.  I could have caused
subscripting to call malloc, and preserved that behavior, but I
thought that being able to subscript strings without calling malloc
was more useful.

Any comments?  Tested x86_64-linux, no regressions.

-- 
Daniel Jacobowitz
CodeSourcery

2008-03-09  Daniel Jacobowitz  <dan@codesourcery.com>

	* eval.c (evaluate_subexp_for_address): Clarify error message.
	Use value_must_coerce_to_target.
	* infcall.c (value_arg_coerce): Call value_coerce_to_target.
	* valops.c (value_assign): Call value_coerce_to_target when
	assigning to anything but internalvars.  Leave GDB-side arrays
	as arrays when assigning to internalvars.
	(value_must_coerce_to_target, value_coerce_to_target): New.
	(value_coerce_array, value_addr): Call value_coerce_to_target.
	(value_array): Create the array in GDB's memory instead of
	the inferior's.
	* value.h (value_must_coerce_to_target, value_coerce_to_target):
	Declare.

2008-03-09  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.texinfo (Expressions): Update description of malloced arrays.

2008-03-09  Daniel Jacobowitz  <dan@codesourcery.com>

	* gdb.base/printcmds.exp (test_print_array_constants): Do not expect
	*& to work on created array elements.
	(Top level): Test print $pc with a file.  Test string operations
	without a target.
	* gdb.base/ptype.exp: Do not expect *& to work on created array
	elements.

Index: eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.80
diff -u -p -r1.80 eval.c
--- eval.c	4 Feb 2008 00:23:04 -0000	1.80
+++ eval.c	9 Mar 2008 15:47:31 -0000
@@ -2204,14 +2204,14 @@ evaluate_subexp_for_address (struct expr
 	{
 	  struct type *type = check_typedef (value_type (x));
 
-	  if (VALUE_LVAL (x) == lval_memory)
+	  if (VALUE_LVAL (x) == lval_memory || value_must_coerce_to_target (x))
 	    return value_zero (lookup_pointer_type (value_type (x)),
 			       not_lval);
 	  else if (TYPE_CODE (type) == TYPE_CODE_REF)
 	    return value_zero (lookup_pointer_type (TYPE_TARGET_TYPE (type)),
 			       not_lval);
 	  else
-	    error (_("Attempt to take address of non-lval"));
+	    error (_("Attempt to take address of value not located in memory."));
 	}
       return value_addr (x);
     }
Index: infcall.c
===================================================================
RCS file: /cvs/src/src/gdb/infcall.c,v
retrieving revision 1.94
diff -u -p -r1.94 infcall.c
--- infcall.c	8 Jan 2008 19:28:08 -0000	1.94
+++ infcall.c	9 Mar 2008 15:47:31 -0000
@@ -111,6 +111,12 @@ value_arg_coerce (struct value *arg, str
   if (current_language->la_language == language_ada)
     arg = ada_convert_actual (arg, type, sp);
 
+  /* Force the value to the target if we will need its address.  At
+     this point, we could allocate arguments on the stack instead of
+     calling malloc if we knew that their addresses would not be
+     saved by the called function.  */
+  arg = value_coerce_to_target (arg);
+
   switch (TYPE_CODE (type))
     {
     case TYPE_CODE_REF:
Index: valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.183
diff -u -p -r1.183 valops.c
--- valops.c	4 Feb 2008 00:23:04 -0000	1.183
+++ valops.c	9 Mar 2008 15:47:31 -0000
@@ -600,9 +600,18 @@ value_assign (struct value *toval, struc
 
   type = value_type (toval);
   if (VALUE_LVAL (toval) != lval_internalvar)
-    fromval = value_cast (type, fromval);
+    {
+      toval = value_coerce_to_target (toval);
+      fromval = value_cast (type, fromval);
+    }
   else
-    fromval = coerce_array (fromval);
+    {
+      /* Coerce arrays and functions to pointers, except for arrays
+	 which only live in GDB's storage.  */
+      if (!value_must_coerce_to_target (fromval))
+	fromval = coerce_array (fromval);
+    }
+
   CHECK_TYPEDEF (type);
 
   /* Since modifying a register can trash the frame chain, and
@@ -852,6 +861,50 @@ value_of_variable (struct symbol *var, s
   return val;
 }
 
+/* Return one if VAL does not live in target memory, but should in order
+   to operate on it.  Otherwise return zero.  */
+
+int
+value_must_coerce_to_target (struct value *val)
+{
+  struct type *valtype;
+
+  /* The only lval kinds which do not live in target memory.  */
+  if (VALUE_LVAL (val) != not_lval
+      && VALUE_LVAL (val) != lval_internalvar)
+    return 0;
+
+  valtype = check_typedef (value_type (val));
+
+  switch (TYPE_CODE (valtype))
+    {
+    case TYPE_CODE_ARRAY:
+    case TYPE_CODE_STRING:
+      return 1;
+    default:
+      return 0;
+    }
+}
+
+/* Make sure that VAL lives in target memory if it's supposed to.  For instance,
+   strings are constructed as character arrays in GDB's storage, and this
+   function copies them to the target.  */
+
+struct value *
+value_coerce_to_target (struct value *val)
+{
+  LONGEST length;
+  CORE_ADDR addr;
+
+  if (!value_must_coerce_to_target (val))
+    return val;
+
+  length = TYPE_LENGTH (check_typedef (value_type (val)));
+  addr = allocate_space_in_inferior (length);
+  write_memory (addr, value_contents (val), length);
+  return value_at_lazy (value_type (val), addr);
+}
+
 /* Given a value which is an array, return a value which is a pointer
    to its first element, regardless of whether or not the array has a
    nonzero lower bound.
@@ -881,6 +934,11 @@ value_coerce_array (struct value *arg1)
 {
   struct type *type = check_typedef (value_type (arg1));
 
+  /* If the user tries to do something requiring a pointer with an
+     array that has not yet been pushed to the target, then this would
+     be a good time to do so.  */
+  arg1 = value_coerce_to_target (arg1);
+
   if (VALUE_LVAL (arg1) != lval_memory)
     error (_("Attempt to take address of value not located in memory."));
 
@@ -926,6 +984,10 @@ value_addr (struct value *arg1)
   if (TYPE_CODE (type) == TYPE_CODE_FUNC)
     return value_coerce_function (arg1);
 
+  /* If this is an array that has not yet been pushed to the target,
+     then this would be a good time to force it to memory.  */
+  arg1 = value_coerce_to_target (arg1);
+
   if (VALUE_LVAL (arg1) != lval_memory)
     error (_("Attempt to take address of value not located in memory."));
 
@@ -1016,7 +1078,7 @@ value_ind (struct value *arg1)
   return 0;			/* For lint -- never reached.  */
 }
 
-/* Create a value for an array by allocating space in the inferior,
+/* Create a value for an array by allocating space in GDB, copying
    copying the data into that space, and then setting up an array
    value.
 
@@ -1074,24 +1136,15 @@ value_array (int lowbound, int highbound
       return val;
     }
 
-  /* Allocate space to store the array in the inferior, and then
-     initialize it by copying in each element.  FIXME: Is it worth it
-     to create a local buffer in which to collect each value and then
-     write all the bytes in one operation?  */
+  /* Allocate space to store the array, and then initialize it by
+     copying in each element.  */
 
-  addr = allocate_space_in_inferior (nelem * typelength);
+  val = allocate_value (arraytype);
   for (idx = 0; idx < nelem; idx++)
-    {
-      write_memory (addr + (idx * typelength),
-		    value_contents_all (elemvec[idx]),
-		    typelength);
-    }
-
-  /* Create the array type and set up an array value to be evaluated
-     lazily.  */
-
-  val = value_at_lazy (arraytype, addr);
-  return (val);
+    memcpy (value_contents_writeable (val) + (idx * typelength),
+	    value_contents_all (elemvec[idx]),
+	    typelength);
+  return val;
 }
 
 /* Create a value for a string constant by allocating space in the
Index: value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.108
diff -u -p -r1.108 value.h
--- value.h	4 Feb 2008 00:23:04 -0000	1.108
+++ value.h	9 Mar 2008 15:47:31 -0000
@@ -332,6 +332,10 @@ extern struct value *value_add (struct v
 
 extern struct value *value_sub (struct value *arg1, struct value *arg2);
 
+extern int value_must_coerce_to_target (struct value *arg1);
+
+extern struct value *value_coerce_to_target (struct value *arg1);
+
 extern struct value *value_coerce_array (struct value *arg1);
 
 extern struct value *value_coerce_function (struct value *arg1);
Index: doc/gdb.texinfo
===================================================================
RCS file: /cvs/src/src/gdb/doc/gdb.texinfo,v
retrieving revision 1.472
diff -u -p -r1.472 gdb.texinfo
--- doc/gdb.texinfo	3 Mar 2008 13:24:12 -0000	1.472
+++ doc/gdb.texinfo	9 Mar 2008 15:47:34 -0000
@@ -5567,8 +5567,10 @@ you compiled your program to include thi
 @cindex arrays in expressions
 @value{GDBN} supports array constants in expressions input by
 the user.  The syntax is @{@var{element}, @var{element}@dots{}@}.  For example,
-you can use the command @code{print @{1, 2, 3@}} to build up an array in
-memory that is @code{malloc}ed in the target program.
+you can use the command @code{print @{1, 2, 3@}} to create an array
+of three integers.  If you pass an array to a function or assign it
+to a program variable, @value{GDBN} copies the array to memory that
+is @code{malloc}ed in the target program.
 
 Because C is so widespread, most of the expressions shown in examples in
 this manual are in C.  @xref{Languages, , Using @value{GDBN} with Different
Index: testsuite/gdb.base/printcmds.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/printcmds.exp,v
retrieving revision 1.20
diff -u -p -r1.20 printcmds.exp
--- testsuite/gdb.base/printcmds.exp	9 Jan 2008 13:47:59 -0000	1.20
+++ testsuite/gdb.base/printcmds.exp	9 Mar 2008 15:47:34 -0000
@@ -651,7 +651,7 @@ proc test_print_array_constants {} {
     gdb_test_escape_braces "print {(long)0,(long)1,(long)2}"  " = {0, 1, 2}"
     gdb_test_escape_braces "print {{0,1,2},{3,4,5}}"  " = {{0, 1, 2}, {3, 4, 5}}"
     gdb_test "print {4,5,6}\[2\]"	" = 6"
-    gdb_test "print *&{4,5,6}\[1\]"	" = 5"
+    gdb_test "print *&{4,5,6}\[1\]"	"Attempt to take address of value not located in memory."
 }
 
 proc test_printf {} {
@@ -735,11 +735,19 @@ gdb_start
 gdb_reinitialize_dir $srcdir/$subdir
 
 gdb_test "print \$pc" "No registers\\."
-# FIXME: should also test "print $pc" when there is an execfile but no
-# remote debugging target, process or corefile.
+
+# Some simple operations on strings should work even without a target
+# (and therefore without calling malloc).
+gdb_test "print \"abc\"" " = \"abc\""
+gdb_test "print sizeof (\"abc\")" " = 4"
+gdb_test "ptype \"abc\"" " = char \\\[4\\\]"
+gdb_test "print \$cvar = \"abc\"" " = \"abc\""
+gdb_test "print sizeof (\$cvar)" " = 4"
 
 gdb_load ${binfile}
 
+gdb_test "print \$pc" "No registers\\." "print \$pc (with file)"
+
 gdb_test "set print sevenbit-strings" ""
 gdb_test "set print address off" ""
 gdb_test "set width 0" ""
Index: testsuite/gdb.base/ptype.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/ptype.exp,v
retrieving revision 1.14
diff -u -p -r1.14 ptype.exp
--- testsuite/gdb.base/ptype.exp	30 Jan 2008 18:48:07 -0000	1.14
+++ testsuite/gdb.base/ptype.exp	9 Mar 2008 15:47:34 -0000
@@ -638,7 +638,7 @@ if [runto_main] then {
   gdb_test "ptype {(float)0,(float)1,(float)2}" "type = float \\\[3\\\]"
   gdb_test "ptype {{0,1,2},{3,4,5}}"	"type = int \\\[2\\\]\\\[3\\\]"
   gdb_test "ptype {4,5,6}\[2\]"	"type = int"
-  gdb_test "ptype *&{4,5,6}\[1\]"	"type = int"
+  gdb_test "ptype *&{4,5,6}\[1\]"	"Attempt to take address of value not located in memory."
 
   # Test ptype of user register
   gdb_test "ptype \$pc" "void \\(\\*\\)\\(\\)" "ptype \$pc"