[CodeGen] Clear the registers the stack clear leaves data in - #13
[CodeGen] Clear the registers the stack clear leaves data in#13claude[bot] wants to merge 1 commit into
Conversation
|
|
|
Hello @claude[bot] 👋 Thank you for submitting a Pull Request (PR) to the LLVM Project. Since this is your first PR, here are a few useful links covering our main contribution policies and review practices.
Please reply to this message to confirm that you have read these policies, especially the LLVM AI Tool Use Policy, and that any AI tool usage has been noted in the PR description. Frequently asked questionsHow do I add reviewers? This PR will be automatically labeled, and the relevant teams will be notified. For some parts of the project, reviewers may also be added automatically. You can also add reviewers manually using the Reviewers section on this page. If you cannot use that section, it is probably because you do not have write permissions for the repository. In that case, you can request a review by tagging reviewers in a comment using What if there are no comments? If you have not received any comments on your PR after a week, you can request a review by pinging the PR with a comment such as “Ping”. The common courtesy ping rate is once a week. Please remember that you are asking for volunteer time from other developers. Are any special GitHub settings required to contribute to LLVM? We only require contributors to have a public email address associated with their GitHub commits, see this section of LLVM Developer Policy for details. If you have questions, feel free to leave a comment on this PR, or ask on LLVM Discord or LLVM Discourse. Thank you, |
|
That inverts the principle this code states for itself — "silence is indistinguishable from having zeroed the registers" ( At an absolute minimum the flag must not bypass Detail, and the rest of the findings on this patch, in the write-up: https://claude.ai/code/artifact/accb9215-36b5-4022-ae0f-249ee54e99cb Generated by Claude Code |
4116012 to
3d68722
Compare
9c4ac60 to
3e2a3e5
Compare
3d68722 to
abdc7d6
Compare
The clearing sequence runs the stack clear in front of the register clear because the stack clear cannot do its work without registers: it reads the frame through one and writes zeroes back through another, so when it finishes, the registers it worked through hold what it has just destroyed -- the value it overwrote, or the address inside the frame it overwrote it at. Leaving with those in registers discloses exactly what leaving with them on the stack would have. That order is already fixed. This is the coverage, which the order does not give on its own. What the register clear clears is chosen by "zero-call-used-regs", and every mode of it is a statement about the function: which registers the function used, which of them are argument registers, which are general purpose. A register the clearing machinery itself dirtied is none of those things. A "used" mode does not select it, because the sweep that computes the used set runs while the plan is made, before the stack clear has been emitted, and so cannot see it. An "arg" mode does not select it unless it happens to be an argument register. And a function that asked for its frame to be cleared need not have asked for its registers to be cleared at all, in which case there is no mode to select anything. So the coverage is declared rather than inferred. A step of the sequence records the registers it used, and the register clear adds what has been recorded to what it was already going to clear. Three things follow, and each is the point rather than a detail of how it is written. The declaration is per exit. A step is emitted once at each in-scope exit and need not use the same registers at each one, so the record is built as the sequence runs at an exit and read by the register clear at that same exit, rather than being settled for the function the way the plan is. The declarations are folded in after the exit has narrowed the candidate set and not before. That narrowing exists to spare what the exit still needs, and it would take a declared register straight back out again: a declared register is not one the function used, it is one the sequence dirtied on the way here. The register clear stops being optional once a step in front of it declares anything. A function with "zeroize-stack" and no "zero-call-used-regs" gets a register clear anyway, over nothing but the declared registers, because a request to clear the frame is not discharged while the frame's contents are sitting in registers. The same holds for a function that wrote "zero-call-used-regs"="skip", which declines a clear of what the function left in its registers and says nothing about what clearing its frame put there. It follows that a target that cannot clear registers cannot clear the frame either, and it now says so rather than emitting the half of the sequence it can do. A step may only declare a register whose value at the exit nothing depends on. That rules out the registers the exit itself names -- the return value, a tail call's outgoing arguments, the exception object an unwind resume is passed -- and it rules out the callee-saved registers, which have to reach the exit holding what the caller left in them whether or not the exit names them. A step that needs such a register has to save and restore it rather than declare it, because what is declared is cleared. Builds with assertions check both halves; a build without them clears what it was told to, which is the direction the rest of this machinery errs in. What is not here is stack clearing itself, which is trailofbits/vspells-ct-internal-notes#26 and which no target implements. The step that would declare registers is a placeholder: it emits nothing, uses nothing, and so declares nothing, and with no producer there is nothing to exercise the consumer with. A hidden option, -pei-stack-clear-scratch-regs, stands in for one. It makes the placeholder behave as a target that clears the frame using the registers it names, declaring them and emitting nothing else, which is the part of a real implementation the rest of this file has to cope with. It is inert unless a test asks for it, and it is the only thing that can reach this code today. That fixes what the tests can honestly show, and it is one thing: a register declared by a step in front of the register clear is cleared by it, in cases where nothing else would have cleared it. On X86 that is %r11 cleared under "used-gpr", which does not select it because the function does not use it; under no register attribute at all; under "skip"; and at each of a function's two returns rather than at one. The control is a function whose frame is not being cleared, where %r11 is left alone, so that the other cases could not pass on some unrelated reason for clearing it. The sequence printer reports the declared registers at each exit, so the declaration is visible without reading it back out of the emitted code. On ARM, which implements neither capability, a function that asks only for its frame to be cleared is refused for the register clear it did not ask for and needs, while a function that did ask for one is still refused on its own terms. The registers a real stack clear would pick, and the code that picks them, are not tested here, because they do not exist yet. No existing test changes. CodeGen/X86 and CodeGen/ARM pass unchanged, as do the tests this stack has added. Each new test was confirmed load-bearing by breaking the implementation once and restoring it: dropping the declared registers from what the register clear clears failed the X86 test, and leaving the register clear off in a function that did not ask for one failed both. The assertion cannot run in a build without assertions, so its predicate was checked by turning it into a hard error for one build: declaring a callee-saved register or the return-value register was caught, and declaring a register that is dead at the exit was not. This is trailofbits/vspells-ct-internal-notes#20, under the umbrella trailofbits/vspells-ct-internal-notes#17.
3e2a3e5 to
3405cff
Compare
2 participants
Requested by Francesco Bertolaccini · Slack thread
The clearing sequence runs the stack clear in front of the register clear
because the stack clear cannot do its work without registers: it reads the frame
through one and writes zeroes back through another, so when it finishes, the
registers it worked through hold what it has just destroyed -- the value it
overwrote, or the address inside the frame it overwrote it at. Leaving with
those in registers discloses exactly what leaving with them on the stack would
have. That order is already fixed. This is the coverage, which the order does
not give on its own.
What the register clear clears is chosen by "zero-call-used-regs", and every
mode of it is a statement about the function: which registers the function used,
which of them are argument registers, which are general purpose. A register the
clearing machinery itself dirtied is none of those things. A "used" mode does
not select it, because the sweep that computes the used set runs while the plan
is made, before the stack clear has been emitted, and so cannot see it. An "arg"
mode does not select it unless it happens to be an argument register. And a
function that asked for its frame to be cleared need not have asked for its
registers to be cleared at all, in which case there is no mode to select
anything.
So the coverage is declared rather than inferred. A step of the sequence records
the registers it used, and the register clear adds what has been recorded to
what it was already going to clear. Three things follow, and each is the point
rather than a detail of how it is written.
The declaration is per exit. A step is emitted once at each in-scope exit and
need not use the same registers at each one, so the record is built as the
sequence runs at an exit and read by the register clear at that same exit,
rather than being settled for the function the way the plan is.
The declarations are folded in after the exit has narrowed the candidate set and
not before. That narrowing exists to spare what the exit still needs, and it
would take a declared register straight back out again: a declared register is
not one the function used, it is one the sequence dirtied on the way here.
The register clear stops being optional once a step in front of it declares
anything. A function with "zeroize-stack" and no "zero-call-used-regs" gets a
register clear anyway, over nothing but the declared registers, because a
request to clear the frame is not discharged while the frame's contents are
sitting in registers. The same holds for a function that wrote
"zero-call-used-regs"="skip", which declines a clear of what the function left
in its registers and says nothing about what clearing its frame put there. It
follows that a target that cannot clear registers cannot clear the frame either,
and it now says so rather than emitting the half of the sequence it can do.
A step may only declare a register whose value at the exit nothing depends on.
That rules out the registers the exit itself names -- the return value, a tail
call's outgoing arguments, the exception object an unwind resume is passed --
and it rules out the callee-saved registers, which have to reach the exit
holding what the caller left in them whether or not the exit names them. A step
that needs such a register has to save and restore it rather than declare it,
because what is declared is cleared. Builds with assertions check both halves; a
build without them clears what it was told to, which is the direction the rest
of this machinery errs in.
What is not here is stack clearing itself, which is
trailofbits/vspells-ct-internal-notes#26 and which no target implements. The
step that would declare registers is a placeholder: it emits nothing, uses
nothing, and so declares nothing, and with no producer there is nothing to
exercise the consumer with. A hidden option, -pei-stack-clear-scratch-regs,
stands in for one. It makes the placeholder behave as a target that clears the
frame using the registers it names, declaring them and emitting nothing else,
which is the part of a real implementation the rest of this file has to cope
with. It is inert unless a test asks for it, and it is the only thing that can
reach this code today.
That fixes what the tests can honestly show, and it is one thing: a register
declared by a step in front of the register clear is cleared by it, in cases
where nothing else would have cleared it. On X86 that is %r11 cleared under
"used-gpr", which does not select it because the function does not use it; under
no register attribute at all; under "skip"; and at each of a function's two
returns rather than at one. The control is a function whose frame is not being
cleared, where %r11 is left alone, so that the other cases could not pass on
some unrelated reason for clearing it. The sequence printer reports the declared
registers at each exit, so the declaration is visible without reading it back
out of the emitted code. On ARM, which implements neither capability, a function
that asks only for its frame to be cleared is refused for the register clear it
did not ask for and needs, while a function that did ask for one is still
refused on its own terms. The registers a real stack clear would pick, and the
code that picks them, are not tested here, because they do not exist yet.
No existing test changes. CodeGen/X86 and CodeGen/ARM pass unchanged, as do the
tests this stack has added. Each new test was confirmed load-bearing by breaking
the implementation once and restoring it: dropping the declared registers from
what the register clear clears failed the X86 test, and leaving the register
clear off in a function that did not ask for one failed both. The assertion
cannot run in a build without assertions, so its predicate was checked by
turning it into a hard error for one build: declaring a callee-saved register or
the return-value register was caught, and declaring a register that is dead at
the exit was not.
This is trailofbits/vspells-ct-internal-notes#20, 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