From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 8868 invoked by alias); 10 Oct 2003 01:27:16 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 8857 invoked from network); 10 Oct 2003 01:27:14 -0000 Received: from unknown (HELO localhost.redhat.com) (207.219.125.105) by sources.redhat.com with SMTP; 10 Oct 2003 01:27:14 -0000 Received: from redhat.com (localhost [127.0.0.1]) by localhost.redhat.com (Postfix) with ESMTP id 3C0AC2B89; Thu, 9 Oct 2003 21:27:12 -0400 (EDT) Message-ID: <3F860AF0.5040104@redhat.com> Date: Fri, 10 Oct 2003 01:27:00 -0000 From: Andrew Cagney User-Agent: Mozilla/5.0 (X11; U; NetBSD macppc; en-US; rv:1.0.2) Gecko/20030820 X-Accept-Language: en-us, en MIME-Version: 1.0 To: Kevin Buettner Cc: gdb-patches@sources.redhat.com Subject: Re: [rfa:ppc64] Fix 64-bit PPC ELF function calls References: <3F6E368C.30009@redhat.com> <3F6F388D.5020706@redhat.com> <1031003212231.ZM26624@localhost.localdomain> <3F81EA72.4030601@redhat.com> <1031009050157.ZM11014@localhost.localdomain> Content-Type: multipart/mixed; boundary="------------030207080102010601080902" X-SW-Source: 2003-10/txt/msg00342.txt.bz2 This is a multi-part message in MIME format. --------------030207080102010601080902 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Content-length: 3708 > On Oct 6, 6:19pm, Andrew Cagney wrote: > > >> > I notice some 80+ character lines in ppc64_sysv_abi_push_dummy_call(). >> > Could you adjust these so that they're 80 characters or less? > >> >> I'll run the file through gdb_indent.sh, as a separate commit. > > > I see that you've already done it. Yes, obvious. > Don't forget to do it again > after you commit your ppc64_sysv_abi_push_dummy_call() changes. The attached has already been re-indented. Otherwize the diff ends up containing unnecessary white space changes. > Here's another typo: [...] > s/ment/meant/ [...] > s/Fortunatly/Fortunately/ Fixed, along with a few others. > Not yet. > > Please add > > gdb_assert (tdep->wordsize == 8) > > somewhere early in ppc64_sysv_abi_push_dummy_call(). Added, along with your comments. > Anyway, in this instance, I think it's advisable to avoid the names > "top" and "bottom" altogether. Perhaps ``param_end'' is a more > appropriate name? I replaced that variable with vparam_size and gparam_size. > + /* Address, on the stack, of the next general (integer, struct, > + float, ...) parameter. */ > + CORE_ADDR gparam = 0; > + /* Address, on the stack, of the next vector. */ > + CORE_ADDR vparam = 0; > > These are addresses when write_pass == 1, but merely a running > total of the number of words needed when write_pass == 0. Calling > them addresses is misleading. > > + /* Go through the argument list twice. > + > + Pass 1: Figure out how much stack space is required for arguments > + and pushed values. > + > + Pass 2: Replay the same computation but this time also write the > + values out to the target. */ > > Perhaps you could explain the dual roles of gparam and vparam in the > above comment? Yes but slightly further down where they are computed. > + if (!write_pass) > + { > + vparam = align_down (param_top - vparam, 16); > + gparam = align_down (vparam - gparam, 16); > + sp = align_down (gparam - 48, 16); > + } > > The ABI says: > > The parameter save area, which is located at a fixed offset of 48 > bytes from the stack pointer, is reserved in each stack frame for use > as an argument list. A minimum of 8 doublewords is always reserved. > > I don't see where you've accounted for this 8 doubleword minimum. > Perhaps revise this to something along the following lines? Good point. > if (!write_pass) > { > vparam = align_down (param_top - vparam, 16); > if (gparam < 8 * tdep->wordsize) > gparam = 8 * tdep->wordsize; > gparam = align_down (vparam - gparam, 16); > sp = align_down (gparam - 48, 16); > } > > I'm not sure this is right either since that alters the vector save > area. I don't happen to have a copy of the Altivec ABI though. Done (but also rearanged). The vector save area can be zero in size. > + if (greg <= 10) > + { > + /* It goes into the register, left aligned (as > + far as I can tell). */ > > I can't find anything in the ABI to contradict this, but I found it > surprising that floating point arguments would be left aligned. If > I'm not mistaken (for big endian), this'll mean that the the float > is stored in the most significant half of the register. It now reads: /* The ABI states "Single precision floating point values are mapped to the first word in a single doubleword" and "... floating point values mapped to the first eight doublewords of the parameter save area are also passed in general registers"). This code interprets that to mean: store it, left aligned, in the general register. */ Andrew --------------030207080102010601080902 Content-Type: text/plain; name="diffs" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="diffs" Content-length: 14085 2003-10-09 Andrew Cagney * Makefile.in (ppc-sysv-tdep.o): Add $(gdb_assert_h). * rs6000-tdep.c (rs6000_gdbarch_init): When 64 bit SysV ABI, set push_dummy_call to ppc64_sysv_abi_push_dummy_call. * ppc-sysv-tdep.c: Include "gdb_assert.h". (ppc64_sysv_abi_push_dummy_call): New function. (ppc64_sysv_abi_broken_push_dummy_call): New function. * ppc-tdep.h (ppc64_sysv_abi_push_dummy_call): Declare. (ppc64_sysv_abi_broken_push_dummy_call): Declare. Index: Makefile.in =================================================================== RCS file: /cvs/src/src/gdb/Makefile.in,v retrieving revision 1.452 diff -u -r1.452 Makefile.in --- Makefile.in 6 Oct 2003 21:56:40 -0000 1.452 +++ Makefile.in 10 Oct 2003 01:25:03 -0000 @@ -2114,7 +2114,7 @@ $(target_h) $(breakpoint_h) $(value_h) $(osabi_h) $(ppc_tdep_h) \ $(ppcnbsd_tdep_h) $(nbsd_tdep_h) $(solib_svr4_h) ppc-sysv-tdep.o: ppc-sysv-tdep.c $(defs_h) $(gdbcore_h) $(inferior_h) \ - $(regcache_h) $(value_h) $(gdb_string_h) $(ppc_tdep_h) + $(regcache_h) $(value_h) $(gdb_string_h) $(gdb_assert_h) $(ppc_tdep_h) printcmd.o: printcmd.c $(defs_h) $(gdb_string_h) $(frame_h) $(symtab_h) \ $(gdbtypes_h) $(value_h) $(language_h) $(expression_h) $(gdbcore_h) \ $(gdbcmd_h) $(target_h) $(breakpoint_h) $(demangle_h) $(valprint_h) \ Index: ppc-sysv-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/ppc-sysv-tdep.c,v retrieving revision 1.14 diff -u -r1.14 ppc-sysv-tdep.c --- ppc-sysv-tdep.c 6 Oct 2003 22:23:47 -0000 1.14 +++ ppc-sysv-tdep.c 10 Oct 2003 01:25:03 -0000 @@ -26,7 +26,7 @@ #include "regcache.h" #include "value.h" #include "gdb_string.h" - +#include "gdb_assert.h" #include "ppc-tdep.h" /* Pass the arguments in either registers, or in the stack. Using the @@ -315,6 +315,295 @@ return 0; return (TYPE_LENGTH (value_type) > 8); +} + +/* Pass the arguments in either registers, or in the stack. Using the + ppc 64 bit SysV ABI. + + This implements a dumbed down version of the ABI. It always writes + values to memory, GPR and FPR, even when not necessary. Doing this + greatly simplifies the logic. */ + +CORE_ADDR +ppc64_sysv_abi_push_dummy_call (struct gdbarch *gdbarch, CORE_ADDR func_addr, + struct regcache *regcache, CORE_ADDR bp_addr, + int nargs, struct value **args, CORE_ADDR sp, + int struct_return, CORE_ADDR struct_addr) +{ + struct gdbarch_tdep *tdep = gdbarch_tdep (current_gdbarch); + /* By this stage in the proceedings, SP has been decremented by "red + zone size" + "struct return size". Fetch the stack-pointer from + before this and use that as the BACK_CHAIN. */ + const CORE_ADDR back_chain = read_sp (); + /* See for-loop comment below. */ + int write_pass; + /* Size of the Altivec's vector parameter region, the final value is + computed in the for-loop below. */ + LONGEST vparam_size = 0; + /* Size of the general parameter region, the final value is computed + in the for-loop below. */ + LONGEST gparam_size = 0; + /* Kevin writes ... I don't mind seeing tdep->wordsize used in the + calls to align_up(), align_down(), etc. because this makes it + easier to reuse this code (in a copy/paste sense) in the future, + but it is a 64-bit ABI and asserting that the wordsize is 8 bytes + at some point makes it easier to verify that this function is + correct without having to do a non-local analysis to figure out + the possible values of tdep->wordsize. */ + gdb_assert (tdep->wordsize == 8); + + /* Go through the argument list twice. + + Pass 1: Compute the function call's stack space and register + requirements. + + Pass 2: Replay the same computation but this time also write the + values out to the target. */ + + for (write_pass = 0; write_pass < 2; write_pass++) + { + int argno; + /* Next available floating point register for float and double + arguments. */ + int freg = 1; + /* Next available general register for non-vector (but possibly + float) arguments. */ + int greg = 3; + /* Next available vector register for vector arguments. */ + int vreg = 2; + /* The address, at which the next general purpose parameter + (integer, struct, float, ...) should be saved. */ + CORE_ADDR gparam; + /* Address, at which the next Altivec vector parameter should be + saved. */ + CORE_ADDR vparam; + + if (!write_pass) + { + /* During the first pass, GPARAM and VPARAM are more like + offsets (start address zero) than addresses. That way + the accumulate the total stack space each region + requires. */ + gparam = 0; + vparam = 0; + } + else + { + /* Decrement the stack pointer making space for the Altivec + and general on-stack parameters. Set vparam and gparam + to their corresponding regions. */ + vparam = align_down (sp - vparam_size, 16); + gparam = align_down (vparam - gparam_size, 16); + /* Add in space for the TOC, link editor double word, + compiler double word, LR save area, CR save area. */ + sp = align_down (gparam - 48, 16); + } + + /* If the function is returning a `struct', then there is an + extra hidden parameter (which will be passed in r3) + containing the address of that struct.. In that case we + should advance one word and start from r4 register to copy + parameters. This also consumes one on-stack parameter slot. */ + if (struct_return) + { + if (write_pass) + regcache_cooked_write_signed (regcache, + tdep->ppc_gp0_regnum + greg, + struct_addr); + greg++; + gparam = align_up (gparam + tdep->wordsize, tdep->wordsize); + } + + for (argno = 0; argno < nargs; argno++) + { + struct value *arg = args[argno]; + struct type *type = check_typedef (VALUE_TYPE (arg)); + char *val = VALUE_CONTENTS (arg); + if (TYPE_CODE (type) == TYPE_CODE_FLT && TYPE_LENGTH (type) <= 8) + { + /* Floats and Doubles go in f1 .. f13. They also + consume a left aligned GREG,, and can end up in + memory. */ + if (write_pass) + { + if (ppc_floating_point_unit_p (current_gdbarch) + && freg <= 13) + { + char regval[MAX_REGISTER_SIZE]; + struct type *regtype = register_type (gdbarch, + FP0_REGNUM); + convert_typed_floating (val, type, regval, regtype); + regcache_cooked_write (regcache, FP0_REGNUM + freg, + regval); + } + if (greg <= 10) + { + /* The ABI states "Single precision floating + point values are mapped to the first word in + a single doubleword" and "... floating point + values mapped to the first eight doublewords + of the parameter save area are also passed in + general registers"). + + This code interprets that to mean: store it, + left aligned, in the general register. */ + char regval[MAX_REGISTER_SIZE]; + memset (regval, 0, sizeof regval); + memcpy (regval, val, TYPE_LENGTH (type)); + regcache_cooked_write (regcache, + tdep->ppc_gp0_regnum + greg, + regval); + } + write_memory (gparam, val, TYPE_LENGTH (type)); + } + /* Always consume parameter stack space. */ + freg++; + greg++; + gparam = align_up (gparam + TYPE_LENGTH (type), tdep->wordsize); + } + else if (TYPE_LENGTH (type) == 16 && TYPE_VECTOR (type) + && TYPE_CODE (type) == TYPE_CODE_ARRAY + && tdep->ppc_vr0_regnum >= 0) + { + /* In the Altivec ABI, vectors go in the vector + registers v2 .. v13, or when that runs out, a vector + annex which goes above all the normal parameters. + NOTE: cagney/2003-09-21: This is a guess based on the + PowerOpen Altivec ABI. */ + if (vreg <= 13) + { + if (write_pass) + regcache_cooked_write (regcache, + tdep->ppc_vr0_regnum + vreg, val); + vreg++; + } + else + { + if (write_pass) + write_memory (vparam, val, TYPE_LENGTH (type)); + vparam = align_up (vparam + TYPE_LENGTH (type), 16); + } + } + else if ((TYPE_CODE (type) == TYPE_CODE_INT + || TYPE_CODE (type) == TYPE_CODE_ENUM) + && TYPE_LENGTH (type) <= 8) + { + /* Scalars get sign[un]extended and go in gpr3 .. gpr10. + They can also end up in memory. */ + if (write_pass) + { + /* Sign extend the value, then store it unsigned. */ + ULONGEST word = unpack_long (type, val); + if (greg <= 10) + regcache_cooked_write_unsigned (regcache, + tdep->ppc_gp0_regnum + + greg, word); + write_memory_unsigned_integer (gparam, tdep->wordsize, + word); + } + greg++; + gparam = align_up (gparam + TYPE_LENGTH (type), tdep->wordsize); + } + else + { + int byte; + for (byte = 0; byte < TYPE_LENGTH (type); + byte += tdep->wordsize) + { + if (write_pass && greg <= 10) + { + char regval[MAX_REGISTER_SIZE]; + int len = TYPE_LENGTH (type) - byte; + if (len > tdep->wordsize) + len = tdep->wordsize; + memset (regval, 0, sizeof regval); + /* WARNING: cagney/2003-09-21: As best I can + tell, the ABI specifies that the value should + be left aligned. Unfortunately, GCC doesn't + do this - it instead right aligns even sized + values and puts odd sized values on the + stack. Work around that by putting both a + left and right aligned value into the + register (hopefully no one notices :-^). + Arrrgh! */ + /* Left aligned (8 byte values such as pointers + fill the buffer). */ + memcpy (regval, val + byte, len); + /* Right aligned (but only if even). */ + if (len == 1 || len == 2 || len == 4) + memcpy (regval + tdep->wordsize - len, + val + byte, len); + regcache_cooked_write (regcache, greg, regval); + } + greg++; + } + if (write_pass) + /* WARNING: cagney/2003-09-21: Strictly speaking, this + isn't necessary, unfortunately, GCC appears to get + "struct convention" parameter passing wrong putting + odd sized structures in memory instead of in a + register. Work around this by always writing the + value to memory. Fortunately, doing this + simplifies the code. */ + write_memory (gparam, val, TYPE_LENGTH (type)); + /* Always consume parameter stack space. */ + gparam = align_up (gparam + TYPE_LENGTH (type), tdep->wordsize); + } + } + + if (!write_pass) + { + /* Save the true region sizes ready for the second pass. */ + vparam_size = vparam; + /* Make certain that the general parameter save area is at + least the minimum 8 registers (or doublewords) in size. */ + if (greg < 8) + gparam_size = 8 * tdep->wordsize; + else + gparam_size = gparam; + } + } + + /* Update %sp. */ + regcache_cooked_write_signed (regcache, SP_REGNUM, sp); + + /* Write the backchain (it occupies WORDSIZED bytes). */ + write_memory_signed_integer (sp, tdep->wordsize, back_chain); + + /* Point the inferior function call's return address at the dummy's + breakpoint. */ + regcache_cooked_write_signed (regcache, tdep->ppc_lr_regnum, bp_addr); + + /* Find a value for the TOC register. Every symbol should have both + ".FN" and "FN" in the minimal symbol table. "FN" points at the + FN's descriptor, while ".FN" points at the entry point (which + matches FUNC_ADDR). Need to reverse from FUNC_ADDR back to the + FN's descriptor address. */ + { + /* Find the minimal symbol that corresponds to FUNC_ADDR (should + have the name ".FN"). */ + struct minimal_symbol *dot_fn = lookup_minimal_symbol_by_pc (func_addr); + if (dot_fn != NULL && SYMBOL_LINKAGE_NAME (dot_fn)[0] == '.') + { + /* Now find the corresponding "FN" (dropping ".") minimal + symbol's address. */ + struct minimal_symbol *fn = + lookup_minimal_symbol (SYMBOL_LINKAGE_NAME (dot_fn) + 1, NULL, + NULL); + if (fn != NULL) + { + /* Got the address of that descriptor. The TOC is the + second double word. */ + CORE_ADDR toc = + read_memory_unsigned_integer (SYMBOL_VALUE_ADDRESS (fn) + + tdep->wordsize, tdep->wordsize); + regcache_cooked_write_unsigned (regcache, + tdep->ppc_gp0_regnum + 2, toc); + } + } + } + + return sp; } Index: ppc-tdep.h =================================================================== RCS file: /cvs/src/src/gdb/ppc-tdep.h,v retrieving revision 1.19 diff -u -r1.19 ppc-tdep.h --- ppc-tdep.h 3 Oct 2003 21:11:39 -0000 1.19 +++ ppc-tdep.h 10 Oct 2003 01:25:03 -0000 @@ -42,6 +42,13 @@ struct value **args, CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr); +CORE_ADDR ppc64_sysv_abi_push_dummy_call (struct gdbarch *gdbarch, + CORE_ADDR func_addr, + struct regcache *regcache, + CORE_ADDR bp_addr, int nargs, + struct value **args, CORE_ADDR sp, + int struct_return, + CORE_ADDR struct_addr); int ppc_linux_memory_remove_breakpoint (CORE_ADDR addr, char *contents_cache); struct link_map_offsets *ppc_linux_svr4_fetch_link_map_offsets (void); void ppc_linux_supply_gregset (char *buf); Index: rs6000-tdep.c =================================================================== RCS file: /cvs/src/src/gdb/rs6000-tdep.c,v retrieving revision 1.166 diff -u -r1.166 rs6000-tdep.c --- rs6000-tdep.c 3 Oct 2003 21:11:39 -0000 1.166 +++ rs6000-tdep.c 10 Oct 2003 01:25:04 -0000 @@ -2959,6 +2959,8 @@ revisited. */ if (sysv_abi && wordsize == 4) set_gdbarch_push_dummy_call (gdbarch, ppc_sysv_abi_push_dummy_call); + else if (sysv_abi && wordsize == 8) + set_gdbarch_push_dummy_call (gdbarch, ppc64_sysv_abi_push_dummy_call); else set_gdbarch_push_dummy_call (gdbarch, rs6000_push_dummy_call); --------------030207080102010601080902--