From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 72778 invoked by alias); 7 Aug 2018 16:05:44 -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 72749 invoked by uid 89); 7 Aug 2018 16:05:44 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=versa, HContent-Transfer-Encoding:8bit X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 07 Aug 2018 16:05:42 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id w77G5aYv012525 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Tue, 7 Aug 2018 12:05:40 -0400 Received: by simark.ca (Postfix, from userid 112) id 323111EF36; Tue, 7 Aug 2018 12:05:36 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 498271E183; Tue, 7 Aug 2018 12:05:34 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Date: Tue, 07 Aug 2018 16:05:00 -0000 From: Simon Marchi To: Alan Hayward Cc: Simon Marchi , GDB Patches , nd Subject: Re: [PATCH v2 1/3] Add min size to regset section iterations In-Reply-To: <13DC373A-4E12-46C4-AA97-C11E82224056@arm.com> References: <20180730092528.98739-1-alan.hayward@arm.com> <20180730092528.98739-2-alan.hayward@arm.com> <045ba9b9-6f37-0eeb-5609-f895e29f894f@ericsson.com> <13DC373A-4E12-46C4-AA97-C11E82224056@arm.com> Message-ID: <4caa36e74ca613047db83f1a9868ea3e@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.6 X-IsSubscribed: yes X-SW-Source: 2018-08/txt/msg00118.txt.bz2 On 2018-08-07 07:01, Alan Hayward wrote: > It’s probably clearer if I explain the SVE specific case: > > With SVE when we read the section from the core file it will give us > one of two things: > 1) SVE header followed by a full SVE register dump (size dependant on > register size) > Or > 2) SVE header followed by a neon register dump (I usually refer to > this as a fpsimd dump). > > The second is a shortcut used by the kernel if the process hadn't run > any SVE code, and > all the SVE state is null. This structure is significantly smaller > than the SVE dump. > > In the common gdb supply code, it will assert if the size of the > section read from the core > file is smaller than the given size. So for SVE I need to set the size > to the fpsimd size > to prevent this. > > Ok, I could peak ahead into the core file to see what type of dump it > is. But, at the > point of the _iterate_over_regset_sections() we don’t have any access > to the core file. > > In the common collect code, it uses the size to allocate memory for > writing the dump into. > For this we always want to write out a full SVE dump, so need the size > of the first dump. > > > If that makes sense now, I can rework it into the comments. Ok, it kind of make sense but I still find the naming (min_size and size) confusing. I can't quickly tell what is used when. From what I understand, when supplying, you only care about min_size. If REGSET_VARIABLE_SIZE is not set, min_size is actually not a min_size, but the expected size (anything else than this size will be rejected). When collecting, you only care about "size", to allocate the memory for the dump. Therefore, I am starting to think the semantic is more straightforward (to me at least) if we named them supply_size and collect_size (which you mentioned in the original patch message). This would make it somewhat clear that if you are in a supply scenario, collect_size is meaningless (and vice versa). It becomes a bit simpler to explain: - When supplying fixed-size regsets (!REGSET_VARIABLE_SIZE), supply_size is the exact expected size. - When supplying variable-size regsets (REGSET_VARIABLE_SIZE) supply_size is actually just a minumum, because we don't know what we will actually find in the section yet. - When collecting, we know the size in advance (since we know what we will dump), so collect_size is always the exact size that will be dumped. In core_target::get_core_register_section (or maybe somewhere else), we can assert that if (!variable_size_section) gdb_assert (supply_size == collect_size); On a different track, did you consider keeping a single "size" parameter to gdbarch_iterate_over_regset_sections, but add one to indicate whether the caller intends to supply or collect registers? And then, in aarch64's implementation, pass different sizes in the supply/collect cases? Most other arch implementations would simply ignore this parameter and always pass the same size, as they do today. Simon