You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
What we have here is two calls to mypanic, one with 7 args, one with 8. One call does two pushes, and the other does one. If you interpret the .cfi_* directives as written, they are only correct if the program executes the second 'if' branch, not the first. If the program executes the first 'if' and attempts to unwind the stack, the wrong return address will be used.
Perhaps the CFI inserter pass could fix this by adding the full cfa offset to each .cfi_adjust_cfa_offset instruction, or by adding some kind of no-op marker instruction to the end of the MBB that encodes the cfa. This would prevent tail merging from considering these blocks to be identical.
The text was updated successfully, but these errors were encountered:
Thank you for a concise description of the problem; I agree with the description (though I don't know that the noreturn attribute is required to reproduce).
Perhaps the CFI inserter pass could fix this by adding the full cfa offset
to each .cfi_adjust_cfa_offset instruction, or by adding some kind of no-op
marker instruction to the end of the MBB that encodes the cfa. This would
prevent tail merging from considering these blocks to be identical.
ComputeCommonTailLength() in llvm/lib/CodeGen/BranchFolding.cpp calls skipBackwardPastNonInstructions() calls countsAsInstruction() to skip over debug instructions and CFI instructions. Perhaps it should not?
If you interpret the .cfi_* directives as written, they are only correct if the program executes the second 'if' branch, not the first. If the program executes the first 'if' and attempts to unwind the stack, the wrong return address will be used.
llvm/lib/Target/X86/X86FrameLowering.cpp seems to be doing a lot of .cfi_* directive emission. Perhaps a bug in there?
(side note: looking at the generated asm from this case, I also noticed that it seems that in the parameter setup for the call, by pushing the stack args last, we pessimize our ability to zip common tails together. If we pushed %r10 first, then we could zip more of the common tails)
Extended Description
This comes up in the Linux kernel, as reported here:
ClangBuiltLinux/linux#612
https://lkml.org/lkml/2019/7/16/759
Consider:
$ cat t.c
$ clang -Os t.c -S -o -
What we have here is two calls to mypanic, one with 7 args, one with 8. One call does two pushes, and the other does one. If you interpret the .cfi_* directives as written, they are only correct if the program executes the second 'if' branch, not the first. If the program executes the first 'if' and attempts to unwind the stack, the wrong return address will be used.
Perhaps the CFI inserter pass could fix this by adding the full cfa offset to each .cfi_adjust_cfa_offset instruction, or by adding some kind of no-op marker instruction to the end of the MBB that encodes the cfa. This would prevent tail merging from considering these blocks to be identical.
The text was updated successfully, but these errors were encountered: