From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1451 invoked by alias); 11 Dec 2017 21:07:18 -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 885 invoked by uid 89); 11 Dec 2017 21:07:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.9 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=Hx-languages-length:4123 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, 11 Dec 2017 21:07:16 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8A1C152779; Mon, 11 Dec 2017 21:07:15 +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 46EA960605; Mon, 11 Dec 2017 21:07:15 +0000 (UTC) From: Sergio Durigan Junior To: Simon Marchi Cc: GDB Patches , Tom Tromey , Eli Zaretskii Subject: Re: [PATCH v2] Implement pahole-like 'ptype /o' option References: <20171121160709.23248-1-sergiodj@redhat.com> <20171128212137.15655-1-sergiodj@redhat.com> <87o9n5drbn.fsf@redhat.com> <99286acb-ce9f-42f0-41c3-ef10e03171ff@ericsson.com> Date: Mon, 11 Dec 2017 21:07:00 -0000 In-Reply-To: <99286acb-ce9f-42f0-41c3-ef10e03171ff@ericsson.com> (Simon Marchi's message of "Mon, 11 Dec 2017 15:45:35 -0500") Message-ID: <87tvwxc6t9.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2017-12/txt/msg00250.txt.bz2 On Monday, December 11 2017, Simon Marchi wrote: >>>> +/* Use 'print_spaces_filtered', but take into consideration the >>>> + type_print_options FLAGS in order to determine how many whitespaces >>>> + will be printed. */ >>>> + >>>> +static void >>>> +print_spaces_filtered_with_print_options (int level, struct ui_file *stream, >>>> + const struct type_print_options *flags) >>> >>> Missing spaces here. >> >> This is actually on purpose. If I indent the line, it will have more >> than 80 chars. I believe this is a well known method for avoiding this >> problem...? > > I am not aware of that. In this case I would put the parameter list on the next, > I'm not sure if it's 100% GNU-style approved, but nobody complained when I did it > so far :) > > static void > print_spaces_filtered_with_print_options > (int level, struct ui_file *stream, const struct type_print_options *flags); > > It helps with long function names. In this case, I would probably just drop the > "struct" to save a few chars, because C++. Fair enough. I use this trick for function prototypes, but not for the definitions. >>>> +if { [prepare_for_testing "failed to prepare" $testfile $srcfile \ >>>> + { debug c++ optimize=-O0 }] } { >>> >>> optimize=-O0 seems unnecessary to me, I've never seen it specified explicitly in a test. >> >> There are very few tests that use it (2, currently). I understand it's >> not very common to explicitly specify -O0, but I put it there because I >> decided to be on the safe side. If the compiler performs any >> optimization at all, it could mess with the layout of the structs. > > If GCC decided to optimize by default, so many things would break in the GDB testsuite. > We would then probably make gdb_compile add -O0, so that we wouldn't need to do it in > all tests. The point is that IMO, tests should expect no optimization by default. OK. >>> I also noticed that the offset is not shown in front of the struct-in-union, >>> as show above, but it is in the case of struct-in-struct: >>> >>> /* offset | size */ >>> type = struct my_struct_3 { >>> /* 0 | 8 */ struct my_struct_1 { >>> /* 0 | 4 */ int a; >>> /* 4 | 4 */ int b; >>> } /* total size: 8 bytes */ s1; >>> /* 8 | 8 */ struct my_struct_2 { >>> /* 8 | 4 */ int c; >>> /* 12 | 4 */ int d; >>> } /* total size: 8 bytes */ s2; >>> } /* total size: 16 bytes */ >>> >>> Is this difference on purpose? >> >> Yes; offsets are not shown for fields inside unions (not only structs, >> but all types of fields), because it doesn't make much sense: they'd be >> 0 every time. This is also inspired from pahole's output. > > Not if that union is itself in a struct. For example with this: > > struct hello > { > int i; > union { > struct { > int x, y; > } a; > struct { > int x, y; > } b; > }; > }; > > (gdb) ptype /o struct hello > /* offset | size */ > type = struct hello { > /* 0 | 4 */ int i; > /* 4 | 8 */ union { > /* 8 */ struct { > /* 4 | 4 */ int x; > /* 8 | 4 */ int y; > } /* total size: 8 bytes */ a; > /* 8 */ struct { > /* 4 | 4 */ int x; > /* 8 | 4 */ int y; > } /* total size: 8 bytes */ b; > } /* total size: 8 bytes */; > } /* total size: 12 bytes */ > > > But I don't mind it, it just stuck out as a little inconsistency. I don't see the inconsistency. If a field is inside a struct, it has its offset *and* size printed. No matter if the field is an int, another struct, or an union. If a field is inside an union, it has only its size printed. In the case above, it makes sense to have the offsets printed for the fields inside the two structs (inside the union), because there might be holes to report (well, one can argue that it doesn't matter if there are holes or not in this case, because if the other struct is bigger then the union size will stay the same). However, it doesn't make sense to print the offsets for the two structs themselves, because they are members of the union. I hope it makes more sense now. 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/