From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 28716 invoked by alias); 16 Sep 2013 20:57:10 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 28703 invoked by uid 89); 16 Sep 2013 20:57:10 -0000 Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 16 Sep 2013 20:57:10 +0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.9 required=5.0 tests=ALL_TRUSTED,BAYES_00 autolearn=ham version=3.3.2 X-HELO: rock.gnat.com Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 70C681165DD; Mon, 16 Sep 2013 16:57:20 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id xN5Geq2v1aHw; Mon, 16 Sep 2013 16:57:20 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id EDAAD1165D5; Mon, 16 Sep 2013 16:57:19 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 77973E1290; Mon, 16 Sep 2013 13:57:05 -0700 (PDT) Date: Mon, 16 Sep 2013 20:57:00 -0000 From: Joel Brobecker To: Pierre Muller Cc: 'Mark Kettenis' , gdb-patches@sourceware.org Subject: Re: [RFC-v2] amd64-windows: Fix funcall with by-pointer arguments Message-ID: <20130916205705.GC3132@adacore.com> References: <1351099417-18960-1-git-send-email-brobecker@adacore.com> <201210251317.q9PDHrnJ026193@glazunov.sibelius.xs4all.nl> <20130116115840.GS6143@adacore.com> <003101ceb0ce$22cfd340$686f79c0$@muller@ics-cnrs.unistra.fr> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <003101ceb0ce$22cfd340$686f79c0$@muller@ics-cnrs.unistra.fr> User-Agent: Mutt/1.5.21 (2010-09-15) X-SW-Source: 2013-09/txt/msg00469.txt.bz2 Hi Pierre, On Sat, Sep 14, 2013 at 12:10:59AM +0200, Pierre Muller wrote: > As explained in the other thread, > I merged Joel's patch in this thread with my patch > in http://www.sourceware.org/ml/gdb-patches/2013-09/msg00145.html > > I attached the difference between two runs of: > make check RUNTESTFKLAGS="gdb.base/*.exp" > with current CVS GDB and with the patch below applied > (plus a few other submitted patches concerning > testsuite runs for *-*-ming* builds). > > The number of failure in gdb.base > drops from 326 to 243 FAILs. Thanks for doing that. I haven't completely finished reviewing the patch yet, but I wanted to send a message so that you and everyone else knows that I am on it. The patch looked good, minus some trivial formatting issues which I will fix, but I noticed a problem with function returning arrays. I am not sure where it is coming from, whether from me incorrectly applying your change, or missing something else, or whether it's a regression. I ran out of time for today, but will try to pursue this further tomorrow. Just one comment below: > 2013-09-12 Joel Brobecker > Pierre Muller > > * amd64-windows-tdep.c: #include "value.h" and "inferior.h". > (amd64_windows_classify): Delete. > (amd64_windows_passed_by_xmm_register) > (amd64_windows_return_in_xmm0_register) > (amd64_windows_passed_by_integer_register) > (amd64_windows_passed_by_pointer) > (amd64_windows_adjust_args_passed_by_pointer) > (amd64_windows_store_arg_in_reg, amd64_windows_push_arguments) > (amd64_windows_push_dummy_call): New functions. > (amd64_return_value): Use new functions above. Add "debug infrum" > information. > (amd64_windows_init_abi): Remove setting of > tdep->call_dummy_num_integer_regs, tdep->call_dummy_integer_regs, > tdep->classify, tdep->memory_args_by_pointer and > tdep->integer_param_regs_saved_in_caller_frame. > Add call to set_gdbarch_push_dummy_call. > /* The registers used to pass integer arguments during a function call. */ > static int amd64_windows_dummy_call_integer_regs[] = > { > AMD64_RCX_REGNUM, /* %rcx */ > AMD64_RDX_REGNUM, /* %rdx */ > - 8, /* %r8 */ > - 9 /* %r9 */ > + AMD64_R8_REGNUM, /* %r8 */ > + AMD64_R9_REGNUM /* %r9 */ This change can be checked in independently of this patch series, I think. > }; > > -/* Implement the "classify" method in the gdbarch_tdep structure > - for amd64-windows. */ > +/* Return nonzero if an argument of type TYPE should be passed > + via one of the XMM registers. */ > > -static void > -amd64_windows_classify (struct type *type, enum amd64_reg_class class[2]) > +static int > +amd64_windows_passed_by_xmm_register (struct type *type) > { > switch (TYPE_CODE (type)) > { > - case TYPE_CODE_ARRAY: > - /* Arrays are always passed by memory. */ > - class[0] = class[1] = AMD64_MEMORY; > - break; > + case TYPE_CODE_TYPEDEF: > + if (TYPE_TARGET_TYPE (type)) > + return amd64_windows_passed_by_xmm_register ( > + TYPE_TARGET_TYPE (type)); > + else > + return 0; > + case TYPE_CODE_STRUCT: > + case TYPE_CODE_UNION: > + if (TYPE_NFIELDS (type) == 1) > + return amd64_windows_passed_by_xmm_register ( > + TYPE_FIELD_TYPE (type, 0)); > + else > + return 0; > + case TYPE_CODE_FLT: > + case TYPE_CODE_DECFLOAT: > + return (TYPE_LENGTH (type) == 4 || TYPE_LENGTH (type) == 8); > + default: > + return 0; > + } > +} > + > +/* Return nonzero if an return value of type TYPE should be passed > + in XMM0 register. */ > + > +static int > +amd64_windows_return_in_xmm0_register (struct type *type) > +{ > + if (amd64_windows_passed_by_xmm_register (type)) > + return 1; > + else > + { > + /* __m128, __m128d and __m128i types are returned in $xmm0 also. > + But they are not stored in XMM registers as arguments, > + at least not in GCC 4.7.6 x86_64-w64-mingw32 target. > + Note: The TYPE_VECTOR check should prevent other arrays from > + being treated as special xmm types. > + Note: This all works for the "unique" __fastcall calling > convention > + as defined by Microsoft. The newer __vectorcall convention > + does support passing __m128X type parameters in xmm registers > + using /Gv option of Microsoft compilers. */ > + struct type *ltype = check_typedef (type); > + if (ltype && TYPE_CODE (ltype) == TYPE_CODE_ARRAY > + && (TYPE_CODE (TYPE_TARGET_TYPE (ltype)) == TYPE_CODE_FLT > + || TYPE_CODE (TYPE_TARGET_TYPE (ltype)) == TYPE_CODE_INT) > + && TYPE_VECTOR (ltype) && TYPE_LENGTH (ltype) == 16) > + return 1; > + else > + return 0; > + } > +} > + > +/* Return nonzero if an argument of type TYPE should be passed > + via one of the integer registers. */ > > +static int > +amd64_windows_passed_by_integer_register (struct type *type) > +{ > + switch (TYPE_CODE (type)) > + { > + case TYPE_CODE_TYPEDEF: > + if (TYPE_TARGET_TYPE (type)) > + return amd64_windows_passed_by_integer_register ( > + TYPE_TARGET_TYPE (type)); > + else > + return 0; > case TYPE_CODE_STRUCT: > case TYPE_CODE_UNION: > - /* Struct/Union types whose size is 1, 2, 4, or 8 bytes > - are passed as if they were integers of the same size. > - Types of different sizes are passed by memory. */ > - if (TYPE_LENGTH (type) == 1 > - || TYPE_LENGTH (type) == 2 > - || TYPE_LENGTH (type) == 4 > - || TYPE_LENGTH (type) == 8) > + if (TYPE_NFIELDS (type) == 1) > { > - class[0] = AMD64_INTEGER; > - class[1] = AMD64_NO_CLASS; > + /* If only one field, exclude float type accepted as xmm reg. > */ > + if (amd64_windows_passed_by_xmm_register ( > + TYPE_FIELD_TYPE (type, 0))) > + return 0; > + else > + return amd64_windows_passed_by_integer_register ( > + TYPE_FIELD_TYPE (type, 0)); > } > - else > - class[0] = class[1] = AMD64_MEMORY; > - break; > + /* Struct/Union with several fields: Pass Through. */ > + case TYPE_CODE_INT: > + case TYPE_CODE_ENUM: > + case TYPE_CODE_BOOL: > + case TYPE_CODE_RANGE: > + case TYPE_CODE_CHAR: > + case TYPE_CODE_PTR: > + case TYPE_CODE_REF: > + case TYPE_CODE_COMPLEX: > + return (TYPE_LENGTH (type) == 1 > + || TYPE_LENGTH (type) == 2 > + || TYPE_LENGTH (type) == 4 > + || TYPE_LENGTH (type) == 8); > > default: > - /* For all the other types, the conventions are the same as > - with the System V ABI. */ > - amd64_classify (type, class); > + return 0; > } > } > > +/* Return non-zero iff an argument of the given TYPE should be passed > + by pointer. */ > + > +static int > +amd64_windows_passed_by_pointer (struct type *type) > +{ > + if (amd64_windows_passed_by_integer_register (type)) > + return 0; > + > + if (amd64_windows_passed_by_xmm_register (type)) > + return 0; > + > + return 1; > +} > + > +/* For each argument that should be passed by pointer, reserve some > + stack space, store a copy of the argument on the stack, and replace > + the argument by its address. Return the new Stack Pointer value. > + > + NARGS is the number of arguments. ARGS is the array containing > + the value of each argument. SP is value of the Stack Pointer. */ > + > +static CORE_ADDR > +amd64_windows_adjust_args_passed_by_pointer (struct gdbarch *gdbarch, > + struct value **args, > + int nargs, CORE_ADDR sp) > +{ > + int i; > + > + for (i = 0; i < nargs; i++) > + if (amd64_windows_passed_by_pointer (value_type (args[i]))) > + { > + struct type *type = value_type (args[i]); > + const gdb_byte *valbuf = value_contents (args[i]); > + const int len = TYPE_LENGTH (type); > + > + /* Store a copy of that argument on the stack, aligned to > + a 16 bytes boundary, and then use the copy's address as > + the argument. */ > + > + sp -= len; > + sp &= ~0xf; > + write_memory (sp, valbuf, len); > + > + args[i] = > + value_addr (value_from_contents_and_address (type, valbuf, sp)); > + if (debug_infrun) > + printf_filtered (_("Parameter #%d, length %d copied on stack " > + "at address %s, and passed as pointer\n"), > + i, len, paddress (gdbarch, sp)); > + } > + > + return sp; > +} > + > +/* Store the value of ARG in register REGNO (right-justified). > + REGCACHE is the register cache. */ > + > +static void > +amd64_windows_store_arg_in_reg (struct gdbarch *gdbarch, > + struct regcache *regcache, > + struct value *arg, int regno, int i) > +{ > + struct type *type = value_type (arg); > + const gdb_byte *valbuf = value_contents (arg); > + gdb_byte buf[8]; > + > + gdb_assert (TYPE_LENGTH (type) <= 8); > + memset (buf, 0, sizeof buf); > + memcpy (buf, valbuf, min (TYPE_LENGTH (type), 8)); > + regcache_cooked_write (regcache, regno, buf); > + if (debug_infrun) > + printf_filtered (_("Parameter #%d, length %d copied to register %s\n"), > + i, TYPE_LENGTH (type), i386_pseudo_register_name( > + gdbarch, regno)); > +} > + > +/* Push the arguments for an inferior function call, and return > + the updated value of the SP (Stack Pointer). > + > + All arguments are identical to the arguments used in > + amd64_windows_push_dummy_call. */ > + > +static CORE_ADDR > +amd64_windows_push_arguments (struct gdbarch *gdbarch, > + struct regcache *regcache, int nargs, > + struct value **args, CORE_ADDR sp, > + int struct_return) > +{ > + int reg_idx = 0; > + int i; > + struct value **stack_args = alloca (nargs * sizeof (struct value *)); > + int num_stack_args = 0; > + int num_elements = 0; > + int element = 0; > + > + /* First, handle the arguments passed by pointer. > + > + These arguments are replaced by pointers to a copy we are making > + in inferior memory. So use a copy of the ARGS table, to avoid > + modifying the original one. */ > + { > + struct value **args1 = alloca (nargs * sizeof (struct value *)); > + > + memcpy (args1, args, nargs * sizeof (struct value *)); > + sp = amd64_windows_adjust_args_passed_by_pointer (gdbarch, args1, > + nargs, sp); > + args = args1; > + } > + > + /* Reserve a register for the "hidden" argument. */ > + if (struct_return) > + reg_idx++; > + > + for (i = 0; i < nargs; i++) > + { > + struct type *type = value_type (args[i]); > + int len = TYPE_LENGTH (type); > + int on_stack_p = 1; > + > + if (reg_idx < ARRAY_SIZE (amd64_windows_dummy_call_integer_regs)) > + { > + if (amd64_windows_passed_by_integer_register (type)) > + { > + amd64_windows_store_arg_in_reg > + (gdbarch, regcache, args[i], > + amd64_windows_dummy_call_integer_regs[reg_idx], i); > + on_stack_p = 0; > + reg_idx++; > + } > + else if (amd64_windows_passed_by_xmm_register (type)) > + { > + amd64_windows_store_arg_in_reg > + (gdbarch, regcache, args[i], AMD64_XMM0_REGNUM + reg_idx, > i); > + /* In case of varargs, these parameters must also be > + passed via the integer registers. */ > + amd64_windows_store_arg_in_reg > + (gdbarch, regcache, args[i], > + amd64_windows_dummy_call_integer_regs[reg_idx], i); > + on_stack_p = 0; > + reg_idx++; > + } > + } > + > + if (on_stack_p) > + { > + num_elements += ((len + 7) / 8); > + stack_args[num_stack_args++] = args[i]; > + } > + } > + > + /* Allocate space for the arguments on the stack, keeping it > + aligned on a 16 byte boundary. */ > + sp -= num_elements * 8; > + sp &= ~0xf; > + > + /* Write out the arguments to the stack. */ > + for (i = 0; i < num_stack_args; i++) > + { > + struct type *type = value_type (stack_args[i]); > + const gdb_byte *valbuf = value_contents (stack_args[i]); > + > + write_memory (sp + element * 8, valbuf, TYPE_LENGTH (type)); > + if (debug_infrun) > + printf_filtered (_("Parameter #%d, length %d copied on stack" > + " at address %s\n"), > + i, TYPE_LENGTH (type), > + paddress (gdbarch, sp + element * 8)); > + element += ((TYPE_LENGTH (type) + 7) / 8); > + } > + > + return sp; > +} > + > +/* Implement the "push_dummy_call" gdbarch method. */ > + > +static CORE_ADDR > +amd64_windows_push_dummy_call > + (struct gdbarch *gdbarch, struct value *function, > + struct regcache *regcache, CORE_ADDR bp_addr, > + int nargs, struct value **args, > + CORE_ADDR sp, int struct_return, CORE_ADDR struct_addr) > +{ > + enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); > + gdb_byte buf[8]; > + > + /* Pass arguments. */ > + sp = amd64_windows_push_arguments (gdbarch, regcache, nargs, args, sp, > + struct_return); > + > + /* Pass "hidden" argument". */ > + if (struct_return) > + { > + /* The "hidden" argument is passed throught the first argument > + register. */ > + const int arg_regnum = amd64_windows_dummy_call_integer_regs[0]; > + > + store_unsigned_integer (buf, 8, byte_order, struct_addr); > + regcache_cooked_write (regcache, arg_regnum, buf); > + } > + > + /* Reserve some memory on the stack for the integer-parameter > + registers, as required by the ABI. */ > + sp -= ARRAY_SIZE (amd64_windows_dummy_call_integer_regs) * 8; > + > + /* Store return address. */ > + sp -= 8; > + store_unsigned_integer (buf, 8, byte_order, bp_addr); > + write_memory (sp, buf, 8); > + > + /* Update the stack pointer... */ > + store_unsigned_integer (buf, 8, byte_order, sp); > + regcache_cooked_write (regcache, AMD64_RSP_REGNUM, buf); > + > + /* ...and fake a frame pointer. */ > + regcache_cooked_write (regcache, AMD64_RBP_REGNUM, buf); > + > + return sp + 16; > +} > + > /* Implement the "return_value" gdbarch method for amd64-windows. */ > > static enum return_value_convention > @@ -90,22 +379,10 @@ amd64_windows_return_value (struct gdbar > > /* See if our value is returned through a register. If it is, then > store the associated register number in REGNUM. */ > - switch (TYPE_CODE (type)) > - { > - case TYPE_CODE_FLT: > - case TYPE_CODE_DECFLOAT: > - /* __m128, __m128i, __m128d, floats, and doubles are returned > - via XMM0. */ > - if (len == 4 || len == 8 || len == 16) > - regnum = AMD64_XMM0_REGNUM; > - break; > - default: > - /* All other values that are 1, 2, 4 or 8 bytes long are returned > - via RAX. */ > - if (len == 1 || len == 2 || len == 4 || len == 8) > - regnum = AMD64_RAX_REGNUM; > - break; > - } > + if (amd64_windows_return_in_xmm0_register (type)) > + regnum = AMD64_XMM0_REGNUM; > + else if (amd64_windows_passed_by_integer_register (type)) > + regnum = AMD64_RAX_REGNUM; > > if (regnum < 0) > { > @@ -117,8 +394,10 @@ amd64_windows_return_value (struct gdbar > regcache_raw_read_unsigned (regcache, AMD64_RAX_REGNUM, &addr); > read_memory (addr, readbuf, TYPE_LENGTH (type)); > } > + if (debug_infrun) > + printf_filtered (_("Return value as memory address in RAX\n")); > return RETURN_VALUE_ABI_RETURNS_ADDRESS; > - } > + } > else > { > /* Extract the return value from the register where it was stored. > */ > @@ -126,6 +405,9 @@ amd64_windows_return_value (struct gdbar > regcache_raw_read_part (regcache, regnum, 0, len, readbuf); > if (writebuf) > regcache_raw_write_part (regcache, regnum, 0, len, writebuf); > + if (debug_infrun) > + printf_filtered (_("Return value in register %s\n"), > + gdbarch_register_name (gdbarch, regnum)); > return RETURN_VALUE_REGISTER_CONVENTION; > } > } > @@ -976,12 +1258,7 @@ amd64_windows_init_abi (struct gdbarch_i > set_gdbarch_long_bit (gdbarch, 32); > > /* Function calls. */ > - tdep->call_dummy_num_integer_regs = > - ARRAY_SIZE (amd64_windows_dummy_call_integer_regs); > - tdep->call_dummy_integer_regs = amd64_windows_dummy_call_integer_regs; > - tdep->classify = amd64_windows_classify; > - tdep->memory_args_by_pointer = 1; > - tdep->integer_param_regs_saved_in_caller_frame = 1; > + set_gdbarch_push_dummy_call (gdbarch, amd64_windows_push_dummy_call); > set_gdbarch_return_value (gdbarch, amd64_windows_return_value); > set_gdbarch_skip_main_prologue (gdbarch, amd64_skip_main_prologue); > set_gdbarch_skip_trampoline_code (gdbarch, -- Joel