[PATCH] D124642: [WIP] Add support for return from an SEH __finally block.

[Eli Friedman via Phabricator via cfe-commits]

efriedma created this revision.
efriedma added reviewers: asmith, tentzen, rnk, mstorsjo, JosephTremoulet, dpaoliello, rjmccall.
Herald added subscribers: pengfei, hiraditya.
Herald added a project: All.
efriedma requested review of this revision.
Herald added a subscriber: jdoerfert.
Herald added projects: clang, LLVM.

MSVC has a warning C4532 for jumping out of a finally block. However, that only specifically covers "break", "continue", and "goto".  There's no warning for "__leave" and "return".

It's not completely clear to me why MSVC considers "__leave" and "return" special, compared to other sorts of control flow.  My best guess is that return-in-finally is so pervasive across drivers that the warning would be way too noisy.  An alternative theory is that MSVC generates a call to _local_unwind in the normal path for certain uses of break/continue/goto, and that's somehow a hazard for unwinding.

In any case, the end result is that "return" shows up somewhat frequently in "__finally" blocks in Windows drivers, but I haven't seen any of the others.

This patch generates code that's similar to what MSVC generates: it branches directly to the return block for normal code, and uses _local_unwind to break out of exception handling.

Currently, this only handles "return". Adding support for other ways of exiting a __finally blocks should be straightforward on top of this; it's mostly a question of managing the different possible destinations.

Some basic testing shows this seems to do the right thing in simple cases.

__try/__finally inside the __try of a __try/__finally is broken because I can't figure out how to get _local_unwind to actually stop unwinding before it runs the outer __finally funclet. See comment in WinEHPrepare.cpp.  I feel like I'm not understanding something, but there isn't really any documentation I can refer to for _local_unwind.  I'm hoping someone from Microsoft can explain how that's supposed to work.

__try/__finally inside a __finally block is broken because recoverAddrOfEscapedLocal can't recover the address of a variable allocated inside a __finally block.  (Here specifically, we compute the wrong value for SEHRetNowParent, but it's a general issue affecting any variable declared inside a __finally block.)  We probably need to store the frame pointer in an alloca in the parent function, and then load it later so we can recover the pointer we want, or something like that.

I'm not happy with the way _local_unwind unwinding is represented. Primarily, I don't like the way it abuses "blockaddress" to compute the address of the catchpad. Suggestions welcome. I don't want to spend too much time thinking about it before I'm sure I understand the "__try/__finally inside __try" issue, though.

Based on the patch by Ten Tzen, particularly the code to call _local_unwind.  I rewrote a bunch of the code to simplify it, and avoid using _local_unwind in cases where it isn't necessary.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124642

Files:
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/CodeGen/WinEHPrepare.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124642.425911.patch
Type: text/x-patch
Size: 19865 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220428/06c0fc76/attachment-0001.bin>