From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 40466 invoked by alias); 27 Nov 2017 19:54:11 -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 40453 invoked by uid 89); 27 Nov 2017 19:54:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=BAYES_00,KB_WAM_FROM_NAME_SINGLEWORD,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=hacked, hole X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 27 Nov 2017 19:54:09 +0000 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A65EAC0587CA; Mon, 27 Nov 2017 19:54:08 +0000 (UTC) Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7FDD262676; Mon, 27 Nov 2017 19:54:08 +0000 (UTC) From: Sergio Durigan Junior To: Tom Tromey Cc: GDB Patches Subject: Re: [PATCH] Implement pahole-like 'ptype /o' option References: <20171121160709.23248-1-sergiodj@redhat.com> <87tvxgvo18.fsf@tromey.com> Date: Mon, 27 Nov 2017 19:54:00 -0000 In-Reply-To: <87tvxgvo18.fsf@tromey.com> (Tom Tromey's message of "Sun, 26 Nov 2017 12:27:15 -0700") Message-ID: <87a7z7pkf4.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2017-11/txt/msg00698.txt.bz2 On Sunday, November 26 2017, Tom Tromey wrote: >>>>>> "Sergio" == Sergio Durigan Junior writes: > > Sergio> This commit implements the pahole-like '/o' option for 'ptype', which > Sergio> prints the offsets and sizes of struct fields, reporting whenever > Sergio> there is a hole found. > > Thanks for doing this! Thanks for the insights :-). > Sergio> The idea is to print offsets and sizes only for structs, so unions and > Sergio> other types are mostly ignored. > > I tried out the patch, since I want to add support for this feature to > the Rust language code. I have two comments. > > First, I think it would be valuable to descend into embedded structures. > This would make it simpler to review layout of the entire outer struct. > That is, like this: > > struct t { > int x; > int y; > }; > > struct s { > int x; > char c; > struct t t; > } s; > > The old pahole.py did do this, like > > (gdb) pahole struct s > struct s { > /* 0 4 */ int x > /* 4 1 */ char c > /* XXX 24 bit hole, try to pack */ > /* 8 8 */ struct t { > /* 0 4 */ int x > /* 4 4 */ int y > } t > } I agree. In order to test the patch, I've even hacked GDB and made 'ptype' print more levels instead of just 1. However, due to the idiosincrasies of the formatting function + the fact that this feature would be better off as a second patch, I left it as TODO. I think for 'ptype /o' the best would be to stick with 2 levels, because otherwise the output gets too messed up. But I've been thinking about sending another patch to enable the multi-level ptype anyway. > ... though the old code's output is kind of confusing, restarting the > offset at 0 in the embedded struct. The way my code is written, it also restarts the offset at 0. Do you think it'd be better to keep using the offset from the outter struct? > Second, and similarly, descending into unions does seem to make sense > sometimes (pahole.py didn't do this, but it seems like it should have). > > Continuing the above example, consider: > > union u { > struct s s; > int i; > }; > > Here ptype shows: > > (gdb) ptype/o union u > type = union u { > struct s s; > int i; > } > > (The formatting looks weird without the layout info here...) > > I think one specific case where it is interesting to see the union's > layout is when you want to know why a union field in a struct is as > large as it is. That is, what is the largest member of the union, in > case you want to try to shrink it? Good point. I've implemented this on the patch. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/