mkovacevic99 wrote:

> > With the patch, clang's CodeGen emits a call to `__cxa_guard_abort` that 
> > shouldn't be emitted when the following code is compiled:
> 
> I think it's okay to emit the call to `__cxa_guard_abort` in the IR, but the 
> function shouldn't be called at runtime when `mayThrow` throws.

Fix is working and verified on both ABIs, with and without exceptions. Two 
things are fixed: the ICE (#188058), and the 
__cxa_guard_abort-on-the-wrong-path bug you found.

Mechanically: I wrap the guarded initialization in a ConditionalEvaluation, so 
the block-capture cleanups take the conditional pushLifetimeExtendedDestroy 
path — they get a saved (dominating) address and an active flag, which fixes 
the dominance error and keeps them alive to end of scope (so the 
single-activation b() case you mentioned stays correct). To fix the guard-abort 
itself, I push it inactive and ActivateCleanupBlock after __cxa_guard_acquire, 
so its active flag defaults to false on the already-initialized path.

The one wart I want your call on: because the init is now a 
ConditionalEvaluation, that guard-abort active flag gets emitted on every 
thread-safe guarded static init with a throwing constructor — including simple 
non-block ones that previously had no flag. It's correctness-neutral but it's 
real codegen churn. I couldn't find a clean way to gate the flag to only the 
case where the guard cleanup actually gets buried by the block-capture 
cleanups, since we don't know about the burial until after the initializer runs.

So: is that churn acceptable, or do you see a clean way to limit the flag to 
the buried case? And more generally — is this the approach you'd take, or do 
you have a different one in mind / an existing mechanism I should be reusing? 
You know this code far better than I do.

(If it turns out we don't need the single-activation escape case after all — 
efriedma was skeptical it's ever what anyone wants — there's a much smaller 
alternative: just destroy the captures at the end of the initializer, no 
conditional-eval and no churn. Happy to go that way instead if you both prefer.)
[guarded-init-block-capture.patch](https://github.com/user-attachments/files/29101634/guarded-init-block-capture.patch)


https://github.com/llvm/llvm-project/pull/199508
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to