From patchwork Tue Oct 3 12:21:29 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 56193 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 B765438582AC for ; Tue, 3 Oct 2023 12:21:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org B765438582AC DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1696335707; bh=vQTQB9GSgd+dMlW+XS5YLz995ZZhiXJNIhS+w/u/v7o=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Help: List-Subscribe:From:Reply-To:Cc:From; b=Eucs3mMaFEPKTEwBlCXsMWbrGtEJwmh9ND4OlpozQzaVIEY/cMms7BYFzMB4o0clI VzQLPSJPHcJYejDkPvBL+afysRA7f1/NPyEeaPqe76GoeezxDwsX+HwJfvNBplmGf3 DrrinYJsSPUyjKmkdG1fnptVQmYRopzOrlQp2sQk= 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.129.124]) by sourceware.org (Postfix) with ESMTPS id 5ACCB3858D38 for ; Tue, 3 Oct 2023 12:21:40 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5ACCB3858D38 Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-401-1KPRtOVtPauRIzOUk2CW0g-1; Tue, 03 Oct 2023 08:21:34 -0400 X-MC-Unique: 1KPRtOVtPauRIzOUk2CW0g-1 Received: by mail-qt1-f197.google.com with SMTP id d75a77b69052e-4180c303ef1so10111051cf.0 for ; Tue, 03 Oct 2023 05:21:33 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696335693; x=1696940493; h=mime-version:user-agent:message-id:date:organization:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=vQTQB9GSgd+dMlW+XS5YLz995ZZhiXJNIhS+w/u/v7o=; b=qZQHrIk+ZCWPKiEoB0Twfj5Q7Sj3oa1RweucnbhccPs4RRcPXRi+QlzkS03IZVNmhz LpH4oRu6sfJ+s/u1r2WJTGsvR6I2Uh4jNCac3TjOcSd8PUG68nkTFVGcp1wO9oy9Kx07 0kU1VtXKEM1CFzr83C8VOQBK0N31F/x2OAQpTjLjOEmHFeOe++O2pLRnfbY/GDnBRfkl QiKaFISHsElBEEKGqQWH9yOLJbj8mmpbiYRAzvYqikbJPjYLem2bdy93WOtSuE8WGPL2 jb4611S/TiSFNSyVToiLBFUZOtBjuJaqUu1yK0a8bK/zUqtL/qfAwWhStb6R1cpWOn3m dl/g== X-Gm-Message-State: AOJu0Yy+OJg4yCWhbLUkXZVA1ikrV864grXWZxeWlo0/gYzMSVe7ks0w kDJj4NL25b8VLsJMfLNGfzt2USO2J5kDHVaW7AdQnLBSbtpkKXCR0EnsVYfIwV5f6hiyLIJdo1S D6HbWNaVQb3lHB0IF9mVDqU8cMrY3 X-Received: by 2002:ac8:5846:0:b0:418:1272:7c6e with SMTP id h6-20020ac85846000000b0041812727c6emr17199983qth.7.1696335693292; Tue, 03 Oct 2023 05:21:33 -0700 (PDT) X-Google-Smtp-Source: AGHT+IEDR8anYnYtuO4O16wF2+wI+lZbLNdS/J9M1/+eGznQhb5TilAdNxI33sGClZucCbNxVX2PYQ== X-Received: by 2002:ac8:5846:0:b0:418:1272:7c6e with SMTP id h6-20020ac85846000000b0041812727c6emr17199958qth.7.1696335692894; Tue, 03 Oct 2023 05:21:32 -0700 (PDT) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id he37-20020a05622a602500b00419801b1094sm391585qtb.13.2023.10.03.05.21.32 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 03 Oct 2023 05:21:32 -0700 (PDT) Received: by localhost (Postfix, from userid 1000) id E55D5B6E87; Tue, 3 Oct 2023 14:21:29 +0200 (CEST) To: libabigail@sourceware.org Subject: [PATCH 0/4, RFC] For WIP branch check-uapi-support: Fix comparing non-reachable anonymous enums Organization: Red Hat / France X-Operating-System: CentOS Stream release 9 X-URL: http://www.redhat.com Date: Tue, 03 Oct 2023 14:21:29 +0200 Message-ID: <874jj7hp6e.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=-5.9 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_NONE, 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 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-Help: List-Subscribe: , X-Patchwork-Original-From: Dodji Seketeli via Libabigail From: Dodji Seketeli Reply-To: Dodji Seketeli Cc: dodji@redhat.com Errors-To: libabigail-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libabigail" Hello, This patch set is intended for the check-uapi-support branch dedicated to supporting the check-uapi.sh script [1]. The description of the task of supporting the comparison of non-reachable anonymous enums can be read on the email https://inbox.sourceware.org/libabigail/340b33bd-2b43-9f99-58e1-f1b77a51b48a@quicinc.com/: Another issue that comes up when comparing ABI across wide swaths of kernel commit history are changes to anonymous enums. From what I can tell, there's not a great way to handle this. If someone adds a new anonymous enum, the tag abidiff gives (e.g. enum __anonymous_enum__6) can change, so abidiff reports a new anonymous enum with all the same fields even though it was basically just a "rename". For reference, this file is full of anonymous enums: https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/ethtool_netlink.h#L15. Basically any change in there is triggering a failure from the script. So, this patch set allows changes to anonymous enums that are not used by any function or global to be represented as we would expect them. For instance, suppose that a translation unit has the anonymous enums below: enum { E1_0, E1_1 }; enum { E2_0, E2_1 }; Suppose also that a subsequent version of that translation unit had some changes to that anonymous enum, making it become: enum { E1_0, E1_1, E1_2 }; enum { E2_0, E2_1, E2_2 }; Before this patch, compiling the files with the gcc option -fno-eliminate-unused-debug-types and using abidiff --non-reachable-types on the output would yield something like: Unreachable types summary: 0 removed, 1 changed, 1 added types 1 changed type unreachable from any public interface: [C] 'enum __anonymous_enum__1' changed: type name changed from '__anonymous_enum__1' to '__anonymous_enum__' type size hasn't changed 2 enumerator deletions: '__anonymous_enum__1::E2_0' value '0' '__anonymous_enum__1::E2_1' value '1' 3 enumerator insertions: '__anonymous_enum__::E1_O' value '0' '__anonymous_enum__::E1_1' value '1' '__anonymous_enum__::E1_2' value '2' 1 added type unreachable from any public interface: [A] 'enum __anonymous_enum__1' at test1-v1.c:9:1 What happens here is that: 1/ the internal name used to designate anonymous enums for the purpose of type canonicalization is used for user-facing reporting, which arguably makes very little sense from the point of view of the users. 2/ The generation of the internal names for anonymous enums is not meant to be "stable" across translation units. So the internal name of the first anonymous enum in the first version of the translation unit doesn't necessary match the one in the second version of the translation unit. And a number of downstream issues originates from 1/ and 2/. The third patch of the set designates anonymous enums by their "flat textual representation". Said otherwise, the name of the firs enum is going to be: "enum {E1_0, E1_1,}". This should solve 1/ and 2/. In the reporting engine, the second patch uses that "flat representation" to name anonymous types in general (and enums in particular), instead of trying to use their qualified name (and thus their internal meaningless names). The fourth patch teaches the comparison engine that if an anonymous enum is deleted and added again (with the two version of the anonymous enum differing by a few enumerators), then we are probably looking at a change of anonymous enum. The first patch fixes a subtle issue where corpus_diff::has_incompatible_changes() fails to accurately say if an unreachable enum has an incompatible change. That function was invariability saying that any change to an unreachable type was an incompatible change. Which is false. Note that the fourth patch has a test case that covers this fix. Now that I am writing this, I am thinking that I should probably come up with a separate test case for this patch alone, and maybe this patch should independently go into the master branch. Oh well. In any case, with this patch set applied, abidiff says the following about the example given earlier: Unreachable types summary: 0 removed, 2 changed, 0 added types 2 changed types unreachable from any public interface: [C] 'enum {E2_0=0, E2_1=1, }' changed: type size hasn't changed 1 enumerator insertion: 'E2_2' value '2' [C] 'enum {E1_1=1, E1_O=0, }' changed: type size hasn't changed 1 enumerator insertion: 'E1_2' value '2' This is much closer to what we'd intuitively expect. I am thus asking a for a review for merging this work-in-progress set into the branch "check-uapi-support" https://sourceware.org/git/?p=libabigail.git;a=shortlog;h=refs/heads/users/dodji/check-uapi-support, so that it becomes somewhat testable. [1]: The bug that tracks the work to support the check-uapi.sh script is https://sourceware.org/bugzilla/show_bug.cgi?id=30931. Dodji Seketeli (4): abg-comparison[-priv]: Detect incompatible unreachable type changes default-reporter,reporter-priv: Do not report names of anonymous enums ir,comparison,corpus: Better support anonymous enums comparison ir,comparison: Represent changed anonymous enums include/abg-fwd.h | 27 + include/abg-ir.h | 7 + src/abg-comparison-priv.h | 3 + src/abg-comparison.cc | 140 +- src/abg-corpus.cc | 4 +- src/abg-default-reporter.cc | 12 +- src/abg-ir.cc | 205 +- src/abg-reporter-priv.cc | 3 +- tests/data/Makefile.am | 6 + .../test-anonymous-enums-change-report-v0.txt | 16 + .../test-anonymous-enums-change-report-v1.txt | 21 + .../test-anonymous-enums-change-v0.c | 36 + .../test-anonymous-enums-change-v0.o | Bin 0 -> 3296 bytes .../test-anonymous-enums-change-v1.c | 41 + .../test-anonymous-enums-change-v1.o | Bin 0 -> 3336 bytes tests/data/test-annotate/libtest23.so.abi | 224 +- .../test-anonymous-members-0.o.abi | 22 +- .../data/test-annotate/test15-pr18892.so.abi | 8 +- ...st18-pr19037-libvtkRenderingLIC-6.1.so.abi | 54 +- ...19-pr19023-libtcmalloc_and_profiler.so.abi | 10 +- ...st20-pr19025-libvtkParallelCore-6.1.so.abi | 2 +- .../test43-PR22913-report-0.txt | 4 +- .../test-read-dwarf/PR22122-libftdc.so.abi | 12416 ++++++++-------- tests/data/test-read-dwarf/libtest23.so.abi | 216 +- .../test-read-dwarf/test-libandroid.so.abi | 44 +- .../test-read-dwarf/test16-pr18904.so.abi | 1816 +-- ...st18-pr19037-libvtkRenderingLIC-6.1.so.abi | 40 +- .../test22-pr19097-libstdc++.so.6.0.17.so.abi | 60 +- tests/test-abidiff-exit.cc | 32 + 29 files changed, 7927 insertions(+), 7542 deletions(-) create mode 100644 tests/data/test-abidiff-exit/test-anonymous-enums-change-report-v0.txt create mode 100644 tests/data/test-abidiff-exit/test-anonymous-enums-change-report-v1.txt create mode 100644 tests/data/test-abidiff-exit/test-anonymous-enums-change-v0.c create mode 100644 tests/data/test-abidiff-exit/test-anonymous-enums-change-v0.o create mode 100644 tests/data/test-abidiff-exit/test-anonymous-enums-change-v1.c create mode 100644 tests/data/test-abidiff-exit/test-anonymous-enums-change-v1.o