From patchwork Tue Apr 16 15:53:33 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 88552 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2007E3858C98 for ; Tue, 16 Apr 2024 15:53:50 +0000 (GMT) X-Original-To: libabigail@sourceware.org Delivered-To: libabigail@sourceware.org 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 E5B8C3858D37 for ; Tue, 16 Apr 2024 15:53:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org E5B8C3858D37 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 E5B8C3858D37 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=1713282824; cv=none; b=iOQOx0N70a8fxfNDcjpu5CU/uISuaelFcux9iRTJmDb5iXYXLBNfVTXTbyX3QOS9VYYUPLlv/rZGzz4DlxMwXgwlUkBgSMHkuppjdFq5VayFOlEL3kHqNDkzz/pTxNPdGxFKzapBNM/LJ3KHnvh7DvXq+EQaEj+qutBKCEE2/zk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1713282824; c=relaxed/simple; bh=ejgP2UlEx+WQoU6fLS5BIq3Q9irSqeRRd7I5FkjY0sA=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=W1LytjpPQH4Wlrf/h40h3hZ7qCBc+eepxYJKXGUGNiLy/EBD6nkCjHbM5b/ACd2NXpJsYDfBkVeWVSIt7XysU6GykMQmNx9H/uyvWXixQt416pEEi5n68j/3UrhCTF3kU9aPt8xlnk43Bmdqo7XtVmLwZ1NNg9kuGmH67dTHTt4= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1713282819; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type; bh=tnzMyyDDNQWtOrwT/tCIAOqaBDz5StfmmCxurdsg2BE=; b=XHeQRqFigwTO5WVASe45T/p0YzQ6IE7Sv0Co9lGm19HJqRqAKopLp5O9SCTKa21A9nFUZe 4kQ//1iL3TpaD+w00spYV3IFIHOTJDwk+r8Ca7Dwm7K1W+VnLpiG4xP8pcCdjUATKM2Hsx cuRU3by3Cw/kOOOrtqKAmv2toWrJvlE= Received: from mail-qv1-f72.google.com (mail-qv1-f72.google.com [209.85.219.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-6-2rDGHXarNne9BcehXCrLJA-1; Tue, 16 Apr 2024 11:53:37 -0400 X-MC-Unique: 2rDGHXarNne9BcehXCrLJA-1 Received: by mail-qv1-f72.google.com with SMTP id 6a1803df08f44-69b20caadcaso48419766d6.0 for ; Tue, 16 Apr 2024 08:53:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713282817; x=1713887617; h=mime-version:user-agent:message-id:date:organization:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=tnzMyyDDNQWtOrwT/tCIAOqaBDz5StfmmCxurdsg2BE=; b=kD0RTxLh/pJMBGDp6AR/ukWLMKPSOR/bn0m9bpU8inuGKtQste5MBAw3GDuczl1+f6 ot5qDK6YKHjBtAFTqmMRTOv+1lCnU5cujO4kYhhl8k4+Q3tySB5VsAju3/wmVbkXlwNn t/9DTOZFteMqVbk4GIxYlYh7+tZK/dJWWlK/wEjxmeKY1vnkwsO4uHoJVQLutu9fPdT3 3Hv1M3OckSkrkP4BzUPZDFQ1pAW9UnJyRAdhY4F4KPR0Yu89fuVwMUJjiRAEV3ZEeGi3 PTCvLkFgcG7atSkdzkQC4C7aP6anReT4cubvDEb/vdCb2NI1W6ld/Cs+VmxdtmHO6UVj 0PWw== X-Gm-Message-State: AOJu0Yyn8OuT2YmTVI8VUrUTJXMfothLdyoL2lEvlZtD3hSIetBRYZDF D5FZ6V9NbRaeamIVYaZpFB0T6qCkv6xN/njpcFix+9f3QJ0D9PMToD94SiRA28FY/RjiIYQaO29 2BcfyTRXvW6vlVi0vu6sbhqssvMwMG9vPGUVbfEeB0a9DgApg1NtUf0sSV1ZhS1XK5TTL/GYVSA LvKpkHJVRhq21prEqgZt4D08T+4dnPb8F3huxhmw== X-Received: by 2002:ad4:5993:0:b0:699:3883:5474 with SMTP id ek19-20020ad45993000000b0069938835474mr11090366qvb.33.1713282816710; Tue, 16 Apr 2024 08:53:36 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFdgeIel3VEK9AmRi1H1AU3fiLKgtVYnJzsT1rjpaxiL+o+Jrb7cNKQJTRooz4tDI50K4F5GQ== X-Received: by 2002:ad4:5993:0:b0:699:3883:5474 with SMTP id ek19-20020ad45993000000b0069938835474mr11090326qvb.33.1713282815652; Tue, 16 Apr 2024 08:53:35 -0700 (PDT) Received: from localhost (88-120-130-27.subs.proxad.net. [88.120.130.27]) by smtp.gmail.com with ESMTPSA id i16-20020a0cfcd0000000b006994062299dsm7492781qvq.33.2024.04.16.08.53.34 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 16 Apr 2024 08:53:35 -0700 (PDT) Received: by localhost (Postfix, from userid 1000) id 159295077C43; Tue, 16 Apr 2024 17:53:33 +0200 (CEST) From: Dodji Seketeli To: libabigail@sourceware.org Subject: [PATCH, applied] Bug 31646: Fix type suppression tactics for webkit2gtk3 Organization: Red Hat / France X-Operating-System: AlmaLinux 9.3 X-URL: http://www.redhat.com Date: Tue, 16 Apr 2024 17:53:33 +0200 Message-ID: <87ttk1i9s2.fsf@redhat.com> 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 X-Spam-Status: No, score=-12.6 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_NUMSUBJECT, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, TXREP, T_FILL_THIS_FORM_SHORT autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Hello, By default, using abidw --abidiff on the binary /usr/lib64/libwebkit2gtk-4.0.so.37 on the ppc64le platform takes more than 50GB of RAM and ooms on my system. That is why a number of suppression specifications have been put in place in the default.abignore file that is installed by default on the system. With that suppression specification however, abidw --abidiff hits an assert and crashes. That is because the suppression specification rule below is too eager and thus removes parameters from some function's signature, in the final internal representation: [suppress_type] soname_regexp = libwebkit2?gtk-.*\\.so.* name_regexp = (^std::.*|WebCore::.*|WebKit::.*) drop = true An example of function signature that got changed (in the IR) because of this suppression specification is: This function signature: WebKitAuthenticationScheme webkit_authentication_request_get_scheme(WebKitAuthenticationRequest*) becomes (after applying the suppression specification above): void webkit_authentication_request_get_scheme() Woops. That is not good. To address that problem, this patch changes that suppression specification to make libabigail transform those types into opaque types. An opaque struct, class or enum is essentially a declaration-only type of the same kind and name. So the suppression specification above becomes: [suppress_type] # Drop all standard c++ types on the floor like before. soname_regexp = libwebkit2?gtk-.*\\.so.* name_regexp = ^std::.* drop = true [suppress_type] # Transform all C++ types in the WebCore and WebKit namespaces into # opaque types. These are essentially internal types of libwebkit2 # anyway. This greatly reduces the size of the in-memory working set. label = libabigail::OPAQUE_TYPE_LABEL soname_regexp = libwebkit2?gtk-.*\\.so.* name_regexp = (WebCore::.*|WebKit::.*) drop = true Notice the introduction of the new special label "libabigail::OPAQUE_TYPE_LABEL". This new special label is what makes the suppression engine of libabigail transform a type (that matches that suppression specification) into an opaque type, rather than dropping it on the floor. The patch then adapts the code of the suppression engine to detect this special label for the purpose of transforming types into opaque types. The patch normalizes the use of the term "opaque type" instead of "private type" that was used previously in the code. The patch fixes a bug in the code of get_opaque_version_of_type that prevents this function from working on types with no location information. While working on this, I have noticed that the abipkgdiff has a bug that was making it ignore suppression specifications provided via the --suppression option. The patch fixes that. I have noticed that abipkgdiff --self-check completely ignores all suppression specifications provided either via --suppression or indirectly via devel packages. The patch fixes that. Last but not least, fedabipkgdiff --self-compare completely ignores suppression specifications too! The patch fixes that as well. With all those fixes, the system now takes around 7GB of RAM and ~ 30 minutes to complete the analysis of the webkit2gtk3 package, using a non-optimized libabigail build. The command I use is: $ time tools/fedabipkgdiff --debug --self-compare --abipkgdiff build/tools/abipkgdiff --suppressions default.abignore -a --from fc39 webkit2gtk3 [...] [DEBUG] Result from run_abipkgdiff: 0, in: 0:29:00.088735 [DEBUG] Result from self_compare_rpms_from_distro: 0, in: 0:29:20.721748 real 29m20,846s user 29m4,561s sys 2m41,611s $ The culprit package was webkit2gtk3-2.40.1-1.fc36.ppc64le.rpm. To reproduce the issue on that package, I do: $ time ~/git/libabigail/rhbz2273891/build/tools/abipkgdiff --self-check --suppr ~/git/libabigail/rhbz2273891/default.abignore --d1 webkit2gtk3-debuginfo-2.40.1-1.fc36.ppc64le.rpm --devel1 webkit2gtk3-devel-2.40.1-1.fc36.ppc64le.rpm webkit2gtk3-2.40.1-1.fc36.ppc64le.rpm ==== SELF CHECK SUCCEEDED for 'libwebkit2gtk-4.0.so.37.63.2' ==== ==== SELF CHECK SUCCEEDED for 'WebKitWebDriver' ==== ==== SELF CHECK SUCCEEDED for 'WebKitNetworkProcess' ==== ==== SELF CHECK SUCCEEDED for 'WebKitWebProcess' ==== real 8m25,895s user 8m46,433s sys 0m15,683s $ * default.abignore: Split the libwebkit2gtk suppression specification for types whose names match the regexp ^std::.*|WebCore::.*|WebKit::.* into two. One that drops type with names being ^std::.* and another one that turns type being WebCore::.*|WebKit::.* into opaque types. * doc/manuals/libabigail-concepts.rst: Add documentation for the new special label libabigail::OPAQUE_TYPE_LABEL that transforms types matching a [suppress_type] into opaque types. * include/abg-suppression.h (is_opaque_type_suppr_spec): Renamed is_private_type_suppr_spec into this. (get_opaque_types_suppr_spec_label): Renamed get_private_types_suppr_spec_label. * src/abg-comparison.cc (diff::is_suppressed): Adjust. * src/abg-dwarf-reader.cc (type_is_suppressed): Adjust. Change the name of the type_is_private parameter into type_is_opaque. (get_opaque_version_of_type): Do not return early if the type has no associated location. What was I thinking. (build_ir_node_from_die): For enums, struct and classes adjust call to type_is_suppressed. * src/abg-suppression.cc (type_suppression::suppresses_diff): Adjust to using is_opaque_type_suppr_spec and get_opaque_version_of_type in lieu of is_private_type_suppr_spec and get_private_types_suppr_spec_label. (get_opaque_types_suppr_spec_label): Rename get_private_types_suppr_spec_label into this. Also, rename the name of the special label that specifies opaque types from "Artificial private types suppression specification" to "libabigail::OPAQUE_TYPE_LABEL". (is_opaque_type_suppr_spec): Rename is_private_type_suppr_spec into this. (is_type_suppressed): Rename the "type_is_private" parameter into "type_is_opaque". * src/abg-tools-utils.cc (handle_file_entry): Adjust to using get_opaque_types_suppr_spec_label rather than get_private_types_suppr_spec_label. * tools/abipkgdiff.cc (compare): Use the supprs variable where all the suppression specifications got accumulated, not just the priv_types_supprs1. (compare_to_self): Add a suppression specifications variable for private types. Add those private types specs to the user-provided ones. * tools/fedabipkgdiff (abipkgdiff): In the self comparison mode, take into account devel packages and suppression specifications. Pass those to the abipkgdiff tool's invocation. * tests/data/test-diff-suppr/PR31646/test-PR31646-result-[1-3].txt: New reference test output files. * tests/data/test-diff-suppr/PR31646/test-PR31646-v{0,1}.cc: Source code of binary inputs below. * tests/data/test-diff-suppr/PR31646/test-PR31646-v{0,1}.o: Binary input files. * tests/data/test-diff-suppr/PR31646/test-PR31646.2.abignore: Suppression specification file. * tests/data/test-diff-suppr/PR31646/test-PR31646.abignore: Likewise. * tests/data/Makefile.am: Add the new test material above to source distribution. * tests/test-diff-suppr.cc (in_out_specs): Add the new test input to this harness. Signed-off-by: Dodji Seketeli Applied to the master branch. --- default.abignore | 21 +++++++++- doc/manuals/libabigail-concepts.rst | 36 ++++++++++++++---- include/abg-suppression.h | 6 +-- src/abg-comparison.cc | 2 +- src/abg-dwarf-reader.cc | 26 ++++++------- src/abg-suppression.cc | 32 ++++++++-------- src/abg-tools-utils.cc | 2 +- tests/data/Makefile.am | 9 +++++ .../PR31646/test-PR31646-result-1.txt | 21 ++++++++++ .../PR31646/test-PR31646-result-2.txt | 0 .../PR31646/test-PR31646-result-3.txt | 10 +++++ .../PR31646/test-PR31646-v0.cc | 17 +++++++++ .../test-diff-suppr/PR31646/test-PR31646-v0.o | Bin 0 -> 3200 bytes .../PR31646/test-PR31646-v1.cc | 18 +++++++++ .../test-diff-suppr/PR31646/test-PR31646-v1.o | Bin 0 -> 3312 bytes .../PR31646/test-PR31646.2.abignore | 4 ++ .../PR31646/test-PR31646.abignore | 3 ++ tests/test-diff-suppr.cc | 30 +++++++++++++++ tools/abipkgdiff.cc | 27 ++++++++++--- tools/fedabipkgdiff | 2 + 20 files changed, 217 insertions(+), 49 deletions(-) create mode 100644 tests/data/test-diff-suppr/PR31646/test-PR31646-result-1.txt create mode 100644 tests/data/test-diff-suppr/PR31646/test-PR31646-result-2.txt create mode 100644 tests/data/test-diff-suppr/PR31646/test-PR31646-result-3.txt create mode 100644 tests/data/test-diff-suppr/PR31646/test-PR31646-v0.cc create mode 100644 tests/data/test-diff-suppr/PR31646/test-PR31646-v0.o create mode 100644 tests/data/test-diff-suppr/PR31646/test-PR31646-v1.cc create mode 100644 tests/data/test-diff-suppr/PR31646/test-PR31646-v1.o create mode 100644 tests/data/test-diff-suppr/PR31646/test-PR31646.2.abignore create mode 100644 tests/data/test-diff-suppr/PR31646/test-PR31646.abignore new file mode 100644 index 00000000..25733562 new file mode 100644 index 00000000..44ba7ba8 diff --git a/default.abignore b/default.abignore index 2df7f515..a963d5b3 100644 --- a/default.abignore +++ b/default.abignore @@ -91,7 +91,26 @@ [suppress_type] soname_regexp = libwebkit2?gtk-.*\\.so.* - name_regexp = (^std::.*|WebCore::.*|WebKit::.*) + name_regexp = ^std::.* + drop = true + +[suppress_type] +# Transform all C++ types in the WebCore and WebKit namespaces into +# opaque types. These are essentially internal types of libwebkit2 +# anyway. This greatly reduces the size of the in-memory working set. + label = libabigail::OPAQUE_TYPE_LABEL + soname_regexp = libwebkit2?gtk-.*\\.so.* + name_regexp = (^WebCore::.*|^WebKit::.*) + drop = true + +[suppress_type] +# All structs that don't start with either WebKit* or _WebKit are +# considered internal types and so are transform into opaque types. +# This helps to reduce the size of the in-memory working set further. + label = libabigail::OPAQUE_TYPE_LABEL + soname_regexp = libwebkit2?gtk-.*\\.so.* + type_kind = struct + name_not_regexp = ^_?WebKit.* drop = true ####################################################### diff --git a/doc/manuals/libabigail-concepts.rst b/doc/manuals/libabigail-concepts.rst index 7b73aaa8..27ac8178 100644 --- a/doc/manuals/libabigail-concepts.rst +++ b/doc/manuals/libabigail-concepts.rst @@ -242,7 +242,8 @@ The potential properties of this sections are listed below: Define a label for the section. A label is just an informative string that might be used by the tool to refer to a type suppression - in error messages. + in error messages. There can also be some special label names that + make the suppression system behave a certain way. * ``soname_regexp`` @@ -390,8 +391,17 @@ The potential properties of this sections are listed below: ABI being analyzed. This property makes its enclosing suppression specification to be applied in the :ref:`early suppression specification mode `. The net effect - is that it potentially reduces the memory used to represent the ABI - being analyzed. + is that it potentially reduces the amount of memory used to represent + the ABI being analyzed. + + Please note that for ``struct``, ``class`` or``enum`` types matching + a suppression specification with this property having a value set to + "yes" (or to "true"), if the specification also has a ``label`` + property with a value set to ``libabigail::OPAQUE_TYPE_LABEL``, then + the type is transformed into an opaque type, rather than being just + dropped on the floor. That also reduces the amount of memory used to + represent the ABI being analyzed, but with potentially less + disruption in the resulting ABI representation. Please note that for this property to be effective, the enclosing suppression specification must have at least one of the following @@ -665,10 +675,22 @@ changes, unless the ``has_size_changes`` property is set to ``yes``. ``label`` ``=`` - Define a label for the section. A label is just an informative - string that might be used by a tool to refer to a type suppression in - error messages. - + Define a label for the section. In general, a label is just an + informative string that might be used by a tool to refer to a type + suppression in error messages. + + Note however that there are some special label values that can + trigger a special kind of behavior. Below are those special label + values: + + * ``libabigail::OPAQUE_TYPE_LABEL``: A struct, class or enum type + that matches a ``[suppress_type]`` section with this label + property value is going to be replaced by an opaque type of the + same kind. Note that an opaque type is essentially a + declaration-only type, thus with no member. Also note that for a + ``[suppress_type]`` section with this label to trigger the + transformation of a type into an opaque type, the section must + have the ``drop`` property value set to ``yes|true``. * ``name`` diff --git a/include/abg-suppression.h b/include/abg-suppression.h index dd0870bc..500a96a8 100644 --- a/include/abg-suppression.h +++ b/include/abg-suppression.h @@ -949,13 +949,13 @@ suppression_matches_soname_or_filename(const string& soname, const suppression_base& suppr); const char* -get_private_types_suppr_spec_label(); +get_opaque_types_suppr_spec_label(); bool -is_private_type_suppr_spec(const type_suppression&); +is_opaque_type_suppr_spec(const type_suppression&); bool -is_private_type_suppr_spec(const suppression_sptr& s); +is_opaque_type_suppr_spec(const suppression_sptr& s); bool suppression_can_match(const fe_iface&, diff --git a/src/abg-comparison.cc b/src/abg-comparison.cc index 06298868..c9df2f0d 100644 --- a/src/abg-comparison.cc +++ b/src/abg-comparison.cc @@ -2620,7 +2620,7 @@ diff::is_suppressed(bool &is_private_type) const if (d->suppresses_diff(this)) { do_suppress = true; - if (is_private_type_suppr_spec(d)) + if (is_opaque_type_suppr_spec(d)) is_private_type = true; break; } diff --git a/src/abg-dwarf-reader.cc b/src/abg-dwarf-reader.cc index 5d5353ea..07596933 100644 --- a/src/abg-dwarf-reader.cc +++ b/src/abg-dwarf-reader.cc @@ -15259,9 +15259,9 @@ variable_is_suppressed(const reader& rdr, /// /// @param type_die the DIE that designates the type to consider. /// -/// @param type_is_private out parameter. If this function returns +/// @param type_is_opaque out parameter. If this function returns /// true (the type @p type_die is suppressed) and if the type was -/// suppressed because it's private then this parameter is set to +/// suppressed because it's opaque then this parameter is set to /// true. /// /// @return true iff the type designated by the DIE @p type_die, in @@ -15271,7 +15271,7 @@ static bool type_is_suppressed(const reader& rdr, const scope_decl* scope, Dwarf_Die *type_die, - bool &type_is_private) + bool &type_is_opaque) { if (type_die == 0 || (dwarf_tag(type_die) != DW_TAG_enumeration_type @@ -15288,7 +15288,7 @@ type_is_suppressed(const reader& rdr, return suppr::is_type_suppressed(rdr, qualified_name, type_location, - type_is_private, + type_is_opaque, /*require_drop_property=*/true); } @@ -15310,8 +15310,8 @@ type_is_suppressed(const reader& rdr, const scope_decl* scope, Dwarf_Die *type_die) { - bool type_is_private = false; - return type_is_suppressed(rdr, scope, type_die, type_is_private); + bool type_is_opaque = false; + return type_is_suppressed(rdr, scope, type_die, type_is_opaque); } /// Get the opaque version of a type that was suppressed because it's @@ -15355,8 +15355,6 @@ get_opaque_version_of_type(reader &rdr, string type_name, linkage_name; location type_location; die_loc_and_name(rdr, type_die, type_location, type_name, linkage_name); - if (!type_location) - return result; string qualified_name = build_qualified_name(scope, type_name); @@ -15874,10 +15872,10 @@ build_ir_node_from_die(reader& rdr, case DW_TAG_enumeration_type: { - bool type_is_private = false; + bool type_is_opaque = false; bool type_suppressed = - type_is_suppressed(rdr, scope, die, type_is_private); - if (type_suppressed && type_is_private) + type_is_suppressed(rdr, scope, die, type_is_opaque); + if (type_suppressed && type_is_opaque) { // The type is suppressed because it's private. If other // non-suppressed and declaration-only instances of this @@ -15906,11 +15904,11 @@ build_ir_node_from_die(reader& rdr, case DW_TAG_class_type: case DW_TAG_structure_type: { - bool type_is_private = false; + bool type_is_opaque = false; bool type_suppressed= - type_is_suppressed(rdr, scope, die, type_is_private); + type_is_suppressed(rdr, scope, die, type_is_opaque); - if (type_suppressed && type_is_private) + if (type_suppressed && type_is_opaque) { // The type is suppressed because it's private. If other // non-suppressed and declaration-only instances of this diff --git a/src/abg-suppression.cc b/src/abg-suppression.cc index 7c7a4305..aba4da14 100644 --- a/src/abg-suppression.cc +++ b/src/abg-suppression.cc @@ -926,7 +926,7 @@ type_suppression::suppresses_diff(const diff* diff) const // public -- depending on, e.g, if the typedef is defined in a // public header or not. So if we are in the context of a // private type suppression let's *NOT* peel typedefs away. - if (!is_private_type_suppr_spec(*this)) + if (!is_opaque_type_suppr_spec(*this)) { ft = peel_typedef_type(ft); st = peel_typedef_type(st); @@ -1346,7 +1346,7 @@ suppression_matches_type_location(const type_suppression& s, // the declaration. If we reach this place, it // means the class has no definition at this point. ABG_ASSERT(!cl->get_definition_of_declaration()); - if (s.get_label() == get_private_types_suppr_spec_label()) + if (s.get_label() == get_opaque_types_suppr_spec_label()) // So this looks like what really amounts to an // opaque type. So it's not defined in the public // headers. So we want to filter it out. @@ -4910,12 +4910,12 @@ suppression_matches_soname_or_filename(const string& soname, /// specification that is auto-generated by libabigail to suppress /// change reports about types that are not defined in public headers. const char* -get_private_types_suppr_spec_label() +get_opaque_types_suppr_spec_label() { - static const char *PRIVATE_TYPES_SUPPR_SPEC_NAME = - "Artificial private types suppression specification"; + static const char *OPAQUE_TYPES_SUPPR_SPEC_NAME = + "libabigail::OPAQUE_TYPE_LABEL"; - return PRIVATE_TYPES_SUPPR_SPEC_NAME; + return OPAQUE_TYPES_SUPPR_SPEC_NAME; } /// Test if a type suppression specification represents a private type @@ -4926,8 +4926,8 @@ get_private_types_suppr_spec_label() /// /// @return true iff @p s is a private type suppr spec. bool -is_private_type_suppr_spec(const type_suppression& s) -{return s.get_label() == get_private_types_suppr_spec_label();} +is_opaque_type_suppr_spec(const type_suppression& s) +{return s.get_label() == get_opaque_types_suppr_spec_label();} /// Test if a type suppression specification represents a private type /// suppression automatically generated by libabigail from the user @@ -4937,11 +4937,11 @@ is_private_type_suppr_spec(const type_suppression& s) /// /// @return true iff @p s is a private type suppr spec. bool -is_private_type_suppr_spec(const suppression_sptr& s) +is_opaque_type_suppr_spec(const suppression_sptr& s) { type_suppression_sptr type_suppr = is_type_suppression(s); return (type_suppr - && type_suppr->get_label() == get_private_types_suppr_spec_label()); + && type_suppr->get_label() == get_opaque_types_suppr_spec_label()); } // @@ -5275,9 +5275,9 @@ is_variable_suppressed(const fe_iface& fe, /// /// @param type_location the source location of the type. /// -/// @param type_is_private output parameter. This is set to true if +/// @param type_is_opaque output parameter. This is set to true if /// the type was matched by one suppression specification, and if the -/// suppression was for private types. +/// suppression was for opaque types. /// /// @param require_drop_property if true, this type requires the /// suppression specification to contain the "drop" property to match @@ -5289,7 +5289,7 @@ bool is_type_suppressed(const fe_iface& fe, const string& type_name, const location& type_location, - bool& type_is_private, + bool& type_is_opaque, bool require_drop_property) { for (auto i : fe.suppressions()) @@ -5301,14 +5301,14 @@ is_type_suppressed(const fe_iface& fe, type_name, type_location)) { - if (is_private_type_suppr_spec(*suppr)) - type_is_private = true; + if (is_opaque_type_suppr_spec(*suppr)) + type_is_opaque = true; return true; } } - type_is_private = false; + type_is_opaque = false; return false; } diff --git a/src/abg-tools-utils.cc b/src/abg-tools-utils.cc index 2115113b..33a1e8af 100644 --- a/src/abg-tools-utils.cc +++ b/src/abg-tools-utils.cc @@ -2003,7 +2003,7 @@ handle_file_entry(const string& file_path, { if (!suppr) { - suppr.reset(new type_suppression(get_private_types_suppr_spec_label(), + suppr.reset(new type_suppression(get_opaque_types_suppr_spec_label(), /*type_name_regexp=*/"", /*type_name=*/"")); diff --git a/tests/data/Makefile.am b/tests/data/Makefile.am index 8b33644b..2ec0bb96 100644 --- a/tests/data/Makefile.am +++ b/tests/data/Makefile.am @@ -2033,6 +2033,15 @@ test-diff-suppr/test-has-data-member-inserted-at-3-v0.c \ test-diff-suppr/test-has-data-member-inserted-at-3-v0.o \ test-diff-suppr/test-has-data-member-inserted-at-3-v1.c \ test-diff-suppr/test-has-data-member-inserted-at-3-v1.o \ +test-diff-suppr/PR31646/test-PR31646.2.abignore \ +test-diff-suppr/PR31646/test-PR31646.abignore \ +test-diff-suppr/PR31646/test-PR31646-result-1.txt \ +test-diff-suppr/PR31646/test-PR31646-result-2.txt \ +test-diff-suppr/PR31646/test-PR31646-result-3.txt \ +test-diff-suppr/PR31646/test-PR31646-v0.cc \ +test-diff-suppr/PR31646/test-PR31646-v0.o \ +test-diff-suppr/PR31646/test-PR31646-v1.cc \ +test-diff-suppr/PR31646/test-PR31646-v1.o \ \ test-lookup-syms/test0.cc \ test-lookup-syms/test0.o \ diff --git a/tests/data/test-diff-suppr/PR31646/test-PR31646-result-1.txt b/tests/data/test-diff-suppr/PR31646/test-PR31646-result-1.txt new file mode 100644 index 00000000..a2216256 --- /dev/null +++ b/tests/data/test-diff-suppr/PR31646/test-PR31646-result-1.txt @@ -0,0 +1,21 @@ +Functions changes summary: 0 Removed, 1 Changed, 0 Added function +Variables changes summary: 0 Removed, 0 Changed, 0 Added variable + +1 function with some indirect sub-type change: + + [C] 'function opaque* fun(type&)' at test-PR31646-v0.cc:15:1 has some indirect sub-type changes: + return type changed: + entity changed from 'opaque*' to 'void' + type size changed from 64 to 0 (in bits) + parameter 1 of type 'type&' has sub-type changes: + in referenced type 'struct type' at test-PR31646-v1.cc:8:1: + type size hasn't changed + 1 data member change: + type of 'opaque* m1' changed: + in pointed to type 'struct opaque' at test-PR31646-v1.cc:1:1: + type size changed from 64 to 96 (in bits) + 1 data member insertion: + 'char m_inserted', at offset 32 (in bits) at test-PR31646-v1.cc:4:1 + 1 data member change: + 'int m1' offset changed from 32 to 64 (in bits) (by +32 bits) + diff --git a/tests/data/test-diff-suppr/PR31646/test-PR31646-result-2.txt b/tests/data/test-diff-suppr/PR31646/test-PR31646-result-2.txt new file mode 100644 index 00000000..e69de29b diff --git a/tests/data/test-diff-suppr/PR31646/test-PR31646-result-3.txt b/tests/data/test-diff-suppr/PR31646/test-PR31646-result-3.txt new file mode 100644 index 00000000..2433dcd2 --- /dev/null +++ b/tests/data/test-diff-suppr/PR31646/test-PR31646-result-3.txt @@ -0,0 +1,10 @@ +Functions changes summary: 0 Removed, 1 Changed, 0 Added function +Variables changes summary: 0 Removed, 0 Changed, 0 Added variable + +1 function with some indirect sub-type change: + + [C] 'function opaque* fun(type&)' at test-PR31646-v0.cc:15:1 has some indirect sub-type changes: + return type changed: + entity changed from 'opaque*' to 'void' + type size changed from 64 to 0 (in bits) + diff --git a/tests/data/test-diff-suppr/PR31646/test-PR31646-v0.cc b/tests/data/test-diff-suppr/PR31646/test-PR31646-v0.cc new file mode 100644 index 00000000..53cc6ad0 --- /dev/null +++ b/tests/data/test-diff-suppr/PR31646/test-PR31646-v0.cc @@ -0,0 +1,17 @@ +struct opaque +{ + int m0; + int m1; +}; + +struct type +{ + int m0; + opaque* m1; + int m2; +}; + +opaque* +fun(type&) +{ +} diff --git a/tests/data/test-diff-suppr/PR31646/test-PR31646-v0.o b/tests/data/test-diff-suppr/PR31646/test-PR31646-v0.o new file mode 100644 index 0000000000000000000000000000000000000000..31766f769918c2154c0a432cacde9ada2b2bce8c GIT binary patch literal 3200 zcmb_e&1+*-6hAjFY0`PqByB`HR%jxM^I={-T089w9V5k=76&PXK@o@elDsBQn-7!M z(y5Aqq6}_EaV0KX2;xp~F(@tsH*Wkh-0Mzoq367JPIB{jMNtpro^yWpeBFEQy)SP+ zd-HlsDG*X{4JMkP0PiFxa?KZO@BrlC*7n_Bw(q|6`@P#gevfbhRElN|8VWS%WNJ!` zYcJGz*E8h0x6YVGmTs zUk8~8Nq+U&w+JO^h<((7RO$t?P;+3Ut`yW&bHm)pt)p=)r;M~|23R4ba=5U0^>{jG z8p}(|^J*cPeFz}_7?|cY)4+u%GAQn#*^H&0K8Mjv3f4>SVez`{221lnFiu9W&CJx7cdj6W2X{k6x`ax4>s_5BhGX2+_L zQSZhKH1_Tc9Ow`1*AE@I@%*j))}>38%lS&hTD2}(#duNBu;i$~Rbu?_W?RRR!r zY&Dxu>h!x#snu`2>XzEBS2}PTcEfGk?m=nTX`EE6msi#|Dkbt9m0GrE`(m->?(Y{z zhl9be#PR;eND*a5KyYJg>q`DSZrFu?uvd6j0#YY?ppLU@>1;A}51aG|!t^8ZoBd2W z`+jU|Zss}~1aRa?{f)zKe@0DWQ=?$Kl;h@4%^SgWox(^lFhdj>FcSr%z4l0G76!~} z+C(%?Io<}Gofv770clN}hz4XPiY6H_r)hymeOS=;XMDjTT0C`lIg<3`4C?wWNG8H* zwX%W*_9L8rLJzVaI6ezLJq1GX8lv<;k>d& z$}6$SG<%Buqm0YCZnI7g9FgE(1}apT3;rGJSDA5R7>&RhItRAZavF#2y4&0DSt zz1i<}9sF`xy}svI_(Bv%p51(n13R7i{?P6^ypaFMme8N}NctqV%Yx)oTT&kZ~dwN;h2<9^8Y}JMqyhzSM*BNG#e7jd{}>Fd>%R3$=$)P zuTPsehGWu%$>5v5)no|U`u|@h(_`raHY|^kKnVNOI-}~(b3?_S>ZjK=Tz?C(a7={Y zK7%(&(I{+N$GNEdyPW?O_NDyvzJ&7+5TlqpRm8o3-5#vG*5`1*nNcpVN-7r>f*mHhRkLE1pWersQv$%_g~H_ z?LU3x#9#2J+!XtBuD`wGLsVCk|4&^1H)G2{2>bs9 D{7v`h literal 0 HcmV?d00001 diff --git a/tests/data/test-diff-suppr/PR31646/test-PR31646-v1.cc b/tests/data/test-diff-suppr/PR31646/test-PR31646-v1.cc --- /dev/null +++ b/tests/data/test-diff-suppr/PR31646/test-PR31646-v1.cc @@ -0,0 +1,18 @@ +struct opaque +{ + int m0; + char m_inserted; + int m1; +}; + +struct type +{ + int m0; + opaque* m1; + int m2; +}; + +void +fun(type&) +{ +} diff --git a/tests/data/test-diff-suppr/PR31646/test-PR31646-v1.o b/tests/data/test-diff-suppr/PR31646/test-PR31646-v1.o new file mode 100644 index 0000000000000000000000000000000000000000..d12f31fd03ae57009026f0a5b3baaaecf7fcfbbb GIT binary patch literal 3312 zcmcIm&1)n@6n{0H$#gQwWRelh8X+B7akFdpd?aSGS+YjL?oM=(C9oG^8GB}WKHT}T zrza#4@gN9`mk1&rJc+CyH}U34@FXbsC+NYu-UJW&Ud^kSp0XE3v0&=G_j@1p>bsd%#!z_geq3+cRUen7_PJUetI`(%

G>&D2 zkxC|q*qSLaxY0%NR4S7+R##RR#Zn^u80L+Bsh#B5Eq)%;x|TGsGSkea01^v`9$*O5 z5zn2&m83@}MPc@qPtQ^GnK&F|V#@%tX*PyuC`n2WWT7QTliJG&V5W#Pj(nUQFu#al50joYPWr|0yT{vG_SU9-RIuziWEpp>zU<|Kw1THUe1Ri>1LRtf6LeiKu9&+dnm>U~~I0Nw7MGONL zB2dbI{x3N7VJTFf(Y1?c^u)d3NUA5ZD2J<{RwA5syUzAYObMso3oJjX;Vi=A%uolz z-MSdES2+-r4dJwIn&s;n&LY$`{5;Do4Oew}pyBvF%JDfGR5o*{6` z{XRIp*K(bzS02>y0j+gWCwaGA_PwJCwd=c`x;N?4YIZ#3f=eit;F6Rp-FDl<=Dkpk4cIZ z2H%%f<4L!@jVc`@>n|A0ja?%YRjv_8E@`uaV@D5lC2SvhRJoF@uks@7NZb;P6mAF%)Tf8+noL;U-EAk}-S z!l-o>eUJT>UeS;A1I-HMr|dpMhOmjZ2<1=Ne_I(cRrOQw$H+w0-=b#YQ0J8DPhUml yukfg~$?r#AzrhL9cZ!xl{r?2*(*unF literal 0 HcmV?d00001 diff --git a/tests/data/test-diff-suppr/PR31646/test-PR31646.2.abignore b/tests/data/test-diff-suppr/PR31646/test-PR31646.2.abignore --- /dev/null +++ b/tests/data/test-diff-suppr/PR31646/test-PR31646.2.abignore @@ -0,0 +1,4 @@ +[suppress_type] + label = libabigail::OPAQUE_TYPE_LABEL + name = opaque + drop = yes \ No newline at end of file diff --git a/tests/data/test-diff-suppr/PR31646/test-PR31646.abignore b/tests/data/test-diff-suppr/PR31646/test-PR31646.abignore new file mode 100644 index 00000000..81c00a77 --- /dev/null +++ b/tests/data/test-diff-suppr/PR31646/test-PR31646.abignore @@ -0,0 +1,3 @@ +[suppress_type] + name = opaque + drop = yes \ No newline at end of file diff --git a/tests/test-diff-suppr.cc b/tests/test-diff-suppr.cc index 119be55b..297bfb0b 100644 --- a/tests/test-diff-suppr.cc +++ b/tests/test-diff-suppr.cc @@ -2396,6 +2396,36 @@ InOutSpec in_out_specs[] = "data/test-diff-suppr/test-has-strict-flexible-array-data-member-conversion-report-2.txt", "output/test-diff-suppr/test-has-strict-flexible-array-data-member-conversion-report-2.txt", }, + { + "data/test-diff-suppr/PR31646/test-PR31646-v0.o", + "data/test-diff-suppr/PR31646/test-PR31646-v1.o", + "", + "", + "", + "--no-default-suppression ", + "data/test-diff-suppr/PR31646/test-PR31646-result-1.txt", + "output/test-diff-suppr/PR31646/test-PR31646-result-1.txt", + }, + { + "data/test-diff-suppr/PR31646/test-PR31646-v0.o", + "data/test-diff-suppr/PR31646/test-PR31646-v1.o", + "", + "", + "data/test-diff-suppr/PR31646/test-PR31646.abignore", + "--no-default-suppression ", + "data/test-diff-suppr/PR31646/test-PR31646-result-2.txt", + "output/test-diff-suppr/PR31646/test-PR31646-result-2.txt", + }, + { + "data/test-diff-suppr/PR31646/test-PR31646-v0.o", + "data/test-diff-suppr/PR31646/test-PR31646-v1.o", + "", + "", + "data/test-diff-suppr/PR31646/test-PR31646.2.abignore", + "--no-default-suppression ", + "data/test-diff-suppr/PR31646/test-PR31646-result-3.txt", + "output/test-diff-suppr/PR31646/test-PR31646-result-3.txt", + }, // This should be the last entry {NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL} }; diff --git a/tools/abipkgdiff.cc b/tools/abipkgdiff.cc index 3a601075..ed9efc9b 100644 --- a/tools/abipkgdiff.cc +++ b/tools/abipkgdiff.cc @@ -1483,7 +1483,7 @@ compare(const elf_file& elf1, opts.show_all_types); ABG_ASSERT(reader); - reader->add_suppressions(priv_types_supprs1); + reader->add_suppressions(supprs); set_generic_options(*reader, opts); corpus1 = reader->read_corpus(c1_status); @@ -1664,6 +1664,9 @@ compare(const elf_file& elf1, /// /// @param debug_dir the debug directory of the ELF file. /// +/// @param priv_types_supprs type suppression specification that +/// suppress private types. +/// /// @param opts the options passed the user. /// /// @param env the environment to use for the comparison. @@ -1681,6 +1684,7 @@ compare(const elf_file& elf1, static abidiff_status compare_to_self(const elf_file& elf, const string& debug_dir, + const suppressions_type& priv_types_supprs, const options& opts, abigail::ir::environment& env, corpus_diff_sptr& diff, @@ -1707,6 +1711,14 @@ compare_to_self(const elf_file& elf, << elf.path << " ...\n"; + ctxt.reset(new diff_context); + set_diff_context_from_opts(ctxt, opts); + suppressions_type& supprs = ctxt->suppressions(); + + // Add the opaque type suppressions set to the set of suppressions. + for (auto& suppr : priv_types_supprs) + supprs.push_back(suppr); + corpus_sptr corp; abigail::elf_based_reader_sptr reader; { @@ -1726,6 +1738,7 @@ compare_to_self(const elf_file& elf, opts.show_all_types); ABG_ASSERT(reader); + reader->add_suppressions(supprs); corp = reader->read_corpus(c_status); if (!(c_status & abigail::fe_iface::STATUS_OK)) @@ -2291,6 +2304,7 @@ public: abigail::fe_iface::STATUS_UNKNOWN; status |= compare_to_self(args->elf1, args->debug_dir1, + args->private_types_suppr1, args->opts, env, diff, ctxt, out, &detailed_status); @@ -2915,7 +2929,7 @@ compare_prepared_userspace_packages(package& first_package, if (iter != second_package.path_elf_file_sptr_map().end() && (iter->second->type == abigail::elf::ELF_TYPE_DSO || iter->second->type == abigail::elf::ELF_TYPE_EXEC - || iter->second->type == abigail::elf::ELF_TYPE_PI_EXEC + || iter->second->type == abigail::elf::ELF_TYPE_PI_EXEC || iter->second->type == abigail::elf::ELF_TYPE_RELOCATABLE)) { if (iter->second->type != abigail::elf::ELF_TYPE_RELOCATABLE) @@ -3069,7 +3083,6 @@ self_compare_prepared_userspace_package(package& pkg, pkg.debug_info_packages().front()->extracted_dir_path() + relative_debug_path; - suppressions_type supprs; for (map::iterator it = pkg.path_elf_file_sptr_map().begin(); it != pkg.path_elf_file_sptr_map().end(); @@ -3078,7 +3091,7 @@ self_compare_prepared_userspace_package(package& pkg, if (it != pkg.path_elf_file_sptr_map().end() && (it->second->type == abigail::elf::ELF_TYPE_DSO || it->second->type == abigail::elf::ELF_TYPE_EXEC - || it->second->type == abigail::elf::ELF_TYPE_PI_EXEC + || it->second->type == abigail::elf::ELF_TYPE_PI_EXEC || it->second->type == abigail::elf::ELF_TYPE_RELOCATABLE)) { if (it->second->type != abigail::elf::ELF_TYPE_RELOCATABLE) @@ -3086,10 +3099,12 @@ self_compare_prepared_userspace_package(package& pkg, compare_args_sptr args (new compare_args(*it->second, debug_dir, - supprs, + create_private_types_suppressions + (pkg, opts), *it->second, debug_dir, - supprs, + create_private_types_suppressions + (pkg, opts), opts)); self_compare_task_sptr t(new self_compare_task(args)); self_compare_tasks.push_back(t); diff --git a/tools/fedabipkgdiff b/tools/fedabipkgdiff index ca94a3f9..de494d17 100755 --- a/tools/fedabipkgdiff +++ b/tools/fedabipkgdiff @@ -1155,7 +1155,9 @@ def abipkgdiff(cmp_half1, cmp_half2): abipkgdiff_tool, '--dso-only' if global_config.dso_only else '', '--self-check', + suppressions, debuginfo_pkg1, + devel_pkg1, cmp_half1.subject.downloaded_file, ] else: