[PATCH] D143108: [Asan] Add "funclet" OpBundle to Asan calls that are generated inside a funclet

[Reid Kleckner via Phabricator via llvm-commits]

rnk added inline comments.


================
Comment at: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp:706
+        continue;
+     assert(Colors.size() == 1 && "Expected monochromatic BB!");
+
----------------
saudi wrote:
> aeubanks wrote:
> > I'm unfamiliar with this code, why is this true?
> This is a constraint on the funclets in the Windows EH model.
> 
> An illustration in code is in WinEHPrepare pass `verifyPreparedFunclets` method here : https://github.com/llvm/llvm-project/blob/llvmorg-18-init/llvm/lib/CodeGen/WinEHPrepare.cpp#L1167
>  (from which I copied the assert)
> 
> In the documentation I found this: https://llvm.org/docs/ExceptionHandling.html#funclet-parent-tokens
> It explains that the funclet structure must represent a tree.
> 
> I'm not an expert in this, but my understanding is that in WinEH model, the funclets need to be structured as a tree; the coloring allows to identify which "node" each EH-related BasicBlock belongs to, having the constraint that the BB must belong to one and only one node.
> 
> @rnk can you confirm?
I think it is possible to construct cases where valid transforms will lead to multi-color blocks. Consider patches like https://reviews.llvm.org/D29428

I would actually strengthen the assert here to report an error via the LLVMContext. That seems to be the behavior that folks want.


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

https://reviews.llvm.org/D143108