public inbox for libabigail@sourceware.org
 help / color / mirror / Atom feed
From: John Moon <quic_johmoo@quicinc.com>
To: Masahiro Yamada <masahiroy@kernel.org>,
	Nathan Chancellor <nathan@kernel.org>,
	Nick Desaulniers <ndesaulniers@google.com>,
	"Nicolas Schier" <nicolas@fjasle.eu>
Cc: John Moon <quic_johmoo@quicinc.com>,
	<linux-kbuild@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-arm-msm@vger.kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Randy Dunlap <rdunlap@infradead.org>,
	"Arnd Bergmann" <arnd@arndb.de>,
	Bjorn Andersson <andersson@kernel.org>,
	Todd Kjos <tkjos@google.com>,
	Matthias Maennich <maennich@google.com>,
	Giuliano Procida <gprocida@google.com>, <kernel-team@android.com>,
	<libabigail@sourceware.org>, Jordan Crouse <jorcrous@amazon.com>,
	Trilok Soni <quic_tsoni@quicinc.com>,
	Satya Durga Srinivasu Prabhala <quic_satyap@quicinc.com>,
	Elliot Berman <quic_eberman@quicinc.com>
Subject: [PATCH v2 2/2] docs: dev-tools: Add UAPI checker documentation
Date: Tue, 28 Feb 2023 23:54:02 -0800	[thread overview]
Message-ID: <20230301075402.4578-3-quic_johmoo@quicinc.com> (raw)
In-Reply-To: <20230301075402.4578-1-quic_johmoo@quicinc.com>

Add detailed documentation for scripts/check-uapi.sh.
---
 Documentation/dev-tools/checkuapi.rst | 258 ++++++++++++++++++++++++++
 Documentation/dev-tools/index.rst     |   1 +
 2 files changed, 259 insertions(+)
 create mode 100644 Documentation/dev-tools/checkuapi.rst

diff --git a/Documentation/dev-tools/checkuapi.rst b/Documentation/dev-tools/checkuapi.rst
new file mode 100644
index 000000000000..2255066658e3
--- /dev/null
+++ b/Documentation/dev-tools/checkuapi.rst
@@ -0,0 +1,258 @@
+.. SPDX-License-Identifier: GPL-2.0-only
+
+============
+UAPI Checker
+============
+
+The UAPI checker (scripts/check-uapi.sh) is a shell script which checks UAPI
+header files for userspace backwards-compatibility across the git tree.
+
+The script can produce false positives in some cases, so developers are
+encouraged to use their best judgement when interpreting the results. Please
+refer to kernel documentation on the topic of IOCTL stability for more
+information (Documentation/process/botching-up-ioctls.rst).
+
+Options
+=======
+
+This section will describe the options check-uapi.sh can be run with.
+
+Usage::
+
+    ./scripts/check-uapi.sh [-b BASE_REF] [-r COMP_REF] [-a] [-j N] [-l ERROR_LOG]
+
+Available options::
+
+    -b BASE_REF    Base git reference to use for comparison. If unspecified or empty,
+                   will use any dirty changes in tree to UAPI files. If there are no
+                   dirty changes, HEAD will be used.
+    -r COMP_REF    Compare BASE_REF to COMP_REF (e.g. -r v6.1). If unspecified or empty,
+                   will use BASE_REF^1.
+    -a             Check all UAPI headers for backwards compatibility.
+    -j JOBS        Number of checks to run in parallel (default: number of CPU cores)
+    -l ERROR_LOG   Write error log to file (default: "$KERNEL_SOURCE/abi_error_log.txt")
+
+Environmental args::
+
+    ABIDIFF  Custom path to abidiff binary
+    CC       C compiler (default is "gcc")
+
+Examples
+========
+
+First, let's try making a change to a UAPI header file that obviously won't
+break userspace::
+
+    cat << 'EOF' | patch -l -p1
+    --- a/include/uapi/linux/acct.h
+    +++ b/include/uapi/linux/acct.h
+    @@ -21,7 +21,9 @@
+     #include <asm/param.h>
+     #include <asm/byteorder.h>
+
+    -/*
+    +#define FOO
+    +
+    +/*
+      *  comp_t is a 16-bit "floating" point number with a 3-bit base 8
+      *  exponent and a 13-bit fraction.
+      *  comp2_t is 24-bit with 5-bit base 2 exponent and 20 bit fraction
+    diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
+    EOF
+
+Now, let's use the script to validate::
+
+    % ./scripts/check-uapi.sh
+    Installing sanitized UAPI headers from dirty tree... OK
+    Saving current tree state... OK
+    Installing sanitized UAPI headers from HEAD... OK
+    Restoring current tree state... OK
+    Checking changes to UAPI headers starting from dirty tree
+    No ABI differences detected in include/uapi/linux/acct.h from HEAD -> dirty tree
+    All 1 UAPI headers modified between HEAD and dirty tree are backwards compatible!
+
+Let's add another change that *would* break userspace::
+
+    cat << 'EOF' | patch -l -p1
+    --- a/include/uapi/linux/bpf.h  2023-02-28 13:32:36.505591077 -0800
+    +++ b/include/uapi/linux/bpf.h  2023-02-28 13:32:57.033494020 -0800
+    @@ -73,7 +73,7 @@
+            __u8    dst_reg:4;      /* dest register */
+            __u8    src_reg:4;      /* source register */
+            __s16   off;            /* signed offset */
+    -       __s32   imm;            /* signed immediate constant */
+    +       __u32   imm;            /* unsigned immediate constant */
+     };
+
+     /* Key of an a BPF_MAP_TYPE_LPM_TRIE entry */
+    EOF
+
+The script should catch this incompatibility::
+
+    % ./scripts/check-uapi.sh
+    Installing sanitized UAPI headers from dirty tree... OK
+    Saving current tree state... OK
+    Installing sanitized UAPI headers from HEAD... OK
+    Restoring current tree state... OK
+    Checking changes to UAPI headers starting from dirty tree
+    No ABI differences detected in include/uapi/linux/acct.h from HEAD -> dirty tree
+    !!! ABI differences detected in include/uapi/linux/bpf.h from HEAD -> dirty tree !!!
+
+        [C] 'struct bpf_insn' changed:
+          type size hasn't changed
+          1 data member change:
+            type of '__s32 imm' changed:
+              typedef name changed from __s32 to __u32 at int-ll64.h:27:1
+              underlying type 'int' changed:
+                type name changed from 'int' to 'unsigned int'
+                type size hasn't changed
+
+    Header file diff (after headers_install):
+    --- HEAD/include/uapi/linux/bpf.h       2023-02-28 22:24:44.068898545 -0800
+    +++ dirty/include/uapi/linux/bpf.h      2023-02-28 22:24:42.132909241 -0800
+    @@ -73,7 +73,7 @@
+            __u8    dst_reg:4;      /* dest register */
+            __u8    src_reg:4;      /* source register */
+            __s16   off;            /* signed offset */
+    -       __s32   imm;            /* signed immediate constant */
+    +       __u32   imm;            /* unsigned immediate constant */
+     };
+
+     /* Key of an a BPF_MAP_TYPE_LPM_TRIE entry */
+
+    error - 1/2 UAPI headers modified between HEAD and dirty tree are not backwards compatible
+    error - UAPI header ABI check failed
+
+Note: the script is operating on UAPI header files which have changed in the
+dirty git tree. If there were no changes in the tree, it would look for UAPI
+changes introduced by the latest HEAD commit.
+
+Let's commit the breaking change, then commit the good change::
+
+    % git commit -m 'Breaking UAPI change' include/uapi/linux/bpf.h
+    [detached HEAD f758e574663a] Breaking UAPI change
+     1 file changed, 1 insertion(+), 1 deletion(-)
+    % git commit -m 'Innocuous UAPI change' include/uapi/linux/acct.h
+    [detached HEAD 2e87df769081] Innocuous UAPI change
+     1 file changed, 3 insertions(+), 1 deletion(-)
+
+Now, what happens if we run the script again with no arguments?::
+
+    % ./scripts/check-uapi.sh
+    Saving current tree state... OK
+    Installing sanitized UAPI headers from HEAD... OK
+    Installing sanitized UAPI headers from HEAD^1... OK
+    Restoring current tree state... OK
+    Checking changes to UAPI headers starting from HEAD
+    No ABI differences detected in include/uapi/linux/acct.h from HEAD^1 -> HEAD
+    All 1 UAPI headers modified between HEAD^1 and HEAD are backwards compatible!
+
+It doesn't catch any breaking change because by default, it only compares HEAD
+to HEAD^1. If we wanted the search scope to go back further, we'd have to use
+the "-r" option to pass other references to compare to. In this case, let's
+pass "-r HEAD~2" to the script so it checks UAPI changes between HEAD~2 and
+HEAD::
+
+    % ./scripts/check-uapi.sh -r "HEAD~2"
+    Saving current tree state... OK
+    Installing sanitized UAPI headers from HEAD... OK
+    Installing sanitized UAPI headers from HEAD~2... OK
+    Restoring current tree state... OK
+    Checking changes to UAPI headers starting from HEAD
+    No ABI differences detected in include/uapi/linux/acct.h from HEAD~2 -> HEAD
+    !!! ABI differences detected in include/uapi/linux/bpf.h from HEAD~2 -> HEAD !!!
+
+        [C] 'struct bpf_insn' changed:
+          type size hasn't changed
+          1 data member change:
+            type of '__s32 imm' changed:
+              typedef name changed from __s32 to __u32 at int-ll64.h:27:1
+              underlying type 'int' changed:
+                type name changed from 'int' to 'unsigned int'
+                type size hasn't changed
+
+    Header file diff (after headers_install):
+    --- HEAD~2/include/uapi/linux/bpf.h     2023-02-28 22:27:40.311925004 -0800
+    +++ HEAD/include/uapi/linux/bpf.h       2023-02-28 22:27:39.743928142 -0800
+    @@ -73,7 +73,7 @@
+            __u8    dst_reg:4;      /* dest register */
+            __u8    src_reg:4;      /* source register */
+            __s16   off;            /* signed offset */
+    -       __s32   imm;            /* signed immediate constant */
+    +       __u32   imm;            /* unsigned immediate constant */
+     };
+
+     /* Key of an a BPF_MAP_TYPE_LPM_TRIE entry */
+
+    error - 1/2 UAPI headers modified between HEAD~2 and HEAD are not backwards compatible
+    error - UAPI header ABI check failed
+
+If you want to check different chunks of the tree, you can also change your
+base commit with the "-b" option. For example, to check all changed UAPI header
+files between v6.0 and v6.1, you'd run::
+
+    % ./scripts/check-uapi.sh -b v6.1 -r v6.0
+    Saving current tree state... OK
+    Installing sanitized UAPI headers from v6.1... OK
+    Installing sanitized UAPI headers from v6.0... OK
+    Restoring current tree state... OK
+    Checking changes to UAPI headers starting from v6.1
+    -- snip --
+    error - 42/106 UAPI headers modified between v6.0 and v6.1 are not backwards compatible
+    error - UAPI header ABI check failed
+
+Aren't kernel UAPIs stable forever? Why is the script reporting errors?
+
+False Positives
+===============
+
+The UAPI checker is very aggressive in detecting ABI changes, so some false
+positives may appear. For example, if you check all UAPI headers between v6.0
+and v6.1, many breakages will be flagged. Run the following::
+
+    ./scripts/check-uapi.sh -b v6.1 -r v6.0
+
+The errors will be logged to "abi_error_log.txt". Here, we'll find examples of
+several types of false positives.
+
+Enum Expansion::
+
+    !!! ABI differences detected in include/uapi/linux/openvswitch.h from v6.0 -> v6.1 !!!
+
+        [C] 'enum ovs_datapath_attr' changed:
+          type size hasn't changed
+          1 enumerator insertion:
+            'ovs_datapath_attr::OVS_DP_ATTR_IFINDEX' value '9'
+          1 enumerator change:
+            'ovs_datapath_attr::__OVS_DP_ATTR_MAX' from value '9' to '10' at openvswitch.h.current:85:1
+
+In this case, an enum was expanded. Consequently, the "MAX" value was
+incremented. This is not considered a breaking change because it's assumed
+userspace programs are using the MAX value in a sane fashion.
+
+Expanding into reserved/padding fields::
+
+    !!! ABI differences detected in include/uapi/linux/perf_event.h from v6.0 -> v6.1 !!!
+
+        [C] 'struct perf_branch_entry' changed:
+          type size hasn't changed
+          3 data member insertions:
+            '__u64 spec', at offset 152 (in bits) at perf_event.h.current:1420:1
+            '__u64 new_type', at offset 154 (in bits) at perf_event.h.current:1421:1
+            '__u64 priv', at offset 158 (in bits) at perf_event.h.current:1422:1
+          1 data member change:
+            '__u64 reserved' offset changed from 152 to 161 (in bits) (by +9 bits)
+
+In this case, a reserved field was expanded into. Previously, the reserved
+field occupied 40 bits in the struct. After the change, three new members
+were added that took up 9 bits, so the size of the reserved field was
+reduced to 31.
+
+As the size of the struct did not change and none of the fields a userspace
+program could have been using were removed/changed/relocated, this change is
+not considered breaking.
+
+In the future, as tooling improves, we may be able to filter out more of these
+false positives. There may also be additional examples of false positives not
+listed here. Use your best judgement, and ideally a unit test in userspace, to
+test your UAPI changes!
diff --git a/Documentation/dev-tools/index.rst b/Documentation/dev-tools/index.rst
index 6b0663075dc0..0876f5a2cf55 100644
--- a/Documentation/dev-tools/index.rst
+++ b/Documentation/dev-tools/index.rst
@@ -34,6 +34,7 @@ Documentation/dev-tools/testing-overview.rst
    kselftest
    kunit/index
    ktap
+   checkuapi


 .. only::  subproject and html
--
2.17.1


  parent reply	other threads:[~2023-03-01  7:54 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-01  7:54 [PATCH v2 0/2] Validating UAPI backwards compatibility John Moon
2023-03-01  7:54 ` [PATCH v2 1/2] check-uapi: Introduce check-uapi.sh John Moon
2023-03-03 21:09   ` Dodji Seketeli
2023-03-05  4:22   ` Masahiro Yamada
2023-03-05 17:08     ` John Moon
2023-03-01  7:54 ` John Moon [this message]
2023-03-05  6:20   ` [PATCH v2 2/2] docs: dev-tools: Add UAPI checker documentation Masahiro Yamada
2023-03-06 16:56     ` John Moon
2023-03-01 17:50 ` [PATCH v2 0/2] Validating UAPI backwards compatibility Nick Desaulniers
2023-03-01 18:03   ` John Moon
2023-03-01 19:24     ` John Moon
2023-03-01 22:33       ` John Moon
2023-03-01 23:33         ` Nick Desaulniers
2023-03-01 23:35           ` Trilok Soni
2023-03-01 23:52         ` Mark Wielaard
2023-03-01 23:59           ` John Moon
2023-03-10  8:09 ` Christoph Hellwig
2023-03-10 18:20   ` Trilok Soni
2023-03-20  6:26     ` Christoph Hellwig

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230301075402.4578-3-quic_johmoo@quicinc.com \
    --to=quic_johmoo@quicinc.com \
    --cc=andersson@kernel.org \
    --cc=arnd@arndb.de \
    --cc=gprocida@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jorcrous@amazon.com \
    --cc=kernel-team@android.com \
    --cc=libabigail@sourceware.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maennich@google.com \
    --cc=masahiroy@kernel.org \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=nicolas@fjasle.eu \
    --cc=quic_eberman@quicinc.com \
    --cc=quic_satyap@quicinc.com \
    --cc=quic_tsoni@quicinc.com \
    --cc=rdunlap@infradead.org \
    --cc=tkjos@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).