[PATCH] D90216: [stack-clash] Fix probing of dynamic alloca

Josh Stone via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Oct 27 13:26:26 PDT 2020


cuviper added a comment.

I tried the C reproducer from that Rust bug, and it turns out that `alloca(0)` was already fixed by D88548 <https://reviews.llvm.org/D88548>. But if I change that to a larger size, then it was skipping right over the probe loop. I confirmed that your change here does make it go through the loop.

It would be nice to have tests that actually execute this stuff, rather than just relying on manual reviews of the expected assembly, but I don't know if that's feasible in the current infrastructure.



================
Comment at: llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll:29-30
 ; CHECK-X86-64-NEXT:  .LBB0_2: # =>This Inner Loop Header: Depth=1
 ; CHECK-X86-64-NEXT:  	movq	$0, (%rsp)
 ; CHECK-X86-64-NEXT:  	subq	$4096, %rsp # imm = 0x1000
 ; CHECK-X86-64-NEXT:  	cmpq	%rsp, %rax
----------------
Shouldn't this `sub` before `mov 0`? I think right now, the first iteration is going to clobber the most recent thing on the stack, in this case the saved value from `pushq %rbp`.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90216/new/

https://reviews.llvm.org/D90216



More information about the llvm-commits mailing list