[CodeGen] Ask the target whether it can clear registers or the stack - #7
[CodeGen] Ask the target whether it can clear registers or the stack#7claude[bot] wants to merge 3 commits into
Conversation
|
|
cf198e8 to
2812359
Compare
413a04b to
8ab6b64
Compare
2812359 to
4fa2552
Compare
8ab6b64 to
a8c76e7
Compare
|
A full The cause is the new "not supported by this target" diagnostic added here at Build: Generated by Claude Code |
4fa2552 to
384fc0f
Compare
a8c76e7 to
171f6f9
Compare
384fc0f to
7f0f07f
Compare
171f6f9 to
b23c975
Compare
7f0f07f to
4f1842c
Compare
b23c975 to
58f4c4a
Compare
4f1842c to
5a12178
Compare
58f4c4a to
e6aaa20
Compare
5a12178 to
027b84f
Compare
e6aaa20 to
23d1fcc
Compare
027b84f to
8bd995b
Compare
7a49e65 to
ea0fad5
Compare
8bd995b to
6028d5b
Compare
e0763e3 to
b37645a
Compare
ea0fad5 to
9cee239
Compare
9cee239 to
8fdf75d
Compare
A request to clear call-used registers is dispatched through emitZeroCallUsedRegs, whose default body is empty. A target that has not implemented it therefore compiles the request into nothing at all, and nothing says so: the caller asked for the registers to be destroyed, the object file leaves them holding what was in them, and the only way to find out is to read the disassembly. ARM32 is the live case. The driver refuses the command-line flag there through a hard-coded list of triples, but the function attribute that flag turns into is accepted, carried through the whole pipeline, and dropped in the frame lowering, so anything that sets the attribute directly, including LTO and inlining of code compiled elsewhere, silently gets nothing. Add a capability query the target answers instead of inferring support from an emission that may do nothing. supportsZeroCallUsedRegs defaults to false, so a target is unsupported until it says otherwise, and prologue-epilogue insertion asks before computing what to clear: a target that answers false gets a diagnostic and no code, rather than no diagnostic and no code. The three targets that implement the emission today, X86, AArch64 and RISCV, answer true, which is what the driver's triple list already assumed, so no target changes what it generates. supportsZeroizeStack is the same query for the "zeroize-stack" attribute, and today every target answers false, because none of them clears the frame yet. That attribute has had no backend consumer at all since it was added, which is the same silence in a worse form, so it now reports itself as unsupported everywhere until an implementation exists. DiagnosticInfoUnsupported is what the backend already uses to refuse a request it cannot compile, including in frame lowering, where RISCV reports a reserved stack or frame pointer through it. It names the function, is an error rather than a warning, and leaves llc exiting non-zero, which is what fails closed means here. The scope is the query surface and the refusal. Clearing the stack is trailofbits/vspells-ct-internal-notes#26 and the set of registers to clear is unchanged, on trailofbits/vspells-ct-internal-notes#27. The frontend diagnostic that replaces the driver's triple list is trailofbits/vspells-ct-internal-notes#67, which will consult these queries. The names are recommendations awaiting sign-off on trailofbits/vspells-ct-internal-notes#64. This is trailofbits/vspells-ct-internal-notes#23, under the umbrella trailofbits/vspells-ct-internal-notes#17.
Prologue-epilogue insertion asked the target about "zero-call-used-regs" and "zeroize-stack" from inside insertPrologEpilogCode, which runs only for functions that get a prologue. A naked function carries the attribute but gets no prologue, so its request was dropped in the silence these queries were added to break. Ask before the naked check instead, from one place that answers for both attributes. The "zero-call-used-regs" mode parse moves into a helper so the report and the emission agree on what "skip" means. Report an unsupported "zeroize-stack" as a warning rather than an error. No target clears the frame, so an error fired everywhere and left an attribute that LangRef, the verifier, bitcode and the inliner all already accept impossible to compile anywhere, which removes it rather than reports on it. "zero-call-used-regs" stays an error: three targets implement it, so the error names a target choice the user can change. Promote the stack warning to an error once a target clears the frame. Drop a reference to emitZeroizeStack from supportsZeroizeStack's comment. No such function exists; the query has no emission hook paired with it yet.
8fdf75d to
67d249a
Compare
A naked function has no compiler-generated frame or epilogue. Prologue-epilogue insertion lays out no frame and writes no epilogue, so "zeroize-stack" has nothing to clear and "zero-call-used-regs", emitted into the epilogue before the callee-saved registers are restored, has nowhere to go. Servicing either would mean generating the prologue and epilogue that "naked" exists to suppress, so both requests are dropped. Report them, so the caller learns the attribute had no effect. The message names the attribute rather than the target. Neither request can be honored on any target however capable, and neither becomes honorable once a target implements clearing, so "not supported by this target" would be the wrong reason today and a false one later. For the same reason the naked case does not also get the target-support report. Both are warnings, not errors: a request that cannot be met on a function written without a frame is no reason to refuse to compile it. This also retires the error a naked function on a target without register clearing began producing when the capability queries moved outside the Naked check. "skip" asks for no clearing, so nothing is ignored and nothing is reported. Keeping it silent matters: the attribute is often set to "skip" wholesale, and warning on it would make a naked function unwritable in a translation unit that opts out. The register half is not new breakage. Clearing has always been emitted from insertPrologEpilogCode, which is skipped for a naked function, so such a function has always compiled without the clearing and without a word; on x86_64, where the emission exists, it is dropped in complete silence today. Shorten the comments added with the capability queries while here.
kumarak
left a comment
There was a problem hiding this comment.
Looks good to me. I would like to see an improved handling of diagnostic in the later PR as we have some of the pipeline working. Instead of having seperate dianostic for stack and register. It should be one.
Requested by Francesco Bertolaccini · Slack thread
A request to clear call-used registers is dispatched through
emitZeroCallUsedRegs, whose default body is empty. A target that has not
implemented it therefore compiles the request into nothing at all, and nothing
says so: the caller asked for the registers to be destroyed, the object file
leaves them holding what was in them, and the only way to find out is to read
the disassembly. ARM32 is the live case. The driver refuses the command-line
flag there through a hard-coded list of triples, but the function attribute
that flag turns into is accepted, carried through the whole pipeline, and
dropped in the frame lowering, so anything that sets the attribute directly,
including LTO and inlining of code compiled elsewhere, silently gets nothing.
Add a capability query the target answers instead of inferring support from an
emission that may do nothing. supportsZeroCallUsedRegs defaults to false, so a
target is unsupported until it says otherwise, and prologue-epilogue insertion
asks before computing what to clear: a target that answers false gets a
diagnostic and no code, rather than no diagnostic and no code. The three
targets that implement the emission today, X86, AArch64 and RISCV, answer true,
which is what the driver's triple list already assumed, so no target changes
what it generates.
supportsZeroizeStack is the same query for the "zeroize-stack" attribute, and
today every target answers false, because none of them clears the frame yet.
That attribute has had no backend consumer at all since it was added, which is
the same silence in a worse form, so it now reports itself as unsupported
everywhere until an implementation exists.
DiagnosticInfoUnsupported is what the backend already uses to refuse a request
it cannot compile, including in frame lowering, where RISCV reports a reserved
stack or frame pointer through it. It names the function, is an error rather
than a warning, and leaves llc exiting non-zero, which is what fails closed
means here.
The scope is the query surface and the refusal. Clearing the stack is
trailofbits/vspells-ct-internal-notes#26 and the set of registers to clear is
unchanged, on trailofbits/vspells-ct-internal-notes#27. The frontend
diagnostic that replaces the driver's triple list is
trailofbits/vspells-ct-internal-notes#67, which will consult these queries.
The names are recommendations awaiting sign-off on
trailofbits/vspells-ct-internal-notes#64.
This is trailofbits/vspells-ct-internal-notes#23, under the umbrella
trailofbits/vspells-ct-internal-notes#17.
AI tool use
This pull request contains AI-generated content. It was prepared with the assistance of Claude Code; the contributor has reviewed the generated code and text, is the author of the contribution, and is accountable for it, per the LLVM AI Tool Use Policy.
Generated by Claude Code