[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