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

[Mark Kettenis <mark.kettenis@xs4all.nl>]

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