[PATCH] D143739: [WinEH][NFC] Assert for dynamic alloca in WinEH BBs

Wed May 10 09:50:34 PDT 2023

rnk added a subscriber: hans.
rnk added a comment.

Sorry for the delay, I was OOO, and David Majnemer has moved on to other projects. In the future, consider adding @hans for more visibility for WinEH related changes to help find a reviewer.

I think maybe it would be best to handle this in WinEHPrepare, which is where we check if the third argument operand of the `catchpad` instruction is an alloca. We could check there that the alloca is static if it exists, and it might also be healthy to report a backend error if some non-null, non-alloca value exists. Currently we silently ignore the value if it is not an alloca. It would be consistent with our current behavior to silently ignore dynamic allocas there, if you don't want to go through the trouble of reporting an error.

The next best place to report an error would be the verifier, but the validity of the arguments to the catchpad depend on the personality, so that would require teaching the verifier about more WinEH semantics.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp:173-174
           // Inform the Frame Information that we have variable-sized objects.
+          assert(!IsFuncletEHPersonality &&
+                 "CatchObjects must be fixed objects");
           MF->getFrameInfo().CreateVariableSizedObject(
----------------
This seems overly strict, I think it will assert if there is any dynamic alloca in a function using WinEH. I don't think that's what we want, we only want to assert if the catch object itself is a dynamic alloca, right?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143739