From 4bba0576a5961cf79f978fb8d8a0e916d5c5247a Mon Sep 17 00:00:00 2001 From: Mark Schaller Date: Sat, 26 Sep 2026 02:28:58 -0400 Subject: [PATCH] Compare link-interface flags from CMake's libraries fragments The CMake File API reports a target's link line in fragments by role. Flags that a dependency carries on its link interface (Python's -Xlinker -export-dynamic, OpenMP's -fopenmp, Threads' -pthread) arrive in the "libraries" fragments, not "flags". extract_cmake.py read "libraries" only for dependency names, so these flags reached neither the dep closure nor the link-flag comparison, and a Bazel binary missing one converged. Observed in any2bazel-lab: on a Blender migration, the lab verified that -Xlinker -export-dynamic sits only in the blender target's libraries fragment. By the migrating agent's own report, the diff converged without it and Python extension modules then failed to resolve CPython symbols at run time, caught only by its functional test; the lab did not reproduce the failure, because that tree was not kept. A "libraries" fragment that is not a library and starts with "-" is now added to the target's link argv, so it is compared like any link flag. "-Xlinker X" pairs are folded into "-Wl,X" first, so the fragment is classified by what it passes to the linker. Excluded, because they position or locate the libraries around them or name the CMake build tree: -L, -framework, rpaths in every spelling (-Wl,-rpath, -Wl,--rpath, -Wl,-R, and -Xlinker -rpath -Xlinker ), the whole-archive, push/pop-state, group, -Bstatic/-Bdynamic and as-needed toggles, and -force_load. canonicalize_link_flags also folds "-Xlinker X" into "-Wl,X" on both sides, before its noise filters, so the pair compares as one flag, and then joins an rpath given as two linker arguments (`-Wl,-rpath` then `-Wl,DIR`) into `-Wl,-rpath,DIR`, which is what the linker receives. So these three spellings of one rpath compare equal: `-Xlinker -rpath -Xlinker DIR` (what CMake's LINKER:-rpath,DIR becomes under Clang), `-Wl,-rpath -Wl,DIR` and `-Wl,-rpath,DIR`. `-Wl,-rpath=DIR`, `-Wl,--rpath,DIR` and `-Wl,-R,DIR` are not unified with them, as before. Bazel spells its own rpaths the first way, as $ORIGIN/, $EXEC_ORIGIN/ or @loader_path/ paths into its _solib_ directories; on the Bazel side exactly those (an anchored match) are toolchain link noise, otherwise they would appear as Bazel-only link flags on every binary. A project's own rpath (-Wl,-rpath,$ORIGIN/../lib, -Wl,-rpath=/opt/lib, -Wl,-rpath-link,...) is kept and compared. One limitation: any non-library "libraries" fragment becomes a link flag the Bazel side must have, including one that carries a CMake build-tree path Bazel cannot reproduce (-Wl,--version-script,/sym.map; this holds when the map file's basename does not start with "lib", since a fragment ending in lib. is still read as the external dependency , as before). A version script decides which symbols are exported, so such a flag is not benign: its exact CMake spelling may go in ignore.link_flags only after confirming the Bazel link passes an equivalent script with the same content, and SKILL.md's link_flags_diff row now says so. A multi-token fragment is classified as a whole, as before, so `-Wl,-Bstatic -lfoo -Wl,-Bdynamic` in one fragment still loses its -lfoo; splitting fragments per token is left out because rpath pairs would then need their own pairing logic. A behavior change: a migration that converged before may now show a link_flags_diff for such a flag, most often -pthread (Threads::Threads with THREADS_PREFER_PTHREAD_FLAG) or -fopenmp. That is a real difference in the link command; add the flag to linkopts or record it in ignore.link_flags. SKILL.md says these flags are compared. No migration run exercised this change; it was checked on the stored Blender models. Lab-Proposal: P25 Co-Authored-By: Claude Opus 5.5 --- SKILL.md | 7 +-- scripts/canonicalize.py | 50 ++++++++++++++++++-- scripts/extract_cmake.py | 40 +++++++++++++++- tests/test_extractors.py | 100 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 189 insertions(+), 8 deletions(-) diff --git a/SKILL.md b/SKILL.md index 9c1380d..effe05d 100644 --- a/SKILL.md +++ b/SKILL.md @@ -53,8 +53,9 @@ Bazel aquery jsonproto ──extract_bazel.py───────┘ includes (project-wide TU-set), and the external link-dependency closure matches. - **Link consistency** — every name-aligned executable links with - equivalent link flags. The per-binary stage: the *artifacts* agree, not just - the TUs. + equivalent link flags, including those a dependency adds through its link + interface (`-fopenmp`, `-Xlinker -export-dynamic`). The per-binary stage: + the *artifacts* agree, not just the TUs. Done when both stages converge. The comparison is **asymmetric**: every correctness-relevant reference flag must be present on the Bazel side; extra @@ -366,7 +367,7 @@ correctness flags). | `defines_diff` | add each `cmake_only` define to `defines`, or `ignore.defines` it | | `includes_diff` | add the missing reference include root to `includes`; for a dep whose root is spelled differently each side, `ignore.include_map` it (preferred) or `ignore.include_prefixes` it | | `flags_diff` | add each `cmake_only` flag to `copts`, or `ignore.flags` it | -| `link_flags_diff`| add each `cmake_only` flag to the target's `linkopts`, or `ignore.link_flags` it if benign (a compile flag the reference repeats at link that is not already reported as `link_compile_only_flags`) | +| `link_flags_diff`| add each `cmake_only` flag to the target's `linkopts`, or `ignore.link_flags` it if benign (a compile flag the reference repeats at link that is not already reported as `link_compile_only_flags`). A flag that differs only by a reference build-tree path (`-Wl,--version-script,/x.map`) is still a correctness flag: `ignore.link_flags` that exact spelling only after confirming the Bazel link passes an equivalent script with the same content | | `missing_dep` | add the missing external/system dep to the target's `deps`/`linkopts`; if it's just a name spelled differently per build (`Catch2Main` vs `catch2_main`, `OpenSSL::SSL` vs `ssl`), add a `dep_map` entry. External deps are captured from both `-l` flags and archive-file inputs (e.g. `external/catch2+/libcatch2_main.a`) | | `missing_test_tu`| (tests on) add the test source to a `cc_test`, or `exclude_targets` if out of scope | | `test_binary_count` | (tests on, warning) differing number of test executables — investigate which side has the extra/missing binary | diff --git a/scripts/canonicalize.py b/scripts/canonicalize.py index a6f5552..824cbaf 100644 --- a/scripts/canonicalize.py +++ b/scripts/canonicalize.py @@ -32,6 +32,7 @@ import os import posixpath +import re from typing import Iterable, List, Tuple # Flags Bazel's C++ toolchain injects that are PURE NOISE: they never have a @@ -115,6 +116,20 @@ "-Wl,-rpath,__BAZEL", # bazel sandbox rpaths ) + +def _is_bazel_solib_rpath(tok: str) -> bool: + """Bazel's own rpaths, into its `_solib_` directories (spelled + `-Xlinker -rpath -Xlinker $ORIGIN/.../_solib_k8/...`, which fold_xlinker + turns into one `-Wl,-rpath,`). Only those are toolchain noise; a + project's own rpath (-Wl,-rpath,$ORIGIN/../lib, -Wl,-rpath=/opt/lib, + -Wl,-rpath-link,...) is kept and compared, even one whose path happens to + contain `_solib_` elsewhere.""" + return _BAZEL_SOLIB_RPATH.match(tok) is not None + + +_BAZEL_SOLIB_RPATH = re.compile( + r"-Wl,-rpath,(\$ORIGIN|\$EXEC_ORIGIN|@loader_path)(/.*)?/_solib_[^/]*(/|$)") + # Link flags with no correctness meaning on either side (build-type / debug). IGNORABLE_LINK_PREFIXES = ( "-g", @@ -316,6 +331,30 @@ def canonicalize_flags( return canon_defines, tuple(canon_includes), canon_other +def fold_xlinker(tokens: List[str]) -> List[str]: + """`-Xlinker X` -> `-Wl,X`: the same linker argument in one token. Then an + rpath given as two linker arguments (`-Wl,-rpath` followed by `-Wl,DIR`, + whichever spelling produced them) is joined into `-Wl,-rpath,DIR`, which + is what the linker receives, so every spelling of one rpath compares + equal.""" + folded: List[str] = [] + i = 0 + while i < len(tokens): + if tokens[i] == "-Xlinker" and i + 1 < len(tokens): + folded.append("-Wl," + tokens[i + 1]); i += 2 + else: + folded.append(tokens[i]); i += 1 + out: List[str] = [] + i = 0 + while i < len(folded): + if (folded[i] == "-Wl,-rpath" and i + 1 < len(folded) + and folded[i + 1].startswith("-Wl,")): + out.append("-Wl,-rpath," + folded[i + 1][4:]); i += 2 + else: + out.append(folded[i]); i += 1 + return out + + def canonicalize_link_flags(raw: List[str], *, is_bazel: bool) -> Tuple[str, ...]: """Extract correctness-relevant LINK flags from a link command. @@ -332,16 +371,21 @@ def canonicalize_link_flags(raw: List[str], *, is_bazel: bool) -> Tuple[str, ... """ out: List[str] = [] i = 0 - n = len(raw) - if is_bazel and n and _is_link_input(raw[0]): + if is_bazel and raw and _is_link_input(raw[0]): i = 1 # leading linker/wrapper path + # `-Xlinker X` and `-Wl,X` pass the same argument to the linker; fold the + # two-token form first, so it compares as one flag on either side and goes + # through the same noise filters (Bazel's rpaths, below). + raw = list(raw[:i]) + fold_xlinker(list(raw[i:])) + n = len(raw) while i < n: tok = raw[i] if tok in _LINK_PAIR_FLAGS and i + 1 < n: i += 2; continue if _is_link_input(tok): i += 1; continue - if is_bazel and _matches_any(tok, BAZEL_DEFAULT_LINK_PREFIXES): + if is_bazel and (_is_bazel_solib_rpath(tok) + or _matches_any(tok, BAZEL_DEFAULT_LINK_PREFIXES)): i += 1; continue if _matches_any(tok, IGNORABLE_LINK_PREFIXES): i += 1; continue diff --git a/scripts/extract_cmake.py b/scripts/extract_cmake.py index 3ee6fe7..09c7d41 100644 --- a/scripts/extract_cmake.py +++ b/scripts/extract_cmake.py @@ -40,6 +40,7 @@ import sys from typing import Dict, List, Optional +from canonicalize import fold_xlinker from extract_configure import parse_configure_trace from model import (Action, BuildSystem, CanonicalModel, Dependency, Target, TargetKind, TargetRole) @@ -176,8 +177,9 @@ def _attach_deps(target: Target, tobj: dict, id_to_name: Dict[str, str]) -> None dep_name = id_to_name.get(dep["id"]) if dep_name: target.deps.append(Dependency(dep_name, external=False)) - # link fragments: role "libraries" -> external deps (annotation); role - # "flags" -> a CppLink Action's argv (the differ canonicalizes them). + # link fragments: role "libraries" -> external deps (annotation), plus any + # link-interface FLAGS among them; role "flags" -> a CppLink Action's argv + # (the differ canonicalizes them). link = tobj.get("link") or {} flag_tokens: List[str] = [] for frag in link.get("commandFragments", []): @@ -187,6 +189,8 @@ def _attach_deps(target: Target, tobj: dict, id_to_name: Dict[str, str]) -> None ext = _library_identity(fragment) if ext and not any(d.name == ext for d in target.deps): target.deps.append(Dependency(ext, external=True)) + elif ext is None: + flag_tokens.extend(_link_interface_flags(fragment)) elif role == "flags": flag_tokens.extend(_split_fragment(fragment)) if flag_tokens: @@ -194,6 +198,38 @@ def _attach_deps(target: Target, tobj: dict, id_to_name: Dict[str, str]) -> None arguments=tuple(flag_tokens))) +# Flags in a "libraries" fragment that position or locate the libraries around +# them rather than changing how the program links: they belong to the link +# closure (compared as deps), and rpaths name the CMake build tree. Matched +# after `-Xlinker X` pairs are folded to `-Wl,X` (see _link_interface_flags). +_LIBRARY_POSITIONING_PREFIXES = ( + "-L", "-framework", "-Wl,-rpath", "-Wl,--rpath", "-Wl,-R", + "-Wl,--whole-archive", + "-Wl,--no-whole-archive", "-Wl,-whole-archive", "-Wl,-no-whole-archive", + "-Wl,--push-state", "-Wl,--pop-state", "-Wl,--start-group", + "-Wl,--end-group", "-Wl,-(", "-Wl,-)", "-Wl,-Bstatic", "-Wl,-Bdynamic", + "-Wl,--as-needed", "-Wl,--no-as-needed", "-Wl,-force_load", +) + + +def _link_interface_flags(fragment: str) -> List[str]: + """The link FLAGS a non-library "libraries" fragment carries, or []. + + Such a fragment is a flag a dependency carries on its link interface (e.g. + Python's `-Xlinker -export-dynamic`, OpenMP's `-fopenmp`, Threads' + `-pthread`) rather than a library. These change how the program links, so + they are compared with the target's link flags; before, they were dropped + and a missing one could not show up in the diff. `-Xlinker` pairs are + folded first, so `-Xlinker -rpath -Xlinker ` is recognised as the + rpath it is; positioning flags and -l references yield [].""" + tokens = fold_xlinker(_split_fragment(fragment)) + if (not tokens or not tokens[0].startswith("-") + or tokens[0].startswith("-l") + or tokens[0].startswith(_LIBRARY_POSITIONING_PREFIXES)): + return [] + return tokens + + def _library_identity(fragment: str) -> Optional[str]: """Turn a link fragment into an abstract dep name. diff --git a/tests/test_extractors.py b/tests/test_extractors.py index b5ce4e9..1de9275 100644 --- a/tests/test_extractors.py +++ b/tests/test_extractors.py @@ -301,6 +301,106 @@ def test_bazel_extracts_link_flags_from_cpplink(): assert foo is not None and not foo.external, t.deps +def test_cmake_link_interface_flags_become_link_flags(): + # Flags a dependency carries on its LINK INTERFACE arrive in the File API's + # "libraries" fragments, not "flags". They must reach the link-flag + # comparison; library positioning/rpath flags must not. + import extract_cmake as ec + from model import Target, TargetKind + tobj = {"link": {"commandFragments": [ + {"fragment": "-rdynamic", "role": "flags"}, + {"fragment": "-Xlinker -export-dynamic", "role": "libraries"}, + {"fragment": "-fopenmp", "role": "libraries"}, + {"fragment": "-Wl,-rpath,/work/proj/build/lib", "role": "libraries"}, + {"fragment": "-Wl,--rpath,/work/proj/build/lib", "role": "libraries"}, + {"fragment": "-Xlinker -rpath -Xlinker /work/proj/build/lib", + "role": "libraries"}, + {"fragment": "-lz", "role": "libraries"}, + {"fragment": "/usr/lib/libpython3.12.so", "role": "libraries"}, + {"fragment": "-Wl,--whole-archive", "role": "libraries"}, + {"fragment": "lib/libplugins.a", "role": "libraries"}, + {"fragment": "-Wl,--no-whole-archive", "role": "libraries"}, + {"fragment": "-L/opt/lib", "role": "libraryPath"}, + # search paths and frameworks under the "libraries" role are not flags + {"fragment": "-L/opt/other/lib", "role": "libraries"}, + {"fragment": "-framework Cocoa", "role": "libraries"}, + ]}} + t = Target("blender", TargetKind.EXECUTABLE) + ec._attach_deps(t, tobj, {}) + from model import BuildSystem + from reconstruct import reconstruct_target + v = reconstruct_target(t, BuildSystem.CMAKE, REPO) + assert v.link_flags == ("-Wl,-export-dynamic", "-fopenmp", "-rdynamic"), v.link_flags + assert sorted(d.name for d in v.deps) == ["Cocoa", "plugins", "python3", "z"], \ + v.deps + + +def test_bazel_solib_rpaths_are_toolchain_noise(): + # A realistic Bazel CppLink argv: its -Xlinker rpath pairs fold to -Wl, + # tokens, which must not surface as Bazel-only link flags. + from canonicalize import canonicalize_link_flags + argv = ["external/toolchain/cc_wrapper.sh", "-o", "bazel-out/k8-opt/bin/app", + "-Xlinker", "-rpath", "-Xlinker", "$ORIGIN/_solib_k8/_U_S_Sfoo", + "-Xlinker", "-rpath", "-Xlinker", "$EXEC_ORIGIN/_solib_k8/_U_S_Sfoo", + "-Xlinker", "-rpath", "-Xlinker", + "@loader_path/../_solib_darwin_arm64/_U_S_Sfoo", + "-Lbazel-out/k8-opt/bin/_solib_k8/_U_S_Sfoo", "-lfoo", + "bazel-out/k8-opt/bin/_objs/app/main.o", "-Wl,-S", "-pthread", + "-Xlinker", "-export-dynamic"] + assert canonicalize_link_flags(argv, is_bazel=True) == \ + ("-Wl,-export-dynamic", "-pthread") + + +def test_project_rpaths_in_linkopts_are_kept_and_compared(): + # A project's own rpath flags copied into linkopts must survive on the + # Bazel side (only the toolchain's _solib_ rpaths are noise), in any + # spelling, or they become false cmake-only errors. + from canonicalize import canonicalize_link_flags + from config import MigrationConfig + from diff import _diff_link_flags + from reconstruct import TargetView + from model import TargetKind, TargetRole + preamble = ["cc_wrapper.sh", "-o", "bazel-out/k8-opt/bin/app", + "-Xlinker", "-rpath", "-Xlinker", "$ORIGIN/_solib_k8/_U_S_Sfoo"] + + def converges(cmake_flags, bazel_flags): + cmake = canonicalize_link_flags(cmake_flags, is_bazel=False) + bazel = canonicalize_link_flags(preamble + bazel_flags, is_bazel=True) + views = [TargetView("app", TargetKind.EXECUTABLE, TargetRole.PRODUCTION, + link_flags=lf) for lf in (cmake, bazel)] + return _diff_link_flags("app", views[0], views[1], MigrationConfig()) == [] + + project = ["-Wl,-rpath,$ORIGIN/../lib", "-Wl,-rpath=/opt/lib", + "-Wl,-rpath-link,/opt/lib"] + assert canonicalize_link_flags(preamble + project, is_bazel=True) == \ + tuple(sorted(project)) + assert converges(project, project) + # two linker arguments (CMake's LINKER:-rpath,DIR under Clang is exactly + # -Xlinker -rpath -Xlinker DIR), and the -Wl, pair spelling + for two in (["-Xlinker", "-rpath", "-Xlinker", "/opt/lib"], + ["-Wl,-rpath", "-Wl,/opt/lib"], + ["-Xlinker", "-rpath", "-Xlinker", "@executable_path/../Frameworks"]): + assert converges(two, two), two + # across spellings: one rpath, however it is written + assert converges(["-Xlinker", "-rpath", "-Xlinker", "/x"], ["-Wl,-rpath,/x"]) + assert converges(["-Wl,-rpath,/x"], ["-Wl,-rpath", "-Wl,/x"]) + # and a genuinely missing project rpath is still caught + assert not converges(["-Xlinker", "-rpath", "-Xlinker", "/opt/lib"], []) + # only Bazel's own $ORIGIN/.../_solib_/ rpaths are noise, not a + # project path that merely contains _solib_ + assert converges(["-Wl,-rpath,/opt/_solib_/lib"], ["-Wl,-rpath,/opt/_solib_/lib"]) + assert not converges(["-Wl,-rpath,/opt/_solib_/lib"], []) + + +def test_xlinker_pairs_fold_to_wl_on_both_sides(): + from canonicalize import canonicalize_link_flags + assert canonicalize_link_flags(["-Xlinker", "-export-dynamic"], is_bazel=False) \ + == ("-Wl,-export-dynamic",) + assert canonicalize_link_flags(["cc_wrapper.sh", "-o", "out", "-Xlinker", + "-export-dynamic"], is_bazel=True) \ + == ("-Wl,-export-dynamic",) + + def test_bazel_external_archive_is_external_dep(): # An archive linked by PATH from external/ (a bzlmod module, e.g. catch2) # must be captured as an EXTERNAL dep -- previously dropped (only -l deps