[PATCH] D86528: [4/5] [MC] [Win64EH] Fill in FuncletOrFuncEnd if missing

[Martin Storsjö via Phabricator via llvm-commits]

mstorsjo added inline comments.


================
Comment at: llvm/lib/MC/MCWin64EH.cpp:511
+    // the text section, so we weren't able to automatically add a
+    // FuncletOrFuncEnd label.
     RawFuncLength = 0;
----------------
efriedma wrote:
> I don't understand why you're dropping the FIXME; clearly, if we're emitting unwind info that doesn't cover anything, something has gone very wrong.
Right, the FIXME indeed should stay, if we still hit that case, even though it's much less probable after this change.


================
Comment at: llvm/lib/MC/MCWin64EH.cpp:671
+        Streamer.getCurrentSectionOnly() == CFI->TextSection)
+      CFI->FuncletOrFuncEnd = Streamer.emitCFILabel();
     MCSection *XData = Streamer.getAssociatedXDataSection(CFI->TextSection);
----------------
efriedma wrote:
> It seems like you're setting FuncletOrFuncEnd in too many places; there should be exactly one directive that corresponds to the end of the function, I think.
There's actually two different codepaths; either in `MCStreamer::EmitWinCFIEndProc when running into `.seh_endproc`, or in `ARM64UnwindEmitter::EmitUnwindInfo` when `.seh_handlerdata` forces emitting it while halfway through the function. But this one can be left out, as far as I see with the cases I have in mind.

The case with `.seh_handlerdata` is tricky, because it forces writing the xdata entry possibly before the full function has been generated. On x86 this is not an issue, because the xdata entry doesn't contain the full function length but that's referenced in the .pdata via relocations to the function start/end. But on arm, the xdata entry needs to know the full length. So for cases with `.seh_handlerdata`, we can place `FuncletOrFuncEnd` at the place where we are, and hope that it covers the parts of the function that unwinding will occur in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86528