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

Mark Kettenis
2009-02-11T21:58:25Z

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