From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net [IPv6:2001:4b98:dc4:8::225]) by sourceware.org (Postfix) with ESMTPS id 681433858407 for ; Tue, 29 Nov 2022 11:53:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 681433858407 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=seketeli.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=seketeli.org Received: (Authenticated sender: dodji@seketeli.org) by mail.gandi.net (Postfix) with ESMTPSA id 92F4F1C000A; Tue, 29 Nov 2022 11:53:04 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=seketeli.org; s=gm1; t=1669722785; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=vI8GptD/cHIt4J3LmRDJukdWZSBeYNpoWdkM0g8R5Cs=; b=nRhx2w9+V4aRdcOlcW0wfZp4EfTqA+MVt5dVdEa7GZvHlEDapbiAfJBcfFJVQ0ZTHX4GIf ecFbHJxUQoBexFfG5mWYuBk7fegOuKSwNpe7guhYHFhu5M/WQj5hVc8q+Ils9AMxco2xZV Ox6q+EazJxoiffK7xShiXRg5+YBk1uEMJF9r2tfjd6Y+OeSRnIWeM8bZsvV75dIAtwGYvi DLD5oQH1cEnX+0XcuZo0eVTZHorN+mDydeyVLmCfevHJ3tTAA6XBUMKdiYXp+FeE7F9rF1 zw209wGm2wsAj8VcCROopROh0sdCbf50aUVSpTaAxJ1CK9q7MrEVzFSg7TTW6g== Received: by localhost (Postfix, from userid 1000) id 641B4581C59; Tue, 29 Nov 2022 12:53:03 +0100 (CET) From: Dodji Seketeli To: "Guillermo E. Martinez via Libabigail" Cc: "Guillermo E. Martinez" Subject: Re: [PATCH 2/5] ctf-front-end: Fix size and name for underlying types Organization: Me, myself and I References: <20221117034305.184864-1-guillermo.e.martinez@oracle.com> <20221117034305.184864-3-guillermo.e.martinez@oracle.com> X-Operating-System: Fedora 38 X-URL: http://www.seketeli.net/~dodji Date: Tue, 29 Nov 2022 12:53:03 +0100 In-Reply-To: <20221117034305.184864-3-guillermo.e.martinez@oracle.com> (Guillermo E. Martinez via Libabigail's message of "Wed, 16 Nov 2022 21:43:02 -0600") Message-ID: <871qpmou3k.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-10.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,JMQ_SPF_NEUTRAL,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hello Guillermo, "Guillermo E. Martinez via Libabigail" a =C3=A9crit: > It fixed an incorrect representation in size and name for underlying > enum and struct types when it has a bitfield members: > > struct foo > { > unsigned bar : 2; > unsigned baz : 1; > }; > > > > So, `name' empty property, no showing the right enumerator's type and size > in bits: > > Hmmh, I think here, the name should be "unsigned int", because "unsigned bar" implicitly means 'unsigned int bar". And the size-in-bits should be set to 32. I have thus changed the introductory paragraphs of this commit log to the following: This patch fixes an incorrect representation in size and name of the underlying type of enums as well as underlying types of bitfield data members types. For instance, consider this struct. struct foo { unsigned bar : 2; unsigned baz : 1; }; The data members bar and baz have an underlying type that is "unsigned int". Yet, the CTF front-end represents the underlying type of these data members as: The name property is empty, and it should be "unsigned int". The size in bit is '2', but it should be the size of the underlying "unsigned int", in bits, which is 32. In other words, the underlying type of bar and baz should be: Note that today, libabigail doesn't represent the bitfield properties of the data member. Those bitfield properties are properties of the data member, not of their type. This is a known "Not Yet Implemented" feature request that has been filed upstream at https://sourceware.org/bugzilla/show_bug.cgi?id=3D27334. Similarly, the underlying type of enums is not properly represented by the CTF front-end. Fixed thus. It's a little bit more verbose, but hopefully, that should give a little bit more context for future code maintenance. > > * src/abg-ctf-reader.cc (process_ctf_{base_type,enum_type}): > Look at ctf refence type to build the underlying type if present. > * tests/data/Makefile.am: New test cases. > * tests/data/test-read-ctf/PR27700/test-PR27700.abi: New test input. > * tests/data/test-read-ctf/test-bitfield-enum.abi: Likewise. > * tests/data/test-read-ctf/test-bitfield-enum.c: Likewise. > * tests/data/test-read-ctf/test-bitfield-enum.o: Likewise. > * tests/data/test-read-ctf/test-bitfield.abi: Likewise. > * tests/data/test-read-ctf/test-bitfield.c: Likewise. > * tests/data/test-read-ctf/test-bitfield.o: Likewise. > * tests/data/test-read-ctf/test-enum-many.o.hash.abi: Adjust. > * tests/data/test-read-ctf/test-enum-symbol.o.hash.abi: Likewise. > * tests/data/test-read-ctf/test-enum.o.abi: Likewise: > * tests/data/test-read-ctf/test0.abi: Likewise. > * tests/data/test-read-ctf/test0.hash.abi: Likewise. > * tests/data/test-read-ctf/test1.so.abi: Likewise. > * tests/data/test-read-ctf/test1.so.hash.abi: Likewise. > * tests/data/test-read-ctf/test5.o.abi: Likewise. > * tests/test-read-ctf.cc: Update test suite. > > Signed-off-by: Guillermo E. Martinez Applied to the master branch with the change above. Please find below the actual patch that got applied. Many thanks! [...] >From 217f579bf788a11643c0066a7dc8aa76faa5a05d Mon Sep 17 00:00:00 2001 From: "Guillermo E. Martinez" Date: Wed, 16 Nov 2022 21:43:02 -0600 Subject: [PATCH] ctf-reader: Fix size and name for underlying types This patch fixes an incorrect representation in size and name of the underlying type of enums as well as underlying types of bitfield data members types. For instance, consider this struct. struct foo { unsigned bar : 2; unsigned baz : 1; }; The data members bar and baz have an underlying type that is "unsigned int". Yet, the CTF front-end represents the underlying type of these data members as: The name property is empty, and it should be "unsigned int". The size in bit is '2', but it should be the size of the underlying "unsigned int", in bits, which is 32. In other words, the underlying type of bar and baz should be: Note that today, libabigail doesn't represent the bitfield properties of the data member. Those bitfield properties are properties of the data member, not of their type. This is a known "Not Yet Implemented" feature request that has been filed upstream at https://sourceware.org/bugzilla/show_bug.cgi?id=3D27334. Similarly, the underlying type of enums is not properly represented by the CTF front-end. Fixed thus. * src/abg-ctf-reader.cc (process_ctf_{base_type,enum_type}): Look at ctf refence type to build the underlying type if present. * tests/data/Makefile.am: New test cases. * tests/data/test-read-ctf/PR27700/test-PR27700.abi: New test input. * tests/data/test-read-ctf/test-bitfield-enum.abi: Likewise. * tests/data/test-read-ctf/test-bitfield-enum.c: Likewise. * tests/data/test-read-ctf/test-bitfield-enum.o: Likewise. * tests/data/test-read-ctf/test-bitfield.abi: Likewise. * tests/data/test-read-ctf/test-bitfield.c: Likewise. * tests/data/test-read-ctf/test-bitfield.o: Likewise. * tests/data/test-read-ctf/test-enum-many.o.hash.abi: Adjust. * tests/data/test-read-ctf/test-enum-symbol.o.hash.abi: Likewise. * tests/data/test-read-ctf/test-enum.o.abi: Likewise: * tests/data/test-read-ctf/test0.abi: Likewise. * tests/data/test-read-ctf/test0.hash.abi: Likewise. * tests/data/test-read-ctf/test1.so.abi: Likewise. * tests/data/test-read-ctf/test1.so.hash.abi: Likewise. * tests/data/test-read-ctf/test5.o.abi: Likewise. * tests/test-read-ctf.cc: Update test suite. Signed-off-by: Guillermo E. Martinez Signed-off-by: Dodji Seketeli --- src/abg-ctf-reader.cc | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/abg-ctf-reader.cc b/src/abg-ctf-reader.cc index cbc5cbca..d000556b 100644 --- a/src/abg-ctf-reader.cc +++ b/src/abg-ctf-reader.cc @@ -783,15 +783,17 @@ process_ctf_base_type(reader *rdr, translation_unit_sptr tunit =3D rdr->cur_transl_unit(); type_decl_sptr result; =20 - const char *type_name =3D ctf_type_name_raw(ctf_dictionary, ctf_type); + ctf_id_t ctf_ref =3D ctf_type_reference(ctf_dictionary, ctf_type); + const char *type_name =3D ctf_type_name_raw(ctf_dictionary, + (ctf_ref !=3D CTF_ERR) ? ctf_r= ef : ctf_type); =20 /* Get the type encoding and extract some useful properties of the type from it. In case of any error, just ignore the type. */ ctf_encoding_t type_encoding; if (ctf_type_encoding(ctf_dictionary, - ctf_type, - &type_encoding)) + (ctf_ref !=3D CTF_ERR) ? ctf_ref : ctf_type, + &type_encoding)) return result; =20 /* Create the IR type corresponding to the CTF type. */ @@ -1357,7 +1359,10 @@ process_ctf_enum_type(reader *rdr, { translation_unit_sptr tunit =3D rdr->cur_transl_unit(); enum_type_decl_sptr result; - std::string enum_name =3D ctf_type_name_raw(ctf_dictionary, ctf_type); + ctf_id_t ctf_ref =3D ctf_type_reference(ctf_dictionary, ctf_type); + std::string enum_name =3D ctf_type_name_raw(ctf_dictionary, + (ctf_ref !=3D CTF_ERR) + ? ctf_ref : ctf_type); =20 if (!enum_name.empty()) if (corpus_sptr corp =3D rdr->should_reuse_type_from_corpus_group()) @@ -1367,18 +1372,24 @@ process_ctf_enum_type(reader *rdr, /* Build a signed integral type for the type of the enumerators, aka the underlying type. The size of the enumerators in bytes is specified in the CTF enumeration type. */ - size_t utype_size_in_bits =3D ctf_type_size(ctf_dictionary, ctf_type) * = 8; - type_decl_sptr utype; + size_t utype_size_in_bits =3D ctf_type_size(ctf_dictionary, + (ctf_ref !=3D CTF_ERR) + ? ctf_ref : ctf_type) * 8; + string underlying_type_name =3D + build_internal_underlying_enum_type_name(enum_name, true, + utype_size_in_bits); =20 + type_decl_sptr utype; utype.reset(new type_decl(rdr->env(), - "", - utype_size_in_bits, - utype_size_in_bits, - location())); + underlying_type_name, + utype_size_in_bits, + utype_size_in_bits, + location())); utype->set_is_anonymous(true); utype->set_is_artificial(true); if (!utype) return result; + add_decl_to_scope(utype, tunit->get_global_scope()); canonicalize(utype); =20 --=20 2.38.1 --=20 Dodji