llvm / llvm/llvm-project

[mlir] Liveness queries segfault on blocks created after analysis construction

Open
#208,140 1 comment 0 reactions 0 assignees View on GitHub
crash mlir
Dominant language
LLVM
Stars
40.5k
Forks
18.7k
PR merge metrics
PR metrics pending

Description

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.

Contributor guide

Open the contributing guide

Research direction

Start with the affected query methods in Liveness.cpp:199-254 and the class comment and typo in Liveness.h:46. Review how getLiveness handles blocks created after construction, then add the documented precondition, actionable assertions, and a unit test for its nullptr contract. Done means the stale-block behavior is documented and tested without plain nullptr dereferences.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
compilers
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.