From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id D428E385B53C for ; Tue, 7 Nov 2023 11:22:00 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D428E385B53C Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org D428E385B53C Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699356125; cv=none; b=XJ7pmhIP7VZR0dPiZ7R1szy/91POvbcVZMYHHF2NiM+52NoOrtDnb+Gp3Vku1teY90fCWvnRYRYhRPyS/0bKZfl+Aj2sMVuo2UYOAVORJee0CgqfglTdI8a2r6RCI3g57B4uB79f73wGuPaoPR5P+nUHcbla+Ta8+zEr9q5pXNE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699356125; c=relaxed/simple; bh=hw4hbfT+h69SF80OLgWUZCG2nG/99qKZQYYCCbFNQ4E=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=gWCb6AvloHFrKYv/f9RFoH3X5FexOhVsGLoEjW8CHc67dcBjfze9Xm05vAJ3cAUKaUnuiOXSgBXqbMjlF8Jqjo0sCz0SNcuKzKuUe0PwRqZfyEB28gJqHe+yUX6RfaMW7Fe2rfpC0YNfUwYFags5Sf3jBkIm8m5PY7Rx+abW2mI= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1699356120; 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=bF6fO5NLH/Mpyy87j8XHdINS7u2c9h4vxzsVOMIYJws=; b=Mtx3dy7jf0NZSoumyuPe7CYFZ4MAPX9n6pEEn4y3v2K7N/jbqGMKRTN3jATNDSmMyb8ZR1 sa87UhcUcGKwjwCyjkDQr/YEiaWJh/mVzvAGc2inYlBwtyUuSRleqQJKEn/wAZCdRGSSVa eFAchdIsAlUIJEZ2OgdNUq5j8bQ69eY= Received: from mail-qt1-f199.google.com (mail-qt1-f199.google.com [209.85.160.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-664-YbdAB15QMWm2KxVof8fa-Q-1; Tue, 07 Nov 2023 06:21:58 -0500 X-MC-Unique: YbdAB15QMWm2KxVof8fa-Q-1 Received: by mail-qt1-f199.google.com with SMTP id d75a77b69052e-41cd433742dso69030121cf.2 for ; Tue, 07 Nov 2023 03:21:58 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699356118; x=1699960918; h=content-transfer-encoding:mime-version:user-agent:message-id :in-reply-to:date:references:organization:subject:cc:to:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=USIvWExTCP+r7vRg6MS+C0LRrE01BrqfFj0N94yhqXo=; b=FQp3Emzg+KqSUZCwilzo8jRh48621/iqrYskbYt1+oYiEL687/DsVusdfVylHvEe1m pDFGZmm/jihPt2SpmXeylikunsZeKeiAlp3mlJ0/6plA8cx62YMjVsaG/FSndxBxA+ql bQrp5o2LMn3DKJSLqBeSGZLjPhdlmSF1k9fG1UFpg2OQKYay0XpzJJFcBv0p2Zv7Cas+ YQmDpxm9rr5A9C3pTvkgK85SjWbS4BRQIGW2YR3udHgfiNEe7oT+n2YQRZlOED/B3xJN qtbkHa0kje/Szq1HNG0iuCYZno71nEsZv0+uOkW+9V01wuDbI0kTJr0dr1JKiiHD701o yW4Q== X-Gm-Message-State: AOJu0Yyu0tQL8v2JHTIGxrELtejJMMMhzqWaltWxlcFlvJQGtRzvJBB0 xDTNl3VSOMfWt/WHkT9KIimmbh+Hgtn7ef6iupS0jkn81Q4rTMsHO10+xS404ti5T2ttN1NLxVp 0cTMGOyJzcglBWDfA+UvW X-Received: by 2002:a05:622a:1647:b0:418:1d4f:995c with SMTP id y7-20020a05622a164700b004181d4f995cmr38329043qtj.55.1699356117889; Tue, 07 Nov 2023 03:21:57 -0800 (PST) X-Google-Smtp-Source: AGHT+IGruOrhcuSL8rP7/sKPy+85uLwABzYJ30jxobxVkXXHexoCH3dzVmBT5LHOmaB6ql+ylVWtGg== X-Received: by 2002:a05:622a:1647:b0:418:1d4f:995c with SMTP id y7-20020a05622a164700b004181d4f995cmr38329034qtj.55.1699356117476; Tue, 07 Nov 2023 03:21:57 -0800 (PST) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id z3-20020ac87ca3000000b004182037f655sm4230490qtv.14.2023.11.07.03.21.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 07 Nov 2023 03:21:57 -0800 (PST) Received: by localhost (Postfix, from userid 1000) id 085F65077C76; Tue, 7 Nov 2023 12:21:55 +0100 (CET) From: Dodji Seketeli To: John Moon Cc: , Subject: Re: [PATCH] suppression: Add "has_strict_flexible_array_data_member_conversion" property Organization: Me, myself and I References: <20231104004757.21305-1-quic_johmoo@quicinc.com> X-Operating-System: AlmaLinux 9.2 X-URL: http://www.seketeli.net/~dodji Date: Tue, 07 Nov 2023 12:21:55 +0100 In-Reply-To: <20231104004757.21305-1-quic_johmoo@quicinc.com> (John Moon's message of "Fri, 3 Nov 2023 17:47:57 -0700") Message-ID: <8734xh3j1o.fsf@seketeli.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,KAM_LOTSOFHASH,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE 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, John Moon a =C3=A9crit: > From: Dodji Seketeli First, thank you for putting together a proper patch from my quick initial attempt. That's really appreciated. [...] > > =09* include/abg-fwd.h > =09(ir::has_fake_flexible_array_data_member): Declare new accessor > =09functions. > =09* include/abg-suppression.h > =09(type_suppression::{,set_}has_strict_fam_conversion): Declare new > =09accessor functions. > =09* src/abg-ir.cc > =09(ir::has_fake_flexible_array_data_member): Define new accessor > =09functions. > =09* src/abg-suppression-priv.h > =09(type_suppression::priv::has_strict_fam_conv_): Define new > =09data member. > =09* src/abg-suppression.cc > =09(type_suppression::{,set_}has_strict_fam_conversion): Define new > =09accessor functions. > =09(type_suppression::suppresses_diff): For a type suppression to > =09match a fake flex array conversion, has_size_change must be true > =09and it must change from a fake flex array to a real flex array. > =09(read_type_suppression): Parse the new > =09'has_strict_flexible_array_data_member_conversion' property to > =09set the type_suppression::set_has_strict_fam_conversion property. > =09* doc/manuals/libabigail-concepts.rst: Add an entry for the new > =09'has_strict_flexible_array_data_member_conversion' property. > =09* tests/data/test-diff-suppr/test-has-strict-flexible-array-data-membe= r-conversion-{1,2}.suppr: > =09Add new test suppression files. > =09* tests/data/test-diff-suppr/test-has-strict-flexible-array-data-membe= r-conversion-report-{1,2}.txt: > =09Add new test reference output files. > =09* tests/data/test-diff-suppr/test-has-strict-flexible-array-data-membe= r-conversion-v{0,1}.c: > =09Add source code for new binary test input files. > =09* tests/data/test-diff-suppr/test-has-strict-flexible-array-data-membe= r-conversion-v{0,1}.o: > =09Add new binary test input files. > =09* tests/data/Makefile.am: Add the new test files to the source > =09distribution. Changes to the tests/data/Makefile.am file were not included in the patch. So make distcheck failed. I have added those changes in. [...] > diff --git a/doc/manuals/libabigail-concepts.rst b/doc/manuals/libabigail= -concepts.rst > index 28e71684..f9e27ff9 100644 > --- a/doc/manuals/libabigail-concepts.rst > +++ b/doc/manuals/libabigail-concepts.rst Many thanks for adding documentation here. It's really appreciated! > @@ -619,9 +619,28 @@ names start with the string "private_data_member". > =09 {72, end} > } > =20 > +.. _suppr_has_strict_flexible_array_data_member_conversion_label: > =20 > =20 > - .. _suppr_has_size_change_property_label: > +* ``has_strict_flexible_array_data_member_conversion`` > + > + Usage: > + > + ``has_strict_flexible_array_data_member_conversion`` ``=3D`` yes | no > + > + Suppresses change reports involving a type which has a "fake" > + flexible array member at the end of the struct which is converted > + to a real flexible array member. This would be a member like > + ``data[1]`` being converted to ``data[]``. Nice. > + > + Please note: a conversion to a flex array like this > + will *always* result in the structure changing size, > + so the suppression will not take effect unless the > + :ref:`has_size_change` > + property is present and set to ``yes``. I have just updated this slightly in regard of the discussion we've had at https://sourceware.org/bugzilla/show_bug.cgi?id=3D31017#c3. Here is what I have replaced the hunk above with: + Please note that if the size of the type changed, then the type + change will *NOT* be suppressed by the evaluation of this property, + unless the + :ref:`has_size_change` property + is present and set to ``yes``. [...] > diff --git a/src/abg-ir.cc b/src/abg-ir.cc > index 4a652b0f..7ba7f72d 100644 > --- a/src/abg-ir.cc > +++ b/src/abg-ir.cc > @@ -10822,6 +10822,34 @@ var_decl_sptr > has_flexible_array_data_member(const class_decl_sptr& klass) > {return has_flexible_array_data_member(klass.get());} > =20 > +var_decl_sptr > +has_fake_flexible_array_data_member(const class_decl& klass) I have added doxygen comments to this function as I haven't done it initially. [...] > + > +var_decl_sptr > +has_fake_flexible_array_data_member(const class_decl* klass) Likewise. > +{return has_fake_flexible_array_data_member(*klass);} > + > +var_decl_sptr > +has_fake_flexible_array_data_member(const class_decl_sptr& klass) Likewise. > +{return has_fake_flexible_array_data_member(klass.get());} > + > /// Test wheter a type is a declaration-only class. > /// > /// @param t the type to considier. [...] > --- a/src/abg-suppression.cc > +++ b/src/abg-suppression.cc > @@ -808,6 +808,14 @@ void > type_suppression::set_changed_enumerators_regexp(const vector& n) > {priv_->changed_enumerators_regexp_ =3D n;} > =20 > +bool > +type_suppression::has_strict_fam_conversion () const Likewise. > +{return priv_->has_strict_fam_conv_;} > + > +void > +type_suppression::set_has_strict_fam_conversion(bool f) Likewise. > +{priv_->has_strict_fam_conv_ =3D f;} > + [...] > @@ -1027,6 +1037,28 @@ type_suppression::suppresses_diff(const diff* diff= ) const > =09 else > =09 return false; > =09} > + > + // Support for the > + // "has_strict_flexible_array_data_member_conversion =3D true" > + // clause. > + if (has_strict_fam_conversion()) > +=09{ > +=09 // Let's detect if the first class of the diff has a fake > +=09 // flexible array data member that got turned into a real > +=09 // flexible array data member. > +=09 if (!( > +=09=09(get_has_size_change()) > +=09=09// This situation will always result in a size change, > +=09=09// so check that "has_size_change =3D true" before > +=09=09// processing further > +=09=09&& > +=09=09(has_fake_flexible_array_data_member(first_class) > +=09=09 && has_flexible_array_data_member(second_class)) > +=09=09// A fake flexible array member has been changed into > +=09=09// a real flexible array ... > +=09=09)) > +=09 return false; > +=09} Following our discussion at https://sourceware.org/bugzilla/show_bug.cgi?id=3D31017#c3, I replaced the hunk above with this one: + if (has_strict_fam_conversion()) +=09{ +=09 // Let's detect if the first class of the diff has a fake +=09 // flexible array data member that got turned into a real +=09 // flexible array data member. +=09 if (!((get_has_size_change() || ((first_class->get_size_in_bits() +=09=09=09=09=09 =3D=3D second_class->get_size_in_bits()))) +=09=09&& filtering::has_strict_fam_conversion(klass_diff))) +=09 return false; +=09} The new filtering::has_strict_fam_conversion function was introduced to encapsulate the detection of a strict fam conversion and it's in abg-comp-filter.cc. > } [...] > diff --git a/tests/test-diff-suppr.cc b/tests/test-diff-suppr.cc > index 8c9ad070..119be55b 100644 > --- a/tests/test-diff-suppr.cc > +++ b/tests/test-diff-suppr.cc > @@ -2376,6 +2376,26 @@ InOutSpec in_out_specs[] =3D > "data/test-diff-suppr/test-has-data-member-inserted-at-2-report.3.tx= t", > "output/test-diff-suppr/test-has-data-member-inserted-at-2-report.3.= txt" > }, > + { > + "data/test-diff-suppr/test-has-strict-flexible-array-data-member-con= version-v0.o", > + "data/test-diff-suppr/test-has-strict-flexible-array-data-member-con= version-v1.o", > + "", > + "", > + "data/test-diff-suppr/test-has-strict-flexible-array-data-member-con= version-1.suppr", > + "--drop-private-types --no-default-suppression --non-reachable-types= ", > + "data/test-diff-suppr/test-has-strict-flexible-array-data-member-con= version-report-1.txt", > + "output/test-diff-suppr/test-has-strict-flexible-array-data-member-c= onversion-report-1.txt", > + }, > + { > + "data/test-diff-suppr/test-has-strict-flexible-array-data-member-con= version-v0.o", > + "data/test-diff-suppr/test-has-strict-flexible-array-data-member-con= version-v1.o", > + "", > + "", > + "data/test-diff-suppr/test-has-strict-flexible-array-data-member-con= version-2.suppr", > + "--drop-private-types --no-default-suppression --non-reachable-types= ", > + "data/test-diff-suppr/test-has-strict-flexible-array-data-member-con= version-report-2.txt", > + "output/test-diff-suppr/test-has-strict-flexible-array-data-member-c= onversion-report-2.txt", > + }, > // This should be the last entry > {NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL} > }; Thanks for adding these tests! It's really appreciated! Please find below the amended patch. It passes "make distcheck" and I have updated the WIP branch users/dodji/PR31017 with it. What do you think? >From fe1dbee8e8c5211c910d9b72fccd1230600be4ab Mon Sep 17 00:00:00 2001 From: Dodji Seketeli Date: Fri, 3 Nov 2023 17:47:57 -0700 Subject: [PATCH] suppression: Add "has_strict_flexible_array_data_member_conversion" property In the past, it was common to have a "fake flex array" at the end of a structure. Like this: Nowadays, with improved compiler support, it's more common to use a real flex array. As this is a common change which changes ABI representation in a compatible way, we should have a suppression for it. For example, if you have a change like this: struct foo { int x; int flex[1]; }; ... struct foo { int x; int flex[]; }; abidiff reports: [C] 'struct foo' changed: type size changed from 64 to 32 (in bits) 1 data member change: type of 'int flex[1]' changed: type name changed from 'int[1]' to 'int[]' array type size changed from 32 to 'unknown' array type subrange 1 changed length from 1 to 'unknown' With a new has_strict_flexible_array_data_member_conversion property, users can specify a suppression which stops abidiff from emitting this diff for any "fake" flex arrays being converted to real ones: [suppress_type] type_kind =3D struct has_size_change =3D true has_strict_flexible_array_data_member_conversion =3D true =09* include/abg-comp-filter.h (has_strict_fam_conversion): Declare =09new functions. =09* include/abg-fwd.h =09(ir::has_fake_flexible_array_data_member): Declare new accessor =09functions. =09* include/abg-suppression.h =09(type_suppression::{,set_}has_strict_fam_conversion): Declare new =09accessor functions. =09* src/abg-comp-filter.cc (has_strict_fam_conversion): Define new =09functions. =09* src/abg-ir.cc =09(ir::has_fake_flexible_array_data_member): Define new accessor =09functions. =09* src/abg-suppression-priv.h =09(type_suppression::priv::has_strict_fam_conv_): Define new =09data member. =09* src/abg-suppression.cc =09(type_suppression::{,set_}has_strict_fam_conversion): Define new =09accessor functions. =09(type_suppression::suppresses_diff): For a type suppression to =09match a fake flex array conversion, either the size of the type =09hasn't change or has_size_change must be true and then the type =09must change from a fake flex array to a real flex array. =09(read_type_suppression): Parse the new =09'has_strict_flexible_array_data_member_conversion' property to =09set the type_suppression::set_has_strict_fam_conversion property. =09* doc/manuals/libabigail-concepts.rst: Add an entry for the new =09'has_strict_flexible_array_data_member_conversion' property. =09* tests/data/test-diff-suppr/test-has-strict-flexible-array-data-member-= conversion-{1,2}.suppr: =09Add new test suppression files. =09* tests/data/test-diff-suppr/test-has-strict-flexible-array-data-member-= conversion-report-{1,2}.txt: =09Add new test reference output files. =09* tests/data/test-diff-suppr/test-has-strict-flexible-array-data-member-= conversion-v{0,1}.c: =09Add source code for new binary test input files. =09* tests/data/test-diff-suppr/test-has-strict-flexible-array-data-member-= conversion-v{0,1}.o: =09Add new binary test input files. =09* tests/data/Makefile.am: Add the new test files to the source =09distribution. =09* tests/test-diff-suppr.cc (in_out_specs): Add the new test input =09files to this test harness. Signed-off-by: Dodji Seketeli Signed-off-by: John Moon --- doc/manuals/libabigail-concepts.rst | 26 +++++- include/abg-comp-filter.h | 7 ++ include/abg-fwd.h | 9 ++ include/abg-suppression.h | 6 ++ src/abg-comp-filter.cc | 49 +++++++++++ src/abg-ir.cc | 83 +++++++++++++++++- src/abg-suppression-priv.h | 6 +- src/abg-suppression.cc | 52 +++++++++-- tests/data/Makefile.am | 8 ++ ...xible-array-data-member-conversion-1.suppr | 4 + ...xible-array-data-member-conversion-2.suppr | 3 + ...-array-data-member-conversion-report-1.txt | 4 + ...-array-data-member-conversion-report-2.txt | 14 +++ ...flexible-array-data-member-conversion-v0.c | 11 +++ ...flexible-array-data-member-conversion-v0.o | Bin 0 -> 2440 bytes ...flexible-array-data-member-conversion-v1.c | 11 +++ ...flexible-array-data-member-conversion-v1.o | Bin 0 -> 2432 bytes tests/test-diff-suppr.cc | 20 +++++ 18 files changed, 303 insertions(+), 10 deletions(-) create mode 100644 tests/data/test-diff-suppr/test-has-strict-flexible-arr= ay-data-member-conversion-1.suppr create mode 100644 tests/data/test-diff-suppr/test-has-strict-flexible-arr= ay-data-member-conversion-2.suppr create mode 100644 tests/data/test-diff-suppr/test-has-strict-flexible-arr= ay-data-member-conversion-report-1.txt create mode 100644 tests/data/test-diff-suppr/test-has-strict-flexible-arr= ay-data-member-conversion-report-2.txt create mode 100644 tests/data/test-diff-suppr/test-has-strict-flexible-arr= ay-data-member-conversion-v0.c create mode 100644 tests/data/test-diff-suppr/test-has-strict-flexible-arr= ay-data-member-conversion-v0.o create mode 100644 tests/data/test-diff-suppr/test-has-strict-flexible-arr= ay-data-member-conversion-v1.c create mode 100644 tests/data/test-diff-suppr/test-has-strict-flexible-arr= ay-data-member-conversion-v1.o diff --git a/doc/manuals/libabigail-concepts.rst b/doc/manuals/libabigail-c= oncepts.rst index 28e71684..7b73aaa8 100644 --- a/doc/manuals/libabigail-concepts.rst +++ b/doc/manuals/libabigail-concepts.rst @@ -619,9 +619,28 @@ names start with the string "private_data_member". =09 {72, end} } =20 +.. _suppr_has_strict_flexible_array_data_member_conversion_label: =20 =20 - .. _suppr_has_size_change_property_label: +* ``has_strict_flexible_array_data_member_conversion`` + + Usage: + + ``has_strict_flexible_array_data_member_conversion`` ``=3D`` yes | no + + Suppresses change reports involving a type which has a "fake" + flexible array member at the end of the struct which is converted + to a real flexible array member. This would be a member like + ``data[1]`` being converted to ``data[]``. + + Please note that if the size of the type changed, then the type + change will *NOT* be suppressed by the evaluation of this property, + unless the + :ref:`has_size_change` property + is present and set to ``yes``. + +.. _suppr_has_size_change_property_label: + =20 * ``has_size_change`` =20 @@ -631,9 +650,10 @@ names start with the string "private_data_member". =20 =20 This property is to be used in conjunction with the properties -:ref:`has_data_member_inserted_between` +:ref:`has_data_member_inserted_between`, +:ref:`has_data_members_inserted_between`, and -:ref:`has_data_members_inserted_between`. +:ref:`has_strict_flexible_array_data_member_conversion` Those properties will not match a type change if the size of the type changes, unless the ``has_size_changes`` property is set to ``yes``. =20 diff --git a/include/abg-comp-filter.h b/include/abg-comp-filter.h index cd12b314..8d11fdd2 100644 --- a/include/abg-comp-filter.h +++ b/include/abg-comp-filter.h @@ -98,6 +98,13 @@ bool is_var_1_dim_unknown_size_array_change(const var_decl_sptr& var1, =09=09=09=09 const var_decl_sptr& var2); =20 +bool +has_strict_fam_conversion(const class_decl_sptr& first, +=09=09=09 const class_decl_sptr& second); + +bool +has_strict_fam_conversion(const diff *d); + struct filter_base; /// Convenience typedef for a shared pointer to filter_base typedef shared_ptr filter_base_sptr; diff --git a/include/abg-fwd.h b/include/abg-fwd.h index 7d6637b9..de5b72b0 100644 --- a/include/abg-fwd.h +++ b/include/abg-fwd.h @@ -490,6 +490,15 @@ has_flexible_array_data_member(const class_decl*); var_decl_sptr has_flexible_array_data_member(const class_decl_sptr&); =20 +var_decl_sptr +has_fake_flexible_array_data_member(const class_decl&); + +var_decl_sptr +has_fake_flexible_array_data_member(const class_decl*); + +var_decl_sptr +has_fake_flexible_array_data_member(const class_decl_sptr&); + bool is_declaration_only_class_or_union_type(const type_base *t, =09=09=09=09=09bool look_through_decl_only =3D false); diff --git a/include/abg-suppression.h b/include/abg-suppression.h index 996600bb..dd0870bc 100644 --- a/include/abg-suppression.h +++ b/include/abg-suppression.h @@ -336,6 +336,12 @@ public: void set_changed_enumerators_regexp(const vector&); =20 + bool + has_strict_fam_conversion () const; + + void + set_has_strict_fam_conversion(bool); + virtual bool suppresses_diff(const diff* diff) const; =20 diff --git a/src/abg-comp-filter.cc b/src/abg-comp-filter.cc index e02e9430..82a819d6 100644 --- a/src/abg-comp-filter.cc +++ b/src/abg-comp-filter.cc @@ -712,6 +712,55 @@ is_var_1_dim_unknown_size_array_change(const diff* dif= f) return is_var_1_dim_unknown_size_array_change(f, s); } =20 +/// Test if a class with a fake flexible data member got changed into +/// a class with a real fexible data member. +/// +/// A fake flexible array data member is a data member that is the +/// last of the class/struct which type is an array of one element. +/// This was used before C99 standardized flexible array data members. +/// +/// @param first the first version of the class to consider. +/// +/// @param second the second version of the class to consider. +/// +/// @return true iff @p first has a fake flexible array data member +/// that got changed into @p second with a real flexible array data +/// member. +bool +has_strict_fam_conversion(const class_decl_sptr& first, +=09=09=09 const class_decl_sptr& second) +{ + if (has_fake_flexible_array_data_member(first) + && has_flexible_array_data_member(second)) + // A fake flexible array member has been changed into + // a real flexible array ... + return true; + return false; +} + +/// Test if a diff node carries a change from class with a fake +/// flexible data member into a class with a real fexible data member. +/// +/// A fake flexible array data member is a data member that is the +/// last of the class/struct which type is an array of one element. +/// This was used before C99 standardized flexible array data members. +/// +/// @param the diff node to consider. +/// +/// @return true iff @p dif carries a change from class with a fake +/// flexible data member into a class with a real fexible data member. +/// member. +bool +has_strict_fam_conversion(const diff *dif) +{ + const class_diff* d =3D is_class_diff(dif); + if (!d) + return false; + + return has_strict_fam_conversion(d->first_class_decl(), +=09=09=09=09 d->second_class_decl()); +} + /// Test if a class_diff node has static members added or removed. /// /// @param diff the diff node to consider. diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 4a652b0f..50a4e0ab 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -10822,6 +10822,88 @@ var_decl_sptr has_flexible_array_data_member(const class_decl_sptr& klass) {return has_flexible_array_data_member(klass.get());} =20 +/// Test if the last data member of a class is an array with +/// one element. +/// +/// An array with one element is a way to mimic the flexible data +/// member idiom that was later standardized in C99. +/// +/// To learn more about the flexible data member idiom, please +/// consider reading : +/// https://en.wikipedia.org/wiki/Flexible_array_member. +/// +/// The various ways of representing that idiom pre-standardization +/// are presented in this article: +/// https://developers.redhat.com/articles/2022/09/29/benefits-limitations= -flexible-array-members# +/// +/// @param klass the class to consider. +/// +/// @return the data member which type is a fake flexible array, if +/// any, or nil. +var_decl_sptr +has_fake_flexible_array_data_member(const class_decl& klass) +{ + var_decl_sptr nil; + const class_or_union::data_members& dms =3D klass.get_data_members(); + if (dms.empty()) + return nil; + + if (array_type_def_sptr array =3D is_array_type(dms.back()->get_type())) + {// The type of the last data member is an array. + if (array->get_subranges().size() =3D=3D 1 +=09 && array->get_subranges()[0]->get_length() =3D=3D 1) +=09// The array has a size of one. We are thus looking at a +=09// "fake" flexible array data member. Let's return it. +=09return dms.back(); + } + + return nil; +} + +/// Test if the last data member of a class is an array with +/// one element. +/// +/// An array with one element is a way to mimic the flexible data +/// member idiom that was later standardized in C99. +/// +/// To learn more about the flexible data member idiom, please +/// consider reading : +/// https://en.wikipedia.org/wiki/Flexible_array_member. +/// +/// The various ways of representing that idiom pre-standardization +/// are presented in this article: +/// https://developers.redhat.com/articles/2022/09/29/benefits-limitations= -flexible-array-members# +/// +/// @param klass the class to consider. +/// +/// @return the data member which type is a fake flexible array, if +/// any, or nil. +var_decl_sptr +has_fake_flexible_array_data_member(const class_decl* klass) +{return has_fake_flexible_array_data_member(*klass);} + +/// Test if the last data member of a class is an array with +/// one element. +/// +/// An array with one element is a way to mimic the flexible data +/// member idiom that was later standardized in C99. +/// +/// To learn more about the flexible data member idiom, please +/// consider reading : +/// https://en.wikipedia.org/wiki/Flexible_array_member. +/// +/// The various ways of representing that idiom pre-standardization +/// are presented in this article: +/// https://developers.redhat.com/articles/2022/09/29/benefits-limitations= -flexible-array-members# +/// +/// @param klass the class to consider. +/// +/// @return the data member which type is a fake flexible array, if +/// any, or nil. +var_decl_sptr +has_fake_flexible_array_data_member(const class_decl_sptr& klass) +{return has_fake_flexible_array_data_member(klass.get());} + /// Test wheter a type is a declaration-only class. /// /// @param t the type to considier. @@ -10844,7 +10926,6 @@ is_declaration_only_class_or_union_type(const type_= base *t, return false; } =20 - /// Test wheter a type is a declaration-only class. /// /// @param t the type to considier. diff --git a/src/abg-suppression-priv.h b/src/abg-suppression-priv.h index 351c5965..e4d65df8 100644 --- a/src/abg-suppression-priv.h +++ b/src/abg-suppression-priv.h @@ -586,6 +586,9 @@ class type_suppression::priv mutable regex::regex_t_sptr=09=09source_location_to_keep_regex_; mutable vector=09=09changed_enumerator_names_; mutable vector=09changed_enumerators_regexp_; + // Whether the "has_strict_flexible_array_data_member_conversion" + // property was set. + bool=09=09=09=09=09has_strict_fam_conv_; =20 priv(); =20 @@ -602,7 +605,8 @@ public: type_kind_(type_kind), consider_reach_kind_(consider_reach_kind), reach_kind_(reach_kind), - has_size_change_(false) + has_size_change_(false), + has_strict_fam_conv_(false) {} =20 /// Get the regular expression object associated to the 'type_name_regex= ' diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc index 326d003e..0fb6d057 100644 --- a/src/abg-suppression.cc +++ b/src/abg-suppression.cc @@ -808,6 +808,21 @@ void type_suppression::set_changed_enumerators_regexp(const vector& n) {priv_->changed_enumerators_regexp_ =3D n;} =20 +/// Getter of the "has_string_fam_conversion" property. +/// +/// @return the value of the "has_string_fam_conversion" property. +bool +type_suppression::has_strict_fam_conversion () const +{return priv_->has_strict_fam_conv_;} + +/// Setter of the "has_string_fam_conversion" property. +/// +/// @param f the new value of the "has_string_fam_conversion" +/// property. +void +type_suppression::set_has_strict_fam_conversion(bool f) +{priv_->has_strict_fam_conv_ =3D f;} + /// Evaluate this suppression specification on a given diff node and /// say if the diff node should be suppressed or not. /// @@ -967,6 +982,11 @@ type_suppression::suppresses_diff(const diff* diff) co= nst const class_diff* klass_diff =3D dynamic_cast(d); if (klass_diff) { + const class_decl_sptr& first_class =3D +=09klass_diff->first_class_decl(); + const class_decl_sptr& second_class =3D +=09klass_diff->second_class_decl(); + // We are looking at a class diff ... if (!get_data_member_insertion_ranges().empty()) =09{ @@ -981,9 +1001,6 @@ type_suppression::suppresses_diff(const diff* diff) co= nst =09 // that suppression applies to types that have size =09 // change. =20 -=09 const class_decl_sptr& first_type_decl =3D -=09=09klass_diff->first_class_decl(); - =09 if (klass_diff->inserted_data_members().empty() =09=09 && klass_diff->changed_data_members().empty()) =09=09// So there is a has_data_member_inserted_* clause, @@ -1001,7 +1018,7 @@ type_suppression::suppresses_diff(const diff* diff) c= onst =09=09 for (const auto& range : get_data_member_insertion_ranges()) =09=09 if (is_data_member_offset_in_range(is_var_decl(member), =09=09=09=09=09=09 range, -=09=09=09=09=09=09 first_type_decl.get())) +=09=09=09=09=09=09 first_class.get())) =09=09 matched =3D true; =20 =09=09 if (!matched) @@ -1017,7 +1034,7 @@ type_suppression::suppresses_diff(const diff* diff) c= onst =20 =09=09 for (const auto& range : get_data_member_insertion_ranges()) =09=09 if (is_data_member_offset_in_range(member, range, -=09=09=09=09=09=09 first_type_decl.get())) +=09=09=09=09=09=09 first_class.get())) =09=09 matched =3D true; =20 =09=09 if (!matched) @@ -1027,6 +1044,20 @@ type_suppression::suppresses_diff(const diff* diff) = const =09 else =09 return false; =09} + + // Support for the + // "has_strict_flexible_array_data_member_conversion =3D true" + // clause. + if (has_strict_fam_conversion()) +=09{ +=09 // Let's detect if the first class of the diff has a fake +=09 // flexible array data member that got turned into a real +=09 // flexible array data member. +=09 if (!((get_has_size_change() || ((first_class->get_size_in_bits() +=09=09=09=09=09 =3D=3D second_class->get_size_in_bits()))) +=09=09&& filtering::has_strict_fam_conversion(klass_diff))) +=09 return false; +=09} } =20 const enum_diff* enum_dif =3D dynamic_cast(d); @@ -2321,6 +2352,14 @@ read_type_suppression(const ini::config::section& se= ction) } } =20 + // Support "has_strict_flexible_array_data_member_conversion" + ini::simple_property_sptr has_strict_fam_conv =3D + is_simple_property + (section.find_property("has_strict_flexible_array_data_member_conversi= on")); + string has_strict_fam_conv_str =3D has_strict_fam_conv + ? has_strict_fam_conv->get_value()->as_string() + : ""; + if (section.get_name() =3D=3D "suppress_type") result.reset(new type_suppression(label_str, name_regex_str, name_str)= ); else if (section.get_name() =3D=3D "allow_type") @@ -2388,6 +2427,9 @@ read_type_suppression(const ini::config::section& sec= tion) && !changed_enumerators_regexp.empty()) result->set_changed_enumerators_regexp(changed_enumerators_regexp); =20 + if (has_strict_fam_conv_str =3D=3D "yes" || has_strict_fam_conv_str =3D= =3D "true") + result->set_has_strict_fam_conversion(true); + return result; } =20 diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 945831ff..28d767d5 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -1930,6 +1930,14 @@ test-diff-suppr/test-has-data-member-inserted-at-1-v= 0.o \ test-diff-suppr/test-has-data-member-inserted-at-1-v1.c \ test-diff-suppr/test-has-data-member-inserted-at-1-v1.o \ test-diff-suppr/test-has-data-member-inserted-at-1.1.suppr \ +test-diff-suppr/test-has-strict-flexible-array-data-member-conversion-1.su= ppr \ +test-diff-suppr/test-has-strict-flexible-array-data-member-conversion-2.su= ppr \ +test-diff-suppr/test-has-strict-flexible-array-data-member-conversion-repo= rt-1.txt \ +test-diff-suppr/test-has-strict-flexible-array-data-member-conversion-repo= rt-2.txt \ +test-diff-suppr/test-has-strict-flexible-array-data-member-conversion-v0.c= \ +test-diff-suppr/test-has-strict-flexible-array-data-member-conversion-v1.c= \ +test-diff-suppr/test-has-strict-flexible-array-data-member-conversion-v0.o= \ +test-diff-suppr/test-has-strict-flexible-array-data-member-conversion-v1.o= \ \ test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1 \ test-diff-dwarf-abixml/test0-pr19026-libvtkIOSQL-6.1.so.1.abi \ diff --git a/tests/data/test-diff-suppr/test-has-strict-flexible-array-data= -member-conversion-1.suppr b/tests/data/test-diff-suppr/test-has-strict-fle= xible-array-data-member-conversion-1.suppr new file mode 100644 index 00000000..5cb8d880 --- /dev/null +++ b/tests/data/test-diff-suppr/test-has-strict-flexible-array-data-member= -conversion-1.suppr @@ -0,0 +1,4 @@ +[suppress_type] + type_kind =3D struct + has_size_change =3D true + has_strict_flexible_array_data_member_conversion =3D true diff --git a/tests/data/test-diff-suppr/test-has-strict-flexible-array-data= -member-conversion-2.suppr b/tests/data/test-diff-suppr/test-has-strict-fle= xible-array-data-member-conversion-2.suppr new file mode 100644 index 00000000..384409d0 --- /dev/null +++ b/tests/data/test-diff-suppr/test-has-strict-flexible-array-data-member= -conversion-2.suppr @@ -0,0 +1,3 @@ +[suppress_type] + type_kind =3D struct + has_strict_flexible_array_data_member_conversion =3D true diff --git a/tests/data/test-diff-suppr/test-has-strict-flexible-array-data= -member-conversion-report-1.txt b/tests/data/test-diff-suppr/test-has-stric= t-flexible-array-data-member-conversion-report-1.txt new file mode 100644 index 00000000..b4ea5bf1 --- /dev/null +++ b/tests/data/test-diff-suppr/test-has-strict-flexible-array-data-member= -conversion-report-1.txt @@ -0,0 +1,4 @@ +Functions changes summary: 0 Removed, 0 Changed, 0 Added function +Variables changes summary: 0 Removed, 0 Changed, 0 Added variable +Unreachable types summary: 0 removed, 0 changed (1 filtered out), 0 added = type + diff --git a/tests/data/test-diff-suppr/test-has-strict-flexible-array-data= -member-conversion-report-2.txt b/tests/data/test-diff-suppr/test-has-stric= t-flexible-array-data-member-conversion-report-2.txt new file mode 100644 index 00000000..2352dd4e --- /dev/null +++ b/tests/data/test-diff-suppr/test-has-strict-flexible-array-data-member= -conversion-report-2.txt @@ -0,0 +1,14 @@ +Functions changes summary: 0 Removed, 0 Changed, 0 Added function +Variables changes summary: 0 Removed, 0 Changed, 0 Added variable +Unreachable types summary: 0 removed, 1 changed, 0 added type + +1 changed type unreachable from any public interface: + + [C] 'struct foo' changed: + type size changed from 64 to 32 (in bits) + 1 data member change: + type of 'int flex[1]' changed: + type name changed from 'int[1]' to 'int[]' + array type size changed from 32 to 'unknown' + array type subrange 1 changed length from 1 to 'unknown' + diff --git a/tests/data/test-diff-suppr/test-has-strict-flexible-array-data= -member-conversion-v0.c b/tests/data/test-diff-suppr/test-has-strict-flexib= le-array-data-member-conversion-v0.c new file mode 100644 index 00000000..1397cd52 --- /dev/null +++ b/tests/data/test-diff-suppr/test-has-strict-flexible-array-data-member= -conversion-v0.c @@ -0,0 +1,11 @@ +/* + * Compile this with: + * gcc -g -c test-has-strict-flexible-array-data-member-conversion-v0.c + */ +struct foo +{ + int x; + int flex[1]; +}; + +struct foo S; diff --git a/tests/data/test-diff-suppr/test-has-strict-flexible-array-data= -member-conversion-v0.o b/tests/data/test-diff-suppr/test-has-strict-flexib= le-array-data-member-conversion-v0.o new file mode 100644 index 0000000000000000000000000000000000000000..8d0a755d353d580e6afb96dc55a= 021322caf5a93 GIT binary patch literal 2440 zcmbtV!EVz)5FN)&<6;_GMF^^b* zJ;IGM=3DUzDT1$+hP{sNqkIFy<7Zt5&SNOUCc%)FV|ncbaTA3S{eD5q&4NrP25(Ig7+ zA%7y*xLAWJI0pgBV_Y|Q*jX-%A=3DeB;FtmIPb$t)C0tuHXTSsu*fE@IwG_~m$hj2%H#b`D;U=3D!#WnGU&c0CMsSrq$WVDDC( zI!{FIrB1j&7CTgzat^mfwcaPTX_rtlaTE*YjJh-zn3| z<1#Ifi+00rHto3I>qP+TYirBa{Fc`bl73p7eY?-K>bvTaQ(3s4q9Zmt15wkB!S~|~ zG!XTXos2yqPIBNV1RX7-NEaQh+H@FKA+GDdoA(c!4ikT11n6+)FZj@H8u;jd-Kx-i z)TiOcvFuLCPt!P?IHz18RVZajEh{6OR#n4~XgXzlMTE;|IYdHL9%A5Q`SN_IPrcraXsWol=3D8Ne61JCsLtP7w7sud4tm8Tj=3DJi|K ze$Wij#%0&@BDOnZb|W`vF$_sPI)1>UjI)DN54&B4SH}tPb~$)6@pj>D;}xB1n8j#X zyIilFtR@A8lW&&hP$5jc0ky1CMpommp^eV(f8(ndtIkR3YBV6>gE2&^?@hGQf2cO( z1z@E=3DC-^GxN#q2{UZc02XazrYPQt&?fV7*k!KnH^M~jM)^Sl5pQZl6aK0({4`j15Y zvQDa>UKh3gZN$_$5rW48d`(J*R9|XA!YUab5)XyrHwBmRlGFZ-ir-IvnsDPE@dqMa zaf3qjS+$-M7SY z-6&p&0wD`Rqhkd|5c*CdG+JG@=3Dh$6lSiW!V8%-;;j2`RR%r_dIJ7j*~c&;(5n2jhA zxx@OiZGE*;(I1%i%>~`)g#(u@x0%auM^r7p(OKTBEgFke-DvAZD+sN|uF?0skTpWj zr_`R?hAuApzzv+X%bGe}`#Fj$4+8&g`JLD4d0zR0=3DkEr6tHH`$$F^*zZ8_aCZ61{A zfRSi4omR^T2K~Mdu)eywq~F}O2W~isk7e8&L|WxzrDiTvZ^dZJWycUTTNZvEr#Yyp z9gNMvKcXC?q9u(~-r2HEg)te5bQPkmJ!4a0{0v5b3K#x@pPHhPj|$k*5sn@$*@O0uSK%tov1dj~Dl`}L7WEKKu( z_{Q-GA zagkTxq^G%x_!