[PATCH pve-kernel v4 1/2] d/scripts: correctly compare namespaced ABI exports

[結友 <miagetegorann@gmail.com>]

abi-generate emits a different field order for namespaced exports. Parse
both layouts explicitly so the module name does not become part of the
symbol name. Otherwise, building hv_vmbus into vmlinux falsely reports
three existing namespaced exports as missing.

Compare the actual export type and namespace, preserving hash and missing
symbol checks and symbol/module blacklists. Reject added or changed
namespace requirements, malformed entries and duplicate symbols. Fix the
export-type diagnostic's printf argument separator.

Add 12 ABI regression tests, including end-to-end abi-generate output,
relocation, hash changes, GPL restrictions, namespaces and skip handling.
Document the check and test command, and ignore Python bytecode caches.

All 12 tests pass under WSL2 Ubuntu 24.04. Rechecking the existing
7.0.14-26-pve build artifacts with SKIPABI=0 preserves all 32,726 symbols,
their CRCs and export types, with only the expected 47 VMBus relocations.

Signed-off-by: 結友 <miagetegorann@gmail.com>
---
 .gitignore                     |   1 +
 README                         |   8 ++
 debian/scripts/abi-check       |  56 ++++++++++---
 debian/tests/test_abi_check.py | 140 +++++++++++++++++++++++++++++++++
 4 files changed, 192 insertions(+), 13 deletions(-)
 create mode 100644 debian/tests/test_abi_check.py

diff --git a/.gitignore b/.gitignore
index 295de11..aa1d4e1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -3,6 +3,7 @@
 *.deb
 *.dsc
 *.prepared
+__pycache__/
 /abi-tmp*
 /config*.org
 /proxmox-kernel-[0-9]*/
diff --git a/README b/README
index 9dd855a..00564c8 100644
--- a/README
+++ b/README
@@ -167,6 +167,14 @@ variable in the Makefile, rebuilding the packages and running 'make abiupdate'
 (the 'abiupdate' target in 'Makefile' contains the steps for consistently
 updating the repository).
 
+The ABI check reads both ordinary and namespaced exports. Moving an export
+between a module and vmlinux is reported without failing the check. Missing
+symbols, hash changes and stricter export or namespace requirements fail it
+unless an applicable blacklist or the ABI skip mode permits them.
+
+Run the ABI checker regression tests on a Debian build host with
+'python3 -B -m unittest discover -s debian/tests -v'.
+
 The source package-name is proxmox-kernel-MAJ.MIN, the version of the package
 defaults to MAJ.MIN.PATCH-KREL - but this can be adapted to contain a suffix
 for test-packages (e.g. ~test01). The version in the Makefile needs to be equal
diff --git a/debian/scripts/abi-check b/debian/scripts/abi-check
index 9987d04..ff000c3 100755
--- a/debian/scripts/abi-check
+++ b/debian/scripts/abi-check
@@ -89,17 +89,35 @@ sub is_ignored($$) {
     return 0;
 }
 
+# abi-generate emits four fields for ordinary exports, and five fields for
+# namespaced exports (namespace, type, hash, symbol, module). In the latter
+# format the trailing module must not become part of the symbol name.
+sub parse_symbol {
+    my ($line, $file, $number) = @_;
+    chomp $line;
+    $line =~ s/\r$//;
+
+    if ($line =~ /^(EXPORT_SYMBOL(?:_[A-Z0-9_]+)?)\s+(\S+)\s+(0x[0-9a-f]+)\s+(\S+)$/) {
+	return ($4, $1, $2, $3, '');
+    }
+    if ($line =~ /^(\S+)\s+(EXPORT_SYMBOL(?:_[A-Z0-9_]+)?)\s+(0x[0-9a-f]+)\s+(\S+)\s+(\S+)$/) {
+	return ($4, $2, $5, $3, $1);
+    }
+    die "EE: $file:$number: malformed ABI entry: '$line'\n";
+}
+
 # Read new syms first
 print "    Reading new symbols ($abistr)...";
 $count = 0;
 open(my $NEW_FH, '<', $abinew) or die "Could not open $abinew - $!";
 while (<$NEW_FH>) {
-    chomp;
-    m/^(\S+)\s(.+)\s(0x[0-9a-f]+)\s(.+)$/;
-    $symbols{$4}{'type'} = $1;
-    $symbols{$4}{'loc'} = $2;
-    $symbols{$4}{'hash'} = $3;
-    $module_syms{$2} = 0;
+    my ($sym, $type, $loc, $hash, $namespace) = parse_symbol($_, $abinew, $.);
+    die "EE: $abinew:$.: duplicate symbol '$sym'\n" if defined($symbols{$sym}{'type'});
+    $symbols{$sym}{'type'} = $type;
+    $symbols{$sym}{'loc'} = $loc;
+    $symbols{$sym}{'hash'} = $hash;
+    $symbols{$sym}{'namespace'} = $namespace;
+    $module_syms{$loc} = 0;
     $count++;
 }
 close($NEW_FH);
@@ -110,11 +128,12 @@ print "    Reading old symbols...";
 $count = 0;
 open(my $OLD_FH, '<', $abiold) or die "Could not open $abiold - $!";
 while (<$OLD_FH>) {
-    chomp;
-    m/^(\S+)\s(.+)\s(0x[0-9a-f]+)\s(.+)$/;
-    $symbols{$4}{'old_type'} = $1;
-    $symbols{$4}{'old_loc'} = $2;
-    $symbols{$4}{'old_hash'} = $3;
+    my ($sym, $type, $loc, $hash, $namespace) = parse_symbol($_, $abiold, $.);
+    die "EE: $abiold:$.: duplicate symbol '$sym'\n" if defined($symbols{$sym}{'old_type'});
+    $symbols{$sym}{'old_type'} = $type;
+    $symbols{$sym}{'old_loc'} = $loc;
+    $symbols{$sym}{'old_hash'} = $hash;
+    $symbols{$sym}{'old_namespace'} = $namespace;
     $count++;
 }
 close($OLD_FH);
@@ -158,6 +177,7 @@ $count = 0;
 my $moved = 0;
 my $changed_type = 0;
 my $changed_hash = 0;
+my $changed_namespace = 0;
 for my $sym (keys(%symbols)) {
     if (!defined($symbols{$sym}{'old_type'}) or !defined($symbols{$sym}{'type'})) {
 	next;
@@ -172,12 +192,21 @@ for my $sym (keys(%symbols)) {
     # Changes to export type are only bad if new type isn't
     # EXPORT_SYMBOL. Changing things to GPL are bad.
     if ($symbols{$sym}{'type'} ne $symbols{$sym}{'old_type'}) {
-	printf("    TYPE : %-40s : %s => %s%s\n", $sym, $symbols{$sym}{'old_type'}.
+	printf("    TYPE : %-40s : %s => %s%s\n", $sym, $symbols{$sym}{'old_type'},
 	    $symbols{$sym}{'type'}, is_ignored($symbols{$sym}{'loc'}, $sym)
 	    ? " (ignored)" : "");
 	$changed_type++ if $symbols{$sym}{'type'} ne "EXPORT_SYMBOL" and !is_ignored($symbols{$sym}{'loc'}, $sym);
     }
 
+    # Adding or changing a namespace requires consumers to change their imports.
+    # Removing a namespace relaxes that requirement and is compatible.
+    if ($symbols{$sym}{'namespace'} ne $symbols{$sym}{'old_namespace'}) {
+	printf("    NS   : %-40s : %s => %s%s\n", $sym,
+	    $symbols{$sym}{'old_namespace'} || '(none)', $symbols{$sym}{'namespace'} || '(none)',
+	    is_ignored($symbols{$sym}{'loc'}, $sym) ? " (ignored)" : "");
+	$changed_namespace++ if $symbols{$sym}{'namespace'} ne '' && !is_ignored($symbols{$sym}{'loc'}, $sym);
+    }
+
     # Changes to the hash are always bad
     if ($symbols{$sym}{'hash'} ne $symbols{$sym}{'old_hash'}) {
 	printf("    HASH : %-40s : %s => %s%s\n", $sym, $symbols{$sym}{'old_hash'},
@@ -191,8 +220,9 @@ for my $sym (keys(%symbols)) {
 print "WW: $moved symbols changed location\n" if $moved;
 print "$EE $changed_type symbols changed export type and weren't ignored\n" if $changed_type;
 print "$EE $changed_hash symbols changed hash and weren't ignored\n" if $changed_hash;
+print "$EE $changed_namespace symbols changed namespace and weren't ignored\n" if $changed_namespace;
 
-$errors++ if $changed_hash or $changed_type;
+$errors++ if $changed_hash or $changed_type or $changed_namespace;
 if ($changed_hash) {
     print "II: Module hash change summary...\n";
     for my $mod (sort { $module_syms{$b} <=> $module_syms{$a} } keys %module_syms) {
diff --git a/debian/tests/test_abi_check.py b/debian/tests/test_abi_check.py
new file mode 100644
index 0000000..928c64a
--- /dev/null
+++ b/debian/tests/test_abi_check.py
@@ -0,0 +1,140 @@
+#!/usr/bin/python3
+
+"""Check ABI compatibility using both formats emitted by abi-generate."""
+
+from pathlib import Path
+import subprocess
+import tempfile
+import unittest
+
+
+ROOT = Path(__file__).resolve().parents[2]
+CHECKER = ROOT / "debian/scripts/abi-check"
+GENERATOR = ROOT / "debian/scripts/abi-generate"
+
+
+class AbiCheckTests(unittest.TestCase):
+    def setUp(self):
+        temporary = tempfile.TemporaryDirectory(prefix="kernel-abi-test-")
+        self.addCleanup(temporary.cleanup)
+        self.directory = Path(temporary.name)
+
+    def check(self, old, new, *, skip=False, blacklist=""):
+        (self.directory / "abi-prev-test").write_text(old)
+        (self.directory / "abi-test").write_text(new)
+        (self.directory / "abi-blacklist").write_text(blacklist)
+        return subprocess.run(
+            ["perl", str(CHECKER), "abi-test", "abi-prev-test", str(int(skip))],
+            cwd=self.directory, text=True, stdout=subprocess.PIPE,
+            stderr=subprocess.STDOUT, check=False,
+        )
+
+    def test_namespaced_and_ordinary_exports_can_move_to_vmlinux(self):
+        old = (
+            "EXPORT_SYMBOL drivers/hv/hv_vmbus 0x12345678\tvmbus_open\n"
+            "module:mshv_vtl EXPORT_SYMBOL_GPL 0x853b6fd2\thv_post_message\tdrivers/hv/hv_vmbus\n"
+        )
+        result = self.check(old, old.replace("drivers/hv/hv_vmbus", "vmlinux"))
+        self.assertEqual(result.returncode, 0, result.stdout)
+        self.assertIn("2 symbols changed location", result.stdout)
+        self.assertIn("found 0 missing symbols", result.stdout)
+        self.assertIn("found 0 new symbols", result.stdout)
+
+    def test_namespaced_hash_change_is_an_error(self):
+        old = "DMA_BUF EXPORT_SYMBOL_GPL 0x12345678\tdma_buf_get\tvmlinux\n"
+        result = self.check(old, old.replace("0x12345678", "0x87654321"))
+        self.assertNotEqual(result.returncode, 0, result.stdout)
+        self.assertIn("1 symbols changed hash", result.stdout)
+        self.assertRegex(result.stdout, r"vmlinux\s+: 1")
+
+    def test_namespaced_export_restriction_is_an_error(self):
+        old = "DMA_BUF EXPORT_SYMBOL 0x12345678\tdma_buf_get\tvmlinux\n"
+        result = self.check(old, old.replace("EXPORT_SYMBOL ", "EXPORT_SYMBOL_GPL "))
+        self.assertNotEqual(result.returncode, 0, result.stdout)
+        self.assertIn("EXPORT_SYMBOL => EXPORT_SYMBOL_GPL", result.stdout)
+        self.assertIn("1 symbols changed export type", result.stdout)
+
+    def test_ordinary_exports_keep_hash_and_type_checks(self):
+        old = "EXPORT_SYMBOL vmlinux 0x12345678\tordinary\n"
+        for new in (
+            old.replace("0x12345678", "0x87654321"),
+            old.replace("EXPORT_SYMBOL ", "EXPORT_SYMBOL_GPL "),
+        ):
+            with self.subTest(new=new):
+                result = self.check(old, new)
+                self.assertNotEqual(result.returncode, 0, result.stdout)
+
+    def test_namespace_added_or_changed_requires_consumer_changes(self):
+        new = "DMA_BUF EXPORT_SYMBOL 0x12345678\tdma_buf_get\tvmlinux\n"
+        for old in (
+            "EXPORT_SYMBOL vmlinux 0x12345678\tdma_buf_get\n",
+            new.replace("DMA_BUF", "OLD_NAMESPACE"),
+        ):
+            with self.subTest(old=old):
+                result = self.check(old, new)
+                self.assertNotEqual(result.returncode, 0, result.stdout)
+                self.assertIn("1 symbols changed namespace", result.stdout)
+
+    def test_namespace_and_gpl_restrictions_can_be_removed(self):
+        old = "DMA_BUF EXPORT_SYMBOL_GPL 0x12345678\tdma_buf_get\tvmlinux\n"
+        new = "EXPORT_SYMBOL vmlinux 0x12345678\tdma_buf_get\n"
+        result = self.check(old, new)
+        self.assertEqual(result.returncode, 0, result.stdout)
+        self.assertIn("DMA_BUF => (none)", result.stdout)
+
+    def test_namespaced_symbol_and_module_blacklists_still_apply(self):
+        old = "DMA_BUF EXPORT_SYMBOL 0x12345678\tdma_buf_get\tvmlinux\n"
+        new = old.replace("0x12345678", "0x87654321").replace("DMA_BUF", "NEW_NAMESPACE")
+        for blacklist in ("dma_buf_get\n", "M: vmlinux\n"):
+            with self.subTest(blacklist=blacklist):
+                changed = self.check(old, new, blacklist=blacklist)
+                self.assertEqual(changed.returncode, 0, changed.stdout)
+                missing = self.check(old, "", blacklist=blacklist)
+                self.assertEqual(missing.returncode, 0, missing.stdout)
+
+    def test_missing_namespaced_symbol_is_still_an_error(self):
+        old = "DMA_BUF EXPORT_SYMBOL 0x12345678\tdma_buf_get\tvmlinux\n"
+        result = self.check(old, "")
+        self.assertNotEqual(result.returncode, 0, result.stdout)
+        self.assertIn("found 1 missing symbols", result.stdout)
+
+    def test_skip_flag_still_reports_abi_errors(self):
+        old = "DMA_BUF EXPORT_SYMBOL_GPL 0x12345678\tdma_buf_get\tvmlinux\n"
+        result = self.check(old, old.replace("0x12345678", "0x87654321"), skip=True)
+        self.assertEqual(result.returncode, 0, result.stdout)
+        self.assertIn("WW: 1 symbols changed hash", result.stdout)
+
+    def test_malformed_entries_are_rejected_even_when_skipped(self):
+        valid = "EXPORT_SYMBOL vmlinux 0x12345678\tordinary\n"
+        for old, new in ((valid, valid + "broken\n"), (valid + "broken\n", valid)):
+            with self.subTest(old=old, new=new):
+                result = self.check(old, new, skip=True)
+                self.assertNotEqual(result.returncode, 0, result.stdout)
+                self.assertIn("malformed ABI entry", result.stdout)
+
+    def test_duplicate_entries_are_rejected(self):
+        valid = "EXPORT_SYMBOL vmlinux 0x12345678\tordinary\n"
+        for old, new in ((valid, valid * 2), (valid * 2, valid)):
+            with self.subTest(old=old, new=new):
+                result = self.check(old, new)
+                self.assertNotEqual(result.returncode, 0, result.stdout)
+                self.assertIn("duplicate symbol 'ordinary'", result.stdout)
+
+    def test_real_generator_output_preserves_namespaced_relocations(self):
+        symvers = (
+            "0x12345678\tvmbus_open\tdrivers/hv/hv_vmbus\tEXPORT_SYMBOL\t\n"
+            "0x853b6fd2\thv_post_message\tdrivers/hv/hv_vmbus\tEXPORT_SYMBOL_GPL\tmodule:mshv_vtl\n"
+        )
+        for name, content in (("abi-prev-test", symvers), ("abi-test", symvers.replace("drivers/hv/hv_vmbus", "vmlinux"))):
+            source = self.directory / "Module.symvers"
+            source.write_text(content)
+            subprocess.run(["bash", str(GENERATOR), str(source), str(self.directory / name)], check=True)
+        old = (self.directory / "abi-prev-test").read_text()
+        new = (self.directory / "abi-test").read_text()
+        result = self.check(old, new)
+        self.assertEqual(result.returncode, 0, result.stdout)
+        self.assertIn("2 symbols changed location", result.stdout)
+
+
+if __name__ == "__main__":
+    unittest.main()
-- 
2.56.0.windows.1