Skip to content

Make the queue ownership analysis sound - #3

Closed
ZweZeya wants to merge 1 commit into
fix/lift-lower-crashesfrom
fix/analysis-soundness
Closed

ZweZeya wants to merge 1 commit into
fix/lift-lower-crashesfrom
fix/analysis-soundness

Conversation

@ZweZeya

@ZweZeya ZweZeya commented Oct 18, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #2 (fix/lift-lower-crashes).

What this changes

Closes the three soundness defects in docs/audit.md, each of which could make a queue look single-producer or single-consumer when it is not:

  • Spawn in a loop counted as one thread. A site now counts as possibly many when it is in a loop or a CFG cycle, or in a function that may itself run more than once.
  • main had no thread, so its accesses were dropped. It has one now.
  • Parameter index was ignored (argument zero was taken for every parameter). Each instance records its call site and binds parameter i to argument i.

Also two precision fixes: resolution follows every instance on a thread, not just the first; thread contexts are computed to a fixed point instead of recursion over a cache seeded with empty placeholders.

Indirect calls are still not followed, but no longer invisible: a function whose address escapes, or that nothing calls, runs on an "unknown" thread, and every queue it touches is UNKNOWN. A spawn's entry is the first function-address argument, not operand zero (the sret slot on x86-64). Output follows IR order.

How it was checked

23 of 23 pass, no warnings. The eight new or rewritten defect tests fail against the parent, each for the reason it names (worker pool called SPSC; main's push dropped, SPMC instead of MPMC; b's context lost under mutual recursion; second parameter's queue attributed to the first). The prototype's example, compiled for AArch64 and x86-64, gives SPSC for q1 and MPMC for q2 on both.

Checklist

  • CI is green
  • New behaviour has a test; a fixed bug has a test that failed before
  • No file moves or renames mixed in with substantive changes
  • The pinned LLVM revision is untouched, or this PR does nothing else

🤖 Generated with Claude Code

@ZweZeya
ZweZeya requested a review from apaznikov as a code owner October 18, 2026 05:15
@ZweZeya ZweZeya mentioned this pull request Oct 18, 2026
4 tasks done
Three defects let the analysis call a queue single-producer or
single-consumer when it is not, and substituting on such a verdict
introduces a race:

- A thread id was handed out per static spawn site, so a worker pool
  started in a loop counted as one producer. A site now counts as
  possibly many threads when it sits in a loop or a CFG cycle, or in a
  function that may itself run more than once.
- main had no thread, so its pushes and pops, and those of everything it
  calls, were dropped. It now has one.
- A parameter was resolved by taking argument zero of the call, whatever
  the parameter's position, so a function taking two queues merged them.
  Each instance of a function now records its call site and binds each
  parameter to the argument at its own position.

Two precision problems go with them. Resolution followed only the first
instance on a thread, so a helper called with two queues reached one;
it now follows every instance. Thread contexts were computed by recursion
over a cache seeded with empty placeholders, so mutual recursion left
some functions with none, depending on visiting order; they are now
computed to a fixed point.

Indirect calls are still not followed, but no longer invisible. A
function whose address is used other than as a spawn's entry, or that
nothing in the module calls, runs on an "unknown" thread, and every queue
it touches is UNKNOWN. An access that resolves to no queue at all makes
every verdict UNKNOWN, since it may touch any of them.

A spawn's entry is the first argument that is a function's address, not
operand zero, which is the hidden result slot where the target returns
std::thread indirectly. Output follows IR order, and analyses are marked
preserved.

Verified: 22 of 22 pass, no warnings. The eight new or rewritten defect
tests fail against dev, each for the reason it names. The prototype's
example, compiled from C++ for AArch64 and for x86-64, gives the known
answer on both: SPSC for q1, MPMC for q2.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ZweZeya
ZweZeya force-pushed the fix/analysis-soundness branch from 30eca14 to 25d430f Compare October 18, 2026 05:20
@apaznikov

Copy link
Copy Markdown
Collaborator

Closing as superseded by #10. The three soundness defects this closes (a spawn in a loop counted as one thread, main without a thread, the parameter index ignored) are closed there too, by a shared thread-instance walk (ThreadInstanceWalk) that every analysis on that branch now uses: threads are spawn instances under a call string, a spawn that may run more than once stands for many, and parameters bind by index through the call that produced the instance. The fixed-point thread contexts and the "unknown" thread for address-taken code have their counterparts there as well (speculative instances). Keeping two ownership analyses would have meant two answers to the same question.

@apaznikov apaznikov closed this Oct 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants