differing stack sizes via differing paths to variadic calls that push different number of args to the stack

[nickdesaulniers]

From the discussion with @jpoimboe in https://lore.kernel.org/r/20190716231718.flutou25wemgsfju@treble/:

It seems that LLVM is producing differing stack sizes when two paths eventually call a noreturn function. This wouldn't result in observed runtime behavior, but could mess up unwinding.

I should:

1. figure out how to reproduce
2. file a bug upstream in LLVM's issue tracker

[nickdesaulniers]

@arndb came up with this test case:

int a, b;
void __reiserfs_panic(int, ...) __attribute__((noreturn));
void balance_internal() {
  if (a)
    __reiserfs_panic(0, "", __func__, "", 2, __func__, a);
  if (b)
    __reiserfs_panic(0, "", __func__, "", 5, __func__, a, 0);
}

https://godbolt.org/z/Byfvmx

And mentioned that "The point is that the stack is different
between the two branches leading up to the noreturn call."

[nickdesaulniers]

https://lore.kernel.org/r/CAKwvOdm_MiACJWnRww3tSD7033J6MX2Erzs1xwmd1=taNmyg9A@mail.gmail.com/

[nickdesaulniers]

It looks like "branch-folder" is tail merging blocks that end in calls even when the calls push additional arguments on the stack. It probably shouldn't do that, or do so more carefully. When it computes the longest "chain" between two branches that flow together, it thinks the "call" instructions are identical, which is dubious since if the function accepts or can accept 6 or more arguments then it's only safe to merge if the callers push the same number of arguments.

I don't see any existing interfaces to query the number of function parameters that were pushed on the stack, but it would seem that calls to the same variadic function with differing numbers of args pushed to the stack are not identical and should not be candidates for tail merging. (ie. MachineInstr::isIdenticalTo or ComputeCommonTailLength should check for this).

@topperc do you know? Given a call MachineInstr, how can I find how many parameters were passed on the stack? Or even how many total parameters? MachineInstr::isVariadic() doesn't seem to be what we need here. I assume it's ISEL that decides how many parameters to pass on the stack vs in registers? At the MachineInst layer, is there any way to find out given the call MachineInst? Perhaps ISEL can coordinate and note such info on the MachineInstr?

Conservatively, I can check whether the GlobalValue of the first operand to the call is a variadic function, and not merge these. But for the case where it was variadic with the same number of parameters we'd miss out (or no parameters passed on the stack).

[topperc]

@rnk can you help?

[rnk]

My first reaction is that this is a bug in LLVM. If the same MBB is entered with different stack levels, there's no way to emit proper unwind instructions for it, and that's an issue for everyone, not just LLVM.

As a workaround, try disabling shrink wrapping (I forget the flag, sorry).

[rnk]

I looked at this again, and I guess shrink wrapping isn't involved here. This function doesn't have any prologue to speak of. You can use a different workaround of disabling x86 call frame optimization, -mllvm -no-x86-call-frame-opt. That will disable the store to push conversion optimization, and generally makes stack unwinding much simpler by establishing one level stack pointer throughout the function, which might be important to the Linux kernel.

[rnk]

I filed https://llvm.org/pr49130 for this. I don't think you need to include objtool to see what the issue is, I think the .cfi_* directives that LLVM produces itself aren't quite right. The most straightforward fix is probably to make some changes to the CFI inserter pass.