[m68k] return values

[m68k] return values

[Nathan Sidwell]

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

this is a reworking of my recent return value patch.  this patch just contains 
changes to select which register contains a pointer value, and how structure 
return values are located.

I've added tdep field 'ptr_value_regnum' to accommodate the different ABIs that 
return in %d0 or %a0.

Most of the patch is adding the necessary pieces to the os sniffers to 
initialize that value.

ok?

nathan
-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk


[-- Attachment #2: return-value-1.patch --]
[-- Type: text/x-patch, Size: 9707 bytes --]

2006-06-09  Nathan Sidwell  <nathan@codesourcery.com>

	* configure.tgt (gdb_osabi): uclinux is like linux.
	* m68k-tdep.c (m68k_svr4_extract_return_value): Use
	tdep->ptr_value_regnum for pointer returns.
	(m68k_svr4_store_return_value): Likewise.
	(m68k_svr4_init_abi, m68k_aout_init_abi): Set ptr_value_regnum.
	(m68k_gdbarch_init): Likewise.
	* m68k-tdep.h (struct gdbarch_tdep): Add ptr_value_regnum
	field.
	* m68kbsd-tdep.c (m68kbsd_aout_init_abi): Use m68k_aout_init_abi.
	(m68kbsd_elf_init_abi): Add comment.
	* m68klinux-tdep.c (m68k_linux_init_abi): Just set the struct
	pointer register here.

Index: configure.tgt
===================================================================
RCS file: /cvs/src/src/gdb/configure.tgt,v
retrieving revision 1.172
diff -c -3 -p -r1.172 configure.tgt
*** configure.tgt	20 Apr 2006 23:18:48 -0000	1.172
--- configure.tgt	9 Jun 2006 12:45:08 -0000
*************** esac
*** 223,228 ****
--- 223,229 ----
  case "${target}" in
  *-*-freebsd*)	gdb_osabi=GDB_OSABI_FREEBSD_ELF ;;
  *-*-linux*)	gdb_osabi=GDB_OSABI_LINUX ;;
+ *-*-uclinux*)	gdb_osabi=GDB_OSABI_LINUX ;;
  *-*-nto*)	gdb_osabi=GDB_OSABI_QNXNTO ;;
  m68*-*-openbsd* | m88*-*-openbsd* | vax-*-openbsd*) ;;
  *-*-openbsd*)	gdb_osabi=GDB_OSABI_OPENBSD_ELF ;;
Index: m68k-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/m68k-tdep.c,v
retrieving revision 1.106
diff -c -3 -p -r1.106 m68k-tdep.c
*** m68k-tdep.c	8 Jun 2006 19:11:45 -0000	1.106
--- m68k-tdep.c	9 Jun 2006 12:45:09 -0000
*************** m68k_svr4_extract_return_value (struct t
*** 253,259 ****
        convert_typed_floating (buf, builtin_type_m68881_ext, valbuf, type);
      }
    else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
!     regcache_raw_read (regcache, M68K_A0_REGNUM, valbuf);
    else
      m68k_extract_return_value (type, regcache, valbuf);
  }
--- 253,261 ----
        convert_typed_floating (buf, builtin_type_m68881_ext, valbuf, type);
      }
    else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
!     regcache_raw_read (regcache,
! 		       gdbarch_tdep (current_gdbarch)->ptr_value_regnum,
! 		       valbuf);
    else
      m68k_extract_return_value (type, regcache, valbuf);
  }
*************** m68k_svr4_store_return_value (struct typ
*** 292,301 ****
        regcache_raw_write (regcache, M68K_FP0_REGNUM, buf);
      }
    else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
!     {
!       regcache_raw_write (regcache, M68K_A0_REGNUM, valbuf);
!       regcache_raw_write (regcache, M68K_D0_REGNUM, valbuf);
!     }
    else
      m68k_store_return_value (type, regcache, valbuf);
  }
--- 294,302 ----
        regcache_raw_write (regcache, M68K_FP0_REGNUM, buf);
      }
    else if (TYPE_CODE (type) == TYPE_CODE_PTR && len == 4)
!     regcache_raw_write (regcache,
! 			gdbarch_tdep (current_gdbarch)->ptr_value_regnum,
! 			valbuf);
    else
      m68k_store_return_value (type, regcache, valbuf);
  }
*************** m68k_svr4_return_value (struct gdbarch *
*** 370,394 ****
    if ((code == TYPE_CODE_STRUCT || code == TYPE_CODE_UNION)
        && !m68k_reg_struct_return_p (gdbarch, type))
      {
!       /* The System V ABI says that:
! 
! 	 "A function returning a structure or union also sets %a0 to
! 	 the value it finds in %a0.  Thus when the caller receives
! 	 control again, the address of the returned object resides in
! 	 register %a0."
! 
! 	 So the ABI guarantees that we can always find the return
! 	 value just after the function has returned.  */
! 
!       if (readbuf)
! 	{
! 	  ULONGEST addr;
! 
! 	  regcache_raw_read_unsigned (regcache, M68K_A0_REGNUM, &addr);
! 	  read_memory (addr, readbuf, TYPE_LENGTH (type));
! 	}
! 
!       return RETURN_VALUE_ABI_RETURNS_ADDRESS;
      }
  
    /* This special case is for structures consisting of a single
--- 371,381 ----
    if ((code == TYPE_CODE_STRUCT || code == TYPE_CODE_UNION)
        && !m68k_reg_struct_return_p (gdbarch, type))
      {
!       /* Although they SYSV ABI specifies that a function returning a
! 	 structure this way should preserve %a0, GCC doesn't do that.
! 	 Furthermore there's no point changeing GCC to make it do it,
! 	 as that would just be bloat. */
!       return RETURN_VALUE_STRUCT_CONVENTION;
      }
  
    /* This special case is for structures consisting of a single
*************** m68k_svr4_init_abi (struct gdbarch_info 
*** 1124,1131 ****
    /* SVR4 uses a different calling convention.  */
    set_gdbarch_return_value (gdbarch, m68k_svr4_return_value);
  
!   /* SVR4 uses %a0 instead of %a1.  */
    tdep->struct_value_regnum = M68K_A0_REGNUM;
  }
  \f
  
--- 1111,1137 ----
    /* SVR4 uses a different calling convention.  */
    set_gdbarch_return_value (gdbarch, m68k_svr4_return_value);
  
!   /* SVR4 uses %a0.  */
    tdep->struct_value_regnum = M68K_A0_REGNUM;
+   tdep->struct_return = reg_struct_return;
+   /* Pointers are returned in %a0 */
+   tdep->ptr_value_regnum = M68K_A0_REGNUM;
+ }
+ 
+ /* a.out */
+ 
+ void
+ m68k_aout_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
+ {
+   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
+ 
+   set_gdbarch_return_value (gdbarch, m68k_return_value);
+ 
+   /* aout uses %a1 */
+   tdep->struct_value_regnum = M68K_A1_REGNUM;
+   tdep->struct_return = reg_struct_return;
+   /* Pointers are returned in %a0 */
+   tdep->ptr_value_regnum = M68K_A0_REGNUM;
  }
  \f
  
*************** m68k_gdbarch_init (struct gdbarch_info i
*** 1175,1182 ****
    set_gdbarch_register_to_value (gdbarch,  m68k_register_to_value);
    set_gdbarch_value_to_register (gdbarch, m68k_value_to_register);
  
    set_gdbarch_push_dummy_call (gdbarch, m68k_push_dummy_call);
!   set_gdbarch_return_value (gdbarch, m68k_return_value);
  
    /* Disassembler.  */
    set_gdbarch_print_insn (gdbarch, print_insn_m68k);
--- 1181,1194 ----
    set_gdbarch_register_to_value (gdbarch,  m68k_register_to_value);
    set_gdbarch_value_to_register (gdbarch, m68k_value_to_register);
  
+   /* Function call & return */
    set_gdbarch_push_dummy_call (gdbarch, m68k_push_dummy_call);
!   /* These values are for bare metal -- os specific ABIs can override
!      them */
!   set_gdbarch_return_value (gdbarch, m68k_svr4_return_value);
!   tdep->struct_value_regnum = M68K_A0_REGNUM;
!   tdep->struct_return = reg_struct_return;
!   tdep->ptr_value_regnum = M68K_D0_REGNUM;
  
    /* Disassembler.  */
    set_gdbarch_print_insn (gdbarch, print_insn_m68k);
*************** m68k_gdbarch_init (struct gdbarch_info i
*** 1187,1194 ****
  #else
    tdep->jb_pc = -1;
  #endif
-   tdep->struct_value_regnum = M68K_A1_REGNUM;
-   tdep->struct_return = reg_struct_return;
  
    /* Frame unwinder.  */
    set_gdbarch_unwind_dummy_id (gdbarch, m68k_unwind_dummy_id);
--- 1199,1204 ----
Index: m68k-tdep.h
===================================================================
RCS file: /cvs/src/src/gdb/m68k-tdep.h,v
retrieving revision 1.10
diff -c -3 -p -r1.10 m68k-tdep.h
*** m68k-tdep.h	17 Dec 2005 22:34:01 -0000	1.10
--- m68k-tdep.h	9 Jun 2006 12:45:09 -0000
*************** struct gdbarch_tdep
*** 76,85 ****
--- 76,90 ----
  
    /* Convention for returning structures.  */
    enum struct_return struct_return;
+   
+   /* Register in which pointers are returned.  */
+   int ptr_value_regnum;
  };
  
  /* Initialize a SVR4 architecture variant.  */
  extern void m68k_svr4_init_abi (struct gdbarch_info, struct gdbarch *);
+ /* Initialize a aout architecture variant.   */
+ extern void m68k_aout_init_abi (struct gdbarch_info, struct gdbarch *);
  \f
  
  /* Functions exported from m68kbsd-tdep.c.  */
Index: m68kbsd-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/m68kbsd-tdep.c,v
retrieving revision 1.7
diff -c -3 -p -r1.7 m68kbsd-tdep.c
*** m68kbsd-tdep.c	17 Dec 2005 22:34:01 -0000	1.7
--- m68kbsd-tdep.c	9 Jun 2006 12:45:09 -0000
*************** m68kbsd_aout_init_abi (struct gdbarch_in
*** 206,212 ****
  
    m68kbsd_init_abi (info, gdbarch);
  
!   tdep->struct_return = reg_struct_return;
  
    tramp_frame_prepend_unwinder (gdbarch, &m68kobsd_sigtramp);
  }
--- 206,212 ----
  
    m68kbsd_init_abi (info, gdbarch);
  
!   m68k_aout_init_abi (info, gdbarch);
  
    tramp_frame_prepend_unwinder (gdbarch, &m68kobsd_sigtramp);
  }
*************** m68kbsd_elf_init_abi (struct gdbarch_inf
*** 222,227 ****
--- 222,228 ----
  
    /* NetBSD ELF uses the SVR4 ABI.  */
    m68k_svr4_init_abi (info, gdbarch);
+   /* But with pcc structure return */
    tdep->struct_return = pcc_struct_return;
  
    /* NetBSD ELF uses SVR4-style shared libraries.  */
Index: m68klinux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/m68klinux-tdep.c,v
retrieving revision 1.18
diff -c -3 -p -r1.18 m68klinux-tdep.c
*** m68klinux-tdep.c	20 Apr 2006 17:29:47 -0000	1.18
--- m68klinux-tdep.c	9 Jun 2006 12:45:09 -0000
*************** m68k_linux_init_abi (struct gdbarch_info
*** 291,298 ****
       address to store a structure value.  It also returns small
       structures in registers instead of memory.  */
    m68k_svr4_init_abi (info, gdbarch);
    tdep->struct_value_regnum = M68K_A1_REGNUM;
-   tdep->struct_return = reg_struct_return;
  
    frame_unwind_append_sniffer (gdbarch, m68k_linux_sigtramp_frame_sniffer);
  
--- 291,298 ----
       address to store a structure value.  It also returns small
       structures in registers instead of memory.  */
    m68k_svr4_init_abi (info, gdbarch);
+   /* But the struct pointer is in %a1 */
    tdep->struct_value_regnum = M68K_A1_REGNUM;
  
    frame_unwind_append_sniffer (gdbarch, m68k_linux_sigtramp_frame_sniffer);
  

Re: [m68k] return values

[Mark Kettenis]

> Date: Fri, 09 Jun 2006 14:05:23 +0100
> From: Nathan Sidwell <nathan@codesourcery.com>
> 
> this is a reworking of my recent return value patch.  this patch
> just contains changes to select which register contains a pointer
> value, and how structure return values are located.
> 
> I've added tdep field 'ptr_value_regnum' to accommodate the
> different ABIs that return in %d0 or %a0.
> 
> Most of the patch is adding the necessary pieces to the os sniffers to 
> initialize that value.
> 
> ok?

Please don't check this in until I've reviewed this.  Some time ago
(two years?) I did examine very carefully what ABI's where in you use
for Linux, NetBSD/m68k and OpenBSD/m68k, and some of the things in
your patch seem to contradict what I learned back then.

I will try to look at it ASAP.

Mark

Re: [m68k] return values

[Mark Kettenis]

> Date: Fri, 09 Jun 2006 14:05:23 +0100
> From: Nathan Sidwell <nathan@codesourcery.com>
> 
> this is a reworking of my recent return value patch.  this patch
> just contains changes to select which register contains a pointer
> value, and how structure return values are located.

OK, I see your problem now.  The uCLinux ABI is quite different from
the normal Linux ABI, and different from the "standard" embedded ABI
too.  If I read the GCC code correctly, on uCLinux you end up with:

#undef PCC_STATIC_STRUCT_RETURN
#define DEFAULT_PCC_STRUCT_RETURN 1
#define M68K_STRUCT_VALUE_REGNUM 8
#define FUNCTION_VALUE(VALTYPE, FUNC) gen_rtx_REG (TYPE_MODE (VALTYPE), 0)

whereas normal Linux has

#undef PCC_STATIC_STRUCT_RETURN
#define DEFAULT_PCC_STRUCT_RETURN 0
#define M68K_STRUCT_VALUE_REGNUM 9
#define FUNCTION_VALUE(VALTYPE, FUNC)  m68k_function_value (VALTYPE, FUNC)

This means that on uCLinux:

* Small structures are returned in memory.
* Pointer return values are returned in %d0.
* The memory for returning structures is passed in %a0.

whereas on normal Linux:

* Small structures are returned in registers (%d0/%d1 or %fp0).
* Pointer return values are returned in %a0.
* The memory for returning structures is passed in %a1.

I hope you can distinguish your uCLinux binaries from normal Linux
binaries, otherwise there is no way out of this mess.  You'll then
need to initialize the uCLinux gdbarch vector with something like:

static void
m68k_uclinux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
{
  struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);

  ...

  /* uCLinux uses its own calling convention, where all return values
     are stored in %d0/%d1 and structures are returned in memory at the
     address passed in %a0.  */

  tdep->struct_value_regnum = M68K_A0_REGNUM;
  tdep->struct_return = pcc_struct_return;

  ...

}

Re: [m68k] return values

[Nathan Sidwell]

Mark Kettenis wrote:

> OK, I see your problem now.  The uCLinux ABI is quite different from
> the normal Linux ABI, and different from the "standard" embedded ABI
> too.  If I read the GCC code correctly, on uCLinux you end up with:

I fear I have confused things with the configure.tgt fragment.  The main bulk of 
the patch was to make function return work for m68k-elf, which returns pointers 
in %d0. As that's an embedded target, it has no OS and so no OS sniffer.  As I 
understood gdb's internals, we should default to that ABI and use OS sniffers to 
set things up for OS ABIs that are different.

Is the patch ok without the configure.tgt fragment.  I can deal with uclinux 
separately.

nathan

-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

Re: [m68k] return values

[Mark Kettenis]

> Date: Mon, 19 Jun 2006 15:39:52 +0100
> From: Nathan Sidwell <nathan@codesourcery.com>
> 
> Mark Kettenis wrote:
> 
> > OK, I see your problem now.  The uCLinux ABI is quite different from
> > the normal Linux ABI, and different from the "standard" embedded ABI
> > too.  If I read the GCC code correctly, on uCLinux you end up with:
> 
> I fear I have confused things with the configure.tgt fragment.  The
> main bulk of the patch was to make function return work for
> m68k-elf, which returns pointers in %d0.

Well, I fear you've managed to confuse yourself.  The current m68k
default target uses m68k_extract_return_value(), which reads all
return values from %d0.

> As I understood gdb's internals, we should default to that ABI and
> use OS sniffers to set things up for OS ABIs that are different.

Indeed, although you can override the default ABI by setting gdb_osabi.

> Is the patch ok without the configure.tgt fragment.  I can deal with uclinux 
> separately.

I believe your patch is wrong and unneeded; dealing with uCLinux is
all you need to do.  And I'm afraid it might be better to start doing
that from scratch based on my comments.  Can't really blame you for
being confused though.  The various m68k ABI's are largely
undocumented and the GCC implementation isn't exactly very clear.

Mark

Re: [m68k] return values

[Nathan Sidwell]

Mark Kettenis wrote:

> Well, I fear you've managed to confuse yourself.  The current m68k
> default target uses m68k_extract_return_value(), which reads all
> return values from %d0.

you're right.  ok, the whole problem I'm trying to solve is to get the function 
return working for m68k-elf.  That appears to be a mixture of original and svr4 
features.  Namely:

a) returns always go in %d0 (original)
b) small structures can be passed in registers (svr4)
c) structure return values use the struct_return convention (svr4?)

What approach do you think would be best in tackling this?  The way I'd gone at 
it was to treat the m68k-elf as a variant of svr4 (i.e. #a was the exceptional 
behaviour).  It would be possible to organize it as original ABI, but with #b & 
#c as the exceptions.

Do you think the default should be compatible with m68k-elf, or should the user 
explicitly set the abi in that case?

As you may be aware, #b is implemented by GCC in a horrible way that is 
dependent on internal implementation details -- it's not just that the structure 
is 'small' or even 2^n bytes.  I have a patch that emulate's GCC's behaviour, 
and I was going to post that after this patch was resolved.

It certainly is an adventure trying to figure out what's meant to happen :)

nathan

-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk

Re: [m68k] return values

[Mark Kettenis]

> Date: Mon, 19 Jun 2006 16:28:47 +0100
> From: Nathan Sidwell <nathan@codesourcery.com>
> 
> Mark Kettenis wrote:
> 
> > Well, I fear you've managed to confuse yourself.  The current m68k
> > default target uses m68k_extract_return_value(), which reads all
> > return values from %d0.
> 
> you're right.  ok, the whole problem I'm trying to solve is to get
> the function return working for m68k-elf.  That appears to be a
> mixture of original and svr4 features.  Namely:
> 
> a) returns always go in %d0 (original)
> b) small structures can be passed in registers (svr4)
> c) structure return values use the struct_return convention (svr4?)
> 
> What approach do you think would be best in tackling this?  The way
> I'd gone at it was to treat the m68k-elf as a variant of svr4
> (i.e. #a was the exceptional behaviour).  It would be possible to
> organize it as original ABI, but with #b & #c as the exceptions.

Well, #b basically means reg_struct_return convention, which already
is the default.  And with #c you probably mean that the caller
allocates the memory to store the returned structure instead of having
the callee provide it in a static buffer (which is braindead since it
is non-reentrant).  The current code should cater for both, as long as
a pointer to the memory is returned in %d0.  Note that for SVR4 the
pointer is returned in %a0, but that makes no sense for m68k-elf since
normal pointers are returned in %d0.  So I believe that the default
m68k gdbarch vector should work just fine for m68k-elf.  But perhaps
it is a good idea to verify that the value is indeed returned in %d0.

> Do you think the default should be compatible with m68k-elf, or
> should the user explicitly set the abi in that case?

I believe the default gdbarch vector should be suitable for m68k-aout,
m68k-coff and m68k-elf.

> As you may be aware, #b is implemented by GCC in a horrible way that
> is dependent on internal implementation details -- it's not just
> that the structure is 'small' or even 2^n bytes.  I have a patch
> that emulate's GCC's behaviour, and I was going to post that after
> this patch was resolved.

And unfortunately GCC's implementation is almost a guaranteed to break
the ABI every now and then.  So it wouldn't surprise me at all if your
GCC is broken; or at least different from the GCC that comes with the
latest OpenBSD/mac68k release, which still uses GCC 2.95.3.  So it may
actually be impossible to solve this.  Fortunately you're probably
fixing corner cases in what is already a corner case; structures tend
to be larger than 8 bytes.

Could you send me the testsuite output for your m68k-elf target?  I'll
run the testsuite on my Quadra 800 tonight so we can compare results.

Mark

Re: [m68k] return values

[Nathan Sidwell]

Mark Kettenis wrote:

> Could you send me the testsuite output for your m68k-elf target?  I'll
> run the testsuite on my Quadra 800 tonight so we can compare results.

unfortunately my board has died, so it'll be a little while ...

nathan

-- 
Nathan Sidwell    ::   http://www.codesourcery.com   ::         CodeSourcery
nathan@codesourcery.com    ::     http://www.planetfall.pwp.blueyonder.co.uk