[RFA/RFC/sparc32] SIGSEGV during fun call when no debugging info

[RFA/RFC/sparc32] SIGSEGV during fun call when no debugging info

[Joel Brobecker]

[-- Attachment #1: Type: text/plain, Size: 2788 bytes --]

Hello

Looks like we're no longer able to call functions on sparc target
when the function hasn't been compiled with debugging information.
I noticed this because calls to functions taking strings as an argument
were now causing the debugger to crash - but the problem can be reduced
to calling "malloc":

    (gdb) p malloc (1)
    zsh: 21029 segmentation fault (core dumped)  ../gdb-head/gdb/gdb hello

The problem has been introduced during the enhancement for multiple
calling conventions, for which we added an extra argument (the function
type) to the "struct_return" gdbarch method.  The sparc-tdep.c code
was then changed as follow:

    -  if (using_struct_return (value_type))
    +  if (using_struct_return (SYMBOL_TYPE (find_pc_function (funcaddr)),
    +                          value_type))

Because "malloc" hasn't been compiled with debugging info, find_pc_function
return NULL, and so SYMBOL_TYPE causes a SEGV.

One approach would be to enhance the code above to also handle function
calls when no symbol is found. This is what find_function_in_inferior
does, and we could do something similar.  But seems a bit absurd to do
that, because we don't have multiple calling conventions on sparc.
So finding the type would be useless.

Following on the last observation, another way of fixing the problem
would be to simply pass NULL as the function type to using_struct_return,
since we don't need to check the function type to do what we need
to do in this case.

One final approach is to change the code in sparc32_push_dummy_code
to call sparc32_return_value directly, instead of going through
using_struct_return. That was more appealing that computing the
function type, but that hard code the assumption that the return_value
gdbarch method is in fact sparc32_return_value, and also re-introduces
a new instance of "current_gdbarch".

I think the best way to go in this case is option 2, which is to accept
NULL as the function type, and document that fact.  This is what my 
patch does.

I checked Corinna's patch (which introduced this extra parameter),
and there are only 2 instances that really use that new parameter,
both in sh-tdep.c: sh_return_value_nofpu and sh_return_value_fpu.
They already both handle the case when the function type is NULL
by defaulting to the user-setting "set sh calling-convention".

2008-04-29  Joel Brobecker  <brobecker@adacore.com>

	* gdbarch.sh: Document the return_value method. Explain that
	the FUNCTYPE parameter might be NULL.
	* gdbarch.h: Regenerated.
	* sparc-tdep.c (sparc32_push_dummy_code): Do not pass the function
	type when calling using_struct_return, as this is unnecessary
	on this target.

Tested on sparc-solaris. Fixes many many regressions in gdb.ada,
gdb.base and gdb.gdb.

OK to apply?

Thanks,
-- 
Joel

[-- Attachment #2: struct_return.diff --]
[-- Type: text/plain, Size: 3079 bytes --]

Index: gdbarch.sh
===================================================================
--- gdbarch.sh	(revision 130365)
+++ gdbarch.sh	(working copy)
@@ -474,10 +474,16 @@ f:CORE_ADDR:pointer_to_address:struct ty
 f:void:address_to_pointer:struct type *type, gdb_byte *buf, CORE_ADDR addr:type, buf, addr::unsigned_address_to_pointer::0
 M:CORE_ADDR:integer_to_address:struct type *type, const gdb_byte *buf:type, buf
 
-# It has been suggested that this, well actually its predecessor,
-# should take the type/value of the function to be called and not the
-# return type.  This is left as an exercise for the reader.
-
+# Return the return-value convention that will be used by FUNCTYPE
+# to return a value of type VALTYPE.  FUNCTYPE may be NULL in which
+# case the return convention is computed based only on VALTYPE.
+#
+# If READBUF is not NULL, extract the return value and save it in this buffer.
+#
+# If WRITEBUF is not NULL, it contains a return value which will be
+# stored into the appropriate register.  This can be used when we want
+# to force the value returned by a function (see the "return" command
+# for instance).
 M:enum return_value_convention:return_value:struct type *functype, struct type *valtype, struct regcache *regcache, gdb_byte *readbuf, const gdb_byte *writebuf:functype, valtype, regcache, readbuf, writebuf
 
 m:CORE_ADDR:skip_prologue:CORE_ADDR ip:ip:0:0
Index: gdbarch.h
===================================================================
--- gdbarch.h	(revision 130365)
+++ gdbarch.h	(working copy)
@@ -371,9 +371,16 @@ typedef CORE_ADDR (gdbarch_integer_to_ad
 extern CORE_ADDR gdbarch_integer_to_address (struct gdbarch *gdbarch, struct type *type, const gdb_byte *buf);
 extern void set_gdbarch_integer_to_address (struct gdbarch *gdbarch, gdbarch_integer_to_address_ftype *integer_to_address);
 
-/* It has been suggested that this, well actually its predecessor,
-   should take the type/value of the function to be called and not the
-   return type.  This is left as an exercise for the reader. */
+/* Return the return-value convention that will be used by FUNCTYPE
+   to return a value of type VALTYPE.  FUNCTYPE may be NULL in which
+   case the return convention is computed based only on VALTYPE.
+  
+   If READBUF is not NULL, extract the return value and save it in this buffer.
+  
+   If WRITEBUF is not NULL, it contains a return value which will be
+   stored into the appropriate register.  This can be used when we want
+   to force the value returned by a function (see the "return" command
+   for instance). */
 
 extern int gdbarch_return_value_p (struct gdbarch *gdbarch);
 
Index: sparc-tdep.c
===================================================================
--- sparc-tdep.c	(revision 130365)
+++ sparc-tdep.c	(working copy)
@@ -388,8 +388,7 @@ sparc32_push_dummy_code (struct gdbarch 
   *bp_addr = sp - 4;
   *real_pc = funcaddr;
 
-  if (using_struct_return (SYMBOL_TYPE (find_pc_function (funcaddr)),
-			   value_type))
+  if (using_struct_return (NULL, value_type))
     {
       gdb_byte buf[4];
 

Re: [RFA/RFC/sparc32] SIGSEGV during fun call when no debugging info

[Mark Kettenis]

> Date: Mon, 28 Apr 2008 20:47:05 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> 
> One approach would be to enhance the code above to also handle function
> calls when no symbol is found. This is what find_function_in_inferior
> does, and we could do something similar.  But seems a bit absurd to do
> that, because we don't have multiple calling conventions on sparc.
> So finding the type would be useless.

Makes sense to me.

> 2008-04-29  Joel Brobecker  <brobecker@adacore.com>
> 
> 	* gdbarch.sh: Document the return_value method. Explain that
> 	the FUNCTYPE parameter might be NULL.
> 	* gdbarch.h: Regenerated.
> 	* sparc-tdep.c (sparc32_push_dummy_code): Do not pass the function
> 	type when calling using_struct_return, as this is unnecessary
> 	on this target.
> 
> Tested on sparc-solaris. Fixes many many regressions in gdb.ada,
> gdb.base and gdb.gdb.
> 
> OK to apply?

sure

Re: [RFA/RFC/sparc32] SIGSEGV during fun call when no debugging info

[Joel Brobecker]

Thanks, everyone, for the feedback.

> > 2008-04-29  Joel Brobecker  <brobecker@adacore.com>
> > 
> > 	* gdbarch.sh: Document the return_value method. Explain that
> > 	the FUNCTYPE parameter might be NULL.
> > 	* gdbarch.h: Regenerated.
> > 	* sparc-tdep.c (sparc32_push_dummy_code): Do not pass the function
> > 	type when calling using_struct_return, as this is unnecessary
> > 	on this target.
> > 
> > Tested on sparc-solaris. Fixes many many regressions in gdb.ada,
> > gdb.base and gdb.gdb.
> > 
> > OK to apply?
> 
> sure

Now checked in.

Thank you,
-- 
Joel

Re: [RFA/RFC/sparc32] SIGSEGV during fun call when no debugging info

[Corinna Vinschen]

On Apr 28 20:47, Joel Brobecker wrote:
> I think the best way to go in this case is option 2, which is to accept
> NULL as the function type, and document that fact.  This is what my 
> patch does.
> 
> I checked Corinna's patch (which introduced this extra parameter),
> and there are only 2 instances that really use that new parameter,
> both in sh-tdep.c: sh_return_value_nofpu and sh_return_value_fpu.
> They already both handle the case when the function type is NULL
> by defaulting to the user-setting "set sh calling-convention".

The idea is of course that not only sh uses this at one point.  In
theory, other targets are using different calling conventions in
parallel as well, like x86 on Windows (cdecl, fastcall, stdcall, etc.),
but the Dwarf-2 calling convention isn't used for that so far, mostly
because Dwarf-2 isn't used by default.

In my first approach I set the func_type parameter simply to NULL in
sparc32_push_dummy_code, too, but for some reason I thought that's just
my laziness :}  Allowing NULL for the func_type seems right to me,
though.


Thanks for catching,
Corinna

> 2008-04-29  Joel Brobecker  <brobecker@adacore.com>
> 
> 	* gdbarch.sh: Document the return_value method. Explain that
> 	the FUNCTYPE parameter might be NULL.
> 	* gdbarch.h: Regenerated.
> 	* sparc-tdep.c (sparc32_push_dummy_code): Do not pass the function
> 	type when calling using_struct_return, as this is unnecessary
> 	on this target.


-- 
Corinna Vinschen
Cygwin Project Co-Leader
Red Hat