On 4/10/2023 11:45 AM, Greg Kroah-Hartman wrote:
On Mon, Apr 10, 2023 at 07:03:05PM +0900, Masahiro Yamada wrote:
On Sat, Apr 8, 2023 at 5:35 AM John Moon <quic_johmoo@xxxxxxxxxxx> wrote:
While the kernel community has been good at maintaining backwards
compatibility with kernel UAPIs, it would be helpful to have a tool
to check if a commit introduces changes that break backwards
compatibility.
To that end, introduce check-uapi.sh: a simple shell script that
checks for changes to UAPI headers using libabigail.
libabigail is "a framework which aims at helping developers and
software distributors to spot some ABI-related issues like interface
incompatibility in ELF shared libraries by performing a static
analysis of the ELF binaries at hand."
The script uses one of libabigail's tools, "abidiff", to compile the
changed header before and after the commit to detect any changes.
abidiff "compares the ABI of two shared libraries in ELF format. It
emits a meaningful report describing the differences between the two
ABIs."
The script also includes the ability to check the compatibility of
all UAPI headers across commits. This allows developers to inspect
the stability of the UAPIs over time.
Signed-off-by: John Moon <quic_johmoo@xxxxxxxxxxx>
BTW, is there anybody (except the submitters) who loves this tool?
(or anybody who has ever evaluated this?)
I evaluated the first one, and yes, I do want this, but I haven't tested
it out yet, sorry.
I get patches for header files all the time and hand-verifying that they
don't break the abi is a pain at times
Agreed, this is the way we're using the tool internally. It's great as a
quick spot-check on a change.
According to this tool, it looks like we broke a lot of UAPI
headers in the previous MW (between v6.2 and v6.3-rc1).
That's not ok, and needs to be fixed, otherwise this is useless as no
one can rely on it at all.
Right, there are several classes of false positives that we've
documented and when examining thousands of commits at time, it'll flag
many things.
For some comparison, if you run checkpatch on the same changeset
(v6.2..v6.3-rc1), you get 995 errors and 7,313 warnings. Still,
checkpatch is helpful for spot-checks.
"./scripts/check-uapi.sh -b v6.3-rc1 -p v6.2" flags 36 out of the 911
files checked. Of those 36, 19 fell into the currently documented false
positive categories:
Enum expansion: 17
Expanding into padded/reserved fields: 2
Beyond those, the tool appears to be flagging legitimate breakages.
Some fit into the definition of "intentional breakages" where support is
being dropped or something is being refactored:
File removals:
- include/uapi/drm/i810_drm.h
- include/uapi/drm/mga_drm.h
- include/uapi/drm/r128_drm.h
- include/uapi/drm/savage_drm.h
- include/uapi/drm/sis_drm.h
- include/uapi/drm/via_drm.h
- include/uapi/linux/meye.h
File moves:
- include/uapi/misc/habanalabs.h
Removal of struct:
- include/uapi/linux/uuid.h (5e6a51787fef)
- include/uapi/linux/mei.h (failed due to uuid.h)
- include/uapi/linux/ublk_cmd.h (failed due to uuid.h)
Others do not seem to be intentional:
Addition/use of flex arrays:
- include/uapi/linux/rseq.h (f7b01bb0b57f)
- include/uapi/scsi/scsi_bsg_mpi3mr.h (c6f2e6b6eaaf)
Type change:
- include/uapi/scsi/scsi_bsg_ufs.h (3f5145a615238)
Additions into existing struct:
- include/uapi/drm/amdgpu_drm.h (b299221faf9b)
- include/uapi/linux/perf_event.h (09519ec3b19e)
- include/uapi/linux/virtio_blk.h (95bfec41bd3d)
Is there something I'm missing that makes these changes false positives?
If so, I'd be happy to add on to the documentation and work towards a
way to filter them out.
In the mean time, we will start a thread on the libabigail mailing list
to see if there's a way to add flags such as --ignore-enum-expansion,
--ignore-expansion-into-reserved-fields, etc. Enum expansion seems to be
making up the largest portion of false positives, so would be the best
thing to filter out.
The script takes some time because it builds many objects
internally.
However, once this script starts running, you must not hit Ctrl-C.
If you do it, your repository will be sprinkled with a ton
of untracked files.
That needs to be unwound and fixed.
CAVEAT
This tool runs 'git checkout' a couple of times internally.
If you interrupt it, your worktree might be messed up.
ctrl-c can be properly caught and the git state needs to be restored for
this to be able to be accepted.
Yes, this can be taken care of.
thanks,
greg k-h
