This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


On Fri, 13 Mar 2009 23:46:53 +0100, Joel Brobecker wrote:
> > +	# Test return from a function with no debug info (symbol; still it may
> > +	# have a minimal-symbol) if it does not crash.
> 
> The part in between parens sound strange. I can suggest:
> 
>     # Verify that we do not crash when using "return" from a function
>     # with no debugging info (it may still have an associated minimal
>     # symbol).
> 
> I'm not sure that the side note about having minimal symbols is really
> necessary, though, but it's definitely not harmful in this case...

The statement `no debugging info' itself is too ambiguous.  I had a mess in it
myself some years ago and I still find people do not properly distinguish
.symtab/.dynsym/.debug_info, which of these are ELF vs. DWARF,
gcc (none)/-s/-g/strip and how they are related to GDB
minimal_symbol/partial_symbol/symbol.

As you removed the `symbol' part, used now this text (also in the patch):

# Verify that we do not crash when using "return" from a function with
# no debugging info.  Such function has no `struct symbol'.  It may
# still have an associated `struct minimal_symbol'.


On Sat, 14 Mar 2009 11:21:30 +0100, Eli Zaretskii wrote:
> Thanks, this part is approved, provided that you add the missing
> punctuation and clarify the text, as indicated below.

Tried to address all your objections.


> would your first example still work the same if the return value of the
> function were `double'?

Yes, there is no testcase for float types but it also gets the
implicit/explicit casting rules (and different registers assignments).


>   Normally, both the selected stack frame and the one above it have
>   debug info.

Talking about the debug info of the caller IMO makes the situation needlessly
more difficult.  Only the callee debug info availability is important for
`return'.


>                @value{GDBN} uses the debug info when the value of
>   @var{expression} does not have an explicit data type.

The current approved patch still fully follows the type found in the debug
info if the debug info is present.  The change was that if the debug info is
not present the original code assumed int, the new code requires in the
missing debug info case an explicit cast and uses this type it was casted as
by the GDB user.


Thanks,
Jan


gdb/
2009-03-14  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* stack.c (return_command <retval_exp>): New variables retval_expr
	and old_chain.  Inline parse_and_eval to initialize retval_expr.  Check
	RETVAL_EXPR for UNOP_CAST and set RETURN_TYPE to the RETURN_VALUE type
	if RETURN_TYPE is NULL.

gdb/doc/
2009-03-14  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.texinfo (Returning): New description for missing debug info.

gdb/testsuite/
2009-03-14  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* gdb.base/return-nodebug.exp, gdb.base/return-nodebug.c: New.


--- gdb/stack.c	6 Mar 2009 18:51:05 -0000	1.186
+++ gdb/stack.c	14 Mar 2009 21:34:47 -0000
@@ -1796,18 +1796,27 @@ return_command (char *retval_exp, int fr
      message.  */
   if (retval_exp)
     {
+      struct expression *retval_expr = parse_expression (retval_exp);
+      struct cleanup *old_chain = make_cleanup (xfree, retval_expr);
       struct type *return_type = NULL;
 
       /* Compute the return value.  Should the computation fail, this
          call throws an error.  */
-      return_value = parse_and_eval (retval_exp);
+      return_value = evaluate_expression (retval_expr);
 
       /* 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;
+      	{
+	  if (retval_expr->elts[0].opcode != UNOP_CAST)
+	    error (_("Return value type not available for selected "
+		     "stack frame.\n"
+		     "Please use an explicit cast of the value to return."));
+	  return_type = value_type (return_value);
+	}
+      do_cleanups (old_chain);
       CHECK_TYPEDEF (return_type);
       return_value = value_cast (return_type, return_value);
 
--- gdb/doc/gdb.texinfo	14 Mar 2009 01:38:08 -0000	1.562
+++ gdb/doc/gdb.texinfo	14 Mar 2009 21:34:58 -0000
@@ -12494,6 +12494,53 @@ returned.  In contrast, the @code{finish
 and Stepping, ,Continuing and Stepping}) resumes execution until the
 selected stack frame returns naturally.
 
+@value{GDBN} needs to know how the @var{expression} argument should be set for
+the inferior.  The concrete registers assignment depends on the OS ABI and the
+type being returned by the selected stack frame.  For example it is common for
+OS ABI to return floating point values in FPU registers while integer values in
+CPU registers.  Still some ABIs return even floating point values in CPU
+registers.  Larger integer widths (such as @code{long long int}) also have
+specific placement rules.  @value{GDBN} already knows the OS ABI from its
+current target so it needs to find out also the type being returned to make the
+assignment into the right register(s).
+
+Normally, the selected stack frame has debug info.  @value{GDBN} will always
+use the debug info instead of the implicit type of @var{expression} when the
+debug info is available.  For example, if you type @kbd{return -1}, and the
+function in the current stack frame is declared to return a @code{long long
+int}, @value{GDBN} transparently converts the implicit @code{int} value of -1
+into a @code{long long int}:
+
+@smallexample
+Breakpoint 1, func () at gdb.base/return-nodebug.c:29
+29        return 31;
+(@value{GDBP}) return -1
+Make func return now? (y or n) y
+#0  0x004004f6 in main () at gdb.base/return-nodebug.c:43
+43        printf ("result=%lld\n", func ());
+(@value{GDBP})
+@end smallexample
+
+However, if the selected stack frame does not have a debug info, e.g., if the
+function was compiled without debug info, @value{GDBN} has to find out the type
+to return from user.  Specifying a different type by mistake may set the value
+in different inferior registers than the caller code expects.  For example,
+typing @kbd{return -1} with its implicit type @code{int} would set only a part
+of a @code{long long int} result for a debug info less function (on 32-bit
+architectures).  Therefore the user is required to specify the return type by
+an appropriate cast explicitly:
+
+@smallexample
+Breakpoint 2, 0x0040050b in func ()
+(@value{GDBP}) return -1
+Return value type not available for selected stack frame.
+Please use an explicit cast of the value to return.
+(@value{GDBP}) return (long long int) -1
+Make selected stack frame return now? (y or n) y
+#0  0x00400526 in main ()
+(@value{GDBP})
+@end smallexample
+
 @node Calling
 @section Calling Program Functions
 
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.base/return-nodebug.c	14 Mar 2009 21:34:58 -0000
@@ -0,0 +1,49 @@
+/* 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
+init (void)
+{
+  return 0;
+}
+
+static TYPE
+func (void)
+{
+  return 31;
+}
+
+static void
+marker (void)
+{
+}
+
+int
+main (void)
+{
+  /* Preinitialize registers to 0 to avoid false PASS by leftover garbage.  */
+  init ();
+
+  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	14 Mar 2009 21:34:58 -0000
@@ -0,0 +1,61 @@
+# 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 {type} {
+    set typenospace [string map {{ } -} $type]
+
+    global pf_prefix
+    set old_prefix $pf_prefix
+    lappend pf_prefix "$typenospace:"
+
+    if {[runto "func"]} {
+	# Verify that we do not crash when using "return" from a function with
+	# no debugging info.  Such function has no `struct symbol'.  It may
+	# still have an associated `struct minimal_symbol'.
+
+    	gdb_test "return -1" \
+		 "Return value type not available for selected stack frame\\.\r\nPlease use an explicit cast of the value to return\\." \
+		 "return from function with no debug info without a cast"
+
+	# Cast of the result to the proper width must be done explicitely.
+	gdb_test "return ($type) -1" "#0 .* main \\(.*"			\
+		 "return from function with no debug info with a cast"	\
+		 "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 {{{signed 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 $type
+    }
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]