REG_STRUCT_HAS_ADDR

REG_STRUCT_HAS_ADDR

[Mark Kettenis]

By defining REG_STRUCT_HAS_ADDR, a target can indicate that structure
and union arguments are passed by reference instead of by value.

While working on the SPARC target, I read the following comment in
infcall.c:call_function_by_hand:

  if (REG_STRUCT_HAS_ADDR_P ())
    {

      ...

	      if (STACK_ALIGN_P ())
		/* MVS 11/22/96: I think at least some of this
		   stack_align code is really broken.  Better to let
		   PUSH_ARGUMENTS adjust the stack in a target-defined
		   manner.  */
		aligned_len = STACK_ALIGN (len);
	      else
		aligned_len = len;

I totally agree with Michael here, so I decided that the new SPARC
stuff shouldn't define REG_STRUCT_HAS_ADDR, and implement the whole
thing in its PUSH_DUMMY_CALL function.

Unfortunately this breaks debugging with stabs, since stabsread.c uses
the same REG_STRUCT_HAS_ADDR to see if a function argument is passed
by value or by reference.  I think we really want to get rid of the
broken code in infcall.c in the long run.  Therefore I looked for a
way to disable it for "modern" targets, i.e. targets that define
PUSH_DUMMY_CALL as opposed to the old PUSH_DUMMY_FRAME & friends.
Looking at the code I found that the following targets (besides SPARC)
are using REG_STRUCT_HAS_ADDR:

* MIPS EABI (32-bit and 64-bit)
* CRIS
* HP PA
* MN10300

(MCore defines REG_STRUCT_HAS_ADDR, but always returns 0, soo it
doesn't actually use the feature.)

Of these targets, only MIPS EABI uses the new PUSH_DUMMY_CALL.
Looking at mips_eabi_push_dummy_call, it seems as if it already
handles tries to pass the larger structures by reference.  That would
mean that it is indeed possible to change the

  if (REG_STRUCT_HAS_ADDR_P ())

into

  if (REG_STRUCT_HAS_ADDR_P ()
      && !gdbarch_push_dummy_call_p (current_gdbarch))

and be done with it.  I'm a little afraid however, that the MIPS code
in mips_eabi_push_dummy_call has never been tested, since in the
current situation it's dead code.  I don't have access to a MIPS
machine myself, is there anyone who could test the attached patch for
me on MIPS EABI?  Look for any regressions in gdb.base/call-ar-st.exp,
in particular:

FAIL: gdb.base/call-ar-st.exp: step into print_long_arg_list

Thanks,

Mark


Index: infcall.c
===================================================================
RCS file: /cvs/src/src/gdb/infcall.c,v
retrieving revision 1.24
diff -u -p -r1.24 infcall.c
--- infcall.c 28 Aug 2003 02:53:08 -0000 1.24
+++ infcall.c 6 Sep 2003 22:02:21 -0000
@@ -661,7 +661,8 @@ You must use a pointer to function type 
       }
   }
 
-  if (REG_STRUCT_HAS_ADDR_P ())
+  if (REG_STRUCT_HAS_ADDR_P ()
+      && !gdbarch_push_dummy_call_p (current_gdbcarch))
     {
       int i;
       /* This is a machine like the sparc, where we may need to pass a

Re: REG_STRUCT_HAS_ADDR

[Andrew Cagney]

> Unfortunately this breaks debugging with stabs, since stabsread.c uses
> the same REG_STRUCT_HAS_ADDR to see if a function argument is passed
> by value or by reference.  I think we really want to get rid of the
> broken code in infcall.c in the long run.  Therefore I looked for a
> way to disable it for "modern" targets, i.e. targets that define
> PUSH_DUMMY_CALL as opposed to the old PUSH_DUMMY_FRAME & friends.
> Looking at the code I found that the following targets (besides SPARC)
> are using REG_STRUCT_HAS_ADDR:

Instead, add a new method:
	STABS_REG_STRUCT_HAS_ADDR
with a default of:
	if (DEPRECATED_REG_STRUCT_HAS_ADDR_P ()
	  return REG_STRUCT ... ();
	else
	  return 0;
(no predicate) (I think I've got that right) (better name?) and 
deprecate REG_STRUCT_HAS_ADDR{,_P}.  I think stabsread.c can then switch 
to the STABS variant.

That should let you safely disentangle the SPARC without breaking 
"stabsread.c".

> -  if (REG_STRUCT_HAS_ADDR_P ())
> +  if (REG_STRUCT_HAS_ADDR_P ()
> +      && !gdbarch_push_dummy_call_p (current_gdbcarch))

The switch to push_dummy_call_p shouldn't cause unexpected side effects 
such as disabling REG_STRUCT_HAS_ADDR_P.  From memory, I've got the 
current code down to just one and even that doesn't really need it.

Andrew

Re: REG_STRUCT_HAS_ADDR

[Mark Kettenis]

   Date: Mon, 08 Sep 2003 14:43:41 -0400
   From: Andrew Cagney <ac131313@redhat.com>

   > Unfortunately this breaks debugging with stabs, since stabsread.c uses
   > the same REG_STRUCT_HAS_ADDR to see if a function argument is passed
   > by value or by reference.  I think we really want to get rid of the
   > broken code in infcall.c in the long run.  Therefore I looked for a
   > way to disable it for "modern" targets, i.e. targets that define
   > PUSH_DUMMY_CALL as opposed to the old PUSH_DUMMY_FRAME & friends.
   > Looking at the code I found that the following targets (besides SPARC)
   > are using REG_STRUCT_HAS_ADDR:

   Instead, add a new method:
	   STABS_REG_STRUCT_HAS_ADDR
   with a default of:
	   if (DEPRECATED_REG_STRUCT_HAS_ADDR_P ()
	     return REG_STRUCT ... ();
	   else
	     return 0;
   (no predicate) (I think I've got that right) (better name?) and 
   deprecate REG_STRUCT_HAS_ADDR{,_P}.  I think stabsread.c can then switch 
   to the STABS variant.

   That should let you safely disentangle the SPARC without breaking 
   "stabsread.c".

That's a good strategy.  However, making this stabs-specific seems
wrong to me.  The new method will simply indicate whether the ABI
silently uses pass by reference instead of pass by value for function
argument of a certain type.  Therefore I don't think we should put
STABS in the name.  What about PASS_ARGUMENT_BY_REFERENCE?  I'm going
to drop the GCC_P argument too, since nobody is using it.  I'll post a
patch later today.

   > -  if (REG_STRUCT_HAS_ADDR_P ())
   > +  if (REG_STRUCT_HAS_ADDR_P ()
   > +      && !gdbarch_push_dummy_call_p (current_gdbcarch))

   The switch to push_dummy_call_p shouldn't cause unexpected side effects 
   such as disabling REG_STRUCT_HAS_ADDR_P.  From memory, I've got the 
   current code down to just one and even that doesn't really need it.

I had doubts whether I could get away with this.  Anyway, this saves
me from building a MIPS cross-environment ;-).

Mark

Re: REG_STRUCT_HAS_ADDR

[Andrew Cagney]

> That's a good strategy.  However, making this stabs-specific seems
> wrong to me.  The new method will simply indicate whether the ABI
> silently uses pass by reference instead of pass by value for function
> argument of a certain type.  Therefore I don't think we should put
> STABS in the name.  What about PASS_ARGUMENT_BY_REFERENCE?  I'm going
> to drop the GCC_P argument too, since nobody is using it.  I'll post a
> patch later today.

To correctly determine if an arument is passed by reference requires the 
complete function prototype (same problem as push dummy call).  I think, 
rather than change the interface (will anyone actually fix/use that 
info?) this method should be burried as stabs only.  Leave the problem 
of defining/implementing a new interface to someone that needs it.

Andrew