From patchwork Mon Feb 18 23:39:12 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andrew Burgess X-Patchwork-Id: 31515 Received: (qmail 71647 invoked by alias); 18 Feb 2019 23:39:19 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 71619 invoked by uid 89); 18 Feb 2019 23:39:18 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_PASS autolearn=ham version=3.3.2 spammy=sparc-like, rv64, Leon3, RV64 X-HELO: mail-wr1-f68.google.com Received: from mail-wr1-f68.google.com (HELO mail-wr1-f68.google.com) (209.85.221.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 18 Feb 2019 23:39:16 +0000 Received: by mail-wr1-f68.google.com with SMTP id n2so8033903wrw.8 for ; Mon, 18 Feb 2019 15:39:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=QJKnlpeZYm0mlqqSb20S17Ae+CKHF9Mb1ormZ6g2zO0=; b=PL3jZIs6Ol2duuDDVwL6iVEVJ1G4tIKQyE2PiAHywNA3TcfTAE49UOkTodOATE1Dxf 7sM5lUOQbispbWl+M1cnw8UWRmWtWxekjIifnhbsmVRar1jetkxY62mP7b9DLAcxmFdm 6Pv91EMba8NoNN1nXuPumhQy7Nawa75uBiQY7AX5xdezvKuZ5O1T1/AsIUDm7LTMhGE4 e0ezZUTDSsLMshPfSM5JMXr023MtbOeAyfEc68hmsra1jezG0kqp3uLLOWnicORTUC9S g8PfNiWpX691iVA0Sa4DNExBRMFRse67rrWds6JcBSnVyT8K9bCRPbDi67j8NGb2lUz5 7MgA== Return-Path: Received: from localhost (host86-135-139-200.range86-135.btcentralplus.com. [86.135.139.200]) by smtp.gmail.com with ESMTPSA id n2sm24398853wrq.58.2019.02.18.15.39.13 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Mon, 18 Feb 2019 15:39:13 -0800 (PST) Date: Mon, 18 Feb 2019 23:39:12 +0000 From: Andrew Burgess To: Tom Tromey Cc: Joel Brobecker , gdb-patches@sourceware.org Subject: Re: [RFA] (riscv/ada) fix error when calling functions with range argument Message-ID: <20190218233912.GK19092@embecosm.com> References: <1549805906-1627-1-git-send-email-brobecker@adacore.com> <87wom56j7u.fsf@tromey.com> <87ftss7ll9.fsf@tromey.com> <20190214032254.GB2945@adacore.com> <20190217124708.GA25164@adacore.com> <87sgwmv6dm.fsf@tromey.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <87sgwmv6dm.fsf@tromey.com> X-Fortune: I guess it was all a DREAM ... or an episode of HAWAII FIVE-O ... X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] User-Agent: Mutt/1.9.2 (2017-12-15) X-IsSubscribed: yes * Tom Tromey [2019-02-17 07:25:57 -0700]: > >>>>> "Joel" == Joel Brobecker writes: > > Joel> Attached is a patch which does just that. > > Joel> gdb/ChangeLog: > > Joel> * gdbtypes.c (type_align): Handle TYPE_CODE_RANGE the same as > Joel> integers and enumeration types. > > Joel> Tested on x86_64-linux. Also tested on a variety of platforms > Joel> (with CPUs being ARM, AArch64, Leon3 (SPARC-like), PowerPC, > Joel> PowerPC64, RV64, Visium, x86, x86_64). > > Joel> OK to push? > > Yes, thanks for doing this. > > I still have a to-do item to convert the various arch-specific > type-alignment functions to use type_align. In some cases I'm not sure > how I'd test this though... So I took a look at doing this switch for RISC-V, and the problem is the handling of vectors. The RISC-V alignment code contains this: case TYPE_CODE_ARRAY: if (TYPE_VECTOR (t)) return std::min (TYPE_LENGTH (t), (unsigned) BIGGEST_ALIGNMENT); /* FALLTHROUGH */ case TYPE_CODE_COMPLEX: return riscv_type_alignment (TYPE_TARGET_TYPE (t)); While the generic type_align just does this: case TYPE_CODE_ARRAY: case TYPE_CODE_COMPLEX: case TYPE_CODE_TYPEDEF: align = type_align (TYPE_TARGET_TYPE (type)); break; So, identical in the non-vector array case, but different in the vector case. The RISC-V vector handling is required, it fixes an issue on RV64 in the gdb.base/gnu_vector.exp case. I can see that at least ARM and AARCH64 also have different rules for vectors than for arrays. I wonder if we should change the relationship between 'type_align' and 'gdbarch_type_align'? Currently 'gdbarch_type_align' is only called for "base" types, int, float, etc. But not compound types, arrays, structs, complex, etc. What if instead, we always called 'gdbarch_type_align' from 'type_align' and we had 'default_type_align' return 0 for compound types, and only provide an alignment for the "base" types. Then 'type_align' would inspect the value returned from 'gdbarch_type_align', and if it sees 0, it breaks the compound type up, and calls itself on the component type. Below is a rough, untested patch of this idea, feedback welcome. Thanks, Andrew diff --git a/gdb/arch-utils.c b/gdb/arch-utils.c index d2e27d95558..86cfe3c7f5b 100644 --- a/gdb/arch-utils.c +++ b/gdb/arch-utils.c @@ -993,7 +993,35 @@ default_in_indirect_branch_thunk (gdbarch *gdbarch, CORE_ADDR pc) ULONGEST default_type_align (struct gdbarch *gdbarch, struct type *type) { - return type_length_units (check_typedef (type)); + ULONGEST align; + + switch (TYPE_CODE (type)) + { + case TYPE_CODE_PTR: + case TYPE_CODE_FUNC: + case TYPE_CODE_FLAGS: + case TYPE_CODE_INT: + case TYPE_CODE_RANGE: + case TYPE_CODE_FLT: + case TYPE_CODE_ENUM: + case TYPE_CODE_REF: + case TYPE_CODE_RVALUE_REF: + case TYPE_CODE_CHAR: + case TYPE_CODE_BOOL: + case TYPE_CODE_DECFLOAT: + case TYPE_CODE_METHODPTR: + case TYPE_CODE_MEMBERPTR: + align = type_length_units (check_typedef (type)); + break; + default: + /* Return 0 for more compound types, the type_align function in + gdbtypes.c will take care of breaking these types up and calling + this method again on the component types. */ + align = 0; + break; + } + + return align; } void diff --git a/gdb/gdbtypes.c b/gdb/gdbtypes.c index 0d293393dae..1d4515d02f3 100644 --- a/gdb/gdbtypes.c +++ b/gdb/gdbtypes.c @@ -2992,11 +2992,17 @@ type_raw_align (struct type *type) unsigned type_align (struct type *type) { + /* Check alignment provided in the debug information. */ unsigned raw_align = type_raw_align (type); if (raw_align != 0) return raw_align; - ULONGEST align = 0; + /* Allow the architecture to provide an alignment. */ + struct gdbarch *arch = get_type_arch (type); + ULONGEST align = gdbarch_type_align (arch, type); + if (align != 0) + return align; + switch (TYPE_CODE (type)) { case TYPE_CODE_PTR: @@ -3013,10 +3019,7 @@ type_align (struct type *type) case TYPE_CODE_DECFLOAT: case TYPE_CODE_METHODPTR: case TYPE_CODE_MEMBERPTR: - { - struct gdbarch *arch = get_type_arch (type); - align = gdbarch_type_align (arch, type); - } + /* We have no further way to compute an alignment for this type. */ break; case TYPE_CODE_ARRAY: