Make signature collection portable across GNU and BSD tools

.github/workflows/library-features.yml

@@ -20,6 +20,14 @@ on:
       - validation/library-features/**
   pull_request:
     paths:
+      - scripts/coff_symbols.py
+      - scripts/collect_fedora_library.py
+      - scripts/fetch_imported_sources.py
+      - scripts/migrate_imported_signatures.py
+      - scripts/tests/test_coff_symbols.py
+      - scripts/tests/test_collect_elf_packages.py
+      - scripts/tests/test_collect_fedora_library.py
+      - scripts/tests/test_migrate_imported_signatures.py
       - features/**
       - scripts/*library*feature*
       - scripts/build_libcxx_features.py
@@ -38,7 +46,11 @@ permissions:
 
 jobs:
   verify:
-    runs-on: ubuntu-24.04
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ubuntu-24.04, macos-15]
+    runs-on: ${{ matrix.os }}
     timeout-minutes: 10
     steps:
       - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1

scripts/coff_symbols.py

@@ -40,11 +40,21 @@ def archive_members(data: bytes):
             raise ValueError(f"archive member header at {offset:#x} is malformed")
         name = header[:16].rstrip()
         size = int(header[48:58].decode("ascii").strip())
+        if size < 0 or offset + 60 + size > len(data):
+            raise ValueError(f"archive member at {offset:#x} is truncated")
         body = data[offset + 60 : offset + 60 + size]
+        if name.startswith(b"#1/"):
+            # BSD ar prefixes the object with its padded, extended filename.
+            length = int(name[3:])
+            if length <= 0 or length > len(body):
+                raise ValueError(f"archive member name at {offset:#x} is malformed")
+            name, body = body[:length].rstrip(b"\x00"), body[length:]
         # "/" and "//" are the linker and long-name members, "/<ECSYMBOLS>/"
         # the ARM64EC symbol map, "/SYM64/" GNU ar's 64-bit symbol table;
         # "/123" is a member with a long name.
-        if name not in (b"/", b"//", b"/SYM64/") and not name.startswith(b"/<"):
+        if (name not in (b"/", b"//", b"/SYM64/") and not name.startswith(b"/<")
+                and name.rstrip(b"/") not in (b"__.SYMDEF", b"__.SYMDEF SORTED",
+                                              b"__.SYMDEF_64", b"__.SYMDEF_64 SORTED")):
             yield body
         offset += 60 + size + (size & 1)
 

scripts/collect_fedora_library.py

@@ -101,7 +101,16 @@ def unpack_rpm(package: Path, root: Path) -> None:
     if command is None:
         raise BuildError(f"{package.name}: unknown payload compression")
     archive = subprocess.run(command, input=payload, capture_output=True, check=True).stdout
-    subprocess.run(["cpio", "-idm", "--quiet", "--no-absolute-filenames"], input=archive,
+    version = subprocess.run(["cpio", "--version"], capture_output=True,
+                             text=True, check=True).stdout
+    if "GNU cpio" in version:
+        path_options = ["--no-absolute-filenames"]
+    elif "bsdcpio" in version:
+        # BSD cpio rejects absolute paths and traversal unless --insecure is set.
+        path_options = []
+    else:
+        raise BuildError("unsupported cpio implementation")
+    subprocess.run(["cpio", "-idm", "--quiet", *path_options], input=archive,
                    cwd=root, check=True)
 
 

scripts/fetch_imported_sources.py

@@ -24,11 +24,12 @@
 import struct
 import subprocess
 import sys
+import tarfile
 import tempfile
 import time
 import urllib.parse
 import urllib.request
-from pathlib import Path
+from pathlib import Path, PurePosixPath
 
 SIGDB_RAW = "https://raw.githubusercontent.com/rizinorg/sigdb-source"
 LAUNCHPAD = "https://launchpad.net/ubuntu/+archive/primary/+files/"
@@ -106,8 +107,20 @@ def unpack(package: Path, destination: Path) -> list[Path]:
         subprocess.run(["unzip", "-q", "-o", str(package), "*.a", "*.o", "-d",
                         str(destination)], check=False)
     elif package.name.endswith(".tar.bz2"):
-        subprocess.run(["tar", "-xjf", str(package), "-C", str(destination),
-                        "--wildcards", "*.a", "*.o"], check=False)
+        # Extract only the bytes consumed by the collector. GNU tar's
+        # --wildcards is unavailable in the macOS tar implementation.
+        with tarfile.open(package, "r:bz2") as archive:
+            for member in archive:
+                name = PurePosixPath(member.name)
+                if name.suffix not in (".a", ".o") or not (member.isfile() or member.islnk()):
+                    continue
+                if name.is_absolute() or ".." in name.parts:
+                    raise ValueError(f"unsafe archive member: {member.name}")
+                target = destination.joinpath(*name.parts)
+                target.resolve().relative_to(destination.resolve())
+                target.parent.mkdir(parents=True, exist_ok=True)
+                with archive.extractfile(member) as source, target.open("wb") as output:
+                    shutil.copyfileobj(source, output)
     return sorted(path for path in destination.rglob("*")
                   if path.suffix in (".a", ".o") and path.is_file())
 

scripts/migrate_imported_signatures.py

@@ -265,15 +265,21 @@ def parse_hex(text: str) -> tuple[bytes, bytes]:
 def demangle(names: list[str]) -> list[str]:
     """Demangle Itanium names the way rizin's demangler printed them.
 
-    That is libiberty without DMGL_VERBOSE: `std::string::rbegin() const`,
-    not `std::basic_string<char, ...>::rbegin() const`, which is what plain
-    c++filt prints. `c++filt -i` turns the verbose expansion off.
+    Use the compact `std::string::rbegin() const` spelling. GNU c++filt
+    needs `-i` to disable DMGL_VERBOSE expansion; LLVM's demangler already
+    uses that spelling. ELF names must retain their leading underscore.
     """
 
     if not names:
         return []
+    help_text = subprocess.run(
+        ["c++filt", "--help"], capture_output=True, text=True, check=True,
+    ).stdout
+    options = ["-n"]
+    if "--no-verbose" in help_text:
+        options.append("-i")
     completed = subprocess.run(
-        ["c++filt", "-i"], input="\n".join(names) + "\n", capture_output=True, text=True,
+        ["c++filt", *options], input="\n".join(names) + "\n", capture_output=True, text=True,
         check=True,
     )
     lines = completed.stdout.split("\n")

scripts/tests/test_coff_symbols.py

@@ -64,6 +64,30 @@ def archive(*members: bytes) -> bytes:
 
 
 class CodeSymbolTests(unittest.TestCase):
+    @staticmethod
+    def bsd_member(name: bytes, body: bytes) -> bytes:
+        name += b"\x00" * (-len(name) % 4)
+        payload = name + body
+        header = (f"#1/{len(name)}".encode().ljust(16) + b"0".ljust(12)
+                  + b"".ljust(6) * 2 + b"644".ljust(8)
+                  + str(len(payload)).encode().ljust(10) + b"`\n")
+        return header + payload + (b"\n" if len(payload) & 1 else b"")
+
+    def test_bsd_extended_names_and_symbol_tables(self) -> None:
+        data = (coff_symbols.ARCHIVE_MAGIC
+                + self.bsd_member(b"__.SYMDEF SORTED", b"index, not object code")
+                + self.bsd_member(b"long-object-name.obj", coff_object()))
+        members = list(coff_symbols.archive_members(data))
+        self.assertEqual(members, [coff_object()])
+        self.assertEqual(coff_symbols.object_symbols(members[0]),
+                         {"?Run@@YAXXZ", "_static"})
+
+    def test_truncated_bsd_members_and_names_are_rejected(self) -> None:
+        member = self.bsd_member(b"name.obj", coff_object())
+        for broken in (member[:-10], b"#1/999".ljust(16) + member[16:]):
+            with self.subTest(broken=broken[:16]), self.assertRaises(ValueError):
+                list(coff_symbols.archive_members(coff_symbols.ARCHIVE_MAGIC + broken))
+
     def test_names_defined_in_code_sections(self) -> None:
         for bigobj in (False, True):
             with self.subTest(bigobj=bigobj):

scripts/tests/test_collect_elf_packages.py

@@ -52,6 +52,49 @@ def test_a_package_two_series_carried_is_fetched_once(self) -> None:
                                    ("bb", "android-ndk-r24-linux.zip")])
 
 
+class UnpackTests(unittest.TestCase):
+    def test_tar_members_and_hardlinks_retain_their_bytes(self) -> None:
+        with tempfile.TemporaryDirectory() as scratch:
+            root = Path(scratch)
+            package = root / "fixture.tar.bz2"
+            with tarfile.open(package, "w:bz2") as archive:
+                for name, data in (("pkg/lib.a", b"archive bytes"),
+                                   ("pkg/unit.o", b"object bytes"),
+                                   ("pkg/README", b"unrelated")):
+                    member = tarfile.TarInfo(name)
+                    member.size = len(data)
+                    archive.addfile(member, io.BytesIO(data))
+                link = tarfile.TarInfo("pkg/copy.a")
+                link.type = tarfile.LNKTYPE
+                link.linkname = "pkg/lib.a"
+                archive.addfile(link)
+                link = tarfile.TarInfo("pkg/symlink.a")
+                link.type = tarfile.SYMTYPE
+                link.linkname = "lib.a"
+                archive.addfile(link)
+            destination = root / "unpacked"
+            files = fetcher.unpack(package, destination)
+            self.assertEqual({p.name: p.read_bytes() for p in files}, {
+                "lib.a": b"archive bytes", "copy.a": b"archive bytes",
+                "unit.o": b"object bytes",
+            })
+            self.assertFalse((destination / "pkg/README").exists())
+            self.assertFalse((destination / "pkg/symlink.a").exists())
+
+    def test_tar_members_cannot_escape_the_destination(self) -> None:
+        for name in ("../outside.a", "/outside.a"):
+            with self.subTest(name=name), tempfile.TemporaryDirectory() as scratch:
+                root = Path(scratch)
+                package = root / "fixture.tar.bz2"
+                with tarfile.open(package, "w:bz2") as archive:
+                    member = tarfile.TarInfo(name)
+                    member.size = 1
+                    archive.addfile(member, io.BytesIO(b"x"))
+                with self.assertRaisesRegex(ValueError, "unsafe archive member"):
+                    fetcher.unpack(package, root / "unpacked")
+                self.assertFalse((root / "outside.a").exists())
+
+
 class ObtainTests(unittest.TestCase):
     def obtain(self, transfers: list[bytes]) -> tuple[Path | None, str | None, list[str]]:
         """What obtain() makes of these transfers, and the files it leaves."""

scripts/tests/test_collect_fedora_library.py

@@ -11,6 +11,7 @@
 import sys
 import tempfile
 import unittest
+from unittest import mock
 from pathlib import Path, PurePosixPath
 
 sys.path.insert(0, str(Path(__file__).resolve().parents[1]))
@@ -32,6 +33,38 @@ def rpm(payload: bytes, signature_store: bytes = b"") -> bytes:
     return bytes(96) + signature + padding + header(b"main") + payload
 
 
+class CpioOptionsTests(unittest.TestCase):
+    def test_each_supported_tool_keeps_its_path_checks(self) -> None:
+        for version, options in (("cpio (GNU cpio) 2.15", ["--no-absolute-filenames"]),
+                                 ("bsdcpio 3.5.3", [])):
+            with self.subTest(version=version), tempfile.TemporaryDirectory() as scratch:
+                root = Path(scratch)
+                package = root / "test.rpm"
+                package.write_bytes(rpm(gzip.compress(b"cpio payload")))
+                with mock.patch.object(collect.subprocess, "run", side_effect=[
+                    subprocess.CompletedProcess([], 0, stdout=b"cpio payload"),
+                    subprocess.CompletedProcess([], 0, stdout=version),
+                    subprocess.CompletedProcess([], 0),
+                ]) as run:
+                    collect.unpack_rpm(package, root)
+                self.assertEqual(run.call_args_list[-1].args[0],
+                                 ["cpio", "-idm", "--quiet", *options])
+                self.assertEqual(run.call_args_list[-1].kwargs["input"], b"cpio payload")
+
+    def test_unknown_tool_fails_before_extraction(self) -> None:
+        with tempfile.TemporaryDirectory() as scratch:
+            root = Path(scratch)
+            package = root / "test.rpm"
+            package.write_bytes(rpm(gzip.compress(b"cpio payload")))
+            with mock.patch.object(collect.subprocess, "run", side_effect=[
+                subprocess.CompletedProcess([], 0, stdout=b"cpio payload"),
+                subprocess.CompletedProcess([], 0, stdout="unknown cpio"),
+            ]) as run:
+                with self.assertRaisesRegex(collect.BuildError, "unsupported cpio"):
+                    collect.unpack_rpm(package, root)
+            self.assertEqual(run.call_count, 2)
+
+
 @unittest.skipUnless(shutil.which("cpio"), "needs cpio")
 class RpmTests(unittest.TestCase):
     def test_the_payload_is_unpacked_past_a_padded_signature(self) -> None:

scripts/tests/test_migrate_imported_signatures.py

@@ -237,7 +237,11 @@ def test_uncheckable_crc_still_settles_when_nothing_else_does(self) -> None:
 
 
 @unittest.skipUnless(shutil.which("c++filt"), "needs c++filt")
-class ELFReferenceTests(unittest.TestCase):
+class ELFSpellingIndexTests(unittest.TestCase):
+    def test_compact_string_spelling_and_elf_underscores(self) -> None:
+        self.assertEqual(migrate.demangle(["_ZNKSs6rbeginEv", "_Z3foov", "puts"]),
+                         ["std::string::rbegin() const", "foo()", "puts"])
+
     def test_spelling_index_maps_imports_back_to_mangled_names(self) -> None:
         reference = migrate.Reference("elf")
         reference.add_line(reference_line("_ZNSt6thread4joinEv", "4883EC08C3"))
@@ -707,7 +711,7 @@ def test_pe_and_elf_assets_of_one_architecture_stay_apart(self) -> None:
             self.assertEqual(migrate.collected_releases(assets, "x64", "elf"),
                              {"ubuntu-libc6": False})
 
-    @unittest.skipUnless(shutil.which("cc") and shutil.which("ar"), "needs a C compiler")
+    @unittest.skipUnless(shutil.which("clang") and shutil.which("ar"), "needs Clang and ar")
     def test_elf_archives_name_their_functions(self) -> None:
         with tempfile.TemporaryDirectory() as scratch:
             root = Path(scratch)
@@ -716,7 +720,8 @@ def test_elf_archives_name_their_functions(self) -> None:
                 "static int helper(int x) { return x + 1; }\n"
                 "int use(int x) { return helper(x); }\n"
                 "int data = 1;\n")
-            subprocess.run(["cc", "-O0", "-c", str(root / "a.c"), "-o", str(root / "a.o")],
+            subprocess.run(["clang", "-target", "x86_64-unknown-linux-gnu", "-O0", "-c",
+                            str(root / "a.c"), "-o", str(root / "a.o")],
                            check=True)
             subprocess.run(["ar", "rcs", str(root / "liba.a"), str(root / "a.o")], check=True)
             from coff_symbols import code_symbols