From 0fe57ae804bee2d9f7e4ddf2ccf227a4e6659f7b Mon Sep 17 00:00:00 2001 From: Mateusz Olczyk Date: Tue, 20 Oct 2026 18:23:01 +0200 Subject: [PATCH] Do not treat -c as consuming the next compile argument `-c` takes no argument in gcc/clang; the extractors merely place the source right after it. Two places assumed otherwise: * reconstruct._source_from_compile_args took the first `-c ` as the translation unit's source. * canonicalize listed `-c` in _DRIVER_PAIR_FLAGS and dropped it together with the following token. Real projects can carry a redundant `-c` inside their flags (e.g. `CMAKE_C_FLAGS="... -c -std=c11 ..."`); gcc ignores the repetition. With such flags every TU of a CMake target was keyed by `-std=c11`, so all of them collapsed into a single bogus TU, and `-std=c11` itself was hidden from the flag comparison on both sides. The diff then compared a single TU per target instead of every source, so missing sources and per-file flag gaps went unreported. Now `-c` is dropped on its own, and the next token is only treated as the source (and dropped) when it is not a flag. Source detection skips a `-c` followed by a flag; the existing extension scan remains the fallback. Tests cover canonicalization of a stray `-c`, distinct TUs for two sources under such flags, convergence when both sides set the flag after the stray `-c`, and an error when Bazel is missing it. All three fail without the fix. docs/REFERENCE-cmake.md documents the behavior. --- docs/REFERENCE-cmake.md | 4 ++++ scripts/canonicalize.py | 9 +++++++-- scripts/reconstruct.py | 7 ++++--- tests/test_engine.py | 41 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/docs/REFERENCE-cmake.md b/docs/REFERENCE-cmake.md index 49ed665..c1a277d 100644 --- a/docs/REFERENCE-cmake.md +++ b/docs/REFERENCE-cmake.md @@ -54,6 +54,10 @@ aquery must mirror. - **Link flags**: CMake repeats compile/codegen flags (`-fvisibility=hidden`, `-fno-common`) on the link line, where they're benign, while Bazel doesn't. This is the common `ignore.link_flags` case. +- **Redundant `-c` in project flags**: some projects put `-c` into + `CMAKE__FLAGS` (e.g. `"-c -std=c11"`), which compilers tolerate. `-c` + takes no argument, so it is dropped on its own; the flag after it is still + compared and the TU is still keyed by its real source. ## Tests diff --git a/scripts/canonicalize.py b/scripts/canonicalize.py index f2b8c66..09c8095 100644 --- a/scripts/canonicalize.py +++ b/scripts/canonicalize.py @@ -91,7 +91,7 @@ # These are universal facts (a -o names an output; -c means compile-only), never # migration decisions, so they're stripped at canonicalization, not configured. # Flags that consume the FOLLOWING token (drop both): -_DRIVER_PAIR_FLAGS = {"-o", "-c", "-isysroot", "--sysroot", "-arch", "-target", +_DRIVER_PAIR_FLAGS = {"-o", "-isysroot", "--sysroot", "-arch", "-target", "-gcc-toolchain", "-x", "--serialize-diagnostics"} # Defines the Bazel toolchain injects for reproducible builds (arrive as @@ -218,9 +218,14 @@ def canonicalize_flags( tok = raw[i] # driver flags that consume the next token (-o out.o, -isysroot /sdk, - # -arch arm64, -c src). Pure invocation mechanics -- drop flag + arg. + # -arch arm64). Pure invocation mechanics -- drop flag + arg. if tok in _DRIVER_PAIR_FLAGS and i + 1 < n: i += 2; continue + # -c takes no argument, but the extractors put the source right after + # it: drop that source too. + if tok == "-c": + i += 2 if i + 1 < n and not raw[i + 1].startswith("-") else 1 + continue # bare positional driver tokens (compiler path mid-argv, .o output) if _is_driver_token(tok): i += 1; continue diff --git a/scripts/reconstruct.py b/scripts/reconstruct.py index c186c10..41893b1 100644 --- a/scripts/reconstruct.py +++ b/scripts/reconstruct.py @@ -93,11 +93,12 @@ def _is_header_processing(args) -> bool: def _source_from_compile_args(args) -> Optional[str]: - """The compiled source is the arg to -c (or the lone source arg). Returns - None for header inputs -- not translation units.""" + """The compiled source is the token after -c (or the lone source arg). + Returns None for header inputs -- not translation units. A -c followed by a + flag is a stray, redundant -c from the project's flags, not the source.""" src = None for i, a in enumerate(args): - if a == "-c" and i + 1 < len(args): + if a == "-c" and i + 1 < len(args) and not args[i + 1].startswith("-"): src = args[i + 1] break if src is None: diff --git a/tests/test_engine.py b/tests/test_engine.py index f50ef31..52f3a29 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -28,10 +28,12 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "scripts")) +from canonicalize import canonicalize_flags from config import MigrationConfig from diff import Severity, diff_models, summarize from model import (Action, BuildSystem, CanonicalModel, Dependency, Target, TargetKind, TargetRole) +from reconstruct import reconstruct REPO = "/work/proj" @@ -143,6 +145,45 @@ def test_missing_cmake_flag_is_caught(): assert "NEEDED=1" in dd.cmake_only +# A redundant -c inside the project's own flags (CMAKE_C_FLAGS="-c -std=c11"; +# gcc ignores the repetition). -c takes no argument, so it must neither be read +# as naming the source nor swallow the flag that follows it. +_STRAY_C_RAW = ["-mcpu=cortex-m4", "-c", "-std=c11", "-Wall"] + + +def test_stray_c_does_not_swallow_next_flag(): + _, _, flags = canonicalize_flags(_STRAY_C_RAW + ["-c", "src/a.c"], REPO, + is_bazel=False) + assert flags == ("-Wall", "-mcpu=cortex-m4", "-std=c11"), flags + + +def _stray_c_model(bazel_raw=None): + """Two-TU target; the CMake side carries the stray -c, the Bazel side + `bazel_raw` (None = the CMake model itself).""" + raw = _STRAY_C_RAW if bazel_raw is None else bazel_raw + m = _bs(CanonicalModel(), is_bazel=bazel_raw is not None) + m.add(Target("lib", TargetKind.STATIC, role=TargetRole.PRODUCTION, + actions=[tu_from_raw(s, raw, is_bazel=bazel_raw is not None) + for s in ("src/a.c", "src/b.c")])) + return m + + +def test_stray_c_keeps_tus_keyed_by_source(): + tus = reconstruct(_stray_c_model())["lib"].tu_map() + assert sorted(tus) == ["src/a.c", "src/b.c"], sorted(tus) + + +def test_stray_c_flag_is_compared(): + a = _stray_c_model() + ok = _stray_c_model(["-mcpu=cortex-m4", "-std=c11", "-Wall"]) + assert summarize(diff_models(a, ok))["errors"] == 0 + missing = _stray_c_model(["-mcpu=cortex-m4", "-Wall"]) # no -std=c11 + discs = diff_models(a, missing) + fd = [d for d in discs if d.kind == "flags_diff"] + assert fd and all("-std=c11" in d.cmake_only for d in fd), discs + assert not summarize(discs)["converged"] + + def test_extra_bazel_define_is_warn_not_error(): cmake_raw = ["-DFOO=1"] bazel_raw = ["-DFOO=1", "-DEXTRA=1"]