[RFA] Function return type checking

[RFA] Function return type checking

[Klee Dienes]

(This is basically the same patch I sent last week, just updated
to the latest source base.)

The following patch adds "expected return type" support to the target
function call interface.  The "expected return type" is specified
using cast syntax; it is ignored if actual type information for the
function is available.  If no type information is provided for a
function, the user is now required to provide an expected return type
using the cast syntax, or get an error.

Some examples:

(gdb) print fabs (3.0)                  (no type information present)
Unable to call function at 0x%lx: no return type information available.
To call this function anyway, you can cast the return type explicitly
(e.g. 'print (float) fabs (3.0)').

(gdb) print (float) fabs (-3.12)        (no type information present)
$1 = 3.11999989

(gdb) print (int) fabs (-3.12)          (no type information present)
$2 = 1074329026
(here, the return value of `fabs' has been interpreted as an int
... not cast to one)

(gdb) print (int) fabs (-3.12)          (type information present)
$3 = -3
(since type information was available, the return value was
interpreted as a float, then cast to an int).

(gdb) print (struct stat) fabs (-3.12)  (no type information present)
Cannot access memory at address 0xf5c28f6
(this causes an error, since the struct return convention is being
misused)

   struct s { int i; int j; };
   struct s f (void) { struct s ss = { 3, 1 }; return ss; }

(gdb) print f ()                        (no type information present)
Unable to call function at 0x%lx: no return type information available.
To call this function anyway, you can cast the return type explicitly
(e.g. 'print (float) fabs (3.0)').

(gdb) print (struct s) f (3.0)          (no type information present)
$4 = {i = 3, j = 1}

(gdb) print (float) f (3.0)             (no type information present)
Program received signal SIGSEGV, Segmentation fault.
0x0804841d in f ()
(here we lied to GDB about the return type, causing it to generate
incorrect struct return code).

Compare this to the old behavior:

(gdb) print fabs (3.0)                  (no type information present)
$1 = 1074266112
(GDB has no type information available, so it assumes 'int').

(gdb) print (float) fabs (-3.12)        (no type information present)
$2 = 1.07426611e+09
(GDB still assumes the function returns 'int', then casts the result
to a float).

   struct s { int i; int j; };
   struct s f (void) { struct s ss = { 3, 1 }; return ss; }

(gdb) print f ()
Program received signal SIGSEGV, Segmentation fault.
0x0804841d in f ()
(here there is no way out ... no matter what we pass in as the return
value to f(), GDB will generate the calling convention for a function
returning int)

The reason I suggest 2) might be controversial is that the old
behavior can be convenient in the general case.  A lot of functions
*do* return int, and when debugging programs without symbols, it can
be annoying to have to declare the return type, just to make the rare
case when functions return floats/structs behave correctly.  But I'd
argue that it's not *that* big a deal to cast a return value, and
correctness must always take priority.

2002-02-04  Klee Dienes <kdienes@apple.com>

         * c-valprint.c (c_val_print): print type with typecode 
TYPE_CODE_ERROR
         as "<error type>" and not "<unknown type>".

         * eval.c (evaluate_subexp_standard): when called with
         EVAL_AVOID_SIDE_EFFECTS, return the passed-in `expect_type' if 
the
         type of the evaluated expression is unknown.  Also update to use
         the new `call_function_by_hand_expecting_type' interface.

         * gcore.c (default_derive_heap_segment): update to pass type 
argument
         to find_function_in_inferior.

         * gdbtypes.c, gdbtypes.h: add new type
         builtin_type_voidptrfuncptr.

         * parse.c, parse.h: export msym_{text,data,unknown}_symbol_type.
         Make them of type TYPE_CODE_ERROR instead of TYPE_CODE_INT.

         * parser-defs.h: export msym_{text,data,unknown}_symbol_type.

         * scm-lang.c (scm_lookup_name): update to use new interface to
         find_function_in_inferior.

         * valops.c (find_function_in_inferior): add a default type as 
second
         argument.  This type will be used for the returned value if no
         type information is available for the function (previously, the
         type was assumed to be (char *) (*).
         (hand_function_call): add new `expect_type' parameter.  This type
         will be used as the return type for the function if no type
         information is available.
         (call_function_by_hand_expecting_type): like
         call_function_by_hand, but uses the new `expect_type' interface 
to
         hand_function_call.
         (call_function_by_hand): implement as a wrapper to
         call_function_by_hand_expecting_type.
         (value_allocate_space_in_inferior): use new cached_function
         interface.
         (find_function_addr): if asked the return type of a function with
         type TYPE_CODE_ERROR, return TYPE_CODE_ERROR.

         * value.h (call_function_by_hand_expecting_type): add.
         (find_function_in_inferior) update to new signature.

Index: gdb/c-valprint.c
===================================================================
RCS file: /cvs/src/src/gdb/c-valprint.c,v
retrieving revision 1.12
diff -u -r1.12 c-valprint.c
--- c-valprint.c        2002/02/04 02:14:46     1.12
+++ c-valprint.c        2002/02/05 06:22:27
@@ -453,7 +453,7 @@
        break;

      case TYPE_CODE_ERROR:
-      fprintf_filtered (stream, "<error type>");
+      fprintf_filtered (stream, "<unknown type>");
        break;

      case TYPE_CODE_UNDEF:
Index: gdb/eval.c
===================================================================
RCS file: /cvs/src/src/gdb/eval.c,v
retrieving revision 1.19
diff -u -r1.19 eval.c
--- eval.c      2002/01/04 17:51:38     1.19
+++ eval.c      2002/02/05 06:22:29
@@ -928,15 +928,20 @@
              gdb isn't asked for it's opinion (ie. through "whatis"),
              it won't offer it. */

-         struct type *ftype =
-         TYPE_TARGET_TYPE (VALUE_TYPE (argvec[0]));
-
-         if (ftype)
-           return allocate_value (TYPE_TARGET_TYPE (VALUE_TYPE 
(argvec[0])));
-         else
-           error ("Expression of type other than \"Function 
returning ...\" used as function");
+         struct type *type =
+           TYPE_TARGET_TYPE (VALUE_TYPE (argvec[0]));
+
+         if (type)
+           {
+             if ((TYPE_CODE (type) == TYPE_CODE_ERROR) && expect_type)
+               return allocate_value (expect_type);
+             else
+               return allocate_value (type);
+           }
+         else
+            error ("Expression of type other than \"Function 
returning ...\" used as function");
         }
-      return call_function_by_hand (argvec[0], nargs, argvec + 1);
+      return call_function_by_hand_expecting_type (argvec[0], 
expect_type, nargs, argvec + 1);
        /* pai: FIXME save value from call_function_by_hand, then adjust 
pc by adjust_fn_pc if +ve  */

      case OP_F77_UNDETERMINED_ARGLIST:
Index: gdb/gcore.c
===================================================================
RCS file: /cvs/src/src/gdb/gcore.c,v
retrieving revision 1.3
diff -u -r1.3 gcore.c
--- gcore.c     2002/01/14 20:00:48     1.3
+++ gcore.c     2002/02/05 06:22:30
@@ -274,7 +274,7 @@
         }
      }
    /* Now get the top-of-heap by calling sbrk in the inferior.  */
-  if ((sbrk = find_function_in_inferior ("sbrk")) == NULL)
+  if ((sbrk = find_function_in_inferior ("sbrk", 
builtin_type_voidptrfuncptr)) == NULL)
      return 0;
    if ((zero = value_from_longest (builtin_type_int, (LONGEST) 0)) == 
NULL)
      return 0;
Index: gdb/gdbtypes.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.c,v
retrieving revision 1.40
diff -u -r1.40 gdbtypes.c
--- gdbtypes.c  2002/02/03 23:18:11     1.40
+++ gdbtypes.c  2002/02/05 06:22:33
@@ -99,6 +99,7 @@
  struct type *builtin_type_void_func_ptr;
  struct type *builtin_type_CORE_ADDR;
  struct type *builtin_type_bfd_vma;
+struct type *builtin_type_voidptrfuncptr;

  int opaque_type_resolution = 1;
  int overload_debug = 0;
@@ -3290,6 +3291,8 @@
      init_type (TYPE_CODE_INT, TARGET_BFD_VMA_BIT / 8,
                TYPE_FLAG_UNSIGNED,
                "__bfd_vma", (struct objfile *) NULL);
+  builtin_type_voidptrfuncptr =
+    lookup_pointer_type (lookup_function_type (lookup_pointer_type 
(builtin_type_void)));
  }


Index: gdb/gdbtypes.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbtypes.h,v
retrieving revision 1.26
diff -u -r1.26 gdbtypes.h
--- gdbtypes.h  2002/02/03 22:57:56     1.26
+++ gdbtypes.h  2002/02/05 06:22:35
@@ -934,6 +934,7 @@
     bit address type even though the TARGET has a 64 bit pointer type
     (cf MIPS). */
  extern struct type *builtin_type_bfd_vma;
+extern struct type *builtin_type_voidptrfuncptr;

  /* Explicit sizes - see C9X <intypes.h> for naming scheme */
  extern struct type *builtin_type_int8;
Index: gdb/parse.c
===================================================================
RCS file: /cvs/src/src/gdb/parse.c,v
retrieving revision 1.19
diff -u -r1.19 parse.c
--- parse.c     2002/01/31 02:13:56     1.19
+++ parse.c     2002/02/05 06:22:37
@@ -382,9 +382,9 @@
     based on the language, but they no longer have names like "int", so
     the initial rationale is gone.  */

-static struct type *msym_text_symbol_type;
-static struct type *msym_data_symbol_type;
-static struct type *msym_unknown_symbol_type;
+struct type *msym_text_symbol_type;
+struct type *msym_data_symbol_type;
+struct type *msym_unknown_symbol_type;

  void
  write_exp_msymbol (struct minimal_symbol *msymbol,
@@ -1341,16 +1341,15 @@
  {
    int i;

-  msym_text_symbol_type =
-    init_type (TYPE_CODE_FUNC, 1, 0, "<text variable, no debug info>", 
NULL);
-  TYPE_TARGET_TYPE (msym_text_symbol_type) = builtin_type_int;
-  msym_data_symbol_type =
-    init_type (TYPE_CODE_INT, TARGET_INT_BIT / HOST_CHAR_BIT, 0,
-              "<data variable, no debug info>", NULL);
-  msym_unknown_symbol_type =
-    init_type (TYPE_CODE_INT, 1, 0,
-              "<variable (not text or data), no debug info>",
-              NULL);
+   msym_text_symbol_type =
+    init_type (TYPE_CODE_FUNC, 0, 0, "<text variable, no debug info>", 
NULL);
+   TYPE_TARGET_TYPE (msym_text_symbol_type) = builtin_type_error;
+   msym_data_symbol_type =
+     init_type (TYPE_CODE_ERROR, 0, 0,
+               "<data variable, no debug info>", NULL);
+   msym_unknown_symbol_type =
+     init_type (TYPE_CODE_ERROR, 0, 0,
+               "<variable (not text or data), no debug info>", NULL);

    /* create the std_regs table */

Index: gdb/parser-defs.h
===================================================================
RCS file: /cvs/src/src/gdb/parser-defs.h,v
retrieving revision 1.6
diff -u -r1.6 parser-defs.h
--- parser-defs.h       2001/11/15 01:55:59     1.6
+++ parser-defs.h       2002/02/05 06:22:37
@@ -39,6 +39,10 @@
  extern int expout_size;
  extern int expout_ptr;

+extern struct type *msym_text_symbol_type;
+extern struct type *msym_data_symbol_type;
+extern struct type *msym_unknown_symbol_type;
+
  /* If this is nonzero, this block is used as the lexical context
     for symbol names.  */

Index: gdb/scm-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/scm-lang.c,v
retrieving revision 1.7
diff -u -r1.7 scm-lang.c
--- scm-lang.c  2002/01/04 05:20:08     1.7
+++ scm-lang.c  2002/02/05 06:22:37
@@ -169,7 +169,7 @@
      /* FIXME in this case, we should try lookup_symbol first */
      args[2] = value_from_longest (builtin_type_scm, SCM_EOL);

-  func = find_function_in_inferior ("scm_lookup_cstr");
+  func = find_function_in_inferior ("scm_lookup_cstr", 
builtin_type_voidptrfuncptr);
    val = call_function_by_hand (func, 3, args);
    if (!value_logical_not (val))
      return value_ind (val);
@@ -192,7 +192,7 @@
    write_memory (iaddr, str, len);
    /* FIXME - should find and pass env */
    write_memory (iaddr + len, "", 1);
-  func = find_function_in_inferior ("scm_evstr");
+  func = find_function_in_inferior ("scm_evstr", 
builtin_type_voidptrfuncptr);
    return call_function_by_hand (func, 1, &addr);
  }

Index: gdb/valops.c
===================================================================
RCS file: /cvs/src/src/gdb/valops.c,v
retrieving revision 1.50
diff -u -r1.50 valops.c
--- valops.c    2002/02/04 02:14:46     1.50
+++ valops.c    2002/02/05 06:22:41
@@ -32,6 +32,7 @@
  #include "language.h"
  #include "gdbcmd.h"
  #include "regcache.h"
+#include "parser-defs.h"
  #include "cp-abi.h"

  #include <errno.h>
@@ -46,7 +47,7 @@

  static int typecmp (int staticp, struct type *t1[], struct value *t2[]);

-static CORE_ADDR find_function_addr (struct value *, struct type **);
+CORE_ADDR find_function_addr (struct value *, struct type **);
  static struct value *value_arg_coerce (struct value *, struct type *, 
int);


@@ -89,11 +90,13 @@
  int unwind_on_signal_p = 0;



-
-/* Find the address of function name NAME in the inferior.  */
-
-struct value *
-find_function_in_inferior (char *name)
+/* Find the address of function name NAME in the inferior.  If no type
+   information is available for NAME, use `type' as the type for the
+   resulting value.
+*/
+
+struct value *
+find_function_in_inferior (char *name, struct type *type)
  {
    register struct symbol *sym;
    sym = lookup_symbol (name, 0, VAR_NAMESPACE, 0, NULL);
@@ -111,20 +114,18 @@
        struct minimal_symbol *msymbol = lookup_minimal_symbol (name, 
NULL, NULL);
        if (msymbol != NULL)
         {
-         struct type *type;
-         CORE_ADDR maddr;
-         type = lookup_pointer_type (builtin_type_char);
-         type = lookup_function_type (type);
-         type = lookup_pointer_type (type);
-         maddr = SYMBOL_VALUE_ADDRESS (msymbol);
-         return value_from_pointer (type, maddr);
+         if (type != NULL)
+           return value_from_longest (type, (LONGEST) 
SYMBOL_VALUE_ADDRESS (msymbol));
+         else
+           return value_from_longest (lookup_pointer_type 
(msym_text_symbol_type),
+                                      (LONGEST) SYMBOL_VALUE_ADDRESS 
(msymbol));
         }
        else
         {
           if (!target_has_execution)
             error ("evaluation of this expression requires the target 
program to be active");
           else
-           error ("evaluation of this expression requires the program 
to have a function \"%s\".", name);
+           error ("evaluation of this expression requires the program 
to have a function named \"%s\".", name);
         }
      }
  }
@@ -136,10 +137,10 @@
  value_allocate_space_in_inferior (int len)
  {
    struct value *blocklen;
-  struct value *val = find_function_in_inferior ("malloc");
+  struct value *val = find_function_in_inferior ("malloc", 
builtin_type_voidptrfuncptr);

    blocklen = value_from_longest (builtin_type_int, (LONGEST) len);
-  val = call_function_by_hand (val, 1, &blocklen);
+  val = call_function_by_hand (val, 1, &blocklen);
    if (value_logical_not (val))
      {
        if (!target_has_execution)
@@ -1261,6 +1262,11 @@

        value_type = builtin_type_int;
      }
+  else if (code == TYPE_CODE_ERROR)
+    {
+      value_type = builtin_type_error;
+      funaddr = (CORE_ADDR) -1;
+    }
    else
      error ("Invalid data type for function to be called.");

@@ -1287,7 +1293,8 @@
     ARGS is modified to contain coerced values. */

  static struct value *
-hand_function_call (struct value *function, int nargs, struct value 
**args)
+hand_function_call (struct value *function,
+                   struct type *expect_type, int nargs, struct value 
**args)
  {
    register CORE_ADDR sp;
    register int i;
@@ -1334,6 +1341,33 @@
    inf_status = save_inferior_status (1);
    old_chain = make_cleanup_restore_inferior_status (inf_status);

+  funaddr = find_function_addr (function, &value_type);
+  CHECK_TYPEDEF (value_type);
+
+  if ((value_type == NULL) || (value_type->code == TYPE_CODE_ERROR))
+    value_type = expect_type;
+
+  if ((value_type == NULL) || (value_type->code == TYPE_CODE_ERROR))
+    error ("Unable to call function at 0x%lx: no return type 
information available.\n"
+           "To call this function anyway, you can cast the return type 
explicitly\n"
+          "(e.g. 'print (float) fabs (3.0)')",
+           (unsigned long) funaddr);
+
+  CHECK_TYPEDEF (value_type);
+
+  {
+    struct block *b = block_for_pc (funaddr);
+    /* If compiled without -g, assume GCC 2.  */
+    using_gcc = (b == NULL ? 2 : BLOCK_GCC_COMPILED (b));
+  }
+
+  /* Are we returning a value using a structure return or a normal
+     value return? */
+
+  struct_return = using_struct_return (function, funaddr, value_type,
+                                      using_gcc);
+
+
    /* PUSH_DUMMY_FRAME is responsible for saving the inferior registers
       (and POP_FRAME for restoring them).  (At least on most machines)
       they are saved on the stack in the inferior.  */
@@ -1354,21 +1388,6 @@
        sp += sizeof_dummy1;
      }

-  funaddr = find_function_addr (function, &value_type);
-  CHECK_TYPEDEF (value_type);
-
-  {
-    struct block *b = block_for_pc (funaddr);
-    /* If compiled without -g, assume GCC 2.  */
-    using_gcc = (b == NULL ? 2 : BLOCK_GCC_COMPILED (b));
-  }
-
-  /* Are we returning a value using a structure return or a normal
-     value return? */
-
-  struct_return = using_struct_return (function, funaddr, value_type,
-                                      using_gcc);
-
    /* Create a call sequence customized for this function
       and the number of arguments for it.  */
    for (i = 0; i < (int) (SIZEOF_CALL_DUMMY_WORDS / sizeof (dummy[0])); 
i++)
@@ -1786,16 +1805,24 @@
  }

  struct value *
-call_function_by_hand (struct value *function, int nargs, struct value 
**args)
+call_function_by_hand_expecting_type (struct value *function,
+                                     struct type *expect_type,
+                                     int nargs, struct value **args)
  {
    if (CALL_DUMMY_P)
      {
-      return hand_function_call (function, nargs, args);
+      return hand_function_call (function, expect_type, nargs, args);
      }
    else
      {
        error ("Cannot invoke functions on this machine.");
      }
+}
+
+struct value *
+call_function_by_hand (struct value *function, int nargs, struct value 
**args)
+{
+  return call_function_by_hand_expecting_type (function, NULL, nargs, 
args);
  }



Index: gdb/value.h
===================================================================
RCS file: /cvs/src/src/gdb/value.h,v
retrieving revision 1.26
diff -u -r1.26 value.h
--- value.h     2002/01/04 23:21:38     1.26
+++ value.h     2002/02/05 06:22:42
@@ -551,6 +551,11 @@

  extern struct value *value_slice (struct value *, int, int);

+extern struct value *
+call_function_by_hand_expecting_type (struct value *,
+                                     struct type *, int,
+                                     struct value **);
+
  extern struct value *call_function_by_hand (struct value *, int,
                                             struct value **);

@@ -564,7 +569,9 @@
  extern void find_rt_vbase_offset (struct type *, struct type *, char *, 
int,
                                   int *, int *);

-extern struct value *find_function_in_inferior (char *);
+extern CORE_ADDR find_function_addr (struct value *, struct type **);
+
+extern struct value *find_function_in_inferior (char *, struct type *);

  extern struct value *value_allocate_space_in_inferior (int);

Index: gdb/testsuite/gdb.asm/asm-source.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.asm/asm-source.exp,v
retrieving revision 1.17
diff -u -r1.17 asm-source.exp
--- asm-source.exp      2002/01/18 00:13:00     1.17
+++ asm-source.exp      2002/02/05 06:22:44
@@ -220,10 +220,12 @@
         "Make selected stack frame return now\?.*" "y"

  # See if we can look at a global variable
-gdb_test "print globalvar" ".* = 11" "look at global variable"
+gdb_test "print globalvar" ".* = (11|<unknown type>)" "look at global 
variable"
+gdb_test "print (int) globalvar" ".* = 11" "look at global variable"

  # See if we can look at a static variable
-gdb_test "print staticvar" ".* = 5" "look at static variable"
+gdb_test "print staticvar" ".* = (5|<unknown type>)" "look at static 
variable"
+gdb_test "print (int) staticvar" ".* = 5" "look at static variable"

  # See if we can look at a static function
  gdb_test "disassem foostatic" ".*<foostatic>:.*End of assembler dump." \
Index: gdb/testsuite/gdb.base/nodebug.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.base/nodebug.exp,v
retrieving revision 1.2
diff -u -r1.2 nodebug.exp
--- nodebug.exp 2001/03/06 08:21:50     1.2
+++ nodebug.exp 2002/02/05 06:22:47
@@ -77,23 +77,24 @@
      gdb_test "whatis top" \
         "(<(text variable|function), no debug info>|short 
\\(int\\)|short \\(\\))"
      if {!$gcc_compiled} then { setup_xfail "mips-sgi-irix6*" }
-    gdb_test "ptype top" "(short|int) \\((|void|int|<non-float 
parameter>|<non-float parameter>, <non-float parameter>)\\)"
+    gdb_test "ptype top" "(short|int|<unknown type>) 
\\((|void|int|<non-float parameter>|<non-float parameter>, <non-float 
parameter>)\\)"

      if {!$gcc_compiled} then { setup_xfail "mips-sgi-irix5*" }
      setup_xfail "mips-sgi-irix6*"
      gdb_test "p middle" \
-       "\{(<(text variable|function), no debug info>|short 
\\(int\\)|short \\(\\))\} \[0-9a-fx]* <middle(\\(int\\)|)>"
+       "\{(<(text variable|function), no debug info>|short|<unknown 
type> \\(int\\)|short \\(\\))\} \[0-9a-fx]* <middle(\\(int\\)|)>"
      if {!$gcc_compiled} then { setup_xfail "mips-sgi-irix5*" }
      setup_xfail "mips-sgi-irix6*"
      gdb_test "whatis middle" \
-       "(<(text variable|function), no debug info>|short 
\\(int\\)|short \\(\\))"
+       "(<(text variable|function), no debug info>|short|<unknown type> 
\\(int\\)|short \\(\\))"
      setup_xfail "mips-sgi-irix6*"
-    gdb_test "ptype middle" "(short|int) \\((|void|int|<non-float 
parameter>|<non-float parameter>, <non-float parameter>)\\)"
+    gdb_test "ptype middle" "(short|int|<unknown type>) 
\\((|void|int|<non-float parameter>|<non-float parameter>, <non-float 
parameter>)\\)"

-    gdb_test "p dataglobal" "= 3"
+    gdb_test "p dataglobal" "(= 3|<unknown type>)"
+    gdb_test "p (int) dataglobal" "= 3"
      gdb_test "whatis dataglobal" \
-       "<(data variable|variable), no debug info>|int"
-    gdb_test "ptype dataglobal" "<(data variable|variable), no debug 
info>|int"
+       "<(data variable|variable), no debug info>|int|<unknown type>"
+    gdb_test "ptype dataglobal" "<(data variable|variable), no debug 
info>|int|<unknown type>"

      # The only symbol xcoff puts out for statics is for the TOC entry.
      # Possible, but hairy, for gdb to deal.  Right now it doesn't, it
@@ -102,37 +103,40 @@
      setup_xfail "powerpc*-*-aix*"
      if {!$gcc_compiled} then { setup_xfail "hppa*-*-hpux*" }
      if {$gcc_compiled} then { setup_xfail "mips-sgi-irix6*" }
-    gdb_test "p datalocal" "= 4"
+    gdb_test "p datalocal" "(= 4|<unknown type>)"
+    gdb_test "p (int) datalocal" "4"
      setup_xfail "rs6000*-*-aix*"
      setup_xfail "powerpc*-*-aix*"
      if {!$gcc_compiled} then { setup_xfail "hppa*-*-hpux*" }
      if {$gcc_compiled} then { setup_xfail "mips-sgi-irix6*" }
-    gdb_test "whatis datalocal" "<(data variable|variable), no debug 
info>"
+    gdb_test "whatis datalocal" "<(data variable|variable), no debug 
info>|<unknown type>"
      setup_xfail "rs6000*-*-aix*"
      setup_xfail "powerpc*-*-aix*"
      if {!$gcc_compiled} then { setup_xfail "hppa*-*-hpux*" }
      if {$gcc_compiled} then { setup_xfail "mips-sgi-irix6*" }
-    gdb_test "ptype datalocal" "<(data variable|variable), no debug 
info>"
+    gdb_test "ptype datalocal" "<(data variable|variable), no debug 
info>|<unknown type>"

-    gdb_test "p bssglobal" "= 0"
-    gdb_test "whatis bssglobal" "<(data variable|variable), no debug 
info>|int"
-    gdb_test "ptype bssglobal" "<(data variable|variable), no debug 
info>|int"
+    gdb_test "p bssglobal" "(= 0|<unknown type>)"
+    gdb_test "p (int) bssglobal" "= 0"
+    gdb_test "whatis bssglobal" "<(data variable|variable), no debug 
info>|<unknown type>|int"
+    gdb_test "ptype bssglobal" "<(data variable|variable), no debug 
info>|<unknown type>|int"

      setup_xfail "rs6000*-*-aix*"
      setup_xfail "powerpc*-*-aix*"
      if {!$gcc_compiled} then { setup_xfail "hppa*-*-hpux*" }
      if {$gcc_compiled} then { setup_xfail "mips-sgi-irix6*" }
-    gdb_test "p bsslocal" "= 0"
+    gdb_test "p bsslocal" "(= 0|<unknown type>)"
+    gdb_test "p (int) bsslocal" "= 0"
      setup_xfail "rs6000*-*-aix*"
      setup_xfail "powerpc*-*-aix*"
      if {!$gcc_compiled} then { setup_xfail "hppa*-*-hpux*" }
      if {$gcc_compiled} then { setup_xfail "mips-sgi-irix6*" }
-    gdb_test "whatis bsslocal" "<(data variable|variable), no debug 
info>"
+    gdb_test "whatis bsslocal" "<(data variable|variable), no debug 
info>|<unknown type>"
      setup_xfail "rs6000*-*-aix*"
      setup_xfail "powerpc*-*-aix*"
      if {!$gcc_compiled} then { setup_xfail "hppa*-*-hpux*" }
      if {$gcc_compiled} then { setup_xfail "mips-sgi-irix6*" }
-    gdb_test "ptype bsslocal" "<(data variable|variable), no debug 
info>"
+    gdb_test "ptype bsslocal" "<(data variable|variable), no debug 
info>|<unknown type>"

      if {$gcc_compiled} then { setup_xfail "mips-sgi-irix6*" }
      gdb_test "backtrace 10" 
"#0.*inner.*#1.*middle.*#2.*top.*#3.*main.*" \
@@ -167,7 +171,9 @@
             # We need to up this because this can be really slow on some 
boards.
             # (malloc() is called as part of the test).
             set timeout 60;
-           gdb_test {p/c array_index("abcdef",2)} " = 99 'c'"
+            gdb_test {p/c (int) array_index("abcdef",2)} " = 99 'c'"
+            gdb_test {p/c array_index("abcdef",2)} \
+               "Unable to call function .* no return type information 
available.*"
         }
      }

Re: [RFA] Function return type checking

[Eli Zaretskii]

On Tue, 5 Feb 2002, Klee Dienes wrote:

> The following patch adds "expected return type" support to the target
> function call interface.  The "expected return type" is specified
> using cast syntax; it is ignored if actual type information for the
> function is available.  If no type information is provided for a
> function, the user is now required to provide an expected return type
> using the cast syntax, or get an error.

I don't know whether we want this behavior, but if this patch is 
accepted, these subtleties surely need to be documented in the user's 
manual.

Re: [RFA] Function return type checking

[Daniel Jacobowitz]

On Tue, Feb 05, 2002 at 01:36:41AM -0800, Klee Dienes wrote:
> (This is basically the same patch I sent last week, just updated
> to the latest source base.)
> 
> The following patch adds "expected return type" support to the target
> function call interface.  The "expected return type" is specified
> using cast syntax; it is ignored if actual type information for the
> function is available.  If no type information is provided for a
> function, the user is now required to provide an expected return type
> using the cast syntax, or get an error.

Having thought about this since you last posted it, I don't really like
it.  What you do by overloading the cast syntax for this operation is
very ``cute'', but exceedingly unintuitive.

> Some examples:
> 
> (gdb) print fabs (3.0)                  (no type information present)
> Unable to call function at 0x%lx: no return type information available.
> To call this function anyway, you can cast the return type explicitly
> (e.g. 'print (float) fabs (3.0)').

Except for the syntax of the example, I like this message.  This is a
good message.  We need more warnings about doing things like this.

Fabs is a bad example, though - can I suggest you look over the list of
what GCC defines as builtin functions, and not choose one?

> The reason I suggest 2) might be controversial is that the old
> behavior can be convenient in the general case.  A lot of functions
> *do* return int, and when debugging programs without symbols, it can
> be annoying to have to declare the return type, just to make the rare
> case when functions return floats/structs behave correctly.  But I'd
> argue that it's not *that* big a deal to cast a return value, and
> correctness must always take priority.

Losing the implicit int doesn't bother me especially.

Have you considered casting the function itself?  Something like:
(gdb) print ((float (*)(float)) fabs) (3.0)
$1 = 3.0
(gdb) set fabs

Which, I will note, already works except for the fact that we neglect
the argument types on function pointers.  Or
(gdb) set $fabs = (float (*)(float)) fabs
(gdb) p $fabs(4.0)
$2 = 4.0

Which works with the same caveat.

Specifying just the return type of the function is not useful in the
general case.  If we do not understand the argument types, it will
still crash.  I was trying to find an example where float vs. double
would cause a problem too, but I can't think of one at the moment. 
There's one in the testsuite though, for some targets.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Function return type checking

[Klee Dienes]

On Tuesday, February 5, 2002, at 08:07 AM, Daniel Jacobowitz wrote:

> Have you considered casting the function itself?  Something like:
> (gdb) print ((float (*)(float)) fabs) (3.0)
> $1 = 3.0
> (gdb) set fabs
>
> Which, I will note, already works except for the fact that we neglect
> the argument types on function pointers.  Or
> (gdb) set $fabs = (float (*)(float)) fabs
> (gdb) p $fabs(4.0)
> $2 = 4.0

We have; for a long time that was the answer we gave to people who were 
running into this problem.  Our experience was that it was a nightmare 
to explain to people how this mechanism worked, and that even for folks 
who did undertand it, they found it a major mental burden to use in 
practice.  C function casting syntax is neither intuitive nor pleasant 
to type.

The reason we chose the "cast" syntax wasn't so much to be cute, but 
because it was the first thing everyone tried when they were trying to 
get this to work.  People would try 'print (float) fabs (3.0)', followed 
by 'print {float} fabs (3.0)', usually followed by several unsuccessful 
attemtps to remember the correct syntax to cast the function pointer.

I also think there's a pretty solid rationale behind the syntax, and one 
that generalizes to argument-passing.  The theory goes:

All symbols without debugging information are assumed to be of type 
'unknown' (previously, they were assumed to be 'int', or (int (*) ())).  
When you cast an expression of type 'unknown' to anything else, GDB does 
no conversion, but simply interprets the data (or generates the data) 
according to the specified type.  So if you have 'f' with no symbols,

   print (long long) f ((long long) 7, (float) 3.0)

will generate a function call as if 'f' had been declared as

   long long f (long long, float)

For function arguments, I claim this is both intuitive and matches the 
behavior of the C compiler.  For function return values, wee have to 
choose something for what 'print (type) f ()' is to mean, and I claim 
that it's the best of the two alternatives (the other being "assume 
'int', then cast the 'int' to the specified type).

Re: [RFA] Function return type checking

[Daniel Jacobowitz]

On Wed, Feb 06, 2002 at 01:43:45PM -0800, Klee Dienes wrote:
> On Tuesday, February 5, 2002, at 08:07 AM, Daniel Jacobowitz wrote:
> 
> >Have you considered casting the function itself?  Something like:
> >(gdb) print ((float (*)(float)) fabs) (3.0)
> >$1 = 3.0
> >(gdb) set fabs
> >
> >Which, I will note, already works except for the fact that we neglect
> >the argument types on function pointers.  Or
> >(gdb) set $fabs = (float (*)(float)) fabs
> >(gdb) p $fabs(4.0)
> >$2 = 4.0
> 
> We have; for a long time that was the answer we gave to people who were 
> running into this problem.  Our experience was that it was a nightmare 
> to explain to people how this mechanism worked, and that even for folks 
> who did undertand it, they found it a major mental burden to use in 
> practice.  C function casting syntax is neither intuitive nor pleasant 
> to type.
> 
> The reason we chose the "cast" syntax wasn't so much to be cute, but 
> because it was the first thing everyone tried when they were trying to 
> get this to work.  People would try 'print (float) fabs (3.0)', followed 
> by 'print {float} fabs (3.0)', usually followed by several unsuccessful 
> attemtps to remember the correct syntax to cast the function pointer.
> 
> I also think there's a pretty solid rationale behind the syntax, and one 
> that generalizes to argument-passing.  The theory goes:

OK, I buy that.  I'm still a little unhappy about the two possible
meanings of (int) fabs(3.0) - but it seems like a good compromise to
allow this syntax too.  Thanks for the clarification.

-- 
Daniel Jacobowitz                           Carnegie Mellon University
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] Function return type checking

[Kevin Buettner]

On Feb 5,  1:36am, Klee Dienes wrote:

> The following patch adds "expected return type" support to the target
> function call interface.  The "expected return type" is specified
> using cast syntax; it is ignored if actual type information for the
> function is available.  If no type information is provided for a
> function, the user is now required to provide an expected return type
> using the cast syntax, or get an error.

I've read through your explanation and the ensuing discussion to date. 
Though I haven't reviewed the actual patch, I think this idea makes
sense and recommend that the maintainers give it serious
consideration.

Kevin

Re: [RFA] Function return type checking

[Andrew Cagney]

> On Tuesday, February 5, 2002, at 08:07 AM, Daniel Jacobowitz wrote:
> 
> Have you considered casting the function itself?  Something like:
> (gdb) print ((float (*)(float)) fabs) (3.0)
> $1 = 3.0
> (gdb) set fabs

Only:

	print ((float(*)()) fabs) (3.0)

should be necessary.  GDB will then attempt to pass the arguments 
according to GDB's interpretation of traditional K&R parameter passing 
rules.

> We have; for a long time that was the answer we gave to people who were running into this problem.  Our experience was that it was a nightmare to explain to people how this mechanism worked, and that even for folks who did undertand it, they found it a major mental burden to use in practice.  C function casting syntax is neither intuitive nor pleasant to type.
> 
> The reason we chose the "cast" syntax wasn't so much to be cute, but because it was the first thing everyone tried when they were trying to get this to work.  People would try 'print (float) fabs (3.0)', followed by 'print {float} fabs (3.0)', usually followed by several unsuccessful attemtps to remember the correct syntax to cast the function pointer.
> 
> I also think there's a pretty solid rationale behind the syntax, and one that generalizes to argument-passing.  The theory goes:
> 
> All symbols without debugging information are assumed to be of type 'unknown' (previously, they were assumed to be 'int', or (int (*) ())).  When you cast an expression of type 'unknown' to anything else, GDB does no conversion, but simply interprets the data (or generates the data) according to the specified type.  So if you have 'f' with no symbols,
> 
>   print (long long) f ((long long) 7, (float) 3.0)
> 
> will generate a function call as if 'f' had been declared as
> 
>   long long f (long long, float)
> 
> For function arguments, I claim this is both intuitive and matches the behavior of the C compiler.  For function return values, wee have to choose something for what 'print (type) f ()' is to mean, and I claim that it's the best of the two alternatives (the other being "assume 'int', then cast the 'int' to the specified type).

I've personally got reservations over introducing a change that, given a 
file like:

void
b(void)
{
   double d;
   d = (double) bar ((float) 3);
}

radically alters gdb's behavour given:

	print (double) bar((float) 3)

and rejects (?)

	print bar((float) 3)

However, I do see your point that even calling bar() (when bar() has no 
debug info) is dangerous.

Is this feature intended for C or ObjectiveC developers?

Andrew

Re: [RFA] Function return type checking

[Klee Dienes]

[-- Attachment #1: Type: text/plain, Size: 3091 bytes --]

On Saturday, March 9, 2002, at 08:30 PM, Andrew Cagney wrote:

> I've personally got reservations over introducing a change that, given 
> a file like:
>
> void
> b(void)
> {
>   double d;
>   d = (double) bar ((float) 3);
> }
>
> radically alters gdb's behavour given:
>
> 	print (double) bar((float) 3)
>
> and rejects (?)
>
> 	print bar((float) 3)

I think that's a very valid reservation.  But let me expand the example 
a bit:

$ cat a.c
double foo (float f)
{
   return (f + 1.0);
}

double bar (float f)
{
   return (f + 1.0);
}

$ cat b.c
double foo (float);

int main ()
{
   double d1, d2;

   d1 = (double) foo ((float) 3);
   d2 = (double) bar ((float) 3);
}

$ cc -c a.c
$ cc -g -c b.c
$ cc a.o b.o -o a

In "standard" GDB, you get the following results:

(gdb) print d1
$1 = 4
(gdb) print d2
$2 = 1 							/* generic bogus value */
(gdb) print (double) foo ((float) 3)
$3 = 1 							/* generic bogus value */
(gdb) print (double) bar ((float) 3)
$4 = 1 							/* generic bogus value */

In GDB as patched, you get:

(gdb) print d1
$1 = 4
(gdb) print d2
$2 = 1074266112 					/* generic bogus value */
(gdb) print (double) foo ((float) 3)
$3 = 4
(gdb) print (double) bar ((float) 3)
$4 = 4

I'd argue that the real problem here is that the calling conventions for 
a function call are determined for GCC and for GDB in two very different 
ways.  In the case of GCC, it's based on what signature GCC has seen for 
the function.  In the case of GDB, it's based on what debugging 
information is available for the function being called.  This means that 
it's always going to be possible for there to be a mismatch between 
behavior of GCC and GDB when evaluating expressions, and my patch does 
not try to address that.  My patch only changes the default behavior 
from two "incorrect" results (only one of which matches the behavior of 
GCC), to two "correct" results (only one of which matches the behavior 
of GCC).  I'd argue that it's better to err on the side of returning the 
correct result, if you have to err at all, and that in the case where 
you do want the exact behavior of GCC,

print (float) (int) bar ((float) 3)

is much more intuitive and easy to type than

print ((float (*) ()) bar) ((float) 3)

> and rejects (?)
>
> 	print bar((float) 3)

Correct; this would print:

Unable to call function at 0x1df8: no return type information available.
To call this function anyway, you can cast the return type explicitly 
(e.g. 'print (float) fabs (3.0)')

> Is this feature intended for C or ObjectiveC developers?

I'd intend for this to be used by everyone. We specifically added it in 
response to bug reports from people making heavy use of the system math 
libraries; as well as from Cocoa (Objective-C) developers making heavy 
use of functions returning NSRect objects.  The reason it's of interest 
to me in preparing the Objective-C patches is that much of the 
Objective-C GDB code makes use of  being able to pass 'expected_type' 
arguments to the modified functions, and I'd rather not have to 
re-architect all those calls before submitting the patches.

[-- Attachment #2: Type: text/enriched, Size: 3211 bytes --]

On Saturday, March 9, 2002, at 08:30 PM, Andrew Cagney wrote:


<excerpt>I've personally got reservations over introducing a change
that, given a file like:


void

b(void)

{

  double d;

  d = (double) bar ((float) 3);

}


radically alters gdb's behavour given:


	print (double) bar((float) 3)


and rejects (?)


	print bar((float) 3)

</excerpt>

I think that's a very valid reservation.  But let me expand the
example a bit:


$ cat a.c

double foo (float f)

{

  return (f + 1.0);

}


double bar (float f)

{

  return (f + 1.0);

}


$ cat b.c

double foo (float);


int main ()

{

  double d1, d2;


  d1 = (double) foo ((float) 3);

  d2 = (double) bar ((float) 3);

}


$ cc -c a.c

$ cc -g -c b.c

$ cc a.o b.o -o a


In "standard" GDB, you get the following results:


(gdb) print d1

$1 = 4

(gdb) print d2

$2 = 1 							/* generic bogus value */

(gdb) print (double) foo ((float) 3)

$3 = 1 							/* generic bogus value */

(gdb) print (double) bar ((float) 3)

$4 = 1 							/* generic bogus value */


In GDB as patched, you get:


(gdb) print d1

$1 = 4

(gdb) print d2

$2 = 1074266112 					/* generic bogus value */

(gdb) print (double) foo ((float) 3)

$3 = 4

(gdb) print (double) bar ((float) 3)

$4 = 4


I'd argue that the real problem here is that the calling conventions
for a function call are determined for GCC and for GDB in two very
different ways.  In the case of GCC, it's based on what signature GCC
has seen for the function.  In the case of GDB, it's based on what
debugging information is available for the function being called. 
This means that it's always going to be possible for there to be a
mismatch between behavior of GCC and GDB when evaluating expressions,
and my patch does not try to address that.  My patch only changes the
default behavior from two "incorrect" results (only one of which
matches the behavior of GCC), to two "correct" results (only one of
which matches the behavior of GCC).  I'd argue that it's better to err
on the side of returning the correct result, if you have to err at
all, and that in the case where you <italic>do</italic> want the exact
behavior of GCC,


print (float) (int) bar ((float) 3)


is much more intuitive and easy to type than


print ((float (*) ()) bar) ((float) 3)


<excerpt><color><param>0000,0000,DEDE</param>and rejects (?)


	print bar((float) 3)</color>

</excerpt>

Correct; this would print:


Unable to call function at 0x1df8: no return type information
available.

To call this function anyway, you can cast the return type explicitly
(e.g. 'print (float) fabs (3.0)')


<excerpt>Is this feature intended for C or ObjectiveC developers?

</excerpt>

I'd intend for this to be used by everyone. We specifically added it
in response to bug reports from people making heavy use of the system
math libraries; as well as from Cocoa (Objective-C) developers making
heavy use of functions returning NSRect objects.  The reason it's of
interest to me in preparing the Objective-C patches is that much of
the Objective-C GDB code makes use of  being able to pass
'expected_type' arguments to the modified functions, and I'd rather
not have to re-architect all those calls before submitting the patches.

Re: [RFA] Function return type checking

[Andrew Cagney]

>> Is this feature intended for C or ObjectiveC developers?

> 
> I'd intend for this to be used by everyone. We specifically added it in response to bug reports from people making heavy use of the system math libraries; as well as from Cocoa (Objective-C) developers making heavy use of functions returning NSRect objects. The reason it's of interest to me in preparing the Objective-C patches is that much of the Objective-C GDB code makes use of being able to pass 'expected_type' arguments to the modified functions, and I'd rather not have to re-architect all those calls before submitting the patches.

So there are two reasons for the change?  The infrastructure you need 
for objective C and a user visible interface change.  Can you expand a 
little on the objective C problems.  If objective C has good reason for 
this infrastructure than I can't see why that part shouldn't go in.

enjoy,
Andrew

Re: [RFA] Function return type checking

[Klee Dienes]

Saying that I "need" the infrastructure change for Objective-C would 
probably be a bit strong.

The basic issue arises when evaluating Objective-C expressions like the 
following:

call [[window boundingBox] print]

Here 'window' is of type NSWindow, an opaque type defined in a system 
library, and 'boundingBox' is a method of NSWindow that returns a 
NSRect.  The issue arises because '[window boundingBox]' is really a 
function call to 'objc_msgSend_stret (window, selector)'.  There 
generally isn't symbol information available for 'objc_msgSend_stret', 
but there often is for '[NSWindow boundingBox]'.  So it's nice for 
Objective-C to be able to pass the correct return type information to 
'call_function_by_hand' so that the result returns with the correct 
type, rather than have to cons up a fake function value with fake return 
type to pass to 'call_function_by_hand'.

On Tuesday, March 12, 2002, at 07:55 AM, Andrew Cagney wrote:

> So there are two reasons for the change?  The infrastructure you need 
> for objective C and a user visible interface change.  Can you expand a 
> little on the objective C problems.  If objective C has good reason for 
> this infrastructure than I can't see why that part shouldn't go in.

Re: [RFA] Function return type checking

[Andrew Cagney]

> Saying that I "need" the infrastructure change for Objective-C would probably be a bit strong.

> The basic issue arises when evaluating Objective-C expressions like the following:
> 
> call [[window boundingBox] print]
> 
> Here 'window' is of type NSWindow, an opaque type defined in a system library, and 'boundingBox' is a method of NSWindow that returns a NSRect.  The issue arises because '[window boundingBox]' is really a function call to 'objc_msgSend_stret (window, selector)'.  There generally isn't symbol information available for 'objc_msgSend_stret', but there often is for '[NSWindow boundingBox]'.  So it's nice for Objective-C to be able to pass the correct return type information to 'call_function_by_hand' so that the result returns with the correct type, rather than have to cons up a fake function value with fake return type to pass to 'call_function_by_hand'.

One of the things I've been puzzled by is how a user could get 
themselves in a situtation where the feature was needed.  I was thinking 
that a well written program, having including all relevant headers, 
would have all the relevant information available.

The above is starting to explain why there is a problem.  Can you, humor 
me a little and express it in slightly more concret terms (bits of 
code).  Does C++ have a similar problem?

Keep in mind that Objective C doesn't have the same entrenched 
conventions that normal C suffers from - you've more freedom to define 
its behavour.

> On Tuesday, March 12, 2002, at 07:55 AM, Andrew Cagney wrote:
> 
> So there are two reasons for the change?  The infrastructure you need for objective C and a user visible interface change.  Can you expand a little on the objective C problems.  If objective C has good reason for this infrastructure than I can't see why that part shouldn't go in.

Andrew