scripts/check-uapi.sh: add stgdiff support
Commit Message
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 <gprocida@google.com>
---
scripts/check-uapi.sh | 102 ++++++++++++++++++++++++++++--------------
1 file changed, 69 insertions(+), 33 deletions(-)
Comments
On 7/20/2023 9:10 AM, Giuliano Procida wrote:
> 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.
Thank you Giuliano for trying the script w/ stg. We will review the
modifications below.
Just to update everyone here that John is looking into the libabigail
changes to reduce the false positives as discussed earlier in the email
thread. There is some progress on the libabigail mailing list. Once we
have enough changes made in the libabigail, John will update here with
his results.
We have also submitted abstract in LPC 2023 Android MC as well about the
UAPI checker. We hope to make a good progress before LPC.
---Trilok Soni
@@ -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
}