From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 81729 invoked by alias); 9 Jun 2017 14:29:41 -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 80565 invoked by uid 89); 9 Jun 2017 14:29:39 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.0 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=principles, Hx-languages-length:6566 X-HELO: mail-wr0-f169.google.com Received: from mail-wr0-f169.google.com (HELO mail-wr0-f169.google.com) (209.85.128.169) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 09 Jun 2017 14:29:37 +0000 Received: by mail-wr0-f169.google.com with SMTP id q97so35305260wrb.2 for ; Fri, 09 Jun 2017 07:29:41 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=JhIR+tXrtuumTWdlZn2iMRHdLHAA4M1D3fEdDWt7Zew=; b=XpKW9ro/hONeh+/Ry00P89EC5Remw7nje3gBDXFvZQdFgeE/sIjzWMwscJHoClbwDv UOh81OUd6t/5waOUF/Bm71ZsFAH6nr9MSLSpxod/g55o3OocthhktXhijp9UTSCtNBtF AcspiF+Eg4jmdHvg0dB5INbxQxphCUnv2jctHTj+dY02G0WGZlcEMeg2pO/qGO9UwPpQ pHzVXKAPg8Y8A6WuM6BTy71NexQcz4UhO265FuiHuPUsOPcilpCbJWIrPrK9sYXnEnlA anIgHATE4T56y6dOIlN42svHI3S3ooTKBDgJyFgkoFKYNQ/KLY0G0sY0dqGRnhMhLjAu oABw== X-Gm-Message-State: AODbwcCmh9ESBEt5uqW46kuU3MzZqVOxBCTOWgFEKYPvPIGiVzXmFQPv ZmnsTahFABAkEuFR X-Received: by 10.223.176.122 with SMTP id g55mr30867655wra.172.1497018579865; Fri, 09 Jun 2017 07:29:39 -0700 (PDT) Received: from [192.168.0.102] ([37.189.166.198]) by smtp.gmail.com with ESMTPSA id t27sm1223361wra.35.2017.06.09.07.29.38 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 09 Jun 2017 07:29:38 -0700 (PDT) Subject: Re: [PATCH 3/11] Add MIPS_MAX_REGISTER_SIZE (4/4) To: "Maciej W. Rozycki" References: <3C00280E-37C9-4C0A-9DA6-F3B9DB1A6E8F@arm.com> <86y3v7uf9j.fsf@gmail.com> <0150DDF9-6204-4F4F-99E9-D757C1DBD512@arm.com> <434A7317-C19A-4B53-8CB1-C7B4ACEC7D17@arm.com> Cc: Alan Hayward , Yao Qi , "gdb-patches@sourceware.org" , nd From: Pedro Alves Message-ID: Date: Fri, 09 Jun 2017 14:29:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-06/txt/msg00236.txt.bz2 On 06/09/2017 02:23 PM, Maciej W. Rozycki wrote: > Hardcoding things, and especially with literals, causes a maintenance > pain when things change. Bad code elsewhere is not an excuse; besides, > the other places where `8' is hardcoded are not (buffer) limits, but just > handle specific register sizes, which are not going to change, and which > are a completely different matter. Perhaps you won't be surprised to learn that I subscribe to the same general principles. However, I believe that this isn't, in fact, a completely different matter, because the buffer in question is used to hold the contents of a structure's address, to either put in a general register, or in a stack slot, and those (addresses and general registers), unlike FPRs, aren't expected to grow for a given architecture. > So if you find `alloca' unacceptable, > then the limit has to be a macro, and an assertion check is also due (as > already proposed), preferably using `sizeof', so that a future change does > not break it by accident. I think a patch like the below is likely to clarify things. (Builds, but is otherwise untested). >From 5bdf1c939d72bf2718a542164d99bb396f0526e3 Mon Sep 17 00:00:00 2001 From: Pedro Alves Date: Fri, 9 Jun 2017 15:10:50 +0100 Subject: [PATCH] mips --- gdb/mips-tdep.c | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/gdb/mips-tdep.c b/gdb/mips-tdep.c index 82f91ba..7844c73 100644 --- a/gdb/mips-tdep.c +++ b/gdb/mips-tdep.c @@ -271,6 +271,9 @@ mips_isa_regsize (struct gdbarch *gdbarch) / gdbarch_bfd_arch_info (gdbarch)->bits_per_byte); } +/* Max saved register size. */ +#define MAX_MIPS_ABI_REGSIZE 8 + /* Return the currently configured (or set) saved register size. */ unsigned int @@ -4476,7 +4479,7 @@ mips_eabi_push_dummy_call (struct gdbarch *gdbarch, struct value *function, int stack_offset = 0; enum bfd_endian byte_order = gdbarch_byte_order (gdbarch); CORE_ADDR func_addr = find_function_addr (function, NULL); - int regsize = mips_abi_regsize (gdbarch); + int abi_regsize = mips_abi_regsize (gdbarch); /* For shared libraries, "t9" needs to point at the function address. */ @@ -4499,7 +4502,7 @@ mips_eabi_push_dummy_call (struct gdbarch *gdbarch, struct value *function, than necessary for EABI, because the first few arguments are passed in registers, but that's OK. */ for (argnum = 0; argnum < nargs; argnum++) - len += align_up (TYPE_LENGTH (value_type (args[argnum])), regsize); + len += align_up (TYPE_LENGTH (value_type (args[argnum])), abi_regsize); sp -= align_up (len, 16); if (mips_debug) @@ -4528,7 +4531,9 @@ mips_eabi_push_dummy_call (struct gdbarch *gdbarch, struct value *function, for (argnum = 0; argnum < nargs; argnum++) { const gdb_byte *val; - gdb_byte valbuf[MAX_REGISTER_SIZE]; + /* This holds the address of structures that are passed by + reference. */ + gdb_byte ref_valbuf[MAX_MIPS_ABI_REGISTER_SIZE]; struct value *arg = args[argnum]; struct type *arg_type = check_typedef (value_type (arg)); int len = TYPE_LENGTH (arg_type); @@ -4541,13 +4546,14 @@ mips_eabi_push_dummy_call (struct gdbarch *gdbarch, struct value *function, /* The EABI passes structures that do not fit in a register by reference. */ - if (len > regsize + if (len > abi_regsize && (typecode == TYPE_CODE_STRUCT || typecode == TYPE_CODE_UNION)) { - store_unsigned_integer (valbuf, regsize, byte_order, + gdb_assert (ARRAY_SIZE (ref_valbuf) >= abi_regsize); + store_unsigned_integer (ref_valbuf, abi_regsize, byte_order, value_address (arg)); typecode = TYPE_CODE_PTR; - len = regsize; + len = abi_regsize; val = valbuf; if (mips_debug) fprintf_unfiltered (gdb_stdlog, " push"); @@ -4560,7 +4566,7 @@ mips_eabi_push_dummy_call (struct gdbarch *gdbarch, struct value *function, up before the check to see if there are any FP registers left. Non MIPS_EABI targets also pass the FP in the integer registers so also round up normal registers. */ - if (regsize < 8 && fp_register_arg_p (gdbarch, typecode, arg_type)) + if (abi_regsize < 8 && fp_register_arg_p (gdbarch, typecode, arg_type)) { if ((float_argreg & 1)) float_argreg++; @@ -4626,12 +4632,12 @@ mips_eabi_push_dummy_call (struct gdbarch *gdbarch, struct value *function, /* Copy the argument to general registers or the stack in register-sized pieces. Large arguments are split between registers and stack. */ - /* Note: structs whose size is not a multiple of regsize + /* Note: structs whose size is not a multiple of abi_regsize are treated specially: Irix cc passes them in registers where gcc sometimes puts them on the stack. For maximum compatibility, we will put them in both places. */ - int odd_sized_struct = (len > regsize && len % regsize != 0); + int odd_sized_struct = (len > abi_regsize && len % abi_regsize != 0); /* Note: Floating-point values that didn't fit into an FP register are only written to memory. */ @@ -4639,7 +4645,7 @@ mips_eabi_push_dummy_call (struct gdbarch *gdbarch, struct value *function, { /* Remember if the argument was written to the stack. */ int stack_used_p = 0; - int partial_len = (len < regsize ? len : regsize); + int partial_len = (len < abi_regsize ? len : abi_regsize); if (mips_debug) fprintf_unfiltered (gdb_stdlog, " -- partial=%d", @@ -4657,15 +4663,15 @@ mips_eabi_push_dummy_call (struct gdbarch *gdbarch, struct value *function, stack_used_p = 1; if (gdbarch_byte_order (gdbarch) == BFD_ENDIAN_BIG) { - if (regsize == 8 + if (abi_regsize == 8 && (typecode == TYPE_CODE_INT || typecode == TYPE_CODE_PTR || typecode == TYPE_CODE_FLT) && len <= 4) - longword_offset = regsize - len; + longword_offset = abi_regsize - len; else if ((typecode == TYPE_CODE_STRUCT || typecode == TYPE_CODE_UNION) - && TYPE_LENGTH (arg_type) < regsize) - longword_offset = regsize - len; + && TYPE_LENGTH (arg_type) < abi_regsize) + longword_offset = abi_regsize - len; } if (mips_debug) @@ -4706,7 +4712,7 @@ mips_eabi_push_dummy_call (struct gdbarch *gdbarch, struct value *function, if (mips_debug) fprintf_filtered (gdb_stdlog, " - reg=%d val=%s", argreg, - phex (regval, regsize)); + phex (regval, abi_regsize)); regcache_cooked_write_signed (regcache, argreg, regval); argreg++; } @@ -4721,7 +4727,7 @@ mips_eabi_push_dummy_call (struct gdbarch *gdbarch, struct value *function, only needs to be adjusted when it has been used. */ if (stack_used_p) - stack_offset += align_up (partial_len, regsize); + stack_offset += align_up (partial_len, abi_regsize); } } if (mips_debug) -- 2.5.5