From 217ee9602814ab92787d728cbe599d5bcd174f34 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 11 Aug 2026 21:00:55 +0000 Subject: [PATCH 1/5] [CodeGen] Suppress tail calls in protected functions A tail call replaces the caller's frame with the callee's and jumps, so control never comes back to the caller. A function carrying "zeroize-stack" has undertaken to clear its frame before it returns, and a tail call takes away the point at which it would do that: the frame stays live underneath the callee, and the function reports itself protected while leaving in memory exactly what the attribute exists to destroy. Tail-call optimization is suppressed in a protected function. This is decision DD8, which the design records as settled. The suppression goes where LLVM already decides tail-call eligibility rather than into a check of its own. isInTailCallPosition in CodeGen/Analysis.cpp is the target-independent answer to that question, and everything that forms a tail call out of a call in the IR reaches it: SelectionDAGBuilder through canTailCall, FastISel, and GlobalISel's CallLowering. Those are the same three places that each honor "disable-tail-calls" separately, which is what asking once here avoids. Folding a memcpy, memmove or memset into a tail call to the library routine goes through it too, and replaces the frame just as thoroughly. A libcall the legalizer generates has no call in the IR behind it and never reaches that function. It is asked separately, by the SDNode overload of TargetLowering::isInTailCallPosition, which carries its own "disable-tail-calls" check for the same reason, and the second half of the change sits next to it. The path is not hypothetical: an frem in return position becomes a tail call to fmod on both x86 and ARM, and was the one remaining way a protected function still jumped away from its frame. musttail is diagnosed rather than suppressed. Declining an ordinary tail call is available because forming one is an optimization; musttail is a requirement the caller is not allowed to drop. A function that must be replaced at the call and must clear its frame after it is a function that cannot be generated, so the combination is rejected instead of being honored in one direction without saying so. The rejection is in the Verifier, in verifyMustTailCall, next to "cannot use musttail call with inline asm". That neighbor has the same shape: not malformed IR, but musttail combined with something that makes it impossible to honor, and the Verifier is where that shape already lives. It is also the layer at which the conflict is fully visible without a target. Leaving it to CodeGen would surface as the backend's existing "failed to perform tail call elimination on a call site marked musttail", which is fatal but never names the attribute that caused it, and which is reached per target and twice over for a call FastISel starts and SelectionDAG finishes. The frontend diagnostic for the same conflict written in source is separate work. Rejecting the combination in the Verifier makes it a bug for a pass to build one, and one pass did. MergeFunctions rewrites a merged function into a thunk that tail-calls the body, copies the attributes of the function it replaces onto that thunk, and uses musttail when both functions are swifttailcc, so two identical protected swifttailcc functions became a thunk carrying "zeroize-stack" around a musttail call, aborting the compilation with "Broken module found" from valid input. Protected functions are excluded from merging, which is the answer inlining already gives them: a thunk standing in for a protected function undertakes to clear a frame that no longer holds anything, and where the convention makes its call musttail it cannot discharge the undertaking at all. tailcc and -tailcallopt are covered as well. Both exist to guarantee the optimization rather than to permit it, and the guarantee is over the frame the attribute is about, so a protected function does not obtain it by choosing the convention. The cost is real: a protected tailcc function doing unbounded mutual recursion now grows the stack. Whether that should be rejected the way musttail is, rather than quietly losing the guarantee, is left to trailofbits/vspells-ct-internal-notes#22 rather than settled here. Where this meets the per-exit register clearing is worth stating, because the two can look like they overlap. Clearing at a tail-call exit spares the registers the callee is about to read as outgoing arguments, and a protected function no longer has a tail-call exit: the exit classification for one now reports both exits as returns where it used to report a tail call and a return. That path is unreachable for a protected function. It is not dead. Register clearing is driven by "zero-call-used-regs", a separate attribute that long predates this work and that functions carry without "zeroize-stack"; those functions still tail-call, and the per-exit set is still what makes their tail-call exits correct. The test covering that case carries only "zero-call-used-regs" and is untouched here. The two mechanisms answer for disjoint sets of functions rather than for the same one twice. No existing test changes, in CodeGen/X86, CodeGen/ARM, or anywhere under Transforms. Nothing in the tree combines "zeroize-stack" with a tail call, which is why the suppression arrives without an old test starting to expect less. The new tests are the contrast in both directions on both targets: a protected function that would otherwise jump does not, an unprotected one with the same body still does, and the same pair for a legalizer libcall; the exit classification changing from a tail call to a return; musttail in a protected function rejected under two modes with an unprotected musttail untouched; and the merged pair left unmerged. Each was confirmed load-bearing by breaking the implementation once and restoring it: dropping either half of the suppression failed the CodeGen tests at the corresponding check, dropping the Verifier check failed the musttail test, and dropping the merging exclusion failed the MergeFunc test. This is trailofbits/vspells-ct-internal-notes#22, under the umbrella trailofbits/vspells-ct-internal-notes#17. --- llvm/lib/CodeGen/Analysis.cpp | 10 ++ .../CodeGen/SelectionDAG/TargetLowering.cpp | 6 + llvm/lib/IR/Verifier.cpp | 10 ++ llvm/lib/Transforms/IPO/MergeFunctions.cpp | 6 + llvm/test/CodeGen/ARM/zeroize-tailcall.ll | 43 ++++++++ llvm/test/CodeGen/X86/zeroize-tailcall.ll | 103 ++++++++++++++++++ .../Transforms/MergeFunc/zeroize-stack.ll | 70 ++++++++++++ llvm/test/Verifier/zeroize-stack-musttail.ll | 40 +++++++ 8 files changed, 288 insertions(+) create mode 100644 llvm/test/CodeGen/ARM/zeroize-tailcall.ll create mode 100644 llvm/test/CodeGen/X86/zeroize-tailcall.ll create mode 100644 llvm/test/Transforms/MergeFunc/zeroize-stack.ll create mode 100644 llvm/test/Verifier/zeroize-stack-musttail.ll diff --git a/llvm/lib/CodeGen/Analysis.cpp b/llvm/lib/CodeGen/Analysis.cpp index 9ece9f0c187a2..e144b391a07b2 100644 --- a/llvm/lib/CodeGen/Analysis.cpp +++ b/llvm/lib/CodeGen/Analysis.cpp @@ -538,6 +538,16 @@ static bool nextRealType(SmallVectorImpl &SubTypes, /// This function only tests target-independent requirements. bool llvm::isInTailCallPosition(const CallBase &Call, const TargetMachine &TM, bool ReturnsFirstArg) { + // A tail call replaces the caller's frame and jumps away, so a function that + // promised to clear its frame before returning never gets to. Suppress it at + // this one target-independent point, which SelectionDAGBuilder, FastISel, + // GlobalISel, and memcpy/memmove/memset folding all reach. musttail is + // rejected in the Verifier (a caller cannot drop it); suppressing it here too + // is a backstop for unverified IR, failing closed into the backend's musttail + // error rather than a protected function that keeps its frame. + if (Call.getCaller()->hasFnAttribute("zeroize-stack")) + return false; + const BasicBlock *ExitBB = Call.getParent(); const Instruction *Term = ExitBB->getTerminator(); const ReturnInst *Ret = dyn_cast(Term); diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp index d85bdcadab68f..6d6e75e8d5263 100644 --- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp @@ -66,6 +66,12 @@ bool TargetLowering::isInTailCallPosition(SelectionDAG &DAG, SDNode *Node, if (F.getFnAttribute("disable-tail-calls").getValueAsBool()) return false; + // A protected function cannot clear its frame after a tail call. This is a + // legalizer libcall being folded into one, refused for the same reason as a + // tail call in the IR (see isInTailCallPosition in Analysis.cpp). + if (F.hasFnAttribute("zeroize-stack")) + return false; + // Conservatively require the attributes of the call to match those of // the return. Ignore following attributes because they don't affect the // call sequence. diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp index 4d63d746753d1..1a10d6aef177c 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -4214,6 +4214,16 @@ void Verifier::verifyMustTailCall(CallInst &CI) { Check(!CI.isInlineAsm(), "cannot use musttail call with inline asm", &CI); Function *F = CI.getParent()->getParent(); + + // "zeroize-stack" clears the frame before returning; musttail replaces the + // frame and never returns here to clear it. An ordinary tail call is an + // optimization and is suppressed, but musttail is a requirement the caller + // cannot drop, so the two together describe a function that cannot exist. + Check(!F->hasFnAttribute("zeroize-stack"), + "cannot use musttail call in a function with the \"zeroize-stack\" " + "attribute", + &CI); + FunctionType *CallerTy = F->getFunctionType(); FunctionType *CalleeTy = CI.getFunctionType(); Check(CallerTy->isVarArg() == CalleeTy->isVarArg(), diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp index cd35058d099f6..4ad2488462743 100644 --- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp +++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp @@ -459,6 +459,12 @@ static bool hasDistinctMetadataIntrinsic(const Function &F) { static bool isEligibleForMerging(Function &F) { return !F.isDeclaration() && !F.hasAvailableExternallyLinkage() && !F.hasFnAttribute(Attribute::NoIPA) && + // Merging turns the function into a thunk that tail-calls the merged + // body and keeps its attributes; a "zeroize-stack" thunk would promise + // to clear a frame it no longer owns, and a musttail thunk (swifttailcc) + // cannot clear one at all. Keep protected functions whole, as inlining + // already does. + !F.hasFnAttribute("zeroize-stack") && !hasDistinctMetadataIntrinsic(F); } diff --git a/llvm/test/CodeGen/ARM/zeroize-tailcall.ll b/llvm/test/CodeGen/ARM/zeroize-tailcall.ll new file mode 100644 index 0000000000000..877f50f7925a4 --- /dev/null +++ b/llvm/test/CodeGen/ARM/zeroize-tailcall.ll @@ -0,0 +1,43 @@ +; The suppression is decided in target-independent code, so it reaches a target +; that forms its tail calls differently. ARM turns a call in tail position into +; a branch; a protected function keeps the call and returns through its own +; epilogue instead. + +; RUN: llc -mtriple=armv7-unknown-linux-gnueabi %s -o - 2>/dev/null | FileCheck %s + +; The "zeroize-stack" functions also report that no target clears the frame yet. +; That report is a warning, so llc still succeeds, and stderr is discarded here. + +declare i32 @callee(i32) + +; CHECK-LABEL: protected: +; CHECK: bl callee +; CHECK: pop {r11, pc} +define i32 @protected(i32 %x) "zeroize-stack"="used" { + %r = tail call i32 @callee(i32 %x) + ret i32 %r +} + +; CHECK-LABEL: unprotected: +; CHECK: b callee +define i32 @unprotected(i32 %x) { + %r = tail call i32 @callee(i32 %x) + ret i32 %r +} + +; A libcall the legalizer generates has no call in the IR behind it, and is +; refused at the second place the tail-call question is asked. +; CHECK-LABEL: protected_libcall: +; CHECK: bl fmod +; CHECK: pop {r11, pc} +define double @protected_libcall(double %a, double %b) "zeroize-stack"="used" { + %r = frem double %a, %b + ret double %r +} + +; CHECK-LABEL: unprotected_libcall: +; CHECK: b fmod +define double @unprotected_libcall(double %a, double %b) { + %r = frem double %a, %b + ret double %r +} diff --git a/llvm/test/CodeGen/X86/zeroize-tailcall.ll b/llvm/test/CodeGen/X86/zeroize-tailcall.ll new file mode 100644 index 0000000000000..b40c37f893f2f --- /dev/null +++ b/llvm/test/CodeGen/X86/zeroize-tailcall.ll @@ -0,0 +1,103 @@ +; A tail call replaces the caller's frame with the callee's and jumps, so +; control never comes back to the caller. A function carrying "zeroize-stack" +; has undertaken to clear its frame before it returns, and there is no point +; left at which it can do that once it has jumped away. Tail-call optimization +; is suppressed for a protected function; a function that never asked for the +; undertaking keeps it. + +; RUN: llc -mtriple=x86_64-unknown-linux-gnu %s -o - 2>/dev/null | FileCheck %s +; RUN: llc -mtriple=x86_64-unknown-linux-gnu -pei-print-clearing-sequence %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=EXITS + +; Every "zeroize-stack" function below also reports that no target clears the +; frame yet. Those reports are warnings, so llc succeeds; they are what +; zeroize-stack-unsupported.ll is about and are not matched here. + +declare i32 @callee(i32) +declare tailcc i32 @tcallee(i32) + +; The call is marked `tail` and is in tail position, so it would be turned into +; a jump. It stays a call, and the function returns through its own epilogue. +; CHECK-LABEL: protected: +; CHECK-NOT: TAILCALL +; CHECK: callq callee@PLT +; CHECK: retq +define i32 @protected(i32 %x) "zeroize-stack"="used" { + %r = tail call i32 @callee(i32 %x) + ret i32 %r +} + +; The same body without the attribute. Nothing about the suppression is about +; the call, so this one is still a jump. +; CHECK-LABEL: unprotected: +; CHECK: jmp callee@PLT # TAILCALL +define i32 @unprotected(i32 %x) { + %r = tail call i32 @callee(i32 %x) + ret i32 %r +} + +; tailcc exists to guarantee the optimization rather than to permit it, and the +; guarantee is over the frame the protected function is undertaking to clear. +; The suppression covers it too, so a protected function does not get the +; guarantee by choosing the convention. +; CHECK-LABEL: protected_tailcc: +; CHECK-NOT: TAILCALL +; CHECK: callq tcallee@PLT +define tailcc i32 @protected_tailcc(i32 %x) "zeroize-stack"="used" { + %r = tail call tailcc i32 @tcallee(i32 %x) + ret i32 %r +} + +; A libcall the legalizer generates has no call in the IR behind it, so it is +; not reached by the decision the other cases go through. It replaces the frame +; in exactly the same way, and is refused at the second place the tail-call +; question is asked. +; CHECK-LABEL: protected_libcall: +; CHECK-NOT: TAILCALL +; CHECK: callq fmod@PLT +; CHECK: retq +define double @protected_libcall(double %a, double %b) "zeroize-stack"="used" { + %r = frem double %a, %b + ret double %r +} + +; CHECK-LABEL: unprotected_libcall: +; CHECK: jmp fmod@PLT # TAILCALL +define double @unprotected_libcall(double %a, double %b) { + %r = frem double %a, %b + ret double %r +} + +; What the suppression does to the exits, which is where it meets the per-exit +; register clearing. Without the attribute the function has a tail-call exit and +; a return exit, and the clearing at each of them is computed for that exit. +; EXITS-LABEL: clearing sequence for function 'regs_unprotected': +; EXITS-NEXT: %bb.1 tail-call: clear-stack=not-requested clear-registers=emitted clear-flags=unimplemented +; EXITS-NEXT: %bb.2 return: clear-stack=not-requested clear-registers=emitted clear-flags=unimplemented +define i32 @regs_unprotected(i1 %c, i32 %a, i32 %b) "zero-call-used-regs"="used-gpr" { +entry: + br i1 %c, label %tail, label %plain +tail: + %r = tail call i32 @callee(i32 %a) + ret i32 %r +plain: + %s = add i32 %a, %b + ret i32 %s +} + +; The same function protected. The tail-call exit is gone: both exits are +; returns, and the register clearing at what used to be the tail call is now +; computed for a return. The two mechanisms do not overlap for a protected +; function, because it no longer has the exit the tail-call case was about. +; EXITS-LABEL: clearing sequence for function 'regs_protected': +; EXITS-NEXT: %bb.1 return: clear-stack=unsupported clear-registers=emitted clear-flags=unimplemented +; EXITS-NEXT: %bb.2 return: clear-stack=unsupported clear-registers=emitted clear-flags=unimplemented +define i32 @regs_protected(i1 %c, i32 %a, i32 %b) "zero-call-used-regs"="used-gpr" "zeroize-stack"="used" { +entry: + br i1 %c, label %tail, label %plain +tail: + %r = tail call i32 @callee(i32 %a) + ret i32 %r +plain: + %s = add i32 %a, %b + ret i32 %s +} diff --git a/llvm/test/Transforms/MergeFunc/zeroize-stack.ll b/llvm/test/Transforms/MergeFunc/zeroize-stack.ll new file mode 100644 index 0000000000000..e5f45c3e2e9e3 --- /dev/null +++ b/llvm/test/Transforms/MergeFunc/zeroize-stack.ll @@ -0,0 +1,70 @@ +; Merging rewrites one of the two functions into a thunk that tail-calls the +; other, and the thunk is written from the attributes of the function it +; replaces. A thunk carrying "zeroize-stack" is undertaking to clear a frame +; that no longer holds what the attribute was asked for, and where the shared +; calling convention makes the thunk's call musttail the undertaking cannot be +; discharged at all, because control does not come back to the thunk. A +; protected function is left whole, which is what inlining already does with it. + +; RUN: opt -passes=mergefunc -S < %s | FileCheck %s + +declare void @sink(i32) +declare swifttailcc void @swiftsink(i32) + +; Two identical protected functions stay two functions. +; CHECK: define void @protected_a(i32 %x) #0 { +; CHECK-NEXT: add i32 %x, 7 +; CHECK: define void @protected_b(i32 %x) #0 { +; CHECK-NEXT: add i32 %x, 7 +define void @protected_a(i32 %x) "zeroize-stack"="used" { + %y = add i32 %x, 7 + call void @sink(i32 %y) + ret void +} + +define void @protected_b(i32 %x) "zeroize-stack"="used" { + %y = add i32 %x, 7 + call void @sink(i32 %y) + ret void +} + +; The swifttailcc pair is the case that cannot be expressed at all: the thunk +; would musttail-call the body, and a protected function containing a musttail +; call does not verify. Leaving them unmerged is what keeps the pass from +; producing a module that the Verifier rejects. +; CHECK: define swifttailcc void @protected_swift_a(i32 %x) #0 { +; CHECK-NEXT: add i32 %x, 11 +; CHECK: define swifttailcc void @protected_swift_b(i32 %x) #0 { +; CHECK-NEXT: add i32 %x, 11 +define swifttailcc void @protected_swift_a(i32 %x) "zeroize-stack"="used" { + %y = add i32 %x, 11 + tail call swifttailcc void @swiftsink(i32 %y) + ret void +} + +define swifttailcc void @protected_swift_b(i32 %x) "zeroize-stack"="used" { + %y = add i32 %x, 11 + tail call swifttailcc void @swiftsink(i32 %y) + ret void +} + +; Nothing about merging in general changes: an identical pair without the +; attribute is still merged, so what is excluded is the attribute rather than +; this shape of function. +; CHECK: define void @plain_a(i32 %x) { +; CHECK-NEXT: add i32 %x, 13 +; CHECK: define void @plain_b(i32 %0) { +; CHECK-NEXT: tail call void @plain_a(i32 %0) +define void @plain_a(i32 %x) { + %y = add i32 %x, 13 + call void @sink(i32 %y) + ret void +} + +define void @plain_b(i32 %x) { + %y = add i32 %x, 13 + call void @sink(i32 %y) + ret void +} + +; CHECK: attributes #0 = { "zeroize-stack"="used" } diff --git a/llvm/test/Verifier/zeroize-stack-musttail.ll b/llvm/test/Verifier/zeroize-stack-musttail.ll new file mode 100644 index 0000000000000..f8c6a7efd8186 --- /dev/null +++ b/llvm/test/Verifier/zeroize-stack-musttail.ll @@ -0,0 +1,40 @@ +; An ordinary tail call in a protected function is an optimization, and it is +; suppressed. musttail is not an optimization: the caller is required to be +; replaced by the callee, and no pass is allowed to decide otherwise. A function +; cannot both be replaced at the call and clear its frame after it, so the two +; together describe a function that cannot be generated, and the combination is +; rejected here rather than being honored in one direction without saying so. +; +; The check sits next to the other reasons a musttail call cannot be honored, +; such as inline asm, because it is the same kind of conflict. The frontend +; diagnostic for the same conflict in source is separate: +; trailofbits/vspells-ct-internal-notes#22. + +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +declare i32 @callee(i32) + +; CHECK: cannot use musttail call in a function with the "zeroize-stack" attribute +; CHECK-NEXT: musttail call i32 @callee +define i32 @protected(i32 %x) "zeroize-stack"="used" { + %r = musttail call i32 @callee(i32 %x) + ret i32 %r +} + +; The mode does not enter into it. Any mode is an undertaking to clear the +; frame, and none of them can be kept at a call that does not return. +; CHECK: cannot use musttail call in a function with the "zeroize-stack" attribute +; CHECK-NEXT: musttail call i32 @callee +define i32 @protected_sensitive(i32 %x) "zeroize-stack"="sensitive" { + %r = musttail call i32 @callee(i32 %x) + ret i32 %r +} + +; A musttail call in a function that has not asked for the undertaking is +; untouched, so the attribute is what the rejection is about rather than +; musttail being newly restricted. +; CHECK-NOT: @unprotected +define i32 @unprotected(i32 %x) { + %r = musttail call i32 @callee(i32 %x) + ret i32 %r +} From 97392906b6896b0982922f1ba369229b4dddaeb2 Mon Sep 17 00:00:00 2001 From: AkshayK Date: Mon, 31 Aug 2026 20:38:37 -0400 Subject: [PATCH 2/5] [CodeGen] Close tail-call gaps for protected functions Follow-up to the tail-call suppression, addressing three review findings. The Verifier rejects a musttail call in a "zeroize-stack" function, but CoroSplit synthesizes one: a presplit coroutine is lowered into resume and destroy clones that hand off with musttail calls (symmetric transfer, and the async coro.end), and the coroutine's function attributes are copied onto those clones. A protected coroutine therefore became a protected function holding a musttail call, aborting with "Broken module found" on input that verified. Reject "zeroize-stack" on a presplit coroutine up front, the same way the musttail combination is rejected, so the split never runs on it. The suppression covered SelectionDAG but not GlobalISel. GlobalISel forms legalizer libcalls on its own path and decides the tail call in the legalizer rather than in TargetLowering::isInTailCallPosition, so a protected function returning frem under -global-isel still tail-branched to fmod with no return path to clear its frame. Add the guard to isLibCallInTailPosition, the shared predicate both the general and the memory libcall paths reach. The guarantee was gated on the bare string "zeroize-stack" repeated across every enforcement site, where a single typo would silently drop the protection. Add Function::hasZeroizeStack() and getZeroizeStackMode(), naming the attribute once, and route CodeGen, the Verifier, the IPO passes and the inline-mode check through them. Tests: reject the protected coroutine in the Verifier, and keep the AArch64 GlobalISel libcall in its own frame. This is trailofbits/vspells-ct-internal-notes#22, under the umbrella trailofbits/vspells-ct-internal-notes#17. --- llvm/include/llvm/IR/Function.h | 15 +++++++ llvm/lib/CodeGen/Analysis.cpp | 2 +- .../CodeGen/GlobalISel/LegalizerHelper.cpp | 7 ++++ llvm/lib/CodeGen/PrologEpilogInserter.cpp | 4 +- .../CodeGen/SelectionDAG/TargetLowering.cpp | 2 +- llvm/lib/IR/Attributes.cpp | 10 ++--- llvm/lib/IR/Verifier.cpp | 11 +++++- llvm/lib/Transforms/IPO/MergeFunctions.cpp | 2 +- .../CodeGen/AArch64/zeroize-tailcall-gisel.ll | 26 +++++++++++++ llvm/test/Verifier/zeroize-stack-coroutine.ll | 39 +++++++++++++++++++ 10 files changed, 106 insertions(+), 12 deletions(-) create mode 100644 llvm/test/CodeGen/AArch64/zeroize-tailcall-gisel.ll create mode 100644 llvm/test/Verifier/zeroize-stack-coroutine.ll diff --git a/llvm/include/llvm/IR/Function.h b/llvm/include/llvm/IR/Function.h index 0238c9b352f5f..e8c5044ca6f6c 100644 --- a/llvm/include/llvm/IR/Function.h +++ b/llvm/include/llvm/IR/Function.h @@ -517,6 +517,21 @@ class LLVM_ABI Function : public GlobalObject, public ilist_node { return AttributeSets.getParamNoFPClass(ArgNo); } + /// The "zeroize-stack" attribute: the function's undertaking to clear its own + /// stack frame before it returns. Named once here so the enforcement sites + /// spread across CodeGen, the Verifier and the IPO passes cannot drift, and a + /// misspelling fails to compile rather than silently dropping the guarantee. + static constexpr StringRef ZeroizeStackAttrName = "zeroize-stack"; + + /// Return true if the function carries the "zeroize-stack" attribute. + bool hasZeroizeStack() const { return hasFnAttribute(ZeroizeStackAttrName); } + + /// Return the "zeroize-stack" mode, or an empty string if the attribute is + /// absent. An empty or unrecognized value means the widest mode; see LangRef. + StringRef getZeroizeStackMode() const { + return getFnAttribute(ZeroizeStackAttrName).getValueAsString(); + } + /// Determine if the function is presplit coroutine. bool isPresplitCoroutine() const { return hasFnAttribute(Attribute::PresplitCoroutine); diff --git a/llvm/lib/CodeGen/Analysis.cpp b/llvm/lib/CodeGen/Analysis.cpp index e144b391a07b2..965e0b1825a36 100644 --- a/llvm/lib/CodeGen/Analysis.cpp +++ b/llvm/lib/CodeGen/Analysis.cpp @@ -545,7 +545,7 @@ bool llvm::isInTailCallPosition(const CallBase &Call, const TargetMachine &TM, // rejected in the Verifier (a caller cannot drop it); suppressing it here too // is a backstop for unverified IR, failing closed into the backend's musttail // error rather than a protected function that keeps its frame. - if (Call.getCaller()->hasFnAttribute("zeroize-stack")) + if (Call.getCaller()->hasZeroizeStack()) return false; const BasicBlock *ExitBB = Call.getParent(); diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp index a6c5a267c87db..b4fd433384b03 100644 --- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp +++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp @@ -534,6 +534,13 @@ static bool isLibCallInTailPosition(const CallLowering::ArgInfo &Result, MachineBasicBlock &MBB = *MI.getParent(); const Function &F = MBB.getParent()->getFunction(); + // A protected function cannot clear its frame after a tail call. This is the + // GlobalISel analog of the check in TargetLowering::isInTailCallPosition: a + // legalizer libcall folded into a tail call replaces the frame just the same, + // and is refused for the same reason. + if (F.hasZeroizeStack()) + return false; + // Conservatively require the attributes of the call to match those of // the return. Ignore NoAlias and NonNull because they don't affect the // call sequence. diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp b/llvm/lib/CodeGen/PrologEpilogInserter.cpp index 7c5b4d25a9b86..c6a9a1949de5f 100644 --- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp +++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp @@ -1659,7 +1659,7 @@ void PEIImpl::planClearingSequence(MachineFunction &MF, ClearingDisposition PEIImpl::planClearStack(MachineFunction &MF) { const Function &F = MF.getFunction(); - if (!F.hasFnAttribute("zeroize-stack")) + if (!F.hasZeroizeStack()) return ClearingDisposition::NotRequested; const TargetFrameLowering &TFI = *MF.getSubtarget().getFrameLowering(); @@ -1800,7 +1800,7 @@ void PEIImpl::diagnoseIgnoredZeroizeRequestsOnNakedFunction( if (!F.hasFnAttribute(Attribute::Naked)) return; - if (F.hasFnAttribute("zeroize-stack")) + if (F.hasZeroizeStack()) F.getContext().diagnose(DiagnosticInfoUnsupported{ F, "\"zeroize-stack\" ignored on a \"naked\" function: no frame is " diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp index 6d6e75e8d5263..99e35304d6e00 100644 --- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp @@ -69,7 +69,7 @@ bool TargetLowering::isInTailCallPosition(SelectionDAG &DAG, SDNode *Node, // A protected function cannot clear its frame after a tail call. This is a // legalizer libcall being folded into one, refused for the same reason as a // tail call in the IR (see isInTailCallPosition in Analysis.cpp). - if (F.hasFnAttribute("zeroize-stack")) + if (F.hasZeroizeStack()) return false; // Conservatively require the attributes of the call to match those of diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp index fbe9d0fc7499e..bba4a59ca8452 100644 --- a/llvm/lib/IR/Attributes.cpp +++ b/llvm/lib/IR/Attributes.cpp @@ -2621,12 +2621,12 @@ static bool checkZeroizeStack(const Function &Caller, const Function &Callee) { // frame would have sat below the stack pointer at the caller's return, where // no clear reaches it, and inlining turns those bytes into frame bytes the // caller does clear. - if (!Callee.hasFnAttribute("zeroize-stack")) + if (!Callee.hasZeroizeStack()) return true; // Otherwise the caller has to carry the attribute too, or there is no clear // for the callee's frame bytes to be folded into. - if (!Caller.hasFnAttribute("zeroize-stack")) + if (!Caller.hasZeroizeStack()) return false; // Both are protected, so the caller must not ask for less of its frame than @@ -2637,10 +2637,8 @@ static bool checkZeroizeStack(const Function &Caller, const Function &Callee) { // An absent, an empty, and an unrecognized value all mean "used", so a value // this consumer cannot interpret clears more of the frame than it must, // never less. See llvm/test/Transforms/Inline/zeroize-stack.ll. - return Caller.getFnAttribute("zeroize-stack").getValueAsString() != - ZeroizeStackNarrowestMode || - Callee.getFnAttribute("zeroize-stack").getValueAsString() == - ZeroizeStackNarrowestMode; + return Caller.getZeroizeStackMode() != ZeroizeStackNarrowestMode || + Callee.getZeroizeStackMode() == ZeroizeStackNarrowestMode; } template diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp index 1a10d6aef177c..e693d5a055910 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -3115,6 +3115,15 @@ void Verifier::visitFunction(const Function &F) { for (const Argument &Arg : F.args()) Check(Arg.use_empty(), "cannot use argument of naked function", &Arg); + // CoroSplit lowers a presplit coroutine into resume/destroy clones that hand + // off with musttail calls (symmetric transfer, and the async coro.end) and + // copies the coroutine's function attributes onto those clones. A protected + // coroutine would become a protected function holding a musttail call, which + // verifyMustTailCall rejects, so reject it here before the split for the same + // reason: the frame the attribute must clear is handed off and never cleared. + Check(!F.isPresplitCoroutine() || !F.hasZeroizeStack(), + "cannot use the \"zeroize-stack\" attribute on a coroutine", &F); + // Check that this function meets the restrictions on this calling convention. // Sometimes varargs is used for perfectly forwarding thunks, so some of these // restrictions can be lifted. @@ -4219,7 +4228,7 @@ void Verifier::verifyMustTailCall(CallInst &CI) { // frame and never returns here to clear it. An ordinary tail call is an // optimization and is suppressed, but musttail is a requirement the caller // cannot drop, so the two together describe a function that cannot exist. - Check(!F->hasFnAttribute("zeroize-stack"), + Check(!F->hasZeroizeStack(), "cannot use musttail call in a function with the \"zeroize-stack\" " "attribute", &CI); diff --git a/llvm/lib/Transforms/IPO/MergeFunctions.cpp b/llvm/lib/Transforms/IPO/MergeFunctions.cpp index 4ad2488462743..23c5f8c870bd1 100644 --- a/llvm/lib/Transforms/IPO/MergeFunctions.cpp +++ b/llvm/lib/Transforms/IPO/MergeFunctions.cpp @@ -464,7 +464,7 @@ static bool isEligibleForMerging(Function &F) { // to clear a frame it no longer owns, and a musttail thunk (swifttailcc) // cannot clear one at all. Keep protected functions whole, as inlining // already does. - !F.hasFnAttribute("zeroize-stack") && + !F.hasZeroizeStack() && !hasDistinctMetadataIntrinsic(F); } diff --git a/llvm/test/CodeGen/AArch64/zeroize-tailcall-gisel.ll b/llvm/test/CodeGen/AArch64/zeroize-tailcall-gisel.ll new file mode 100644 index 0000000000000..033a182d29245 --- /dev/null +++ b/llvm/test/CodeGen/AArch64/zeroize-tailcall-gisel.ll @@ -0,0 +1,26 @@ +; A libcall the legalizer generates has no call in the IR behind it, so it is +; not caught where an IR call is. GlobalISel forms these libcalls on its own +; path, separate from SelectionDAG, and decides the tail call in the legalizer +; rather than in TargetLowering::isInTailCallPosition. The suppression is asked +; there too: a protected function keeps the call and returns through its own +; epilogue where an unprotected one branches away. + +; RUN: llc -mtriple=aarch64-unknown-linux-gnu -global-isel %s -o - 2>/dev/null | FileCheck %s + +; The "zeroize-stack" function also reports that no target clears the frame yet. +; That report is a warning, so llc still succeeds, and stderr is discarded here. + +; CHECK-LABEL: protected_libcall: +; CHECK: bl fmod +; CHECK: ret +define double @protected_libcall(double %a, double %b) "zeroize-stack"="used" { + %r = frem double %a, %b + ret double %r +} + +; CHECK-LABEL: unprotected_libcall: +; CHECK: b fmod +define double @unprotected_libcall(double %a, double %b) { + %r = frem double %a, %b + ret double %r +} diff --git a/llvm/test/Verifier/zeroize-stack-coroutine.ll b/llvm/test/Verifier/zeroize-stack-coroutine.ll new file mode 100644 index 0000000000000..caeaa6295a1a7 --- /dev/null +++ b/llvm/test/Verifier/zeroize-stack-coroutine.ll @@ -0,0 +1,39 @@ +; CoroSplit lowers a presplit coroutine into resume/destroy clones that hand off +; with musttail calls (symmetric transfer, and the async coro.end) and copies the +; coroutine's function attributes onto those clones. A protected coroutine would +; therefore become a protected function holding a musttail call, which +; verifyMustTailCall rejects. That is the same conflict written one layer up: a +; function cannot both hand its frame off at a musttail call and clear that frame +; after it, so the combination is rejected before the split rather than aborting +; mid-CoroSplit with "Broken module found" on input that verified. + +; RUN: not llvm-as < %s -o /dev/null 2>&1 | FileCheck %s + +; CHECK: cannot use the "zeroize-stack" attribute on a coroutine +; CHECK-NEXT: ptr @protected_coro +define ptr @protected_coro() "zeroize-stack"="used" presplitcoroutine { + ret ptr null +} + +; The mode does not enter into it. Any mode is an undertaking to clear the frame, +; and none of them survives being handed off at a suspend point. +; CHECK: cannot use the "zeroize-stack" attribute on a coroutine +; CHECK-NEXT: ptr @protected_coro_sensitive +define ptr @protected_coro_sensitive() "zeroize-stack"="sensitive" presplitcoroutine { + ret ptr null +} + +; A coroutine that has not asked for the undertaking is untouched, so the +; attribute is what the rejection is about rather than coroutines being newly +; restricted. +; CHECK-NOT: @plain_coro +define ptr @plain_coro() presplitcoroutine { + ret ptr null +} + +; The attribute on an ordinary function is untouched here; the tail-call +; suppression and the musttail rule cover that function. +; CHECK-NOT: @protected_noncoro +define ptr @protected_noncoro() "zeroize-stack"="used" { + ret ptr null +} From 04bbdf0956210de9880d089efe42997e0a568f1c Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 09:33:18 +0000 Subject: [PATCH 3/5] [CodeGen] Fall back to clearing more when the analysis is incomplete Count implicit register operands when selecting registers for the used zero-call-used-regs modes. This includes inline-assembly clobbers and operands of target instructions and call pseudos that the previous scan ignored. Treat unrecognized zero-call-used-regs modes as all. Include otherwise unclassified terminal instructions in exit clearing, while retaining the explicit exclusions for traps, non-returning calls, and non-local jumps. Diagnose an in-scope exit if a requested clearing sequence cannot be placed there. Add X86 and ARM tests for implicit register operands, unknown modes, and opaque exits. Counting implicit call operands also changes existing RISC-V output. The RV32 and RV64 +F runs of zero-call-used-regs-fp.ll fail their unchanged checks: used now clears fa0, and used_arg_double clears a2/a3 on RV32 or a1 on RV64. These are dead helper-call arguments; return values and ra remain intact. The four +D/+Q runs and the three adjacent scalar/vector runs pass. A separate test commit updates the five affected check lines. The parent passes all six original floating-point configurations. These are focused local results, not a full CodeGen validation. This is trailofbits/vspells-ct-internal-notes#24, under the umbrella trailofbits/vspells-ct-internal-notes#17. Co-Authored-By: Claude Opus 5 --- llvm/lib/CodeGen/PrologEpilogInserter.cpp | 101 ++++++++++++++++-- llvm/test/CodeGen/ARM/zeroize-fallback.ll | 52 +++++++++ .../CodeGen/X86/zeroize-fallback-exits.ll | 88 +++++++++++++++ .../test/CodeGen/X86/zeroize-fallback-mode.ll | 68 ++++++++++++ .../test/CodeGen/X86/zeroize-fallback-regs.ll | 68 ++++++++++++ 5 files changed, 367 insertions(+), 10 deletions(-) create mode 100644 llvm/test/CodeGen/ARM/zeroize-fallback.ll create mode 100644 llvm/test/CodeGen/X86/zeroize-fallback-exits.ll create mode 100644 llvm/test/CodeGen/X86/zeroize-fallback-mode.ll create mode 100644 llvm/test/CodeGen/X86/zeroize-fallback-regs.ll diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp b/llvm/lib/CodeGen/PrologEpilogInserter.cpp index c6a9a1949de5f..524a621be8ca2 100644 --- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp +++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp @@ -1396,7 +1396,19 @@ getZeroCallUsedRegsKind(const Function &F) { .Case("all-gpr-arg", ZeroCallUsedRegsKind::AllGPRArg) .Case("all-gpr", ZeroCallUsedRegsKind::AllGPR) .Case("all-arg", ZeroCallUsedRegsKind::AllArg) - .Case("all", ZeroCallUsedRegsKind::All); + .Case("all", ZeroCallUsedRegsKind::All) + // A mode this version of LLVM does not recognize means the widest + // one. The modes are a scale from "skip" to "all", and a name off + // the end of what is known here is either a mode added later, whose + // author selected it over the ones that already existed, or a + // mistake. "all" is the only answer that is safe under both + // readings, and it is the reading LangRef already fixes for an + // unrecognized "zeroize-stack" mode. + // + // Without this the switch runs off its end: an assertion in a build + // that has them, and in a release compiler an uninitialized mode + // that decides what gets cleared. Neither is a decision. + .Default(ZeroCallUsedRegsKind::All); } /// The name of the callee of \p MI, for a call to a known symbol, or the empty @@ -1454,9 +1466,21 @@ static bool isUnwindResumeCall(const MachineInstr &MI) { /// reaches them. The one it misses is a landing pad that resumes unwinding by a /// call, not a return; no existing predicate reaches it, so this does. /// -/// Null means out of scope, not overlooked: a non-returning call, a non-local -/// jump that reloads another frame's pointers, and a trap all abandon the frame -/// rather than release it, so nothing in the block is the last to touch it. +/// The exits this returns null for are out of scope rather than overlooked, and +/// for the same reason in each case: the frame is abandoned rather than +/// released, so there is no position at which a sequence could run and still be +/// the last thing to touch it. A call that does not return here hands the +/// caller's context back through the unwinder or through a jump with nothing of +/// ours in between; a non-local jump does the same by reloading another frame's +/// stack and frame pointers; and a trap, or an empty block left behind by an +/// unreachable, does not transfer out of the frame at all. +/// +/// A block that ends in none of those is in scope, not out of it. Failing to +/// recognise an instruction is not the same as knowing what it does, and the +/// two errors are not symmetric: an opaque instruction that does leave the +/// function takes the frame and the registers with it, while one that does not +/// costs a dead sequence in a block nothing reaches. The uncertainty is +/// resolved towards clearing. static MachineInstr *getEnforceableExit(MachineBasicBlock &MBB) { // A block with a successor continues in the function, so it is not an exit // however its terminator reads; catchret reaches here carrying isReturn. @@ -1471,8 +1495,34 @@ static MachineInstr *getEnforceableExit(MachineBasicBlock &MBB) { for (MachineInstr &MI : reverse(MBB.instrs())) { if (MI.isMetaInstruction()) continue; - return MI.isCall() && isUnwindResumeCall(MI) ? &MI : nullptr; + + // A call either resumes unwinding, which is an exit a sequence goes in + // front of, or does not come back here at all, which abandons the frame. + if (MI.isCall()) + return isUnwindResumeCall(MI) ? &MI : nullptr; + + // A jump with no successor in this function is a jump out of it. Targets + // spell a longjmp either as an indirect branch, once the jump buffer has + // been reloaded, or as a barrier pseudo that expands to one later. + if (MI.isIndirectBranch() || + (MI.isTerminator() && MI.isBarrier() && !MI.isBranch())) + return nullptr; + + // A trap is where control stops, not where it goes: the target has said so + // by marking the instruction, and it is the one shape left here that can be + // ruled out rather than merely not recognised. + if (MI.getDesc().isTrap()) + return nullptr; + + // Nothing else is known about this block, and not knowing has to be + // recorded as not knowing. Inline assembly can jump, can issue a system + // call that does not come back, and can return into another frame, and + // nothing here can establish that it does not; calling such a block one + // control stops in is the one answer that leaves the frame alone. + return &MI; } + + // A block with nothing left in it has nothing that could transfer anywhere. return nullptr; } @@ -1484,7 +1534,9 @@ static StringRef getExitKindName(const MachineInstr &ExitMI) { return "eh-scope-return"; if (ExitMI.isReturn()) return ExitMI.isCall() ? "tail-call" : "return"; - return "unwind-resume"; + if (ExitMI.isCall()) + return "unwind-resume"; + return "unknown"; } /// Where the clearing sequence goes at \p ExitMI of \p MBB. Every step emits @@ -1578,10 +1630,23 @@ void PEIImpl::insertClearingSequences(MachineFunction &MF) { // An exit that is in scope is one the sequence can be placed at, so it has // a position by construction. Emitting at the end of the block instead // would put the sequence after the instruction control leaves through. + // + // If that construction ever fails, the compilation fails with it. Carrying + // on past an exit the sequence could not be placed at produces the one + // output the attribute exists to rule out: a function that reports itself + // protected and leaves through a point at which nothing was cleared, with + // nothing said about it. There is no way to reach this from IR today; it + // is here so that a later change which introduces one is stopped rather + // than absorbed. MachineBasicBlock::iterator InsertPt = getClearingInsertPoint(MBB, *ExitMI); - assert(InsertPt != MBB.end() && "in-scope exit with nowhere to emit"); - if (InsertPt == MBB.end()) + if (InsertPt == MBB.end()) { + if (Plan.anyStepEmits()) + MF.getFunction().getContext().diagnose(DiagnosticInfoUnsupported{ + MF.getFunction(), + "clearing sequence could not be placed at an exit of this " + "function"}); continue; + } if (PrintClearingSequence) OS << " " << printMBBReference(MBB) << " " @@ -1717,6 +1782,23 @@ PEIImpl::planClearRegisters(MachineFunction &MF, const BitVector AllocatableSet(TRI.getAllocatableSet(MF)); // Mark all used registers. + // + // Every register operand counts, whether the instruction names it or carries + // it implicitly. An implicit operand is how the machine layer writes down a + // register an instruction touches without being told to, which is exactly + // the case where a narrowing this set drives cannot be justified: the + // register was written, the value is there, and the mode's promise is that + // what the function touched does not outlive it. + // + // Inline assembly is the case that made this visible. Every register an asm + // block names -- its clobber list and its physical-register outputs alike -- + // reaches the machine layer as an implicit operand of the INLINEASM + // instruction, so skipping implicit operands made an asm block invisible + // here. A function whose only register traffic was an asm block cleared + // nothing at all under a "used" mode, and the asm's registers carried their + // contents past the return. Opaque target operations behave the same way for + // the same reason: a division's remainder register, a return value that no + // longer has a copy naming it, anything a pseudo defines on the side. BitVector UsedRegs(TRI.getNumRegs()); if (OnlyUsed) for (const MachineBasicBlock &MBB : MF) @@ -1730,8 +1812,7 @@ PEIImpl::planClearRegisters(MachineFunction &MF, continue; MCRegister Reg = MO.getReg(); - if (AllocatableSet[Reg.id()] && !MO.isImplicit() && - (MO.isDef() || MO.isUse())) + if (AllocatableSet[Reg.id()] && (MO.isDef() || MO.isUse())) UsedRegs.set(Reg.id()); } } diff --git a/llvm/test/CodeGen/ARM/zeroize-fallback.ll b/llvm/test/CodeGen/ARM/zeroize-fallback.ll new file mode 100644 index 0000000000000..c4ecf8be5fac7 --- /dev/null +++ b/llvm/test/CodeGen/ARM/zeroize-fallback.ll @@ -0,0 +1,52 @@ +; The fallbacks are not written in terms of any one target's instructions, and +; two of them are visible on a target that cannot clear anything at all: which +; exits are in scope is decided before any target is asked, and an unreadable +; mode is resolved before the target is asked too. +; +; trailofbits/vspells-ct-internal-notes#24. + +; Both runs are under "not", because the widened mode reaches a refusal this +; target has to give and llc exits non-zero for it. That refusal is the second +; half of what is being tested. +; RUN: not llc -mtriple=armv7-unknown-linux-gnueabi -pei-print-clearing-sequence %s -o /dev/null 2>&1 | FileCheck --check-prefix=SEQ %s +; RUN: not llc -mtriple=armv7-unknown-linux-gnueabi %s -o /dev/null 2>&1 | FileCheck --check-prefix=DIAG %s + +@g = external global i32 + +declare void @llvm.trap() + +; A supervisor call written as inline assembly ends the block, and whether +; control comes back from it is not something the compiler can decide. It is +; in scope here for the same reason it is on x86-64. +; SEQ-LABEL: clearing sequence for function 'opaque_asm': +; SEQ-NEXT: %bb.0 unknown: clear-stack=not-requested clear-registers=not-requested clear-flags=unimplemented +; SEQ-NEXT: end clearing sequence for function 'opaque_asm' +define void @opaque_asm(i32 %a, i32 %b) { + %s = add i32 %a, %b + store i32 %s, ptr @g + call void asm sideeffect "svc #0", "~{memory}"() + unreachable +} + +; A trap is a trap on every target that marks one, and stays out of scope. +; SEQ-LABEL: clearing sequence for function 'traps': +; SEQ-NEXT: end clearing sequence for function 'traps' +define void @traps() { + call void @llvm.trap() + unreachable +} + +; An unreadable mode is not "skip". ARM cannot clear registers, so what the +; widened mode reaches here is the target's refusal, which is reported; what it +; does not do is quietly resolve to clearing nothing and say nothing. +; DIAG: error: {{.*}}in function unrecognized_mode i32 (i32): "zero-call-used-regs" is not supported by this target +define i32 @unrecognized_mode(i32 %x) "zero-call-used-regs"="a-mode-from-the-future" { + ret i32 %x +} + +; A mode that says to skip is read and honored, on this target as on any other, +; so it reaches no refusal. +; DIAG-NOT: in function skips_explicitly +define i32 @skips_explicitly(i32 %x) "zero-call-used-regs"="skip" { + ret i32 %x +} diff --git a/llvm/test/CodeGen/X86/zeroize-fallback-exits.ll b/llvm/test/CodeGen/X86/zeroize-fallback-exits.ll new file mode 100644 index 0000000000000..59d1c202e9d2a --- /dev/null +++ b/llvm/test/CodeGen/X86/zeroize-fallback-exits.ll @@ -0,0 +1,88 @@ +; A block with no successors that ends in something the emission cannot account +; for used to be passed over, which is the claim that control stops there and +; nothing needs clearing. Not recognizing an instruction is not the same as +; knowing what it does, and the two answers are not symmetric: an opaque +; instruction that does leave the function takes the frame and the registers +; with it, while one that does not costs a dead sequence in a block nothing +; reaches. The uncertainty resolves towards clearing. +; +; trailofbits/vspells-ct-internal-notes#24. + +; RUN: llc -mtriple=x86_64-unknown-linux-gnu -pei-print-clearing-sequence %s -o /dev/null 2>&1 | FileCheck --check-prefix=SEQ %s +; RUN: llc -mtriple=x86_64-unknown-linux-gnu %s -o - | FileCheck %s + +@g = external global i64 + +declare void @llvm.trap() + +; Inline assembly at the end of a block with no successors. It can jump, it can +; issue a system call that does not come back, and nothing here can tell. +; SEQ-LABEL: clearing sequence for function 'opaque_asm': +; SEQ-NEXT: %bb.0 unknown: clear-stack=not-requested clear-registers=emitted clear-flags=unimplemented +; SEQ-NEXT: end clearing sequence for function 'opaque_asm' +; +; The sequence goes in front of the asm, because after it is after the +; function. The registers the earlier computation used are cleared there; the +; ones the asm declares are left alone, the same as at any other exit, because +; an exit cannot be given a sequence that breaks the instruction it leaves +; through. +; CHECK-LABEL: opaque_asm: +; CHECK: xorl %eax, %eax +; CHECK-NEXT: xorl %edi, %edi +; CHECK-NEXT: xorl %esi, %esi +; CHECK-NEXT: #APP +; CHECK-NEXT: hlt +define void @opaque_asm(i64 %a, i64 %b) "zero-call-used-regs"="used-gpr" { + %s = add i64 %a, %b + store i64 %s, ptr @g + call void asm sideeffect "hlt", "~{memory}"() + unreachable +} + +; A trap is still out of scope, and this is what keeps the change from being a +; blanket "clear everywhere". The target has marked the instruction as a trap, +; so control stopping in the block is something known rather than something +; that could not be ruled out. The list is pinned between the two lines that +; bracket it, so an exit that started being emitted at would show up here. +; SEQ-LABEL: clearing sequence for function 'traps': +; SEQ-NEXT: end clearing sequence for function 'traps' +; +; CHECK-LABEL: traps: +; CHECK-NOT: xorl +; CHECK: ud2 +define void @traps(i64 %a, i64 %b) "zero-call-used-regs"="used-gpr" { + %s = add i64 %a, %b + store i64 %s, ptr @g + call void @llvm.trap() + unreachable +} + +; So is a block with nothing left in it: there is no instruction to be unsure +; about. The return is the only exit the sequence runs at. +; SEQ-LABEL: clearing sequence for function 'empty_unreachable': +; SEQ-NEXT: %bb.1 return: clear-stack=not-requested clear-registers=emitted clear-flags=unimplemented +; SEQ-NEXT: end clearing sequence for function 'empty_unreachable' +define void @empty_unreachable(i32 %x) "zero-call-used-regs"="used-gpr" { +entry: + %c = icmp sgt i32 %x, 0 + br i1 %c, label %ok, label %bad + +ok: + ret void + +bad: + unreachable +} + +; And a call that does not return stays out of scope for a reason of its own: +; the frame is abandoned rather than left, so there is no point at which a +; sequence would run and still be the last thing to touch it. That reason +; survives; only the blocks that had no reason at all have moved. +; SEQ-LABEL: clearing sequence for function 'calls_noreturn': +; SEQ-NEXT: end clearing sequence for function 'calls_noreturn' +define void @calls_noreturn() "zero-call-used-regs"="used-gpr" { + call void @abort() + unreachable +} + +declare void @abort() noreturn diff --git a/llvm/test/CodeGen/X86/zeroize-fallback-mode.ll b/llvm/test/CodeGen/X86/zeroize-fallback-mode.ll new file mode 100644 index 0000000000000..cbe80c26f3494 --- /dev/null +++ b/llvm/test/CodeGen/X86/zeroize-fallback-mode.ll @@ -0,0 +1,68 @@ +; The modes of "zero-call-used-regs" are a scale, from clearing nothing to +; clearing everything call-used. A value that is not one of the names on the +; scale carries no information about where on it the producer meant to be, and +; the only reading that cannot clear less than was asked for is the widest one. +; +; This is the reading LangRef already fixes for an unrecognized "zeroize-stack" +; mode, and the two attributes now agree. Before, the mode switch had no +; default at all: an assertion in a build that has them, and in a release +; compiler an uninitialized mode deciding what gets cleared. +; +; trailofbits/vspells-ct-internal-notes#24. + +; RUN: llc -mtriple=x86_64-unknown-linux-gnu %s -o - | FileCheck %s + +; A name this version of LLVM does not know. It clears the whole call-used set: +; the general-purpose registers, the vector registers and the x87 stack. +; CHECK-LABEL: unrecognized_mode: +; CHECK: fldz +; CHECK: xorl %ecx, %ecx +; CHECK: xorps %xmm15, %xmm15 +; CHECK-NEXT: retq +define i32 @unrecognized_mode(i32 %x) "zero-call-used-regs"="used-gpr-and-a-mode-from-the-future" { + ret i32 %x +} + +; A value that names nothing at all reads the same way. There is no mode here +; to be narrower than "all" either. +; CHECK-LABEL: empty_mode: +; CHECK: fldz +; CHECK: xorl %ecx, %ecx +; CHECK: xorps %xmm15, %xmm15 +; CHECK-NEXT: retq +define i32 @empty_mode(i32 %x) "zero-call-used-regs"="" { + ret i32 %x +} + +; The widest mode written out, for comparison: this is what the two above +; resolve to. +; CHECK-LABEL: widest_mode: +; CHECK: fldz +; CHECK: xorl %ecx, %ecx +; CHECK: xorps %xmm15, %xmm15 +; CHECK-NEXT: retq +define i32 @widest_mode(i32 %x) "zero-call-used-regs"="all" { + ret i32 %x +} + +; A mode that is recognized still means what it says. Widening applies to what +; could not be read, not to everything. +; CHECK-LABEL: narrow_mode: +; CHECK: # %bb.0: +; CHECK-NEXT: movl %edi, %eax +; CHECK-NEXT: xorl %edi, %edi +; CHECK-NEXT: retq +define i32 @narrow_mode(i32 %x) "zero-call-used-regs"="used-gpr" { + ret i32 %x +} + +; And "skip" is a name on the scale, not a failure to read one, so it keeps +; meaning skip. An unrecognized mode is the one case that has to widen, because +; it is the only one where nothing was said. +; CHECK-LABEL: skip_mode: +; CHECK: # %bb.0: +; CHECK-NEXT: movl %edi, %eax +; CHECK-NEXT: retq +define i32 @skip_mode(i32 %x) "zero-call-used-regs"="skip" { + ret i32 %x +} diff --git a/llvm/test/CodeGen/X86/zeroize-fallback-regs.ll b/llvm/test/CodeGen/X86/zeroize-fallback-regs.ll new file mode 100644 index 0000000000000..6ccb7bffe908b --- /dev/null +++ b/llvm/test/CodeGen/X86/zeroize-fallback-regs.ll @@ -0,0 +1,68 @@ +; The set of registers a "used" mode clears is a narrowing: it drops the +; registers the function never touched. A narrowing has to be able to justify +; every register it drops, and this one could not. Registers an instruction +; touches implicitly were not counted as touched, so they were dropped from the +; set and kept their contents past the return. +; +; Inline assembly is the case that made it visible, because every register an +; asm block names reaches the machine layer as an implicit operand: a function +; whose only register traffic was an asm block cleared nothing at all. +; +; trailofbits/vspells-ct-internal-notes#24. + +; RUN: llc -mtriple=x86_64-unknown-linux-gnu %s -o - | FileCheck %s + +; The asm writes a secret into a call-used argument register and declares it in +; the clobber list, which is the whole of what the compiler can know about it. +; That declaration is the function's only mention of %rdi, and it is implicit. +; CHECK-LABEL: asm_clobber: +; CHECK: #APP +; CHECK: #NO_APP +; CHECK-NEXT: xorl %edi, %edi +; CHECK-NEXT: retq +define void @asm_clobber() "zero-call-used-regs"="used-gpr" { + call void asm sideeffect "movq $$0x5ec4e7, %rdi", + "~{rdi},~{dirflag},~{fpsr},~{flags}"() + ret void +} + +; An output bound to a physical register is written down the same way, so it +; was missed the same way even though the asm block has a result in the IR. +; CHECK-LABEL: asm_output: +; CHECK: #APP +; CHECK: #NO_APP +; CHECK-NEXT: xorl %edi, %edi +; CHECK-NEXT: retq +define void @asm_output() "zero-call-used-regs"="used-gpr" { + %v = call i64 asm sideeffect "movq $$0x5ec4e7, $0", + "={rdi},~{dirflag},~{fpsr},~{flags}"() + ret void +} + +; Not only inline assembly: an instruction that defines a register on the side +; is opaque here in the same way. rdtsc leaves the counter in %eax and %edx and +; names neither, and the result is discarded, so nothing else in the function +; mentions them either. +; CHECK-LABEL: rdtsc_discarded: +; CHECK: # %bb.0: +; CHECK-NEXT: rdtsc +; CHECK-NEXT: xorl %eax, %eax +; CHECK-NEXT: xorl %edx, %edx +; CHECK-NEXT: retq +define void @rdtsc_discarded() "zero-call-used-regs"="used-gpr" { + %t = call i64 @llvm.x86.rdtsc() + ret void +} + +; The mode is still a narrowing, and this is what keeps the change honest: a +; function that touches no call-used register still clears none. Counting +; implicit operands widened "used" towards "all"; it did not collapse it into +; it. +; CHECK-LABEL: touches_nothing: +; CHECK: # %bb.0: +; CHECK-NEXT: retq +define void @touches_nothing() "zero-call-used-regs"="used-gpr" { + ret void +} + +declare i64 @llvm.x86.rdtsc() From 6f28360e1be35a4c62f499ab04e4adecc032902a Mon Sep 17 00:00:00 2001 From: Francesco Bertolaccini Date: Thu, 10 Sep 2026 15:47:02 +0200 Subject: [PATCH 4/5] [RISCV][test] Expect clears for implicit helper-call arguments Update zero-call-used-regs-fp.ll for the implicit operands now counted by the used-register scan. With +F and no +D, helper calls consume fa0 in used and a2/a3 (RV32) or a1 (RV64) in used_arg_double. Expect those dead argument registers to be cleared before returning. All six floating-point configurations pass with the updated checks, as do the two scalar configurations and the vector configuration. The new +F checks fail on the parent implementation, confirming they detect the additional clears. All invocations use -verify-machineinstrs. The assembly comparison confirms that return-value registers and ra are preserved. No production-code change is required for this test update. The complete CodeGen suite was not rerun. --- llvm/test/CodeGen/RISCV/zero-call-used-regs-fp.ll | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/llvm/test/CodeGen/RISCV/zero-call-used-regs-fp.ll b/llvm/test/CodeGen/RISCV/zero-call-used-regs-fp.ll index 47a5ff9035af7..a632df92ff8c0 100644 --- a/llvm/test/CodeGen/RISCV/zero-call-used-regs-fp.ll +++ b/llvm/test/CodeGen/RISCV/zero-call-used-regs-fp.ll @@ -33,6 +33,7 @@ define double @used(double noundef %a, float noundef %b) "zero-call-used-regs"=" ; 32-BITS-F-NEXT: .cfi_def_cfa_offset 0 ; 32-BITS-F-NEXT: li a2, 0 ; 32-BITS-F-NEXT: li a3, 0 +; 32-BITS-F-NEXT: fmv.w.x fa0, zero ; 32-BITS-F-NEXT: ret ; ; 32-BITS-D-LABEL: used: @@ -70,6 +71,7 @@ define double @used(double noundef %a, float noundef %b) "zero-call-used-regs"=" ; 64-BITS-F-NEXT: addi sp, sp, 16 ; 64-BITS-F-NEXT: .cfi_def_cfa_offset 0 ; 64-BITS-F-NEXT: li a1, 0 +; 64-BITS-F-NEXT: fmv.w.x fa0, zero ; 64-BITS-F-NEXT: ret ; ; 64-BITS-D-LABEL: used: @@ -187,6 +189,8 @@ define double @used_arg_double(double noundef %a, double noundef %b) "zero-call- ; 32-BITS-F-NEXT: .cfi_restore ra ; 32-BITS-F-NEXT: addi sp, sp, 16 ; 32-BITS-F-NEXT: .cfi_def_cfa_offset 0 +; 32-BITS-F-NEXT: li a2, 0 +; 32-BITS-F-NEXT: li a3, 0 ; 32-BITS-F-NEXT: ret ; ; 32-BITS-D-LABEL: used_arg_double: @@ -212,6 +216,7 @@ define double @used_arg_double(double noundef %a, double noundef %b) "zero-call- ; 64-BITS-F-NEXT: .cfi_restore ra ; 64-BITS-F-NEXT: addi sp, sp, 16 ; 64-BITS-F-NEXT: .cfi_def_cfa_offset 0 +; 64-BITS-F-NEXT: li a1, 0 ; 64-BITS-F-NEXT: ret ; ; 64-BITS-D-LABEL: used_arg_double: From 50f73ffdb8b622726f7b6af1b52fa5bed36786c8 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 12 Aug 2026 09:59:23 +0000 Subject: [PATCH 5/5] [CodeGen] Clear the registers the stack clear leaves data in 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. --- llvm/lib/CodeGen/PrologEpilogInserter.cpp | 250 +++++++++++++++++- llvm/test/CodeGen/ARM/zeroize-scratch-regs.ll | 35 +++ llvm/test/CodeGen/X86/zeroize-scratch-regs.ll | 103 ++++++++ 3 files changed, 376 insertions(+), 12 deletions(-) create mode 100644 llvm/test/CodeGen/ARM/zeroize-scratch-regs.ll create mode 100644 llvm/test/CodeGen/X86/zeroize-scratch-regs.ll diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp b/llvm/lib/CodeGen/PrologEpilogInserter.cpp index 524a621be8ca2..2c57e2d38d565 100644 --- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp +++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp @@ -87,6 +87,20 @@ static cl::opt PrintClearingSequence( cl::desc("Print the clearing sequence emitted at each in-scope exit, in " "the order its steps run")); +// A stand-in for a step that is not written yet. Clearing the stack frame is +// trailofbits/vspells-ct-internal-notes#26 and no target implements it, so the +// step that would declare the registers it worked through declares nothing, +// and the coverage the register clear gives those registers has no producer to +// exercise it. This option supplies one: it stands in for a target that clears +// the frame using the named registers. It makes the stack-clearing step +// declare them and emit nothing else, which is the part of a real +// implementation this file has to cope with. Hidden, and inert unless a test +// asks for it. +static cl::list StandInStackScratchRegs( + "pei-stack-clear-scratch-regs", cl::Hidden, cl::CommaSeparated, + cl::desc("Registers the stack-clearing step is to declare as the scratch " + "it used, standing in for the implementation of that step")); + namespace { //===----------------------------------------------------------------------===// @@ -283,9 +297,12 @@ class PEIImpl { ClearingDisposition planClearStack(MachineFunction &MF); ClearingDisposition planClearRegisters(MachineFunction &MF, BitVector &CandidateRegsToZero); + ClearingDisposition planClearRegistersForScratch( + MachineFunction &MF, BitVector &CandidateRegsToZero); void emitClearingStep(ClearingStep Step, const ExitClearingPlan &Plan, MachineBasicBlock &MBB, - MachineBasicBlock::iterator InsertPt); + MachineBasicBlock::iterator InsertPt, + BitVector &ScratchRegs); void diagnoseIgnoredZeroizeRequestsOnNakedFunction(MachineFunction &MF); public: @@ -1599,6 +1616,124 @@ static BitVector computeRegsToClearAtExit( return RegsToZero; } +//===----------------------------------------------------------------------===// +// Scratch registers. +// +// A step of the sequence that needs registers to do its work is not the last +// word on them. Clearing the frame reads the frame through a register 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. Those are the frame's +// contents by another name, and leaving with them in registers discloses +// exactly what leaving with them on the stack would have. +// +// Running the register clear after the stack clear is what makes destroying +// them possible, and ClearingSequence already fixes that order. Order alone is +// not coverage. What the register clear clears is chosen by the function's +// "zero-call-used-regs" mode, and every mode is a statement about the +// function: which registers it used, which of them are argument registers, +// which are general purpose. A register the clearing machinery 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 cannot be inferred from the function, and is declared by the +// step instead: a step records the registers it used, and the register clear +// adds what has been recorded to what it was already going to clear. Two +// things follow, and both are the point rather than a side effect: +// +// - 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. +// +// - 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 one anyway, over nothing but the declared +// registers: the request to clear the frame is not discharged while the +// frame's contents are sitting in registers. It is also why a target that +// cannot clear registers cannot clear the frame either, and is told so. +// +// A step may only declare a register whose value at the exit nothing depends +// on. That rules out the registers the exit itself needs -- 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 too. +//===----------------------------------------------------------------------===// + +/// The register named \p Name on this target, or a null register if it has no +/// register of that name. +static MCRegister findRegisterByName(const TargetRegisterInfo &TRI, + StringRef Name) { + for (unsigned Reg = 1, E = TRI.getNumRegs(); Reg != E; ++Reg) + if (Name.equals_insensitive(TRI.getName(Reg))) + return MCRegister(Reg); + return MCRegister(); +} + +/// Record in \p ScratchRegs the registers the ClearStack step used at this +/// exit. +/// +/// It used none: no target clears the frame, so the step emits nothing +/// (trailofbits/vspells-ct-internal-notes#26). What is declared here is what +/// -pei-stack-clear-scratch-regs names, which is how the declaration and its +/// consumption are exercised while the step that would make one does not +/// exist. An implementation of the step declares what it actually used, in +/// place of this. +static void declareStackClearScratchRegs(const TargetRegisterInfo &TRI, + BitVector &ScratchRegs) { + for (const std::string &Name : StandInStackScratchRegs) { + MCRegister Reg = findRegisterByName(TRI, Name); + if (!Reg) + report_fatal_error(Twine("unknown register name in " + "-pei-stack-clear-scratch-regs: '") + + Name + "'"); + ScratchRegs.set(Reg.id()); + } +} + +#ifndef NDEBUG +/// Whether \p Regs holds a register whose value at the exit something depends +/// on, and which therefore cannot be declared as scratch by a step of the +/// sequence. +static bool anyRegNeededAtExit(const BitVector &Regs, + const MachineBasicBlock &MBB, + MachineBasicBlock::const_iterator InsertPt, + const TargetRegisterInfo &TRI) { + const MachineFunction &MF = *MBB.getParent(); + + // A callee-saved register has to reach every exit holding what the caller + // left in it. Nothing at the exit names it, so the scan below would not find + // it. + for (const MCPhysReg *CSRegs = TRI.getCalleeSavedRegs(&MF); + MCPhysReg CSReg = *CSRegs; ++CSRegs) + for (MCRegister Reg : TRI.sub_and_superregs_inclusive(CSReg)) + if (Regs.test(Reg.id())) + return true; + + // What the exit needs is what the instructions after the sequence read or + // write, which is the same question computeRegsToClearAtExit answers for the + // candidate set. + for (const MachineInstr &MI : make_range(InsertPt, MBB.end())) + for (const MachineOperand &MO : MI.operands()) { + if (!MO.isReg() || !MO.getReg()) + continue; + for (MCPhysReg SReg : TRI.sub_and_superregs_inclusive(MO.getReg())) + if (Regs.test(SReg)) + return true; + } + + return false; +} +#endif + /// insertClearingSequences - Run the clearing sequence at every exit of \p MF /// that is in scope. /// @@ -1616,6 +1751,8 @@ void PEIImpl::insertClearingSequences(MachineFunction &MF) { if (!Plan.anyStepEmits() && !PrintClearingSequence) return; + const TargetRegisterInfo &TRI = *MF.getSubtarget().getRegisterInfo(); + raw_ostream &OS = errs(); if (PrintClearingSequence) OS << "clearing sequence for function '" << MF.getName() << "':\n"; @@ -1652,17 +1789,33 @@ void PEIImpl::insertClearingSequences(MachineFunction &MF) { OS << " " << printMBBReference(MBB) << " " << getExitKindName(*ExitMI) << ":"; + // What the steps in front of the register clear leave in registers. It is + // built as the sequence runs at this exit and read by the register clear + // at this exit; see the comment on scratch registers above. + BitVector ScratchRegs(TRI.getNumRegs()); + for (ClearingStep Step : ClearingSequence) { ClearingDisposition D = Plan.dispositionOf(Step); if (D == ClearingDisposition::Emit) - emitClearingStep(Step, Plan, MBB, InsertPt); + emitClearingStep(Step, Plan, MBB, InsertPt, ScratchRegs); if (PrintClearingSequence) OS << " " << getClearingStepName(Step) << "=" << getClearingDispositionName(D); } - if (PrintClearingSequence) + if (PrintClearingSequence) { + // Only when there are any, so that the line a function without a step + // that declares registers prints is the line it printed before. + if (ScratchRegs.any()) { + OS << " scratch="; + const char *Sep = ""; + for (unsigned Reg : ScratchRegs.set_bits()) { + OS << Sep << TRI.getName(Reg); + Sep = ","; + } + } OS << "\n"; + } } if (PrintClearingSequence) @@ -1671,31 +1824,52 @@ void PEIImpl::insertClearingSequences(MachineFunction &MF) { /// emitClearingStep - Emit one step of the clearing sequence at \p InsertPt. /// +/// \p ScratchRegs carries the registers the steps already run at this exit +/// used, and so left holding what they destroyed. A step adds the registers it +/// used to it, and the register clear reads it; see the comment on scratch +/// registers above. +/// /// A step that emits nothing today still has its case here, so that the /// implementation of it lands at the position the order gives it rather than /// wherever it is convenient. void PEIImpl::emitClearingStep(ClearingStep Step, const ExitClearingPlan &Plan, MachineBasicBlock &MBB, - MachineBasicBlock::iterator InsertPt) { + MachineBasicBlock::iterator InsertPt, + BitVector &ScratchRegs) { MachineFunction &MF = *MBB.getParent(); const TargetFrameLowering &TFI = *MF.getSubtarget().getFrameLowering(); const TargetRegisterInfo &TRI = *MF.getSubtarget().getRegisterInfo(); switch (Step) { case ClearingStep::ClearStack: - // Nothing emits here yet: no target can clear the frame, so planning has - // already refused every request for it and this step never reaches - // emission. It is first in the order because it needs registers to run, - // and the register clear after it is what destroys those. + // Nothing emits here yet: no target can clear the frame, so nothing is + // used and nothing real is declared; + // trailofbits/vspells-ct-internal-notes#26. The step is first in the order + // because clearing the frame needs registers to run, and it declares them + // here so that the register clear behind it destroys them. + declareStackClearScratchRegs(TRI, ScratchRegs); break; - case ClearingStep::ClearRegisters: + case ClearingStep::ClearRegisters: { // What to clear is settled here rather than in the plan, because it is the // exit that decides it: see computeRegsToClearAtExit. - TFI.emitZeroCallUsedRegs( - computeRegsToClearAtExit(Plan.CandidateRegsToZero, MBB, InsertPt, TRI), - MBB, InsertPt, RS); + BitVector RegsToZero = + computeRegsToClearAtExit(Plan.CandidateRegsToZero, MBB, InsertPt, TRI); + + // On top of that, whatever the steps in front of this one declared. The + // declarations are folded in after the exit has narrowed the candidates + // and not before, because narrowing them away is exactly what would + // happen: a declared register is one the sequence dirtied on the way here, + // not one the function used, and the narrowing is there to spare what the + // exit still needs. + assert(!anyRegNeededAtExit(ScratchRegs, MBB, InsertPt, TRI) && + "a step of the clearing sequence declared as scratch a register " + "whose value at the exit something depends on"); + RegsToZero |= ScratchRegs; + + TFI.emitZeroCallUsedRegs(RegsToZero, MBB, InsertPt, RS); break; + } case ClearingStep::ClearFlags: // Nothing emits here yet. It is last in the order because every step in @@ -1713,6 +1887,21 @@ void PEIImpl::planClearingSequence(MachineFunction &MF, ExitClearingPlan &Plan) { Plan.Stack = planClearStack(MF); Plan.Registers = planClearRegisters(MF, Plan.CandidateRegsToZero); + + // A step that runs in front of the register clear leaves the registers it + // worked through holding what it destroyed, and the register clear is what + // destroys those in turn. So once such a step runs, the register clear runs + // with it, whether or not the function asked for one: "zero-call-used-regs" + // is how a function asks for its own registers to be cleared, and these are + // not its registers, they are the sequence's. That includes a function that + // asked for "skip", which declines a clear of what the function itself left + // in registers and says nothing about what clearing its frame put there. A + // function with no step in front of the register clear is untouched by this. + if (Plan.Stack == ClearingDisposition::Emit && + Plan.Registers == ClearingDisposition::NotRequested) + Plan.Registers = + planClearRegistersForScratch(MF, Plan.CandidateRegsToZero); + // Nothing asks for the flags to be cleared and nothing clears them. The step // is planned all the same, so that the sequence a function runs is described // by the plan in full rather than in the parts that have an implementation. @@ -1727,6 +1916,14 @@ ClearingDisposition PEIImpl::planClearStack(MachineFunction &MF) { if (!F.hasZeroizeStack()) return ClearingDisposition::NotRequested; + // The stand-in for the implementation of this step answers the capability + // question instead of asking it, because what it stands in for is a target + // that has the capability. It emits nothing; what it does is declare the + // registers such a step would have used, so that what the rest of the + // sequence does with them can be exercised. + if (!StandInStackScratchRegs.empty()) + return ClearingDisposition::Emit; + const TargetFrameLowering &TFI = *MF.getSubtarget().getFrameLowering(); if (!TFI.supportsZeroizeStack(MF)) { F.getContext().diagnose(DiagnosticInfoUnsupported{ @@ -1740,6 +1937,35 @@ ClearingDisposition PEIImpl::planClearStack(MachineFunction &MF) { return ClearingDisposition::Unimplemented; } +/// planClearRegistersForScratch - Turn the ClearRegisters step on in a +/// function that did not ask for it, because a step in front of it does run +/// and will leave registers holding what it destroyed. +/// +/// The candidate set is left empty on purpose: nothing about the function +/// selects a register here, and what is cleared at each exit is exactly what +/// the steps in front of the register clear declare at that exit. +ClearingDisposition +PEIImpl::planClearRegistersForScratch(MachineFunction &MF, + BitVector &CandidateRegsToZero) { + const Function &F = MF.getFunction(); + const TargetFrameLowering &TFI = *MF.getSubtarget().getFrameLowering(); + const TargetRegisterInfo &TRI = *MF.getSubtarget().getRegisterInfo(); + + // A target that cannot clear registers cannot finish clearing the frame + // either: the sequence would end with the frame's contents in the registers + // it read them through, which is the disclosure the request was made to + // prevent. Report it rather than emitting the half that works. + if (!TFI.supportsZeroCallUsedRegs(MF)) { + F.getContext().diagnose(DiagnosticInfoUnsupported{ + F, "clearing the stack needs the registers it uses to be cleared " + "afterwards, which is not supported by this target"}); + return ClearingDisposition::Unsupported; + } + + CandidateRegsToZero.resize(TRI.getNumRegs()); + return ClearingDisposition::Emit; +} + /// planClearRegisters - Decide what the ClearRegisters step does in \p MF, and /// compute the registers it is allowed to clear. /// diff --git a/llvm/test/CodeGen/ARM/zeroize-scratch-regs.ll b/llvm/test/CodeGen/ARM/zeroize-scratch-regs.ll new file mode 100644 index 0000000000000..53aded08d066b --- /dev/null +++ b/llvm/test/CodeGen/ARM/zeroize-scratch-regs.ll @@ -0,0 +1,35 @@ +; The register clear is what finishes the stack clear's work: it destroys the +; registers the stack clear read the frame through. A target that cannot clear +; registers therefore cannot clear the frame either, and has to say so rather +; than emit the half of the sequence it can do. ARM implements neither, so it +; is where that can be pinned. +; +; As in the X86 test, -pei-stack-clear-scratch-regs stands in for the step that +; clears the frame, which no target implements +; (trailofbits/vspells-ct-internal-notes#26). + +; RUN: not llc -mtriple=armv7-unknown-linux-gnueabi -pei-stack-clear-scratch-regs=r4 < %s -o /dev/null 2>&1 | FileCheck %s + +; The function asked for its frame to be cleared and said nothing about its +; registers, so the register clear it gets is one it did not ask for. It is +; still a register clear, and this target cannot do one, so the request to +; clear the frame cannot be discharged. +; CHECK: error: {{.*}}in function stack_only i32 (i32): clearing the stack needs the registers it uses to be cleared afterwards, which is not supported by this target +define i32 @stack_only(i32 %x) "zeroize-stack"="used" { + ret i32 %x +} + +; A function that did ask for its registers to be cleared is refused on its own +; terms, by the query that has always answered that request, rather than being +; refused twice or reported as something it did not ask for. +; CHECK: error: {{.*}}in function asked_for_both i32 (i32): "zero-call-used-regs" is not supported by this target +; CHECK-NOT: in function asked_for_both {{.*}}clearing the stack needs +define i32 @asked_for_both(i32 %x) "zeroize-stack"="used" "zero-call-used-regs"="used-gpr" { + ret i32 %x +} + +; A function that asked for neither is not dragged into any of this. +; CHECK-NOT: in function untouched +define i32 @untouched(i32 %x) { + ret i32 %x +} diff --git a/llvm/test/CodeGen/X86/zeroize-scratch-regs.ll b/llvm/test/CodeGen/X86/zeroize-scratch-regs.ll new file mode 100644 index 0000000000000..c3cd6ee741352 --- /dev/null +++ b/llvm/test/CodeGen/X86/zeroize-scratch-regs.ll @@ -0,0 +1,103 @@ +; Clearing the stack frame needs registers to do it with, and leaves them +; holding what it took out of the frame. The register clear runs after it for +; that reason, but running after is not the same as covering: what the register +; clear covers is chosen by "zero-call-used-regs", and no mode selects a +; register the clearing machinery itself dirtied. So a step declares the +; registers it used and the register clear adds them to what it clears. +; +; No target clears the stack yet (trailofbits/vspells-ct-internal-notes#26), so +; the step that would declare anything declares nothing, and there is no +; producer to exercise this with. -pei-stack-clear-scratch-regs stands in for +; one. What that leaves demonstrable is one thing, and it is what is tested +; here: 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. The registers a +; real stack clear picks, and the code that picks them, are not tested here +; because they do not exist yet. + +; RUN: llc -mtriple=x86_64-unknown-linux-gnu -pei-stack-clear-scratch-regs=r11 < %s | FileCheck %s +; RUN: llc -mtriple=x86_64-unknown-linux-gnu -pei-stack-clear-scratch-regs=r11 -pei-print-clearing-sequence < %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=SEQ + +declare i32 @callee(i32) + +; %r11 is not a register this function uses, so "used-gpr" does not select it +; and the register clear would not have touched it. It is cleared because the +; step in front declared it. What the mode does select is still cleared, and +; what the exit needs is still spared: %eax carries the return value out. +; CHECK-LABEL: declared_reaches_the_clear: +; CHECK: movl %edi, %eax +; CHECK-NEXT: xorl %edi, %edi +; CHECK-NEXT: xorl %r11d, %r11d +; CHECK-NEXT: retq +define i32 @declared_reaches_the_clear(i32 %x) "zeroize-stack"="used" "zero-call-used-regs"="used-gpr" { + ret i32 %x +} + +; A function that asked for its frame to be cleared and said nothing about its +; registers still gets a register clear, over nothing but what was declared. +; Asking for the frame to be cleared and leaving its contents in a register is +; not a way of discharging the request. +; CHECK-LABEL: no_register_request: +; CHECK: movl %edi, %eax +; CHECK-NEXT: xorl %r11d, %r11d +; CHECK-NEXT: retq +define i32 @no_register_request(i32 %x) "zeroize-stack"="used" { + ret i32 %x +} + +; The same when the function asked for no register clear in so many words. +; "skip" declines a clear of what the function left in its registers; it says +; nothing about what clearing its frame put there, which is not the function's +; doing. %edi is left alone, which is what "skip" does mean. +; CHECK-LABEL: skip_is_still_covered: +; CHECK: movl %edi, %eax +; CHECK-NEXT: xorl %r11d, %r11d +; CHECK-NEXT: retq +define i32 @skip_is_still_covered(i32 %x) "zeroize-stack"="used" "zero-call-used-regs"="skip" { + ret i32 %x +} + +; The declaration is made and consumed at each exit, not once for the function, +; so every in-scope exit covers what the step used there. +; CHECK-LABEL: every_exit: +; CHECK: movl $1, %eax +; CHECK-NEXT: xorl %r11d, %r11d +; CHECK-NEXT: retq +; CHECK: movl $2, %eax +; CHECK-NEXT: xorl %r11d, %r11d +; CHECK-NEXT: retq +define i32 @every_exit(i32 %x) "zeroize-stack"="used" { +entry: + %c = icmp sgt i32 %x, 0 + br i1 %c, label %pos, label %neg + +pos: + ret i32 1 + +neg: + ret i32 2 +} + +; Nothing declares anything in a function whose frame is not being cleared, so +; the register clear covers what its mode selects and no more. This is the +; control for the tests above: without it they would pass just as well if the +; register clear had started clearing %r11 for some unrelated reason. +; CHECK-LABEL: no_stack_clear: +; CHECK: movl %edi, %eax +; CHECK-NEXT: xorl %edi, %edi +; CHECK-NEXT: retq +; CHECK-NOT: %r11d +define i32 @no_stack_clear(i32 %x) "zero-call-used-regs"="used-gpr" { + ret i32 %x +} + +; The sequence reports what was declared at each exit, so the declaration is +; visible without reading the registers back out of the emitted code. +; SEQ-LABEL: clearing sequence for function 'declared_reaches_the_clear': +; SEQ-NEXT: %bb.0 return: clear-stack=emitted clear-registers=emitted clear-flags=unimplemented scratch=R11 +; SEQ-LABEL: clearing sequence for function 'no_register_request': +; SEQ-NEXT: %bb.0 return: clear-stack=emitted clear-registers=emitted clear-flags=unimplemented scratch=R11 +; SEQ-LABEL: clearing sequence for function 'every_exit': +; SEQ-NEXT: %bb.1 return: clear-stack=emitted clear-registers=emitted clear-flags=unimplemented scratch=R11 +; SEQ-NEXT: %bb.2 return: clear-stack=emitted clear-registers=emitted clear-flags=unimplemented scratch=R11 +; SEQ-LABEL: clearing sequence for function 'no_stack_clear': +; SEQ-NEXT: %bb.0 return: clear-stack=not-requested clear-registers=emitted clear-flags=unimplemented