[PATCH] D135270: [WinEH] Validate funclet tokens in colorEHFunclets()

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 7 12:42:00 PDT 2022


rnk added a comment.

In D135270#3843045 <https://reviews.llvm.org/D135270#3843045>, @sgraenitz wrote:

> In D135270#3837735 <https://reviews.llvm.org/D135270#3837735>, @rnk wrote:
>
>> This is an analysis function, which is called during instrumentation passes, and I don't think we should emit errors from an analysis helper.
>
> Fair point. I moved it into the first caller in WinEHPrepare.
>
>> What I had in mind was to essentially emit an error when the ColorVector has a size of more than one. That represents a situation where we would start cloning basic blocks in WinEHPrepare.
>
> This is done already as a debug check in verifyPreparedFunclets() <https://github.com/llvm/llvm-project/blob/release/15.x/llvm/lib/CodeGen/WinEHPrepare.cpp#L1038> at the end of prepareExplicitEH().
>
> It's not happening in our repro though! All basic blocks are assigned exactly one color. The `catchret.dest` block is marked implausible, because it refers to another color's funclet pad. As far as I can tell, this isn't verified anywhere. I adjusted the patch for this specific situation.

Removing implausible terminators happens before that check, and it's supposed to make it so that the check passes and all blocks have a unique color. I guess what I'm suggesting is:

- let's just delete, or flag off, this call to remove implausible terminators
- always call `verifyPreparedFunclets`
- replace the `report_fatal_error` calls with error diagnostics


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135270



More information about the llvm-commits mailing list