kbuild: derive generated-prerequisite dispatch from parsed rules - #86
Draft
pawelchcki wants to merge 9 commits into
Draft
kbuild: derive generated-prerequisite dispatch from parsed rules#86pawelchcki wants to merge 9 commits into
pawelchcki wants to merge 9 commits into
Conversation
Prepares the parser to answer "which rule generates this object's source",
without any consumer yet. Three deliberate behaviour changes:
- makePatternMatch gained GNU make's length guard. A word must be long
enough to supply both the pattern's prefix and its suffix, so "a%a" no
longer matches "a". This changes $(filter)/$(filter-out) results for
short words; no result in the existing corpus moves. makePatsubst is
refactored onto the same makePatternStem helper, which also returns the
stem the "%" stood for.
- Static pattern rules ("targets: target-pattern: prerequisites") now
parse into KbuildRule.TargetPattern instead of being split on the first
top-level colon, which produced a prerequisite with a colon glued to
it. arch/powerpc/crypto/Makefile and arch/arm64/kernel/vdso32/Makefile
carry this shape in 6.12. KbuildFile.Rules and its JSON change shape.
- cmd_* macros are captured as KbuildFile.Commands. All three perlasm
flavours are literally "$(call if_changed,perlasm)"; what distinguishes
them is the value of cmd_perlasm in that directory, so recipe text
alone cannot tell them apart. Capture happens at assignment rather than
by walking the variable table at end of parse: kbuildVariable keeps no
position, and a map walk would need explicit sorting to stay
deterministic. Both the expanded value and the raw right-hand side are
kept, so a later caller can tell a preserved reference from an erased
one.
Rules also gain Directory, and KbuildFile gains an unexported rootDir
stamped at parse time and copied explicitly through merge, since merge
does not carry unexported fields. Rule prerequisites arrive
"$(src)"-expanded and therefore absolute; interpreting them needs that
root, and only the parser knows it.
The rule target slice is now cloned before prefixing. Parsed files are
cached per path, so prefixing in place mutated the cached copy.
Adds KbuildGeneratedSourceResolver, which answers "which Kbuild rule produces the source for this leaf object" from the rules the parser already holds. Nothing consumes it yet. The API is object-driven rather than enumerative: a pattern rule describes an unbounded target set, so there is nothing to enumerate over. The caller supplies the leaf object, the resolver derives the finite candidate list (dir/base.S, .c, .s) and matches rules against that. Selection follows GNU make where it matters: - Rules with no recipe are skipped. They declare a dependency rather than producing anything; without this drivers/tty/vt's recipe-less "$(obj)/defkeymap.o: $(obj)/defkeymap.c" hijacks defkeymap.o away from its checked-in _shipped source. - An explicit rule beats a pattern rule, last definition winning. This is what makes 6.12's arm64 override resolve sha256-core.S to sha512-armv8.pl; the hardcoded object table says sha2-armv8.pl, which is wrong for that kernel. - Otherwise the shortest-stem pattern rule whose prerequisites all exist, then the last defined. Requiring the prerequisites to exist is what stops a directory-wide "%.S: %.pl" rule from claiming every object in the directory that happens to lack a source. - A genuine tie between distinct rule positions fails, listing both. Paths are normalised back to source-relative form, including inside the command body: script paths there come from "$(src)/..." and are absolute, so leaving them would make the command text depend on where the tree was unpacked. Bare words that are not the known "FORCE" phony fail loudly rather than being dropped, since .PHONY is not tracked and a silently dropped prerequisite is a missing digest input. Expanding a cmd_* macro can erase a reference rather than preserve it: a recognised conditional sibling makes an unknown "$(foo-y)" expand to "" with ok=true, which would quietly turn a three-argument invocation into a two-argument one. Comparing the raw right-hand side against the expanded value catches that and fails. Tests replicate the exact 6.12.96 shapes, verified against the tree: x86 crypto's pattern rule, arm64's pattern plus explicit override, all of raid6's int%.c plus the hostprog-driven tables.c, capflags with its "$(src)/../.." indirection, vt's consolemap and its recipe-less defkeymap rule, powerpc's static pattern rule, a rule inside an inactive conditional, and both failure modes.
Adds the generator registry: given a rule resolved by GeneratedSourceForObject, decide which executor can actually run it and what its inputs and arguments are. Still unused by compact metadata. Discovery is generic, execution cannot be, and this file is where that is admitted. No action in this project has an awk, shell or coreutils toolchain; the only interpreter is Perl, and every other generator is a Go port under internal/cmd/. So a newly discovered rule can be described precisely and still have no way to run. The honest outcome is a failure naming the Makefile line, the command text and the two files to edit. The registry keys on the canonicalised command text rather than the command's name or the object's path. That key is derived from the Makefile instead of transcribed from it, and it is shared across versions and architectures: 6.12 spells the same generator cmd_perlasm on x86 and cmd_perl on arm, one writing "$<" and the other "$(<)", and both canonicalise to "PERL $< > $@". Deriving arm64's "void" argument from the rule's own text is the core of the fix; a future flavour falls out of the data rather than needing a new table row. Classification uses the command template with "$*" intact rather than the stem-substituted text, so the key does not vary per object. The leading program word is tokenised, not string-compared: PERL, AWK and CONFIG_SHELL are absent from the default variable environment only because just arch/<srcarch>/Makefile is parsed, and a consumer can define them through -var or module_make_vars, in which case a resolved interpreter path arrives instead. Both forms must classify the same. Canonicalisation fails closed on any surviving non-automatic "$(...)". Per-kind Primary, ActionInputs, DigestOnlyInputs and Args are declared as functions, never inferred. mkcapflags is why: its $< is cpufeatures.h, but the source that stands for the object is the script. Not implemented, with a comment saying so: "PERL $< <flavour> > $@", used by powerpc and mips. Those scripts spawn an "*-xlate.pl" child the sandbox cannot declare, and neither architecture is supported. Verified against unpacked 6.12.96 and 6.18.39 trees for x86, arm64 and arm. Every object the old hardcoded table covers resolves to the same primary source, except 6.12 arm64 sha256-core.o, where the table says sha2-armv8.pl and the Makefile's explicit rule says sha512-armv8.pl. The same code gets 6.18's relocated lib/crypto layout right, including sha256-core.o resolving to sha2-armv8.pl there. poly1305 and all of raid6, the two reported failures, now resolve on both trees. 6.18's mips and riscv poly1305 rules report as unimplemented with the expected message.
Adds the consumer half of generated-prerequisite dispatch. Nothing emits the new attributes yet, so this commit is a no-op at runtime. It lands before the generator change on purpose. The graph generator is a pinned prebuilt with no local-override hook, so a BUILD file produced by a locally built generator and analysed against a tree without these attributes would fail to load. Landing the permissive side first keeps every intermediate commit buildable. For the same reason _linux_perlasm_kind is renamed _linux_legacy_perlasm_kind and retained rather than deleted; it goes once the pin is bumped. linux_object gains generated_source, generator, generator_args and generator_inputs. The generator attribute uses attr.string(values=...) so an unrecognised kind is a loading-phase error rather than a confusing failure deep inside the rule. _linux_generated_source replaces the three separate object-keyed paths with one dispatcher, one branch per kind, all path_mapped_run. The output filename comes from generated_source rather than from the object name with an assumed ".S": that hardcoded extension is exactly what breaks on raid6's int1.c. The legacy perlasm, capflags and consolemap branches stay and are skipped when a generator is present. Two new executors, both verified byte-for-byte against the real tools on this machine and unit-tested so CI can check them without an awk or C hostprog toolchain: - internal/cmd/unroll ports lib/raid6/unroll.awk. Compared against GNU awk 5.3.2 over both catalogued kernels' five ".uc" templates at unroll factors 0, 1, 2, 4, 8, 16 and 32: identical in all 70 combinations. Three details are load-bearing and commented as such: the substitution order, dropping every "$$" line when n is 0, and treating a trailing newline as terminating the last record rather than starting an empty one. The last one would otherwise insert a blank line inside a function body in int.uc. - internal/cmd/raid6tables ports lib/raid6/mktables.c, byte-identical to the compiled program's output at 471,449 bytes. RAID6 does not link without tables.o, so this is required rather than follow-up work. The Clang LTO module-link exclusion is fixed rather than mechanically translated. It previously keyed on "is this a perlasm object"; it now tests whether the source actually compiled is assembly. Perlasm still takes the non-LTO path because it produces ".S", while raid6's unroll produces ".c" and correctly keeps the LTO path. The metadata validator, the standalone checker and the tests/compact fake rule all learn the four keys. The checker and the Starlark action-recipe key hash them only when a generator is present, so variants without one keep their existing identity. That is required for correctness and not just economy: arm64's sha256-core.o and sha512-core.o share source, flags and arguments and differ only in the target they generate, so without the generator fields they would collapse into one action.
Wires the rule resolver into compact metadata, so an object whose source
is produced by a per-directory Kbuild rule resolves instead of aborting
with "exact input scan cannot resolve a source for leaf object".
Resolution order is unchanged and deliberately so: the on-disk probe runs
first and rule lookup only on its miss path. Consulting rules first would
re-resolve objects that already resolve on disk and churn their
identities for no gain. The plan's stale-generated-file assertion is
dropped rather than half-implemented: it cannot fire on a miss-only path,
and a dirty source tree is not something the hermetic repository rule can
produce.
variant.Source keeps naming a checked-in file. Three invariants require
it: the repository rule checks the source exists in-tree and belongs to
the exact-input group, sourceBuildError rejects an empty source, and
linux_object.source_input_file resolves it. The generated target travels
in GeneratedSource instead, and the compiled language is read from it
wherever that matters, threaded through all of the recipe identity, the
group fallback, grouped BUILD emission, symversion selection and Kbuild
flag selection. For every generator that existed before, the two agree
(".pl" and ".S" are both asm, ".sh"/".uni" and ".c" are both C), so this
changes nothing for them; it is what makes raid6's ".uc" template compile
as C rather than falling through to no language at all.
The four include scans still read the nominal source rather than the
generated target, because they describe how the file being lexed is
parsed, and no checked-in generator input is assembly. lib/raid6/int.uc
is the interesting case and it is already correct: it carries exactly the
include lines the generated int%.c needs. The exception is a host C
program, whose includes are the host's: scanning lib/raid6/mktables.c
fails on <stdio.h>. That kind opts out and declares the generated file's
closure explicitly instead, which was found by running this against the
real tree rather than by reading it.
Non-action prerequisites become digest-only footprint entries through
footprint.sourceInputs, the channel that records a digest without
scanning. That closes a latent hole for consolemap_deftbl.o, which
nothing currently invalidates when drivers/tty/vt/conmakehash.c changes.
All nine schema mirrors are widened, and the generator fields are hashed
only when present in all three places that reconstruct an identity, so
objects without a generator keep byte-identical content IDs.
ObjectDir != "" declines to resolve. Rule targets live in the "$(obj)"
namespace, which coincides with the object namespace only when the Kbuild
traversal starts at the source root; every real invocation does exactly
that, so rather than guess the offset for a configuration that does not
occur, the resolver stays out of it.
capflags and consolemap_deftbl deliberately do not migrate yet:
sourceCandidatesForObject still maps them onto files that exist, so the
on-disk probe never misses. They move when those rows are deleted, which
needs the pinned generator release bumped first. The resolver-level tests
already prove the rule-derived path gives the same answers, so that will
be a deletion rather than a behaviour change.
Verified against unpacked 6.12.96 and 6.18.39 trees. Before the change,
resolving lib/raid6, arch/x86/crypto and arch/arm64/crypto each aborts
with the reported error; after, all three resolve, with arm64
sha256-core.o correctly taking sha512-armv8.pl from the Makefile's
explicit override. Resolving arch/x86/kernel/cpu and drivers/tty/vt on
both kernels produces byte-identical output including content IDs, so
nothing unmigrated drifts.
Adds two analysis cases exercising the new attributes through the real rule rather than through the graph generator, which is a pinned prebuilt and cannot emit them yet. - perlasm_generated is the same generator as the existing perlasm case but reached through generator/generated_source instead of the legacy object table, at the arch/x86/crypto path 6.12 uses rather than the post-6.16 lib/crypto one. - raid6_unroll generates ".c" rather than ".S", which is what catches an output name derived from the object name with an assumed extension. The fixture is named unroll.uc because the source input index is keyed by sorted path: a name sorting before shipped.c_shipped would renumber every existing source_input_file in the suite. Each case now carries its content_id explicitly instead of a nested ternary over the case name, which does not extend past three cases.
Author
E2E follow-up, to land with the
|
Quality-only pass over the rule-derived generated-source work. No behaviour changes: object content IDs, generated output bytes and every Starlark analysis result are unchanged, which is why the analysis tests still return cached results. Reuse: - formatRulePosition was a verbatim reimplementation of Position.String, including the empty-filename fallback. Deleted, along with the throwaway KbuildRule built solely to call it and the third hand-rolled "%s:%d" in the kind-error path. - The generator hash fields were written out twice, once per hasher. They are only comparable while both write them identically, so the sequence moves to one method on compactGeneratorIdentity. - splitKbuildStaticPattern and splitKbuildInlineRecipe each re-walked paren depth to find a top-level delimiter; both now call indexTopLevelByte. - KbuildFile.GeneratedSourceForObject had no caller, not even in tests. Simplification: - Four kind specs repeated the same primary/actionInputs closures. Nil now means "the primary prerequisite". raid6_mktables genuinely wants no action inputs, so it overrides explicitly rather than being caught by an inferred default. - _linux_generated_source carried five copies of identical path_mapped_run wiring. Branches now choose executable, inputs, mnemonic and message; the action is written once. - raid6tables had four copies of the same eight-per-line table loop, now one writeByteTable taking the per-entry callback. Output stays byte-identical, which the pinned byte and line counts check. - The four generator parameters on objectVariantContentID become one struct. All four are strings or string slices, so a mis-ordering among twenty-one positional arguments is silent and surfaces only as a wrong content ID. - Dropped the unused Command field: its stated purpose is served by CommandTemplate, and the tests that asserted it also assert Stem and CommandTemplate, which carry the same information. Efficiency: - _linux_legacy_perlasm_kind rebuilt two list literals and linear-scanned them on every object's analysis; it is now a module-level dict lookup. - The generated-source resolver walked every rule and command once per configuration. It is cached on the parsed file, with a stat cache for prerequisite probes that were otherwise repeated per candidate extension and per sibling object sharing a pattern rule. - Merged the two back-to-back appendUniqueStrings on closureInputs and batched the per-path appendUniqueSourceInputs loop; each call rebuilds and re-sorts the whole slice. - Hoisted a per-call strings.NewReplacer and a loop-invariant strconv.Itoa out of their hot paths. Deliberately left alone: the legacy perlasm, capflags and consolemap tables that coexist with the rule-derived path. They are transitional because the graph generator is a pinned prebuilt, and deleting them before the pin bumps leaves the tree unbuildable.
ForObject tried ".S" before ".c" while sourceForObject, which answers the same question from disk, tries ".c" first. Its own comment claimed the two orders mirrored each other; they did not. scripts/Makefile.build defines $(obj)/%.o: $(obj)/%.c before $(obj)/%.o: $(obj)/%.S and make tries pattern rules in definition order, so a stem that could be produced either way builds from the C source. The on-disk probe already followed that; the rule resolver did not, so an object with rules for both extensions would resolve differently depending on which path answered it. No object in the supported trees has rules for both, so nothing observable changes today. The candidate list is now a single shared variable rather than a literal repeated per call site, which is what the comment was asserting by hand. The new test pins the ordering directly: its Makefile writes the ".S" rule first, so it fails on a first-rule-seen scan and on the previous extension order, and passes only when the candidate order decides.
The resolved rule and its executor were threaded as a pair that is always both-nil or both-non-nil. Fold the rule's target into the executor so one value crosses variantForStack and resolvedKbuildObject.variant, and route the "generated target else source" choice through compactCompiledSourcePath instead of open-coding it for the language, the flags and the symversion decision separately. Also drop what nothing reads: KbuildGeneratedSource.Explicit and .OrderOnly, ruleMatch.explicit, the unreachable lazy init of the resolver's stat cache, and three action_inputs overrides in _linux_generated_source that re-encode contracts generated_source_kinds.go already fixes. Generator inputs now go through _source_tree_file like every other producer, which fails loudly instead of pushing None into inputs[0]. Adds compactGeneratorIdentityFor and CompactObjectVariant.generatorIdentity so the four-field literal has one spelling, and leaves a note at each of the legacy perlasm/capflags/conmakehash sites that they change together and are deleted when the generator pin is bumped.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I was trying to build a patched kernel for a libkrun VM and kept hitting configs where a leaf object has no source in the tree at all — Kbuild generates it first, from a
.plscript, a.uctemplate, or a hostprog it compiles on the fly. Graph generation just stops:exact input scan cannot resolve a source for leaf object "lib/raid6/int1.o".The machinery to run those generators already exists here. What was missing is dispatch: objects are matched to generators by two hardcoded object-path tables, and every entry in them sits under the post-6.16
lib/crypto/…layout, so nothing matches 6.12 (the arm64sha256-core.orow is also wrong there — 6.12's Makefile has an explicit rule overriding the pattern rule). This derives dispatch from the Kbuild rules the parser already holds instead.If the overall shape is wrong, the tests should still be salvageable more or less as-is — they're self-contained and pin real 6.12/6.18 Makefile behaviour:
kbuild_generated_source_test.go— rule resolution against verbatim 6.12 shapes: raid6, x86/arm64 perlasm incl. the explicit override, capflags, vt, powerpc static-patterngenerated_source_kinds_test.go— each generator, plus the exact error text when a rule shape has no implementationcompact_generated_source_test.go— end-to-end, deliberately not using the test helper that fabricates missing sources (it hides this bug)unroll/raid6tables— Go ports ofunroll.awkandmktables.c, checked byte-for-byte against GNU awk and the compiled hostprogFor example: unmodified 6.12.96, x86_64,
CONFIG_RAID6_PQ=y+CONFIG_CRYPTO_POLY1305_X86_64=y. Six objects can't resolve today; with this they build, the generated files come out byte-identical to what awk/perl/mktables produce, and the kernel boots under QEMU.Sorry it's 3.6k lines (1.6k of that tests) — larger than I'd like for the number of objects it actually reaches. Happy to cut it down. Closes #85.