From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 40887 invoked by alias); 13 Dec 2017 16:19: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 40439 invoked by uid 89); 13 Dec 2017 16:19:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-6.9 required=5.0 tests=BAYES_00,GIT_PATCH_3,KAM_SHORT,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= 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; Wed, 13 Dec 2017 16:19:09 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id A466B80462; Wed, 13 Dec 2017 16:19:08 +0000 (UTC) Received: from [127.0.0.1] (ovpn04.gateway.prod.ext.ams2.redhat.com [10.39.146.4]) by smtp.corp.redhat.com (Postfix) with ESMTP id F2F3D600D3; Wed, 13 Dec 2017 16:18:59 +0000 (UTC) Subject: Re: [PATCH v5 2/2] Implement pahole-like 'ptype /o' option To: Sergio Durigan Junior , GDB Patches References: <20171121160709.23248-1-sergiodj@redhat.com> <20171213031724.22721-1-sergiodj@redhat.com> <20171213031724.22721-3-sergiodj@redhat.com> Cc: Tom Tromey , Eli Zaretskii , Simon Marchi , Keith Seitz From: Pedro Alves Message-ID: <614d15fc-3793-8a55-b7cc-67570e8c46d3@redhat.com> Date: Wed, 13 Dec 2017 16:19:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20171213031724.22721-3-sergiodj@redhat.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-SW-Source: 2017-12/txt/msg00311.txt.bz2 On 12/13/2017 03:17 AM, Sergio Durigan Junior wrote: > +/* A struct with an union. */ > + > +struct poi > +{ > + int f1; > + > + union qwe f2; > + > + uint16_t f3; > + > + struct pqr f4; > +}; > + > +/* A struct with bitfields. */ > + > +struct tyu > +{ > + int a1 : 1; > + > + int a2 : 3; > + > + int a3 : 23; > + > + char a4 : 2; > + > + int64_t a5; > + > + int a6 : 5; > + > + int64_t a7 : 3; > +}; I think that the testcase should also make sure to exercise the new offset computations in the case c_print_type_struct_field_offset's 'offset_bitpos' parameter is > 0. Is it already covered? I assume we'll need a test like tyu (struct with bitfields with overlapping underlying objects), but that inherits some other base structure? > +++ b/gdb/testsuite/gdb.base/ptype-offsets.exp > @@ -0,0 +1,192 @@ > +# This testcase is part of GDB, the GNU debugger. > + > +# Copyright 2017 Free Software Foundation, Inc. > + > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 3 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see . > + Please add an intro comment describing what this testcase is about. > +standard_testfile .cc > + > +# Test only works on x86_64 LP64 targets. That's how we guarantee > +# that the expected holes will be present in the struct. > +if { !([istarget "x86_64-*-*"] && [is_lp64_target]) } { > + untested "test work only on x86_64 lp64" > + return 0 > +} I'm mildly worried about whether the bitfield handling is working correctly on big endian machines. We may want to lift this x86-64-only restriction, by using e.g., alignas(N) or __attribute__((aligned(N)) to take care of most of the differences between architectures and end up with few per-arch code in the .exp. But I'm fine with starting with only x86-64 if you confirm manually on e.g., a big endian PPC64 machine on the compile farm, and we can extend the testcase in that direction after this is merged. > + > +if { [prepare_for_testing "failed to prepare" $testfile $srcfile \ > + { debug c++ optimize=-O0 }] } { > + return -1 > +} Weren't you going to remove that optimize thing? :-) > +# Test that the offset is properly reset when we are printing an union > +# and go inside two inner structs. > +# This also tests a struct inside a struct inside an union. "a union". (two times here; there may be other places.) > +gdb_test "ptype /o union qwe" \ > + [multi_line \ > +"/\\\* offset | size \\\*/" \ > +"/\\\* 24 \\\*/ struct tuv {" \ > +"/\\\* 0 | 4 \\\*/ int a1;" \ > +"/\\\* XXX 4-byte hole \\\*/" \ > +"/\\\* 8 | 8 \\\*/ char \\\*a2;" \ > +"/\\\* 16 | 4 \\\*/ int a3;" \ > +" } /\\\* total size: 24 bytes \\\*/ fff1;" \ > +"/\\\* 40 \\\*/ struct xyz {" \ > +"/\\\* 0 | 4 \\\*/ int f1;" \ > +"/\\\* 4 | 1 \\\*/ char f2;" \ > +"/\\\* XXX 3-byte hole \\\*/" \ > +"/\\\* 8 | 8 \\\*/ void \\\*f3;" \ > +"/\\\* 16 | 24 \\\*/ struct tuv {" \ > +"/\\\* 16 | 4 \\\*/ int a1;" \ > +"/\\\* XXX 4-byte hole \\\*/" \ > +"/\\\* 24 | 8 \\\*/ char \\\*a2;" \ > +"/\\\* 32 | 4 \\\*/ int a3;" \ > +" } /\\\* total size: 24 bytes \\\*/ f4;" \ > +" } /\\\* total size: 40 bytes \\\*/ fff2;" \ > +"} /\\\* total size: 40 bytes \\\*/"] \ > + "ptype offset union qwe" Did you try using {} instead of "" for these strings, avoiding all the escaping? > @@ -499,6 +506,11 @@ whatis_exp (const char *exp, int show) > real_type = value_rtti_type (val, &full, &top, &using_enc); > } > > + if (flags.print_offsets && && goes on the next line. > + (TYPE_CODE (type) == TYPE_CODE_STRUCT > + || TYPE_CODE (type) == TYPE_CODE_UNION)) > + fprintf_filtered (gdb_stdout, "/* offset | size */\n"); > + > printf_filtered ("type = "); Thanks, Pedro Alves