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

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

[Jan Kratochvil]

Hi,

the command `return' assumes `int' result when the current function has no
debug info.  This assumption breaks for example `void *' return on x86_64.

Original bugreport: https://bugzilla.redhat.com/show_bug.cgi?id=365111

Regression-tested on x86_64-unknown-linux-gnu.

This testcase also covers the previous fix of the `return' crash:
  http://sourceware.org/ml/gdb-patches/2009-01/msg00213.html


Thanks,
Jan


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

	* stack.c (return_command): Default to the builtin_long_long type now.

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

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

--- gdb/stack.c	11 Feb 2009 16:07:28 -0000	1.185
+++ gdb/stack.c	11 Feb 2009 19:42:10 -0000
@@ -1807,7 +1807,8 @@ return_command (char *retval_exp, int fr
       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
+	  = builtin_type (get_frame_arch (thisframe))->builtin_long_long;
       CHECK_TYPEDEF (return_type);
       return_value = value_cast (return_type, return_value);
 
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.base/return-nodebug.c	11 Feb 2009 19:42:12 -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 19:42:12 -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
+    }
+}

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

[Mark Kettenis]

> Date: Wed, 11 Feb 2009 20:45:11 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> Hi,
> 
> the command `return' assumes `int' result when the current function has no
> debug info.  This assumption breaks for example `void *' return on x86_64.
> 
> Original bugreport: https://bugzilla.redhat.com/show_bug.cgi?id=365111
> 
> Regression-tested on x86_64-unknown-linux-gnu.
> 
> This testcase also covers the previous fix of the `return' crash:
>   http://sourceware.org/ml/gdb-patches/2009-01/msg00213.html
> 
> 
> Thanks,
> Jan

Sorry, this is wrong.  Functions without return type implicitly return int.

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

[Daniel Jacobowitz]

On Wed, Feb 11, 2009 at 09:40:00PM +0100, Mark Kettenis wrote:
> Sorry, this is wrong.  Functions without return type implicitly return int.

I think it's a mistake to bring this rule (from K&R C, and removed in
C99) to GDB.  This is a different context; it's not functions declared
without a return type in source code, but functions with an unknown
return type (and unknown source language).

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.  Most
platforms with int != long will have the return register with
sizeof(long).

-- 
Daniel Jacobowitz
CodeSourcery

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

[Mark Kettenis]

> Date: Wed, 11 Feb 2009 15:50:45 -0500
> From: Daniel Jacobowitz <drow@false.org>
> 
> On Wed, Feb 11, 2009 at 09:40:00PM +0100, Mark Kettenis wrote:
> > Sorry, this is wrong.  Functions without return type implicitly return int.
> 
> I think it's a mistake to bring this rule (from K&R C, and removed in
> C99) to GDB.  This is a different context; it's not functions declared
> without a return type in source code, but functions with an unknown
> return type (and unknown source language).

I disagree.  I think it is the behaviour that makes the most sense in
a historical context.  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.

> 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.  Most
> platforms with int != long will have the return register with
> sizeof(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?

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.

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

[Jan Kratochvil]

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. $" {

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

[Mark Kettenis]

> Date: Wed, 11 Feb 2009 22:46:47 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>

Sorry Jan, are you really just ignoring my remarks and actually making
this diff more unacceptable to me?

> 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. $" {
> 

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

[Jan Kratochvil]

On Wed, 11 Feb 2009 22:22:41 +0100, Mark Kettenis wrote:
# I disagree.  I think it is the behaviour that makes the most sense in
# a historical context.  And I have a (somewhat) vague recollection that

On Wed, 11 Feb 2009 22:57:51 +0100, Mark Kettenis wrote:
> Sorry Jan, are you really just ignoring my remarks and actually making
> this diff more unacceptable to me?

I cannot agree GDB should prefer to simulate the K&R C behavior over ANSI/ISO
C behavior.  Sure I do not have the approval right so I will do anything I am
told with the patch to check it in.  Still FYI so far I has not been convinced
to change my opinion on this (K&R vs. ANSI/ISO C) specific subject.

I am sorry if spending the time to read my patch was not acceptable for you.


Regards,
Jan

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

[Mark Kettenis]

> Date: Wed, 11 Feb 2009 23:08:24 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> On Wed, 11 Feb 2009 22:22:41 +0100, Mark Kettenis wrote:
> # I disagree.  I think it is the behaviour that makes the most sense in
> # a historical context.  And I have a (somewhat) vague recollection that
> 
> On Wed, 11 Feb 2009 22:57:51 +0100, Mark Kettenis wrote:
> > Sorry Jan, are you really just ignoring my remarks and actually making
> > this diff more unacceptable to me?
> 
> I cannot agree GDB should prefer to simulate the K&R C behavior over
> ANSI/ISO C behavior.  Sure I do not have the approval right so I
> will do anything I am told with the patch to check it in.  Still FYI
> so far I has not been convinced to change my opinion on this (K&R
> vs. ANSI/ISO C) specific subject.

We can agree to disagree.  That said, there should be no reason to
unecessarily get rid of the K&R and older ISO C heritage if there is
no good reason to do so.  The bug report you cite provides a reason.
In my reply to Daniel's mail I provided an alternative suggestion:

> ..., would it be an idea to use the type of the return value
> expression given by the user instead of int as a fallback?

Which you seemed to ignore.  I think it actually makes the return
command more powerful, by letting the user (implicitly or explicitly)
specify the return type of a function for which debugging information
is missing.  Can you please consider the suggestion I make?

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

[Jan Kratochvil]

On Wed, 11 Feb 2009 23:37:38 +0100, Mark Kettenis wrote:
> > ..., would it be an idea to use the type of the return value
> > expression given by the user instead of int as a fallback?
> 
> Which you seemed to ignore.  I think it actually makes the return
> command more powerful, by letting the user (implicitly or explicitly)
> specify the return type of a function for which debugging information
> is missing.  Can you please consider the suggestion I make?

Your suggestion ("type of the return value expression given by the user"):
+ improvement over the current state (one _can_ return long or long long)
- may be more tricky to the user (requirement to cast to long or long long if
  the inferior function returns such type)
  (as the redhat.com Bug would be still filed on this implementation by the
   user I did not find it acceptable myself no matter of its technical aspects)

My suggestion (long long cast forced by GDB):
+ improvement over the current state - like yours
+ easier for the user automatically fixing his mistakes
- possibly incompatible with ABI having `int' value placement incompatible with
  `long long' value placement
- possibly incompatible with ABI not having `long' returned in a register
(both ABI requirements are compliant with the common ABIs I am aware of)

You are right I should have been more addressing your opinion.


Regards,
Jan

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

[Tom Tromey]

>>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:

Weighing in on an old-ish thread.  AFAIK this has never reached any
sort of resolution.

Jan> Your suggestion ("type of the return value expression given by the user"):
Jan> + improvement over the current state (one _can_ return long or long long)
Jan> - may be more tricky to the user (requirement to cast to long or long long if
Jan>   the inferior function returns such type)

How about, in the specific case of "return" from a function without
debug info, we simply require the user to supply a cast to the desired
type.  This avoids any problems in choosing a default and it helps
avoid errors introduced by using the type implicit in the expression.

Tom

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

[Mark Kettenis]

> From: Tom Tromey <tromey@redhat.com>
> Date: Tue, 03 Mar 2009 11:10:19 -0700
> 
> >>>>> "Jan" == Jan Kratochvil <jan.kratochvil@redhat.com> writes:
> 
> Weighing in on an old-ish thread.  AFAIK this has never reached any
> sort of resolution.
> 
> Jan> Your suggestion ("type of the return value expression given by the user"):
> Jan> + improvement over the current state (one _can_ return long or long long)
> Jan> - may be more tricky to the user (requirement to cast to long or long long if
> Jan>   the inferior function returns such type)
> 
> How about, in the specific case of "return" from a function without
> debug info, we simply require the user to supply a cast to the desired
> type.  This avoids any problems in choosing a default and it helps
> avoid errors introduced by using the type implicit in the expression.

Do we have a way to figure out if such a cast is present in the
expression passed as an argument to the return command?

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

[Tom Tromey]

>>>>> "Mark" == Mark Kettenis <mark.kettenis@xs4all.nl> writes:

Mark> Do we have a way to figure out if such a cast is present in the
Mark> expression passed as an argument to the return command?

Yeah, it is not too hard.  It just means having return_command call
parse_expression directly, then look at the resulting expression.

Tom

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

[Jan Kratochvil]

On Thu, 05 Mar 2009 01:15:08 +0100, Tom Tromey wrote:
> >>>>> "Mark" == Mark Kettenis <mark.kettenis@xs4all.nl> writes:
> 
> Mark> Do we have a way to figure out if such a cast is present in the
> Mark> expression passed as an argument to the return command?
> 
> Yeah, it is not too hard.  It just means having return_command call
> parse_expression directly, then look at the resulting expression.

Attached, check-in approval requested.


Regards,
Jan

gdb/
2009-03-09  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/testsuite/
2009-03-09  Jan Kratochvil  <jan.kratochvil@redhat.com>

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

--- gdb/stack.c	6 Mar 2009 18:51:05 -0000	1.186
+++ gdb/stack.c	9 Mar 2009 01:52:55 -0000
@@ -1796,18 +1796,26 @@ 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 (_("Selected stack frame (with no associated function) "
+		     "requires 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);
 
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.base/return-nodebug.c	9 Mar 2009 01:52:55 -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	9 Mar 2009 01:52:55 -0000
@@ -0,0 +1,60 @@
+# 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"]} {
+	# 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" \
+		 "Selected stack frame \\(with no associated function\\) requires 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
+    }
+}

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

[Mark Kettenis]

> Date: Mon, 9 Mar 2009 02:55:10 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> On Thu, 05 Mar 2009 01:15:08 +0100, Tom Tromey wrote:
> > >>>>> "Mark" == Mark Kettenis <mark.kettenis@xs4all.nl> writes:
> > 
> > Mark> Do we have a way to figure out if such a cast is present in the
> > Mark> expression passed as an argument to the return command?
> > 
> > Yeah, it is not too hard.  It just means having return_command call
> > parse_expression directly, then look at the resulting expression.
> 
> Attached, check-in approval requested.

I think this is a reasonable compromise, and I think the
implementation is ok.  But I'd like the opinion of one of the other
global maintainers (Daniel?, Joel?) before going this way.

> gdb/
> 2009-03-09  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/testsuite/
> 2009-03-09  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.base/return-nodebug.exp, gdb.base/return-nodebug.S: New.
> 
> --- gdb/stack.c	6 Mar 2009 18:51:05 -0000	1.186
> +++ gdb/stack.c	9 Mar 2009 01:52:55 -0000
> @@ -1796,18 +1796,26 @@ 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 (_("Selected stack frame (with no associated function) "
> +		     "requires 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);
>  
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ gdb/testsuite/gdb.base/return-nodebug.c	9 Mar 2009 01:52:55 -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	9 Mar 2009 01:52:55 -0000
> @@ -0,0 +1,60 @@
> +# 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"]} {
> +	# 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" \
> +		 "Selected stack frame \\(with no associated function\\) requires 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
> +    }
> +}
> 

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

[Joel Brobecker]

> 2009-03-09  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.

This looks good to me, but I don't understand the error message when
the user forgot to cast the result. I wonder if we shouldn't just
preserve the old behavior which is to implicitly cast to "int"?

> >        if (return_type == NULL)
> > -	return_type = builtin_type (get_frame_arch (thisframe))->builtin_int;
> > +      	{
> > +	  if (retval_expr->elts[0].opcode != UNOP_CAST)
> > +	    error (_("Selected stack frame (with no associated function) "
> > +		     "requires an explicit cast of the value to return"));
> > +	  return_type = value_type (return_value);
> > +	}

-- 
Joel

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

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

>> 2009-03-09  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.

Joel> This looks good to me, but I don't understand the error message when
Joel> the user forgot to cast the result. I wonder if we shouldn't just
Joel> preserve the old behavior which is to implicitly cast to "int"?

We've come full circle :-)

The original patch was motivated by a case where casting to int gave a
bad result.  Then Mark had a counter-proposal, which was to use the
expression's type, rather than assuming 'int'.  Finally, I suggested
simply requiring a cast, resulting in this patch.

So, the real question for you is how you think it ought to work.
I think we have implementations of all of the approaches now.

Tom

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

[Joel Brobecker]

> The original patch was motivated by a case where casting to int gave a
> bad result.  Then Mark had a counter-proposal, which was to use the
> expression's type, rather than assuming 'int'.  Finally, I suggested
> simply requiring a cast, resulting in this patch.

Ah, OK - I didn't understand the final decision very well. Sorry about
that.

I'm starting to understand what the error message is trying to say, now.
Can we maybe explore other suggestions? I can suggest for instance:

    Return value type not available for selected stack frame.\n
    Please use an explicit cast of the value to return.

I think we should also probably add a note in the documentation as well.
In case a user still gets confused as to why he gets an error.

> So, the real question for you is how you think it ought to work.

Yes, I think so.

-- 
Joel

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

[Jan Kratochvil]

On Wed, 11 Mar 2009 21:59:01 +0100, Joel Brobecker wrote:
> I'm starting to understand what the error message is trying to say, now.
> Can we maybe explore other suggestions? I can suggest for instance:
> 
>     Return value type not available for selected stack frame.\n
>     Please use an explicit cast of the value to return.

Used this text.


> I think we should also probably add a note in the documentation as well.
> In case a user still gets confused as to why he gets an error.

Written a new part for `Returning'.


Thanks,
Jan


gdb/
2009-03-13  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-13  Jan Kratochvil  <jan.kratochvil@redhat.com>

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

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

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

--- gdb/stack.c	6 Mar 2009 18:51:05 -0000	1.186
+++ gdb/stack.c	13 Mar 2009 16:56:21 -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	5 Mar 2009 23:11:11 -0000	1.561
+++ gdb/doc/gdb.texinfo	13 Mar 2009 16:56:32 -0000
@@ -12494,6 +12494,41 @@ 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.
+
+As the selected stack frame function may not have its debug info available in
+such cases @value{GDBN} needs to find out the type to return from user.  For
+example typing @samp{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.
+Therefore the implicit return type is accepted for debug info featuring
+functions as in this case:
+
+@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
+
+But for functions missing their debug info the user is required to specify the
+return type by an appropriate cast explicitely:
+
+@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	13 Mar 2009 16:56:32 -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	13 Mar 2009 16:56:32 -0000
@@ -0,0 +1,60 @@
+# 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"]} {
+	# 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" \
+		 "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
+    }
+}

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

[Joel Brobecker]

> 2009-03-13  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.

This part looks good to me.

> gdb/testsuite/
> 2009-03-13  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.base/return-nodebug.exp, gdb.base/return-nodebug.S: New.

This part looks good to me too.  Just a couple of things:
  - The name of the C file has a small typo in the extension....
  - A comment seems malformed:

> +	# 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...

(thanks for teaching me about "prepare_for_testing" ;-)

-- 
Joel

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

[Eli Zaretskii]

> Date: Fri, 13 Mar 2009 18:11:35 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: Tom Tromey <tromey@redhat.com>, Mark Kettenis <mark.kettenis@xs4all.nl>,         drow@false.org, gdb-patches@sourceware.org
> 
> gdb/doc/
> 2009-03-13  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.texinfo (Returning): New description for missing debug info.

Thanks, this part is approved, provided that you add the missing
punctuation and clarify the text, as indicated below.

> +        The concrete registers assignment depends on the OS ABI and the
> +type being returned by the selected stack frame.

I'm not sure the reader will understand why this sentence is here.
Perhaps it is important to have this note, but then please add some
(apparently missing) text that would explain how register assignment
is relevant to the issue at hand.  Maybe you need to begin with the
normal case, where both the caller and the callee have debug info, and
take it from there to the two less obvious cases.  See below.

> +As the selected stack frame function may not have its debug info available in
> +such cases @value{GDBN} needs to find out the type to return from user.  For
             ^
Missing comma.

> +example typing @samp{return -1} with its implicit type @code{int} would set
          ^
Missing comma.  Also, please use @kbd{return -1} instead of @samp, as
you are describing keyboard input.

>                                                                    would set
> +only a part of a @code{long long int} result for a debug info less function.

I think it is better to talk about more general mismatch of an `int'
and the function's return type, because the "partial fill" case is
just a special case, and even then only on some platforms.  That's
assuming that there is indeed a place for generalization: would your
first example still work the same if the return value of the function
were `double'?

> +Therefore the implicit return type is accepted for debug info featuring
> +functions as in this case:
> +
> +@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
> +
> +But for functions missing their debug info the user is required to specify the
> +return type by an appropriate cast explicitely:
                                      ^^^^^^^^^^^
A typo.

Anyway, this is confusing: the paragraph begins by talking about
functions without debug info, but then you give an example for a
function _with_ debug info, and say "Therefore", as if the example
where GDB does accept incomplete spec of the return value somehow
_follows_ from the immediately preceeding discussion.

I would begin with describing the normal case explicitly, before the
text you wrote.  Something like:

  Normally, both the selected stack frame and the one above it have
  debug info.  @value{GDBN} uses the debug info when the value of
  @var{expression} does not have an explicit data type.  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}:

<Put here your first example.>

  However, if the selected stack frame does not have a debug info,
  e.g., if the function was compiled without debug info, ...

and now describe the use-case you are talking about and give your
second example.

OK?

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

[Jan Kratochvil]

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
+    }
+}

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

[Eli Zaretskii]

> Date: Sat, 14 Mar 2009 22:44:01 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> Cc: Tom Tromey <tromey@redhat.com>, Mark Kettenis <mark.kettenis@xs4all.nl>,
>         drow@false.org, gdb-patches@sourceware.org
> 
> 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.

Thanks, this version is fine.

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

[Joel Brobecker]

> 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.

No problem with your version. The reason why I wondered whether
this was necessary or not is that I assumed that the testsuite
scripts are going to be read mostly by GDB developers...

-- 
Joel

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

[Jan Kratochvil]

Hi,

finally checked-in: http://sourceware.org/ml/gdb-cvs/2009-03/msg00093.html


Thanks,
Jan

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

[Pedro Alves]

Unfortunatelly, this test is failing against gdbserver:

  Running ../../../src/gdb/testsuite/gdb.base/return-nodebug.exp ...
  FAIL: gdb.base/return-nodebug.exp: signed-char: full width of the returned result
  FAIL: gdb.base/return-nodebug.exp: short: full width of the returned result
  FAIL: gdb.base/return-nodebug.exp: int: full width of the returned result
  FAIL: gdb.base/return-nodebug.exp: long: full width of the returned result
  FAIL: gdb.base/return-nodebug.exp: long-long: full width of the returned result

The issue is that it relies on parsing the output of printf, which
doesn't work against remote targets that don't implement some kind
of semi-hosting io, like gdbserver.

( as a side, but related note, tests that need to rely on inferior output,
should be guarded with "if [target_info exists gdb,noinferiorio]" )

What do you think we rewrite test to not rely on printf at all, like below?

This passes for me on native x86_64-linux, and against x86_64-linux gdbserver.

-- 
Pedro Alves

2009-03-18  Pedro Alves  <pedro@codesourcery.com>

	* return-nodebug.c: Don't include stdio.h.
	(init): Delete.
	(func): Delete definition and provide extern declaration.
	(t): New.
	(main): Don't call printf.  Call func and store its result in t.
	* return-nodebug1.c: New.
	* return-nodebug.exp: Don't expect stdio output.  Instead, print
	the global variable t.  Drop printf formatters and cast types from
	foreach loop.  Don't use prepare_for_testing.  Compile
	return-nodebug.c and return-nodebug1.c in separate steps.  Don't
	define FORMAT or CAST.

---
 gdb/testsuite/gdb.base/return-nodebug.c   |   22 ++-------------
 gdb/testsuite/gdb.base/return-nodebug.exp |   44 ++++++++++++++++++++----------
 gdb/testsuite/gdb.base/return-nodebug1.c  |   22 +++++++++++++++
 3 files changed, 56 insertions(+), 32 deletions(-)

Index: src/gdb/testsuite/gdb.base/return-nodebug.c
===================================================================
--- src.orig/gdb/testsuite/gdb.base/return-nodebug.c	2009-03-18 03:11:16.000000000 +0000
+++ src/gdb/testsuite/gdb.base/return-nodebug.c	2009-03-18 03:49:06.000000000 +0000
@@ -15,34 +15,20 @@
    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;
-}
+extern TYPE func (void);
 
 static void
 marker (void)
 {
 }
 
+TYPE t;
+
 int
 main (void)
 {
-  /* Preinitialize registers to 0 to avoid false PASS by leftover garbage.  */
-  init ();
-
-  printf ("result=" FORMAT "\n", CAST func ());
+  t = func ();
 
-  /* Cannot `next' with no debug info.  */
   marker ();
 
   return 0;
Index: src/gdb/testsuite/gdb.base/return-nodebug.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/return-nodebug.exp	2009-03-18 02:58:19.000000000 +0000
+++ src/gdb/testsuite/gdb.base/return-nodebug.exp	2009-03-18 03:52:58.000000000 +0000
@@ -34,28 +34,44 @@ proc do_test {type} {
 		 "return from function with no debug info with a cast"	\
 		 "Make selected stack frame return now\\? \\(y or n\\) " "y"
 
+	gdb_test "advance marker" "marker \\(.*" \
+		 "advance to marker"
+
 	# 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"
+	gdb_test "print /d t" " = -1"
     }
 
     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]
-
+foreach type {{signed char} {short} {int} {long} {long long}} {
     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
+    set testfile "return-nodebug"
+    set srcfile ${testfile}.c
+    set srcfile1 ${testfile}1.c
+    set binfile ${objdir}/${subdir}/${testfile}-${typenospace}
+
+    set additional_flags "additional_flags=-DTYPE=$typeesc"
+
+    if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}-${typenospace}0.o" object [list debug $additional_flags]] != "" } {
+	continue
+    }
+
+    # This one is compiled without debug info.
+    if { [gdb_compile "${srcdir}/${subdir}/${srcfile1}" "${binfile}-${typenospace}1.o" object [list $additional_flags]] != "" } {
+	continue
+    }
+
+    if { [gdb_compile "${binfile}0.o ${binfile}1.o" "${binfile}" executable {debug}] != "" } {
+	continue
     }
+
+    gdb_exit
+    gdb_start
+    gdb_reinitialize_dir $srcdir/$subdir
+    gdb_load ${binfile}
+
+    do_test $type
 }
Index: src/gdb/testsuite/gdb.base/return-nodebug1.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/return-nodebug1.c	2009-03-18 03:12:13.000000000 +0000
@@ -0,0 +1,22 @@
+/* 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/>.  */
+
+TYPE
+func (void)
+{
+  return 31;
+}

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

[Joel Brobecker]

> The issue is that it relies on parsing the output of printf, which
> doesn't work against remote targets that don't implement some kind
> of semi-hosting io, like gdbserver.

Yeah - the worse part is that I saw the use of printf, frowned on it,
but let it go because I didn't see a quick way of avoiding it. Lessons
learned for me.

> 2009-03-18  Pedro Alves  <pedro@codesourcery.com>
> 
> 	* return-nodebug.c: Don't include stdio.h.
> 	(init): Delete.
> 	(func): Delete definition and provide extern declaration.
> 	(t): New.
> 	(main): Don't call printf.  Call func and store its result in t.
> 	* return-nodebug1.c: New.
> 	* return-nodebug.exp: Don't expect stdio output.  Instead, print
> 	the global variable t.  Drop printf formatters and cast types from
> 	foreach loop.  Don't use prepare_for_testing.  Compile
> 	return-nodebug.c and return-nodebug1.c in separate steps.  Don't
> 	define FORMAT or CAST.

Looks great to me.

In the future, don't feel obliged to fix it yourself unless you want to.
I can help fix my own messes...

-- 
Joel

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

[Jan Kratochvil]

On Wed, 18 Mar 2009 05:20:37 +0100, Pedro Alves wrote:
> Unfortunatelly, this test is failing against gdbserver:
> 
>   Running ../../../src/gdb/testsuite/gdb.base/return-nodebug.exp ...
>   FAIL: gdb.base/return-nodebug.exp: signed-char: full width of the returned result
>   FAIL: gdb.base/return-nodebug.exp: short: full width of the returned result
>   FAIL: gdb.base/return-nodebug.exp: int: full width of the returned result
>   FAIL: gdb.base/return-nodebug.exp: long: full width of the returned result
>   FAIL: gdb.base/return-nodebug.exp: long-long: full width of the returned result

got now a link the gdbserver mode can be tested by:
	http://sourceware.org/gdb/wiki/Native_gdbserver_testing


> ( as a side, but related note, tests that need to rely on inferior output,
> should be guarded with "if [target_info exists gdb,noinferiorio]" )

Thanks for the info.


> What do you think we rewrite test to not rely on printf at all, like below?

Yes, the fix looks fine to me.

Just fixed the gdb_compile lines as it did not compile on a clean sourcetree.


Thanks,
Jan


2009-03-18  Pedro Alves  <pedro@codesourcery.com>

	* return-nodebug.c: Don't include stdio.h.
	(init): Delete.
	(func): Delete definition and provide extern declaration.
	(t): New.
	(main): Don't call printf.  Call func and store its result in t.
	* return-nodebug1.c: New.
	* return-nodebug.exp: Don't expect stdio output.  Instead, print
	the global variable t.  Drop printf formatters and cast types from
	foreach loop.  Don't use prepare_for_testing.  Compile
	return-nodebug.c and return-nodebug1.c in separate steps.  Don't
	define FORMAT or CAST.

--- gdb/testsuite/gdb.base/return-nodebug.c	15 Mar 2009 09:19:40 -0000	1.1
+++ gdb/testsuite/gdb.base/return-nodebug.c	18 Mar 2009 15:05:00 -0000
@@ -15,34 +15,20 @@
    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;
-}
+extern TYPE func (void);
 
 static void
 marker (void)
 {
 }
 
+TYPE t;
+
 int
 main (void)
 {
-  /* Preinitialize registers to 0 to avoid false PASS by leftover garbage.  */
-  init ();
-
-  printf ("result=" FORMAT "\n", CAST func ());
+  t = func ();
 
-  /* Cannot `next' with no debug info.  */
   marker ();
 
   return 0;
--- gdb/testsuite/gdb.base/return-nodebug.exp	15 Mar 2009 09:19:40 -0000	1.1
+++ gdb/testsuite/gdb.base/return-nodebug.exp	18 Mar 2009 15:05:00 -0000
@@ -34,28 +34,44 @@ proc do_test {type} {
 		 "return from function with no debug info with a cast"	\
 		 "Make selected stack frame return now\\? \\(y or n\\) " "y"
 
+	gdb_test "advance marker" "marker \\(.*" \
+		 "advance to marker"
+
 	# 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"
+	gdb_test "print /d t" " = -1"
     }
 
     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]
-
+foreach type {{signed char} {short} {int} {long} {long long}} {
     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
+    set testfile "return-nodebug"
+    set srcfile ${testfile}.c
+    set srcfile1 ${testfile}1.c
+    set binfile ${objdir}/${subdir}/${testfile}-${typenospace}
+
+    set additional_flags "additional_flags=-DTYPE=$typeesc"
+
+    if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}0.o" object [list debug $additional_flags]] != "" } {
+	continue
+    }
+
+    # This one is compiled without debug info.
+    if { [gdb_compile "${srcdir}/${subdir}/${srcfile1}" "${binfile}1.o" object [list $additional_flags]] != "" } {
+	continue
+    }
+
+    if { [gdb_compile "${binfile}0.o ${binfile}1.o" "${binfile}" executable {debug}] != "" } {
+	continue
     }
+
+    gdb_exit
+    gdb_start
+    gdb_reinitialize_dir $srcdir/$subdir
+    gdb_load ${binfile}
+
+    do_test $type
 }
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.base/return-nodebug1.c	18 Mar 2009 15:05:00 -0000
@@ -0,0 +1,22 @@
+/* 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/>.  */
+
+TYPE
+func (void)
+{
+  return 31;
+}

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

[Pedro Alves]

On Wednesday 18 March 2009 13:54:48, Joel Brobecker wrote:
> Looks great to me.

Thanks.

> In the future, don't feel obliged to fix it yourself unless you want to.
> I can help fix my own messes...

It's not a problem.  In this case, I thought it would be faster
for me to go ahead, since I knew exactly what had to be done.  It only
took me a few minutes.


On Wednesday 18 March 2009 15:13:20, Jan Kratochvil wrote:
> Yes, the fix looks fine to me.
> 
> Just fixed the gdb_compile lines as it did not compile on a clean sourcetree.
> 

Ah,

 -+    if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}-${typenospace}0.o" object [list debug $additional_flags]] != "" } {
 ++    if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}0.o" object [list debug $additional_flags]] != "" 

Ooops, early experimentation, then I moved -${typenospace} to ${binfile}, but
the .o files were still around.  Thanks.


I brought back the "full width of the returned result" string that
I removed by mistake.

-gdb_test "print /d t" " = -1"
+gdb_test "print /d t" " = -1" "full width of the returned result"

Now checked in.

-- 
Pedro Alves

2009-03-18  Pedro Alves  <pedro@codesourcery.com>

	* return-nodebug.c: Don't include stdio.h.
	(init): Delete.
	(func): Delete definition and provide extern declaration.
	(t): New.
	(main): Don't call printf.  Call func and store its result in t.
	* return-nodebug1.c: New.
	* return-nodebug.exp: Don't expect stdio output.  Instead, print
	the global variable t.  Drop printf formatters and cast types from
	foreach loop.  Don't use prepare_for_testing.  Compile
	return-nodebug.c and return-nodebug1.c in separate steps.  Don't
	define FORMAT or CAST.

---
 gdb/testsuite/gdb.base/return-nodebug.c   |   22 ++-------------
 gdb/testsuite/gdb.base/return-nodebug.exp |   44 ++++++++++++++++++++----------
 gdb/testsuite/gdb.base/return-nodebug1.c  |   22 +++++++++++++++
 3 files changed, 56 insertions(+), 32 deletions(-)

Index: src/gdb/testsuite/gdb.base/return-nodebug.c
===================================================================
--- src.orig/gdb/testsuite/gdb.base/return-nodebug.c	2009-03-18 15:20:12.000000000 +0000
+++ src/gdb/testsuite/gdb.base/return-nodebug.c	2009-03-18 15:21:35.000000000 +0000
@@ -15,34 +15,20 @@
    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;
-}
+extern TYPE func (void);
 
 static void
 marker (void)
 {
 }
 
+TYPE t;
+
 int
 main (void)
 {
-  /* Preinitialize registers to 0 to avoid false PASS by leftover garbage.  */
-  init ();
-
-  printf ("result=" FORMAT "\n", CAST func ());
+  t = func ();
 
-  /* Cannot `next' with no debug info.  */
   marker ();
 
   return 0;
Index: src/gdb/testsuite/gdb.base/return-nodebug.exp
===================================================================
--- src.orig/gdb/testsuite/gdb.base/return-nodebug.exp	2009-03-18 15:20:12.000000000 +0000
+++ src/gdb/testsuite/gdb.base/return-nodebug.exp	2009-03-18 15:24:00.000000000 +0000
@@ -34,28 +34,44 @@ proc do_test {type} {
 		 "return from function with no debug info with a cast"	\
 		 "Make selected stack frame return now\\? \\(y or n\\) " "y"
 
+	gdb_test "advance marker" "marker \\(.*" \
+		 "advance to marker"
+
 	# 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"
+	gdb_test "print /d t" " = -1" "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]
-
+foreach type {{signed char} {short} {int} {long} {long long}} {
     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
+    set testfile "return-nodebug"
+    set srcfile ${testfile}.c
+    set srcfile1 ${testfile}1.c
+    set binfile ${objdir}/${subdir}/${testfile}-${typenospace}
+
+    set additional_flags "additional_flags=-DTYPE=$typeesc"
+
+    if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}0.o" object [list debug $additional_flags]] != "" } {
+	continue
+    }
+
+    # This one is compiled without debug info.
+    if { [gdb_compile "${srcdir}/${subdir}/${srcfile1}" "${binfile}1.o" object [list $additional_flags]] != "" } {
+	continue
+    }
+
+    if { [gdb_compile "${binfile}0.o ${binfile}1.o" "${binfile}" executable {debug}] != "" } {
+	continue
     }
+
+    gdb_exit
+    gdb_start
+    gdb_reinitialize_dir $srcdir/$subdir
+    gdb_load ${binfile}
+
+    do_test $type
 }
Index: src/gdb/testsuite/gdb.base/return-nodebug1.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.base/return-nodebug1.c	2009-03-18 15:21:35.000000000 +0000
@@ -0,0 +1,22 @@
+/* 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/>.  */
+
+TYPE
+func (void)
+{
+  return 31;
+}

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

[Mark Kettenis]

> Date: Wed, 11 Feb 2009 22:46:47 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> > 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.

Thinking a bit more of this now, things all depend on the calling
convention.  I'm not convinced casting to `long long' is safe in all
cases, especially on 32-bit big-endian machines.  It really might do
the wrong thing there, exposing garbage or the wrong 32 bits of the
64-bit value.

The 'int' case is really special in a sense, very much because of the
K&R heritage.  It has to work for all types that are sizeof(int) or
smaller.

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

[Jan Kratochvil]

On Wed, 11 Feb 2009 23:44:21 +0100, Mark Kettenis wrote:
> Thinking a bit more of this now, things all depend on the calling
> convention.  I'm not convinced casting to `long long' is safe in all
> cases, especially on 32-bit big-endian machines.  It really might do
> the wrong thing there, exposing garbage or the wrong 32 bits of the
> 64-bit value.
> 
> The 'int' case is really special in a sense, very much because of the
> K&R heritage.  It has to work for all types that are sizeof(int) or
> smaller.

OK to check-in this patch?

Regression-tested on x86_64-unknown-linux-gnu.


Thanks,
Jan


gdb/
2009-02-12  Mark Kettenis  <kettenis@gnu.org>

	* stack.c (return_command): Returned type for functions with no debug
	info is now the type of user specified expression.

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

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

--- gdb/stack.c	11 Feb 2009 16:07:28 -0000	1.185
+++ gdb/stack.c	12 Feb 2009 09:24:18 -0000
@@ -1806,6 +1806,8 @@ return_command (char *retval_exp, int fr
          the cast fail, this call throws an error.  */
       if (thisfun != NULL)
 	return_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (thisfun));
+      else
+	return_type = value_type (return_value);
       if (return_type == NULL)
 	return_type = builtin_type (get_frame_arch (thisframe))->builtin_int;
       CHECK_TYPEDEF (return_type);
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.base/return-nodebug.c	12 Feb 2009 09:24:18 -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	12 Feb 2009 09:24:18 -0000
@@ -0,0 +1,55 @@
+# 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"]} {
+	# Test return from a function with no debug info (symbol; still it may
+	# have a minimal-symbol) if it does not crash.
+	# 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"	\
+		 "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 $type
+    }
+}

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

[Pierre Muller]

  Concerning the test, 
wouldn't it be better to set the return value to 
another value than the one that is normally
returned by the function?
  This would allow to FAIL on targets
where the substitution fails and the program
is just continued...

Just use some other value in func definition,
for instance
+static TYPE
+func (void)+{
+  return (TYPE) 31;
+}

  I checked your patch, because I was worried 
about the weird syntax
and thought that this would fail
on my old cygwin dejagnu, but it does
give me 10 PASS, even with the above modification.

Pierre Muller
Pascal language support maintainer for GDB


> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Jan Kratochvil
> Envoyé : Thursday, February 12, 2009 10:42 AM
> À : Mark Kettenis
> Cc : drow@false.org; gdb-patches@sourceware.org
> Objet : Re: [patch] Fix `return' of long/long-long results with no
> debuginfo
> 
> On Wed, 11 Feb 2009 23:44:21 +0100, Mark Kettenis wrote:
> > Thinking a bit more of this now, things all depend on the calling
> > convention.  I'm not convinced casting to `long long' is safe in all
> > cases, especially on 32-bit big-endian machines.  It really might do
> > the wrong thing there, exposing garbage or the wrong 32 bits of the
> > 64-bit value.
> >
> > The 'int' case is really special in a sense, very much because of the
> > K&R heritage.  It has to work for all types that are sizeof(int) or
> > smaller.
> 
> OK to check-in this patch?
> 
> Regression-tested on x86_64-unknown-linux-gnu.
> 
> 
> Thanks,
> Jan
> 
> 
> gdb/
> 2009-02-12  Mark Kettenis  <kettenis@gnu.org>
> 
> 	* stack.c (return_command): Returned type for functions with no
> debug
> 	info is now the type of user specified expression.
> 
> gdb/testsuite/
> 2009-02-12  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.base/return-nodebug.exp, gdb.base/return-nodebug.c: New.
> 
> --- gdb/stack.c	11 Feb 2009 16:07:28 -0000	1.185
> +++ gdb/stack.c	12 Feb 2009 09:24:18 -0000
> @@ -1806,6 +1806,8 @@ return_command (char *retval_exp, int fr
>           the cast fail, this call throws an error.  */
>        if (thisfun != NULL)
>  	return_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (thisfun));
> +      else
> +	return_type = value_type (return_value);
>        if (return_type == NULL)
>  	return_type = builtin_type (get_frame_arch (thisframe))-
> >builtin_int;
>        CHECK_TYPEDEF (return_type);
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ gdb/testsuite/gdb.base/return-nodebug.c	12 Feb 2009 09:24:18 -
> 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	12 Feb 2009 09:24:18 -
> 0000
> @@ -0,0 +1,55 @@
> +# 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"]} {
> +	# Test return from a function with no debug info (symbol; still
> it may
> +	# have a minimal-symbol) if it does not crash.
> +	# 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"	\
> +		 "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 $type
> +    }
> +}

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

[Jan Kratochvil]

On Thu, 12 Feb 2009 15:35:58 +0100, Pierre Muller wrote:
> Just use some other value in func definition,
> for instance
> +static TYPE
> +func (void)+{
> +  return (TYPE) 31;
> +}

I fully agree, it was my mistake.


Thanks,
Jan


gdb/
2009-02-12  Mark Kettenis  <kettenis@gnu.org>

	* stack.c (return_command): Returned type for functions with no debug
	info is now the type of user specified expression.

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

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

--- gdb/stack.c	11 Feb 2009 16:07:28 -0000	1.185
+++ gdb/stack.c	12 Feb 2009 09:24:18 -0000
@@ -1806,6 +1806,8 @@ return_command (char *retval_exp, int fr
          the cast fail, this call throws an error.  */
       if (thisfun != NULL)
 	return_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (thisfun));
+      else
+	return_type = value_type (return_value);
       if (return_type == NULL)
 	return_type = builtin_type (get_frame_arch (thisframe))->builtin_int;
       CHECK_TYPEDEF (return_type);
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.base/return-nodebug.c	12 Feb 2009 09:24:18 -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 31;
+}
+
+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	12 Feb 2009 09:24:18 -0000
@@ -0,0 +1,55 @@
+# 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"]} {
+	# Test return from a function with no debug info (symbol; still it may
+	# have a minimal-symbol) if it does not crash.
+	# 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"	\
+		 "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 $type
+    }
+}

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

[Mark Kettenis]

> Date: Thu, 12 Feb 2009 10:41:34 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> On Wed, 11 Feb 2009 23:44:21 +0100, Mark Kettenis wrote:
> > Thinking a bit more of this now, things all depend on the calling
> > convention.  I'm not convinced casting to `long long' is safe in all
> > cases, especially on 32-bit big-endian machines.  It really might do
> > the wrong thing there, exposing garbage or the wrong 32 bits of the
> > 64-bit value.
> > 
> > The 'int' case is really special in a sense, very much because of the
> > K&R heritage.  It has to work for all types that are sizeof(int) or
> > smaller.
> 
> OK to check-in this patch?
> 
> 2009-02-12  Mark Kettenis  <kettenis@gnu.org>
> 
> 	* stack.c (return_command): Returned type for functions with no debug
> 	info is now the type of user specified expression.

Looks good to me, but please put your own name on the ChangeLog entry.
I just suggested the idea, but I was too lazy to actually write the code ;).

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

[Jan Kratochvil]

On Wed, 11 Feb 2009 23:44:21 +0100, Mark Kettenis wrote:
> Thinking a bit more of this now, things all depend on the calling
> convention.  I'm not convinced casting to `long long' is safe in all
> cases, especially on 32-bit big-endian machines.  It really might do
> the wrong thing there, exposing garbage or the wrong 32 bits of the
> 64-bit value.

OK to check-in this patch?

This patch could have a regression on ABI which defines caller saved vs.
callee saved registers depending on the return type of the function.  I do not
believe there exists such ABI.

Regression-tested on GNU/Linux on:
HEAD{x86_64}, 6.8{x86_64(+inf. i386), i386, s390x(+inf. s390), ia64}


Thanks,
Jan


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

	* stack.c (return_extended_value_cast, return_extended_value): New.
	(return_command): Returned type for functions with no debug info has
	now at least the width of the type of user specified expression while
	it tries to use the largest compatible width.

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

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

--- gdb/stack.c	11 Feb 2009 16:07:28 -0000	1.185
+++ gdb/stack.c	20 Feb 2009 21:47:34 -0000
@@ -1777,7 +1777,75 @@ down_command (char *count_exp, int from_
   down_silently_base (count_exp);
   print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC);
 }
-\f
+
+/* Verify whether if RETURN_VALUE width gets extended to EXT_TYPE it will still
+   be the same value after reading it back using the RETURN_VALUE type.  */
+
+static int
+return_extended_value_cast (struct value *return_value, struct type *ext_type)
+{
+  struct regcache *current_regcache = get_current_regcache ();
+  struct gdbarch *gdbarch = get_regcache_arch (current_regcache);
+  struct type *return_type = value_type (return_value);
+  struct value *ext_value, *compare_value;
+
+  if (gdbarch_return_value (gdbarch, NULL, ext_type, NULL, NULL, NULL)
+      != RETURN_VALUE_REGISTER_CONVENTION)
+    return 0;
+
+  ext_value = value_cast (ext_type, return_value);
+  gdbarch_return_value (gdbarch, NULL, ext_type,
+			current_regcache, NULL /*read*/,
+			value_contents (ext_value) /*write*/);
+
+  compare_value = allocate_value (return_type);
+  gdbarch_return_value (gdbarch, NULL, return_type, current_regcache,
+			value_contents_raw (compare_value) /*read*/,
+			NULL /*write*/);
+
+  return value_equal (return_value, compare_value);
+}
+
+/* Set RETURN_VALUE extended to the largest type width which will still be the
+   same value after reading it back using the RETURN_VALUE type.  */
+
+static void
+return_extended_value (struct value *return_value)
+{
+  struct type *return_type = value_type (return_value);
+  struct regcache *current_regcache = get_current_regcache ();
+  struct gdbarch *gdbarch = get_regcache_arch (current_regcache);
+  const struct builtin_type *builtins = builtin_type (gdbarch);
+  struct type **extp, *ext_tab[] =
+    {
+      builtins->builtin_long_long,
+      builtins->builtin_long,
+      return_type
+    };
+  unsigned width_tried = 0;
+
+  for (extp = ext_tab; extp < ext_tab + ARRAY_SIZE (ext_tab); extp++)
+    {
+      struct type *ext_type = *extp;
+
+      /* Do not retry extension to the integer of an already tried width.  */
+      if (ext_type != return_type && width_tried == TYPE_LENGTH (ext_type))
+	continue;
+
+      /* Do not extend to non-original smaller (or the same size) type.  */
+      if (ext_type != return_type
+	  && TYPE_LENGTH (ext_type) <= TYPE_LENGTH (return_type))
+	continue;
+
+      gdb_assert (TYPE_LENGTH (return_type) <= TYPE_LENGTH (ext_type));
+      width_tried = TYPE_LENGTH (ext_type);
+      if (return_extended_value_cast (return_value, ext_type))
+	break;
+    }
+
+  /* Ensure at least the attempt with original RETURN_TYPE was successful.  */
+  gdb_assert (extp < ext_tab + ARRAY_SIZE (ext_tab));
+}
 
 void
 return_command (char *retval_exp, int from_tty)
@@ -1806,6 +1874,8 @@ return_command (char *retval_exp, int fr
          the cast fail, this call throws an error.  */
       if (thisfun != NULL)
 	return_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (thisfun));
+      else
+	return_type = value_type (return_value);
       if (return_type == NULL)
 	return_type = builtin_type (get_frame_arch (thisframe))->builtin_int;
       CHECK_TYPEDEF (return_type);
@@ -1862,9 +1932,13 @@ If you continue, the return value that y
       gdb_assert (gdbarch_return_value (gdbarch, func_type, return_type, NULL,
 					NULL, NULL)
 		  == RETURN_VALUE_REGISTER_CONVENTION);
-      gdbarch_return_value (gdbarch, func_type, return_type,
-			    get_current_regcache (), NULL /*read*/,
-			    value_contents (return_value) /*write*/);
+
+      if (thisfun != NULL)
+	gdbarch_return_value (gdbarch, func_type, return_type,
+			      get_current_regcache (), NULL /*read*/,
+			      value_contents (return_value) /*write*/);
+      else
+	return_extended_value (return_value);
     }
 
   /* If we are at the end of a call dummy now, pop the dummy frame
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gdb/testsuite/gdb.base/return-nodebug.c	20 Feb 2009 21:47:34 -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	20 Feb 2009 21:47:34 -0000
@@ -0,0 +1,54 @@
+# 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"]} {
+	# 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 {{{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
+    }
+}

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

[Mark Kettenis]

> Date: Sat, 21 Feb 2009 01:04:09 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> On Wed, 11 Feb 2009 23:44:21 +0100, Mark Kettenis wrote:
> > Thinking a bit more of this now, things all depend on the calling
> > convention.  I'm not convinced casting to `long long' is safe in all
> > cases, especially on 32-bit big-endian machines.  It really might do
> > the wrong thing there, exposing garbage or the wrong 32 bits of the
> > 64-bit value.
> 
> OK to check-in this patch?

Hell no!  What is all that extra code about?

> gdb/
> 2009-02-20  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* stack.c (return_extended_value_cast, return_extended_value): New.
> 	(return_command): Returned type for functions with no debug info has
> 	now at least the width of the type of user specified expression while
> 	it tries to use the largest compatible width.
> 
> gdb/testsuite/
> 2009-02-20  Jan Kratochvil  <jan.kratochvil@redhat.com>
> 
> 	* gdb.base/return-nodebug.exp, gdb.base/return-nodebug.S: New.
> 
> --- gdb/stack.c	11 Feb 2009 16:07:28 -0000	1.185
> +++ gdb/stack.c	20 Feb 2009 21:47:34 -0000
> @@ -1777,7 +1777,75 @@ down_command (char *count_exp, int from_
>    down_silently_base (count_exp);
>    print_stack_frame (get_selected_frame (NULL), 1, SRC_AND_LOC);
>  }
> -\f
> +
> +/* Verify whether if RETURN_VALUE width gets extended to EXT_TYPE it will still
> +   be the same value after reading it back using the RETURN_VALUE type.  */
> +
> +static int
> +return_extended_value_cast (struct value *return_value, struct type *ext_type)
> +{
> +  struct regcache *current_regcache = get_current_regcache ();
> +  struct gdbarch *gdbarch = get_regcache_arch (current_regcache);
> +  struct type *return_type = value_type (return_value);
> +  struct value *ext_value, *compare_value;
> +
> +  if (gdbarch_return_value (gdbarch, NULL, ext_type, NULL, NULL, NULL)
> +      != RETURN_VALUE_REGISTER_CONVENTION)
> +    return 0;
> +
> +  ext_value = value_cast (ext_type, return_value);
> +  gdbarch_return_value (gdbarch, NULL, ext_type,
> +			current_regcache, NULL /*read*/,
> +			value_contents (ext_value) /*write*/);
> +
> +  compare_value = allocate_value (return_type);
> +  gdbarch_return_value (gdbarch, NULL, return_type, current_regcache,
> +			value_contents_raw (compare_value) /*read*/,
> +			NULL /*write*/);
> +
> +  return value_equal (return_value, compare_value);
> +}
> +
> +/* Set RETURN_VALUE extended to the largest type width which will still be the
> +   same value after reading it back using the RETURN_VALUE type.  */
> +
> +static void
> +return_extended_value (struct value *return_value)
> +{
> +  struct type *return_type = value_type (return_value);
> +  struct regcache *current_regcache = get_current_regcache ();
> +  struct gdbarch *gdbarch = get_regcache_arch (current_regcache);
> +  const struct builtin_type *builtins = builtin_type (gdbarch);
> +  struct type **extp, *ext_tab[] =
> +    {
> +      builtins->builtin_long_long,
> +      builtins->builtin_long,
> +      return_type
> +    };
> +  unsigned width_tried = 0;
> +
> +  for (extp = ext_tab; extp < ext_tab + ARRAY_SIZE (ext_tab); extp++)
> +    {
> +      struct type *ext_type = *extp;
> +
> +      /* Do not retry extension to the integer of an already tried width.  */
> +      if (ext_type != return_type && width_tried == TYPE_LENGTH (ext_type))
> +	continue;
> +
> +      /* Do not extend to non-original smaller (or the same size) type.  */
> +      if (ext_type != return_type
> +	  && TYPE_LENGTH (ext_type) <= TYPE_LENGTH (return_type))
> +	continue;
> +
> +      gdb_assert (TYPE_LENGTH (return_type) <= TYPE_LENGTH (ext_type));
> +      width_tried = TYPE_LENGTH (ext_type);
> +      if (return_extended_value_cast (return_value, ext_type))
> +	break;
> +    }
> +
> +  /* Ensure at least the attempt with original RETURN_TYPE was successful.  */
> +  gdb_assert (extp < ext_tab + ARRAY_SIZE (ext_tab));
> +}
>  
>  void
>  return_command (char *retval_exp, int from_tty)
> @@ -1806,6 +1874,8 @@ return_command (char *retval_exp, int fr
>           the cast fail, this call throws an error.  */
>        if (thisfun != NULL)
>  	return_type = TYPE_TARGET_TYPE (SYMBOL_TYPE (thisfun));
> +      else
> +	return_type = value_type (return_value);
>        if (return_type == NULL)
>  	return_type = builtin_type (get_frame_arch (thisframe))->builtin_int;
>        CHECK_TYPEDEF (return_type);
> @@ -1862,9 +1932,13 @@ If you continue, the return value that y
>        gdb_assert (gdbarch_return_value (gdbarch, func_type, return_type, NULL,
>  					NULL, NULL)
>  		  == RETURN_VALUE_REGISTER_CONVENTION);
> -      gdbarch_return_value (gdbarch, func_type, return_type,
> -			    get_current_regcache (), NULL /*read*/,
> -			    value_contents (return_value) /*write*/);
> +
> +      if (thisfun != NULL)
> +	gdbarch_return_value (gdbarch, func_type, return_type,
> +			      get_current_regcache (), NULL /*read*/,
> +			      value_contents (return_value) /*write*/);
> +      else
> +	return_extended_value (return_value);
>      }
>  
>    /* If we are at the end of a call dummy now, pop the dummy frame
> --- /dev/null	1 Jan 1970 00:00:00 -0000
> +++ gdb/testsuite/gdb.base/return-nodebug.c	20 Feb 2009 21:47:34 -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	20 Feb 2009 21:47:34 -0000
> @@ -0,0 +1,54 @@
> +# 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"]} {
> +	# 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 {{{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
> +    }
> +}
> 

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

[Jan Kratochvil]

On Wed, 11 Feb 2009 22:22:41 +0100, Mark Kettenis wrote:
> would it be an idea to use the type of the return value expression given by
> the user instead of int as a fallback?
> 
> 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.

It has been proven by the original bugreport even experienced programmers will
make a mistake forgetting about the type cast.  Therefore GDB "will not work"
from the user POV in the most common programming usecase of the `(gdb) return'
command:
* x86_64 as the current+future primary development architecture.
* `void *' (pointer) as the most common return type in modern code.
* `(gdb) return 0' to simulate a failed function
  (`0' because `return NULL' may produce: No symbol "NULL" in current context.)
Screenshot of the problem reproducer is at the bottom of this mail.

While I do not have such machine at hand (could try UAE - m68k - for caller
expecting `long long') I agree with your argument to my former unconditionally
`long long'-casting patch:
On Wed, 11 Feb 2009 23:44:21 +0100, Mark Kettenis wrote:
> I'm not convinced casting to `long long' is safe in all cases, especially on
> 32-bit big-endian machines.  It really might do the wrong thing there,
> exposing garbage or the wrong 32 bits of the 64-bit value.

To find a way out of the both requirements this patch implements:
> On Sat, 21 Feb 2009 01:04:09 +0100, Jan Kratochvil wrote:
> > +/* Set RETURN_VALUE extended to the largest type width which will still be the
> > +   same value after reading it back using the RETURN_VALUE type.  */

So this patch tries as best as it can the original intention of a widest cast
but still it verifies it has not broken the compatibility with the type
specified by user.  Specified either intentionally or unintentionally.

It has no regressions.  The only imperfection I find is on the big endian
32bit arches one has to use an explicitely cast for callers expecting `long
long'.  But it cannot be guessed better and big endian arches are minority.


Thanks for the review,
Jan


Reproducer for http://sourceware.org/ml/gdb-patches/2009-02/msg00260.html
w/the patch at http://sourceware.org/ml/gdb-patches/2009-02/msg00280.html:

cat >/tmp/return.c <<EOH
#include <stdio.h>
#include <stdint.h>

static void *
init (void)
{
  return (void *) (intptr_t) -1;
}

static void *
func (void)
{
  return (void *) 0xdeadf00d00c0ffee;
}

int
main (void)
{
  init ();
  printf ("%p\n", func ());
  return 0;
}
EOH
gcc -o /tmp/return /tmp/return.c -Wall # no -g
./gdb -nx /tmp/return
GNU gdb (GDB) 6.8.50.20090220-cvs
Copyright (C) 2009 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-unknown-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
(no debugging symbols found)
(gdb) break func
Breakpoint 1 at 0x4004dd
(gdb) run
Starting program: /tmp/return 
Breakpoint 1, 0x00000000004004dd in func ()
(gdb) return 0
Make selected stack frame return now? (y or n) y
#0  0x00000000004004f7 in main ()
(gdb) continue
Continuing.
0xffffffff00000000
^^^^^^^^^^^^^^^^^^ -> expected: (nil)
Program exited normally.

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

[Mark Kettenis]

> Date: Sat, 21 Feb 2009 13:47:24 +0100
> From: Jan Kratochvil <jan.kratochvil@redhat.com>
> 
> On Wed, 11 Feb 2009 22:22:41 +0100, Mark Kettenis wrote:
> > would it be an idea to use the type of the return value expression given by
> > the user instead of int as a fallback?
> > 
> > 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.
> 
> It has been proven by the original bugreport even experienced
> programmers will make a mistake forgetting about the type cast.

And I' say it again.  You're doing such users a far bigger favour by
distributing libraries with debug information. than with bloating gdb
with code to work around issues.

> Therefore GDB "will not work" from the user POV in the most common
> programming usecase of the `(gdb) return' command:
> * x86_64 as the current+future primary development architecture.
> * `void *' (pointer) as the most common return type in modern code.

You're pulling that one out of your arse for sure.

> * `(gdb) return 0' to simulate a failed function
>   (`0' because `return NULL' may produce: No symbol "NULL" in current context.)
> Screenshot of the problem reproducer is at the bottom of this mail.

Ah, yes.  Gieven the amd64 ISA zero-extends stores into the low 32
bits of a GPR, amd64_return_value() should do the same.

> While I do not have such machine at hand (could try UAE - m68k - for caller
> expecting `long long') I agree with your argument to my former unconditionally
> `long long'-casting patch:
> On Wed, 11 Feb 2009 23:44:21 +0100, Mark Kettenis wrote:
> > I'm not convinced casting to `long long' is safe in all cases, especially on
> > 32-bit big-endian machines.  It really might do the wrong thing there,
> > exposing garbage or the wrong 32 bits of the 64-bit value.
> 
> To find a way out of the both requirements this patch implements:
> > On Sat, 21 Feb 2009 01:04:09 +0100, Jan Kratochvil wrote:
> > > +/* Set RETURN_VALUE extended to the largest type width which will still be the
> > > +   same value after reading it back using the RETURN_VALUE type.  */
> 
> So this patch tries as best as it can the original intention of a widest cast
> but still it verifies it has not broken the compatibility with the type
> specified by user.  Specified either intentionally or unintentionally.

Well, I think your code violates the KISS principle.

> It has no regressions.  The only imperfection I find is on the big endian
> 32bit arches one has to use an explicitely cast for callers expecting `long
> long'.  But it cannot be guessed better and big endian arches are minority.

Perhaps that's true for the architectures that Red Hat ships products
for.  It's certainly not true for the industry as a whole.

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

[Jan Kratochvil]

On Sat, 21 Feb 2009 14:43:44 +0100, Mark Kettenis wrote:
> > It has been proven by the original bugreport even experienced
> > programmers will make a mistake forgetting about the type cast.
> 
> And I' say it again.  You're doing such users a far bigger favour by
> distributing libraries with debug information. than with bloating gdb
> with code to work around issues.

Fedora GDB already has a custom patch to make this more easy:
http://cvs.fedora.redhat.com/viewvc/rpms/gdb/devel/gdb-6.6-buildid-locate.patch?view=co
$ gdb --args sleep 1h
GNU gdb Fedora (6.8-29.fc10)
[...]
This GDB was configured as "x86_64-redhat-linux-gnu"...
(no debugging symbols found)
Missing separate debuginfos, use: debuginfo-install coreutils-6.12-18.fc10.x86_64
(gdb) q
$ gdb -q ./main
(gdb) r
Starting program: /md1/home/lace/src/main 
Program exited normally.
Missing separate debuginfos, use: debuginfo-install glibc-2.9-3.x86_64
(gdb) q

Still you can find in the bugreport(s) people do not do it:
  https://bugzilla.redhat.com/show_bug.cgi?id=365111

Mostly cited reason is that debuginfo files are currently too big, which is
being actively worked on:
  https://fedoraproject.org/wiki/Features/DebugInfoRevamp

The state of the debugging is not perfect but I hope Fedora does the best
effort it can for it instead of turning down patches without even giving
a reason now.


Regards,
Jan