From patchwork Thu Jul 20 16:10:53 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Giuliano Procida X-Patchwork-Id: 13320735 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 58E1AEB64DA for ; Thu, 20 Jul 2023 16:11:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:From:Subject:Message-ID: References:Mime-Version:In-Reply-To:Date:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=/c/4aL9eU2kNG5gGtImhT2yFdbtnrMjyUv9aBoLGOPQ=; b=OC7VeNd1sB1P+y+TsONpj3IXwh SFdZHsfnda4mFbVIxz653ZSeXlMs+xT9GI0KxKCNi2gMGdJYC3qN+D5cn6dRbXDzSw5fe/geaDU5T RnwYmZB2DoAh1M1CMnW+MkiGi+A0lxdowRM/pUQBVkqkHx7SC1c1Fa/ns55TCM6LbLmMPioOfldKo dbVhJRwklIeFLIBIpb5gtXKLwQFPOkREsy3JdHueIe0gzeIT8P0Wae3ECLEcBtNgdfJZapV6lqch+ CPLNIhiYCUBXNL8t0NjVFJxrGLCekGvdH5RyMxDDN3FJcOfgCPeD7o3JSpbPYbgYkbdY1gXUR5edf qqwP2bxw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.96 #2 (Red Hat Linux)) id 1qMWFE-00Bdob-0o; Thu, 20 Jul 2023 16:11:12 +0000 Received: from mail-yw1-x1149.google.com ([2607:f8b0:4864:20::1149]) by bombadil.infradead.org with esmtps (Exim 4.96 #2 (Red Hat Linux)) id 1qMWFA-00BdnQ-2t for linux-arm-kernel@lists.infradead.org; Thu, 20 Jul 2023 16:11:10 +0000 Received: by mail-yw1-x1149.google.com with SMTP id 00721157ae682-577323ba3d5so33271007b3.0 for ; Thu, 20 Jul 2023 09:11:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1689869465; x=1690474265; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=Qp196USLnAy4LTAkpGzxnXfWsR6U5xBzW1Ke9Zcywm4=; b=0xJVu/W/3xHLwIgQt4AdT6cR/YUtiNdLdmQJXkVP0EBEOh0s1C8pMSYPVmVX56g0qL iY14aVUZXceX1+sSx47/A3Oz42zFfT6/iI72X+dHAkxW9xroRWzw6ItifadoKfASJ3u8 hBpU/RpuzpltTY33o0ZEKscU+ZkWmBRLI+L2oSSkqem1In1o/8kvyeLQtZwigN3E10vS NgK9xlM5rN/Z00gvjRghEprD9TdNy5Vmx8iwjIOd4Y+GZ8uG8ZEDfW1U7NXUAb9hO2fz tMytb0jSh5n09x1tTZWMd313KeCvKYZeX3DaeKAhaFjFGLwW9FuzHttvsRB6eaajnMdp HzOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689869465; x=1690474265; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Qp196USLnAy4LTAkpGzxnXfWsR6U5xBzW1Ke9Zcywm4=; b=le7sfYqQQu7P5JYpihGydJ+HbvETGsO8m2xXychamL36+QCaNZZQ5J+7gBQNTbkxIt ltRcPJXNHc8gjPz1S+Yp2llZ4Y0JNtWbPBdq6e3Jr0EeMz6WM5jS/xDJ+WHT76MlhFVV MbM19AS4/kXwPt8V8Qy2aTyeHtNEJbzaQXwTNe/C33AOq4n+dnoXqnSO244hXAmgh0Z/ Mq9vXm3OzF4TiJFHNAXJh1V8xA9YJSqO4k7QfUzV2I8f+PpiWiMzf4XB7yqzY6/GzZPY o62kOUXfGfsXo0xwSvNi5An9mPvY69xnXduCIr/8up+d3udOkwd96dnsXWXGqXkwQaGW PPQw== X-Gm-Message-State: ABy/qLaUdapKVE93KLrQ7qSfazy+TB53FKTar+PwHZfnWv1c/onFzdjq KA8M/Vlnzv5ma6x8tBQjA0un6N9q9obG3A== X-Google-Smtp-Source: APBJJlEiVEmuesw68z+x28qFD4ReuJRNuf+WB6wQmhi/bG4Ofb/elfxoE7pV67Grv8RRZAJJig+fwLf/UqbQFQ== X-Received: from tef.lon.corp.google.com ([2a00:79e0:d:209:c4c4:9486:83f9:4510]) (user=gprocida job=sendgmr) by 2002:a5b:349:0:b0:ceb:324c:ba8e with SMTP id q9-20020a5b0349000000b00ceb324cba8emr77473ybp.4.1689869465517; Thu, 20 Jul 2023 09:11:05 -0700 (PDT) Date: Thu, 20 Jul 2023 17:10:53 +0100 In-Reply-To: <20230407203456.27141-2-quic_johmoo@quicinc.com> Mime-Version: 1.0 References: <20230407203456.27141-2-quic_johmoo@quicinc.com> X-Mailer: git-send-email 2.41.0.255.g8b1d071c50-goog Message-ID: <20230720161053.1213680-1-gprocida@google.com> Subject: [PATCH] scripts/check-uapi.sh: add stgdiff support From: Giuliano Procida To: quic_johmoo@quicinc.com Cc: masahiroy@kernel.org, nathan@kernel.org, ndesaulniers@google.com, nicolas@fjasle.eu, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-arm-msm@vger.kernel.org, gregkh@linuxfoundation.org, rdunlap@infradead.org, arnd@arndb.de, andersson@kernel.org, tkjos@google.com, maennich@google.com, gprocida@google.com, kernel-team@android.com, libabigail@sourceware.org, jorcrous@amazon.com, quic_tsoni@quicinc.com, quic_satyap@quicinc.com, quic_eberman@quicinc.com, quic_gurus@quicinc.com X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230720_091108_958019_6337E75D X-CRM114-Status: GOOD ( 19.99 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi John. I spent a few minutes adding stgdiff support to the script. It's really just for illustration purposes. As I think you know, STG doesn't yet exist as a project outside of AOSP. Nevertheless, this may be useful to you as-is. STG has quite a different philosophy to libabigil in terms of filtering out certain kinds of differences. Some of the things (like enum enumerator additions) are not considered harmless. The reasoning behind this is basically... https://en.wikipedia.org/wiki/Covariance_and_contravariance_(computer_science) However, it does have --ignore interface_addition (and the related --ignore type_definition_addition) which can be used to detect whether one ABI is a subset of another. I am looking at adding support for macro definitions (gcc -g3) to STG which will then let us cover significantly more of the UAPI surface. Unfortunately, there are some headers which use anonymous enums to define constants (e.g. and ironically BTF). ABI tracking these types would require something a bit hacky. Or we could just name them. Regards, Giuliano. Signed-off-by: Giuliano Procida --- scripts/check-uapi.sh | 102 ++++++++++++++++++++++++++++-------------- 1 file changed, 69 insertions(+), 33 deletions(-) diff --git a/scripts/check-uapi.sh b/scripts/check-uapi.sh index 755187f27be5..982666b48f3b 100755 --- a/scripts/check-uapi.sh +++ b/scripts/check-uapi.sh @@ -32,6 +32,7 @@ Options: -v Verbose operation (print more information about each header being checked). Environmental args: + STGDIFF Custom path to stgdiff binary - use stgdiff instead of abidiff ABIDIFF Custom path to abidiff binary CC C compiler (default is "gcc") ARCH Target architecture of C compiler (default is host arch) @@ -270,43 +271,78 @@ compare_abi() { exit "$FAIL_COMPILE" fi - local ret=0 - "$ABIDIFF" --non-reachable-types "${past_header}.bin" "${base_header}.bin" > "$log" || ret="$?" - if [ "$ret" -eq 0 ]; then - if [ "$VERBOSE" = "true" ]; then - printf "No ABI differences detected in %s from %s -> %s\n" "$file" "$past_ref" "${base_ref:-dirty tree}" + if [ "$STGDIFF" ]; then + local ret=0 + "$STGDIFF" --types --ignore interface_addition --elf "${past_header}.bin" "${base_header}.bin" --format small --output "$log" || ret="$?" + if [ "$ret" -eq 0 ]; then + if [ "$VERBOSE" = "true" ]; then + printf "No ABI differences detected in %s from %s -> %s\n" "$file" "$past_ref" "${base_ref:-dirty tree}" + fi + else + # stgdiff's return code can be used to determine the type of error + if [ $((ret & 0x1)) -gt 0 ]; then + eprintf "error - stgdiff failed\n" + exit 1 + fi + + { + printf "!!! ABI differences detected in %s from %s -> %s !!!\n\n" "$file" "$past_ref" "${base_ref:-dirty tree}" + sed -e 's/^/ /g' "$log" + + if ! cmp "$past_header" "$base_header" > /dev/null 2>&1; then + printf "\nHeader file diff (after headers_install):\n" + diff -Naur "$past_header" "$base_header" \ + | sed -e "s|${past_header}|${past_ref}/${file}|g" \ + -e "s|${base_header}|${base_ref:-dirty}/${file}|g" + printf "\n" + else + printf "\n%s did not change between %s and %s...\n" "$file" "$past_ref" "${base_ref:-dirty tree}" + printf "It's possible a change to one of the headers it includes caused this error:\n" + grep '^#include' "$base_header" + printf "\n" + fi + } | tee "${base_header}.error" >&2 + return 1 fi else - # Bits in abidiff's return code can be used to determine the type of error - if [ $((ret & 0x1)) -gt 0 ]; then - eprintf "error - abidiff did not run properly\n" - exit 1 - fi + local ret=0 + "$ABIDIFF" --non-reachable-types "${past_header}.bin" "${base_header}.bin" > "$log" || ret="$?" + if [ "$ret" -eq 0 ]; then + if [ "$VERBOSE" = "true" ]; then + printf "No ABI differences detected in %s from %s -> %s\n" "$file" "$past_ref" "${base_ref:-dirty tree}" + fi + else + # Bits in abidiff's return code can be used to determine the type of error + if [ $((ret & 0x1)) -gt 0 ]; then + eprintf "error - abidiff did not run properly\n" + exit 1 + fi - # If the only changes were additions (not modifications to existing APIs), then - # there's no problem. Ignore these diffs. - if grep "Unreachable types summary" "$log" | grep -q "0 removed" && - grep "Unreachable types summary" "$log" | grep -q "0 changed"; then - return 0 - fi - { - printf "!!! ABI differences detected in %s from %s -> %s !!!\n\n" "$file" "$past_ref" "${base_ref:-dirty tree}" - sed -e '/summary:/d' -e '/changed type/d' -e '/^$/d' -e 's/^/ /g' "$log" - - if ! cmp "$past_header" "$base_header" > /dev/null 2>&1; then - printf "\nHeader file diff (after headers_install):\n" - diff -Naur "$past_header" "$base_header" \ - | sed -e "s|${past_header}|${past_ref}/${file}|g" \ - -e "s|${base_header}|${base_ref:-dirty}/${file}|g" - printf "\n" - else - printf "\n%s did not change between %s and %s...\n" "$file" "$past_ref" "${base_ref:-dirty tree}" - printf "It's possible a change to one of the headers it includes caused this error:\n" - grep '^#include' "$base_header" - printf "\n" + # If the only changes were additions (not modifications to existing APIs), then + # there's no problem. Ignore these diffs. + if grep "Unreachable types summary" "$log" | grep -q "0 removed" && + grep "Unreachable types summary" "$log" | grep -q "0 changed"; then + return 0 fi - } | tee "${base_header}.error" >&2 - return 1 + { + printf "!!! ABI differences detected in %s from %s -> %s !!!\n\n" "$file" "$past_ref" "${base_ref:-dirty tree}" + sed -e '/summary:/d' -e '/changed type/d' -e '/^$/d' -e 's/^/ /g' "$log" + + if ! cmp "$past_header" "$base_header" > /dev/null 2>&1; then + printf "\nHeader file diff (after headers_install):\n" + diff -Naur "$past_header" "$base_header" \ + | sed -e "s|${past_header}|${past_ref}/${file}|g" \ + -e "s|${base_header}|${base_ref:-dirty}/${file}|g" + printf "\n" + else + printf "\n%s did not change between %s and %s...\n" "$file" "$past_ref" "${base_ref:-dirty tree}" + printf "It's possible a change to one of the headers it includes caused this error:\n" + grep '^#include' "$base_header" + printf "\n" + fi + } | tee "${base_header}.error" >&2 + return 1 + fi fi }