[clang] [llvm] [BasicAA] Fix miscompilation with setjmp/longjmp due to missing longjmp re-entry paths in alias analysis (PR #212297)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 7 23:03:38 PDT 2026
midhuncodes7 wrote:
@efriedma-quic @nikic @antoniofrighetto
Per the discussion above, I've reworked this to drop the `contains_returns_twice_call` attribute entirely and instead cache the check locally on `EarliestEscapeAnalysis: containsReturnsTwiceCall()` lazily computes and memoizes `Function::callsFunctionThatReturnsTwice()` once per `EarliestEscapeAnalysis` instance, replacing the `hasFnAttribute(Attribute::ContainsReturnsTwiceCall)` check in `getCapturesBefore`.
While testing this, two existing Clang tests started failing:
```
- Clang :: CodeGenObjC/exceptions.m
- Clang :: CodeGenObjC/synchronized.m
```
Both exercise the Darwin "fragile" ObjC exception model, where `@try/@synchronized` lower to a hand-built `_setjmp` call in `CGObjCMac.cpp` via EmitNounwindRuntimeCall + SetJmpResult->`setCanReturnTwice()`, bypassing
`CodeGenFunction::EmitCall` entirely. That matters here under the old attribute-based approach, Clang only ever set `contains_returns_twice_call` from inside `EmitCall`, so it was never set on these ObjC functions, and BasicAA never engaged its returns-twice handling for them. With the new scan-based check, `callsFunctionThatReturnsTwice()` finds this `_setjmp` call directly from its IR-level `returns_twice` attribute (set via `setCanReturnTwice()`), regardless of how it was emitted, so BasicAA now correctly treats locals as capturable across it.
Concretely, in `exceptions.m`'s f2(), the test comment for the failing check even documents the assumption being invalidated ("Landing pad. Note that we elide the re-enter.") the old codegen forwarded `x++` to a plain `store i32 6, ptr %x` right after the landing pad, which is only sound if `%x` isn't visibly mutated via a `longjmp` driven re-entry at the `_setjmp` which is the exact assumption issue #[198967](https://github.com/llvm/llvm-project/issues/198967) is about, just reached through ObjC's exception lowering instead of libc `setjmp`/`longjmp` directly.
So this looks like a real instance of the same bug class, now correctly caught by the more thorough scan-based check rather than a regression to work around. My inclination is that the right fix is to update the CHECK lines in both tests to match the new, more conservative codegen but before I do that, I'd like a second opinion: is that the right call here, or is there a preference for handling ObjC's exception-lowering `setjmp` differently (e.g., should `CGObjCMac.cpp`'s hand built call route through the normal attribute machinery instead, for consistency with source-level `setjmp` calls)?
https://github.com/llvm/llvm-project/pull/212297
More information about the cfe-commits
mailing list