Re: [patch] Fix `return' of long/long-long results with no debuginfo

[Jan Kratochvil <jan.kratochvil@redhat.com>]

On Wed, 11 Feb 2009 22:22:41 +0100, Mark Kettenis wrote:
> And I have a (somewhat) vague recollection that there are debug formats or
> compilers where we cant distinguish between functions without a return type
> and functions with "unknown" return type.

As `struct type' for a function type GDB debuginfo readers always create by
make_function_type and this function crashes on NULL `type' we can assume
TYPE_TARGET_TYPE is never NULL.

For DWARF it is clear both according to the standard
3.3.2 Subroutine and Entry Point Return Types
If the subroutine or entry point is a function that returns a value, then its
debugging information entry has a DW_AT_type attribute [...]

and according to the code:
dwarf2read.c:die_type():
      /* A missing DW_AT_type represents a void type.  */


Made the condition in the patch more explicit now.  Removed the original
TYPE_CODE_VOID code emulating the K&R behavior to more follow the modern
ANSI/ISO C compilers refuse `return value;' in a void-returning function
without having typeless/`int'-defaulted functions permitted.


> From: Daniel Jacobowitz <drow@false.org>
> > Everything we pick will be wrong some of the time, but IMO "long" is
> > maximally useful.  "long long" on 32-bit platforms is going to pick
> > up garbage from the next register for int or void * returns.

Garbage would be seen by caller of a `long long' returning callee where GDB
would return something smaller (like `int').  As GDB writes (not "reads") the
register it just will write zero to one excessive but unused register.  This
register must be callee-saved (not caller-saved) by the ABI supporting `long
long' return type and GDB already asserts the return type is
RETURN_VALUE_REGISTER_CONVENTION.

If GDB would return only the `long' size there would be no way how to do the
`return' command on a debuginfo-less function natively returning `long long'.


> The right solution of course is to distribute libraries with debug
> sumbols.  But in cases where that is impossible, would it be an idea
> to use the type of the return value expression given by the user
> instead of int as a fallback?

Any return value gets retyped to `long long' which is intepreted by the caller
even as lower-sized types (not guaranteed by ABI but at lest checked now by
new gdb.base/return-nodebug.exp).


> In the case at hand, "return -1" will still fail, but it would be
> possible to do return (void *)-1 to do what the user wanted.

`return -1' is checked by gdb.base/return-nodebug.exp and it works.


Thanks,
Jan


gdb/
2009-02-11  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* stack.c (return_command): Default to the builtin_long_long or at
	least builtin_long types now.  Assert TYPE_TARGET_TYPE of a typed
	function is non-NULL.  Error on return value for void returning
	functions.

gdb/testsuite/
2009-02-11  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/return-nodebug.exp, gdb.base/return-nodebug.c: New.
	* gdb.base/return2.exp (try to return value in a void function): New.

--- gdb/stack.c	11 Feb 2009 16:07:28 -0000	1.185
+++ gdb/stack.c	11 Feb 2009 21:39:57 -0000
@@ -1805,9 +1805,23 @@ return_command (char *retval_exp, int fr
       /* Cast return value to the return type of the function.  Should
          the cast fail, this call throws an error.  */
       if (thisfun != NULL)
-	return_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (thisfun));
-      if (return_type == NULL)
-	return_type = builtin_type (get_frame_arch (thisframe))->builtin_int;
+      	{
+	  return_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (thisfun));
+	  gdb_assert (return_type != NULL);
+	  if (TYPE_CODE (return_type) == TYPE_CODE_VOID)
+	    error (_("Function %s does not return any value"),
+		   SYMBOL_PRINT_NAME (thisfun));
+	}
+      else
+        {
+	  return_type
+	    = builtin_type (get_frame_arch (thisframe))->builtin_long_long;
+
+	  /* Hopefull each ABI returns at least `long' type in register.  */
+	  if (using_struct_return (NULL, return_type))
+	    return_type
+	      = builtin_type (get_frame_arch (thisframe))->builtin_long;
+	}
       CHECK_TYPEDEF (return_type);
       return_value = value_cast (return_type, return_value);
 
@@ -1816,14 +1830,7 @@ return_command (char *retval_exp, int fr
       if (value_lazy (return_value))
 	value_fetch_lazy (return_value);
 
-      if (TYPE_CODE (return_type) == TYPE_CODE_VOID)
-	/* If the return-type is "void", don't try to find the
-           return-value's location.  However, do still evaluate the
-           return expression so that, even when the expression result
-           is discarded, side effects such as "return i++" still
-           occur.  */
-	return_value = NULL;
-      else if (thisfun != NULL
+      if (thisfun != NULL
 	       && using_struct_return (SYMBOL_TYPE (thisfun), return_type))
 	{
 	  query_prefix = "\
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.base/return-nodebug.c	11 Feb 2009 21:40:00 -0000
@@ -0,0 +1,40 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2009 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#include <stdio.h>
+
+static TYPE
+func (void)
+{
+  return (TYPE) -1;
+}
+
+static void
+marker (void)
+{
+}
+
+int
+main (void)
+{
+  printf ("result=" FORMAT "\n", CAST func ());
+
+  /* Cannot `next' with no debug info.  */
+  marker ();
+
+  return 0;
+}
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.base/return-nodebug.exp	11 Feb 2009 21:40:00 -0000
@@ -0,0 +1,52 @@
+# Copyright (C) 2009 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+proc do_test {typenospace} {
+    global pf_prefix
+    set old_prefix $pf_prefix
+    lappend pf_prefix "$typenospace:"
+
+    if {[runto "func"]} {
+	# Test return from a function with no debug info (symbol; still it may
+	# have a minimal-symbol) if it does not crash.
+	gdb_test "return -1" "#0 .* main \\(.*"			\
+		 "return from function with no debug info"	\
+		 "Make selected stack frame return now\\? \\(y or n\\) " "y"
+
+	# And if it returned the full width of the result.
+	gdb_test "adv marker" "\r\nresult=-1\r\n.* in marker \\(.*" \
+		 "full width of the returned result"
+    }
+
+    set pf_prefix $old_prefix
+}
+
+foreach case {{{char}      %d (int)}	\
+	      {{short}     %d (int)}	\
+	      {{int}       %d}		\
+	      {{long}      %ld}		\
+	      {{long long} %lld}} {
+    set type [lindex $case 0]
+    set format [lindex $case 1]
+    set cast [lindex $case 2]
+
+    set typeesc [string map {{ } {\ }} $type]
+    set typenospace [string map {{ } -} $type]
+
+    if {[prepare_for_testing return-nodebug.exp "return-nodebug-$typenospace" "return-nodebug.c" \
+	 [list "additional_flags=-DFORMAT=\"$format\" -DTYPE=$typeesc -DCAST=$cast"]] == 0} {
+	do_test $typenospace
+    }
+}
--- gdb/testsuite/gdb.base/return2.exp	3 Jan 2009 05:58:03 -0000	1.11
+++ gdb/testsuite/gdb.base/return2.exp	11 Feb 2009 21:40:00 -0000
@@ -70,6 +70,8 @@ proc return_void { } {
 	    "set break on void_func"
     gdb_test "continue" "Breakpoint.* void_func.*" \
 	    "continue to void_func"
+    gdb_test "return 23" "Function void_func does not return any value" \
+	     "try to return value in a void function"
     send_gdb "return \n"
     gdb_expect {
 	-re "Make void_func return now.*y or n. $" {