[PATCH] SEH exceptions on Win64 (LLVM)

Nico Rieck nico.rieck at gmail.com
Fri Apr 25 21:13:44 PDT 2014

On 26.04.2014 00:33, Vadim Chugunov wrote:
> There's been much back-and-forth on the mailing list thread, so I am no longer sure which problems absolutely need to be fixed, and which were mentioned just in passing.   It would be helpful to restate them in one place (here probably).

I'll repeat what I wrote earlier. While I'm not in a position to
greenlight this, the getPrevNode, stack realignment issues and missing
tests *must* be fixed. Otherwise this code *will* segfault on valid input.

--- snip 8< ---
Unwind emission still suffers from the same problems that the old code
did. It doesn't handle realigned stacks, and as I summarized in PR16779,
I believe it's not necessary to change the prologue emission for Win64.
Currently ilist_node::getPrevNode of MachineInstr is unusable because it
segfaults on the first node in a list. I raised this issue before on
llvm-dev and proposed a patch but nobody cared, so you have to do
something like this:

  static const MachineInstr *getPrevNode(const MachineInstr *MI) {
    if (&*MI->getParent()->instr_begin() == MI)
      return nullptr;
    return MI->getPrevNode();

Some small nits:

> +      if (MI->getOperand(1).getImm() != 1 ||
MI->getOperand(2).getImm() != 0 ||
> +          MI->getOperand(4).getImm()  != 0)
Spacing. Also, a comment clarifying that scale/index/segment must be
unused might be helpful here.

+  if (MI->getNextNode()) {
+    const MachineInstr *MI2 = MI->getNextNode();
+    if (!(MI2->getFlag(MachineInstr::FrameSetup) ||
+          (MI2->isCFIInstruction() && MI2->getNextNode() &&
+           MI2->getNextNode()->getFlag(MachineInstr::FrameSetup)))) {
+      OutStreamer.EmitWin64EHEndProlog();
+    }
+  }

I'd simplify this and just skip over all CFI instructions. If there's
still a node left, and it lacks the FrameSetup flag, end the prolog.

> +; WIN64: callq {{__chkstk|___chkstk_ms}}

This check should really differentiate between MSVC and Mingw. You can
add additional FileCheck prefixes so the rest does not have to be

Overall this needs more tests covering __chkstk, alloca, manual
over-alignment and various combinations.
--- >8 snip ---


More information about the llvm-commits mailing list