[PATCH] D53541: [COFF, ARM64] Do not emit x86_seh_recoverfp intrinsic

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

In D53541#1358210 <https://reviews.llvm.org/D53541#1358210>, @rnk wrote:

> I can't compile the example you gave yet because I haven't applied your patches locally, but this is the "three stack pointer" case that I have in mind:
>
>   struct Foo {
>     void (*ptr)();
>     int x, y, z;
>   };
>  
>   void escape(void *);
>   void large_align(int a0, int a1, int a2, int a3, int a4, int a5, int a6, int a7,
>                    int stackarg) {
>     struct Foo __declspec(align(32)) alignedlocal;
>     alignedlocal.x = 42;
>     int vla[a0];
>     escape(&alignedlocal);
>     vla[0] = stackarg;
>     escape(&vla[0]);
>   }
>
>
> This is LLVM's generated code:
>
>   "?large_align@@YAXHHHHHHHHH at Z":         ; @"?large_align@@YAXHHHHHHHHH at Z"
>   .seh_proc "?large_align@@YAXHHHHHHHHH at Z"
>   ; %bb.0:                                ; %entry
>           stp     x21, x22, [sp, #-48]!   ; 16-byte Folded Spill
>           stp     x19, x20, [sp, #16]     ; 16-byte Folded Spill
>           stp     x29, x30, [sp, #32]     ; 16-byte Folded Spill
>           add     x29, sp, #32            ; =32
>           sub     x9, sp, #48             ; =48
>           and     sp, x9, #0xffffffffffffffe0
>           mov     x19, sp
>           mov     w8, #42
>           str     w8, [x19, #8]
>           mov     w8, w0
>           ldr     w22, [x29, #16]
>           lsl     x8, x8, #2
>           add     x8, x8, #15             ; =15
>           lsr     x15, x8, #4
>           mov     x21, sp
>           bl      __chkstk
>           mov     x8, sp
>           sub     x20, x8, x15, lsl #4
>           mov     sp, x20
>           add     x0, x19, #0             ; =0
>           bl      "?escape@@YAXPEAX at Z"
>           mov     x0, x20
>           str     w22, [x20]
>           bl      "?escape@@YAXPEAX at Z"
>           mov     sp, x21
>           sub     sp, x29, #32            ; =32
>           ldp     x29, x30, [sp, #32]     ; 16-byte Folded Reload
>           ldp     x19, x20, [sp, #16]     ; 16-byte Folded Reload
>           ldp     x21, x22, [sp], #48     ; 16-byte Folded Reload
>           ret
>
>
> I see three pointers used to address the stack:
>
> - sp: to address vla
> - x19: to address locals, the so-called "base" pointer
> - x29: to address parameters on the stack, looks like the traditional FP, points to FP+LR pair as well
>
>   At least for x86, the unwind info doesn't describe X19, it just describes X29, since that's what you need to restore CSRs and find the parent stack frame. We saw that the Windows EH runtime passes in some value based on the unwind info. For x86, it was just whatever EBP held, so recoverfp simply aligns that value forward to recover the base pointer (ESI) and then uses that to recover locals. For x64, the runtime passes in the value of RSP after the prologue ends, so we adjust it by the "parent frame offset" to get back the value we put in RBP. It looks like for x64 we never handled the case I'm asking you about, because this program doesn't print the right value: ``` #include <stdio.h> struct Foo { void (*ptr)(); int x, y, z; }; int filter(int n) { printf("o.x: %d\n", n); return 1; } void may_throw() { __builtin_trap(); } int main() { struct Foo __declspec(align(32)) o; o.x = 42; __try { may_throw(); } __except(filter(o.x)) { } } ```
>
>   I get "o.x: 0" instead of 42. I bet we can find something about that in bugzilla somewhere. =/
>
>   So, hopefully that explains the intended purpose of `llvm.x86.seh.recoverfp`, and why we might need to generalize it into something non-x86 specific. Maybe `llvm.eh.recoverfp`. Let me know if I can clarify anything else.


@rnk Thanks a lot for the clarification. Yes, I see o.x: 0 instead of 42. Supporting this case would mean implementing recoverfp as well as support generating the correct parent frame offset for arm64 windows. Do you think this can be done in a follow-up patch? So this patch and D53540 <https://reviews.llvm.org/D53540> would add the basic support for SEH and we can go fix corner/more complex cases in follow-up patches. There are also several more comprehensive test cases in https://github.com/Microsoft/windows_seh_tests which we plan to address next.


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

https://reviews.llvm.org/D53541