Skip to content

Function parameter corruption when using tail call optimization in Windows 64 #9382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
llvmbot opened this issue Jan 20, 2011 · 4 comments
Closed
Labels
backend:X86 bugzilla Issues migrated from bugzilla

Comments

@llvmbot
Copy link
Member

llvmbot commented Jan 20, 2011

Bugzilla Link 9010
Resolution FIXED
Resolved on Mar 09, 2011 05:16
Version trunk
OS Windows XP
Blocks llvm/llvm-bugzilla-archive#9100
Attachments .ll and generated .s file
Reporter LLVM Bugzilla Contributor

Extended Description

Tail call optimization is erroneously applied in Windows 64.
As result stack area which contains function parameters is released (RSP is updated) before the call (replaced by jump in tail call optimization). It may cause to wrong function behavior. See attached .s example. Specifically these 3 lines may explain the problem:

lea R8, QWORD PTR [RSP + 32] ; using stack to for parameter storage
...
add RSP, 232 ;; Stack is freed and allocated parameter with it
jmp testcall # TAILCALL

Attached are .ll and .s file generated with latest llc.

The simple inlined fix is solves the problem.

Index: lib/Target/X86/X86ISelLowering.cpp

--- lib/Target/X86/X86ISelLowering.cpp (revision 2609)
+++ lib/Target/X86/X86ISelLowering.cpp (working copy)
@@ -2501,6 +2501,9 @@
SmallVector<CCValAssign, 16> ArgLocs;
CCState CCInfo(CalleeCC, isVarArg, getTargetMachine(),
ArgLocs, *DAG.getContext());

  • if (Subtarget->isTargetWin64()) {
  • CCInfo.AllocateStack(32, 8); 
    
  • }
    CCInfo.AnalyzeCallOperands(Outs, CCAssignFnForNode(CalleeCC));
    if (CCInfo.getNextStackOffset()) {
    MachineFunction &MF = DAG.getMachineFunction();
@llvmbot
Copy link
Member Author

llvmbot commented Jan 20, 2011

Please refer to my patches! :D
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20110110/114977.html

Even if my patches were applied, your example might not be emitted optimally.
I know my patch would be suboptimal.

Comments and additional patches are welcome!

@llvmbot
Copy link
Member Author

llvmbot commented Jan 20, 2011

Sorry, Nakamura, I didn't see it.
Obviously your patches will solve the bug.

Please refer to my patches! :D
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20110110/114977.html
Even if my patches were applied, your example might not be emitted optimally.
I know my patch would be suboptimal.
Comments and additional patches are welcome!

@llvmbot
Copy link
Member Author

llvmbot commented Mar 9, 2011

It has been fixed in r124272 and around.

I close this. Suboptimal cases (eg. this might be tailcallable) will be filed as another issue.

Thank you.

@llvmbot
Copy link
Member Author

llvmbot commented Nov 27, 2021

mentioned in issue llvm/llvm-bugzilla-archive#9100

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 3, 2021
augusto2112 pushed a commit to augusto2112/llvm-project that referenced this issue Oct 9, 2024
[lldb] Introduce precise Fallback SwiftASTContexts.
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:X86 bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

1 participant