From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 49843 invoked by alias); 18 Sep 2018 14:40:03 -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 49821 invoked by uid 89); 18 Sep 2018 14:40:02 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-25.5 required=5.0 tests=AWL,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=hoping, HContent-Transfer-Encoding:8bit X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 18 Sep 2018 14:40:00 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id 7951456003; Tue, 18 Sep 2018 10:39:58 -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 nZyALNNH3qlY; Tue, 18 Sep 2018 10:39:58 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 520C9117FE3; Tue, 18 Sep 2018 10:39:58 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id 825CB873D9; Tue, 18 Sep 2018 07:39:56 -0700 (PDT) Date: Tue, 18 Sep 2018 14:40:00 -0000 From: Joel Brobecker To: Alan Hayward Cc: "gdb-patches@sourceware.org" , nd Subject: Re: [PATCH] Aarch64 SVE: Fix stack smashing when calling functions Message-ID: <20180918143956.GJ19172@adacore.com> References: <20180917144540.78906-1-alan.hayward@arm.com> <20180917184205.GG19172@adacore.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) X-SW-Source: 2018-09/txt/msg00633.txt.bz2 > >> 2018-09-17 Alan Hayward > >> > >> * aarch64-tdep.c (pass_in_v): Use register size. > >> (aarch64_extract_return_value): Likewise. > >> (aarch64_store_return_value): Likewise. > > > > Do we have a testcase already that demonstrates the problem? > > Otherwise, it would be nice to add one. > > I should have mentioned that in the description. I can add: > “This fixes gdb.base/callfuncs.exp for Aarch64 SVE." I was hoping for something like that. Nice :). > >> diff --git a/gdb/aarch64-tdep.c b/gdb/aarch64-tdep.c > >> index 6993e9061e..516eb138dc 100644 > >> --- a/gdb/aarch64-tdep.c > >> +++ b/gdb/aarch64-tdep.c > >> @@ -1358,7 +1358,10 @@ pass_in_v (struct gdbarch *gdbarch, > >> if (info->nsrn < 8) > >> { > >> int regnum = AARCH64_V0_REGNUM + info->nsrn; > >> - gdb_byte reg[V_REGISTER_SIZE]; > >> + /* Enough space for a full vector register. */ > >> + gdb_byte reg[register_size (gdbarch, regnum)]; > >> + gdb_static_assert (AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM); > >> + gdb_assert (len <= sizeof (reg)); > > > > Could you explain the relationship between making the buffer large > > enough, which is the purpose of this patch, and the assertion that > > AARCH64_V0_REGNUM == AARCH64_SVE_Z0_REGNUM? > > > > I don't see a problem with that assertion, but for archeology > > purposes, it is better to decorelate changes that are independent. > > It helps better document why we introduced changes. > > Thinking about it, that assert should be removed. > I was reusing three lines from aarch64_pseudo_read_value_1, which > passed AARCH64_V0_REGNUM into register_size. There the assert made > sense. When I switched to use regnum I didn’t rethink the assert. > > I’ll remove the three new instances of that assert from this patch. > > Are you happy with those those changes? Yes; pre-approved with the addition of gdb.base/callfuncs.exp in the revision log and the 3 asserts removed. Thank you, -- Joel