| Issue |
208140
|
| Summary |
[mlir] Liveness queries segfault on blocks created after analysis construction
|
| Labels |
|
| Assignees |
|
| Reporter |
JPL11
|
Several `mlir::Liveness` query methods dereference `getLiveness(block)` without checking for null, so they segfault when queried about a block that did not exist when the analysis was constructed:
- `Liveness::getLiveIn` / `Liveness::getLiveOut` (Liveness.cpp:231-238) do `getLiveness(block)->in()` / `->out()` directly.
- `Liveness::isDeadAfter` (Liveness.cpp:241-254) dereferences the `LivenessBlockInfo *` without a check.
- `Liveness::resolveLiveness` (Liveness.cpp:199-219) dereferences both the block's info and `getLiveness(successor)` in its worklist loop.
`getLiveness` itself is null-safe (it returns nullptr for unmapped blocks), but nothing documents that the other queries require a covered block, and the failure mode is a plain nullptr dereference rather than a diagnosable assertion.
This is easy to hit in practice with any transformation that creates blocks after constructing the analysis and then queries liveness for them — e.g. splitting critical edges, or blocks created by signature conversion during dialect conversion. We hit it twice downstream in IREE while replacing its own liveness implementation with upstream `mlir::Liveness` (iree-org/iree#24687): once via blocks created by dialect conversion, and once via a pass that split critical edges between computing liveness and querying it. In both cases the symptom was a segfault deep inside the query with no hint that the analysis was stale.
Proposal (patch ready, will open a PR shortly):
1. Document the precondition on the class and the affected methods: queries expect blocks that were part of the analyzed operation at construction time; `getLiveness` returns nullptr for unknown blocks and can be used to check coverage.
2. Add `assert(blockInfo && "expected block to be covered by the analysis")` at the dereference sites so debug builds fail with an actionable message instead of a segfault.
3. Add a unit test covering `getLiveness`'s nullptr contract for blocks created after construction.
An alternative would be to make the queries gracefully return empty sets / conservative answers for unknown blocks, but that is a semantic change (it can silently hide stale-analysis bugs — that is exactly what IREE's old implementation did, and it masked a real miscompile-adjacent bug for years), so I'd rather keep it a documented precondition unless there's appetite for recomputing on demand.
There is also a small typo in the `Liveness` class doc comment (`bool isDeafAfter = liveness.isDeadAfter(...)`, Liveness.h:46) that the patch fixes in passing.
![]()
_______________________________________________
llvm-bugs mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-bugs
