xilinx: fix use-after-free when a LUT6_2 feeds a MUXF7/8/9 tree

[jasonzeng124]

Summary

• Fixes a use-after-free segfault when a LUT6_2 feeds a MUXF7/MUXF8/MUXF9 input. pack_muxfs()'s constrain_muxf_tree() walks driver.cell across nets with no type check and can set constr_parent/constr_children on a LUT6_2 before split_lut6_2() (previously run inside pack_luts(), after pack_muxfs()) deletes that exact cell — leaving other cells holding a dangling CellInfo* that the HEAP placer later dereferences.
• Fix moves split_lut6_2() to run before pack_muxfs()/pack_carries()/pack_srls() in both the xc7 and Ultrascale flows, so LUT6_2 no longer exists as a type by the time any untyped driver-walk pass runs — this closes the bug class at its root rather than adding a type check to one call site.
• flush_cells() now asserts a cell being deleted has no live constr_parent/constr_children, so any future instance of this class fails loudly at the deletion site instead of segfaulting later, deep in the placer.
• constrain_lut6_2_pairs() no longer overwrites constr_parent on a half already constrained by an earlier pass (e.g. its own mux-tree constraint) — found via adversarial testing on top of the reorder: a LUT6_2 that is both constant-I5 pairable and drives a MUXF7 input hit the same dangling/desynced-constraint problem through a different path. Such a pair is now left unpaired (costs one extra LUT bel — the same tradeoff already accepted for the non-constant-I5 case) instead of corrupting constraint state.
• split_lut6_2() now propagates the original cell's region constraint to both halves (previously silently dropped).
• split_lut6_2() fails loudly (log_error) instead of silently misbehaving in two cases: a split-half name colliding with an existing cell (previously a bare NPNR_ASSERT deep inside flush_cells()), and a BEL-constrained LUT6_2 that drives both O5 and O6 (genuinely ambiguous — no rule for which half should keep the BEL). A BEL-constrained LUT6_2 using only one output now correctly inherits the BEL onto that single half, matching pre-split behaviour (this was a regression caught by an independent review pass — see below).

Scope

xilinx/pack.cc + xilinx/pack.h only. Two commits:

1. 6d4e5df1 — the use-after-free fix (reorder, guards, region propagation, flush_cells assert).
2. 0d9fda6a — follow-up fix for a regression an independent Opus review caught in commit 1: the BEL-constraint guard was over-broad and rejected a previously-working case (BEL-pinned LUT6_2 using only O6).

Testing

Verified against hand-built repro netlists (chipdb: xc7z010clg400), each hitting a distinct code path:

• Original UAF: LUT6_2.O6 -> MUXF7. Segfaults before this fix; places and routes cleanly after.
• Constant-I5 LUT6_2 with no mux involvement: confirms the pair still re-fuses onto one physical LUT (not two).
• Paired LUT6_2 (constant I5) with O6 also feeding a MUXF7: exercises the reordered flow's interaction with mux constraining.
• Same, but with O5 (not O6) feeding the MUXF7: the actual conflicting case for constrain_lut6_2_pairs()'s new guard — confirmed load-bearing by temporarily removing the guard and observing a different placer assertion fire (common/placer_heap.cc:1068).
• Split-half name collision (<name>$LUT5 pre-existing): confirms the new log_error fires with a clear message instead of the old bare assert.
• BEL-constrained LUT6_2 driving only O6: previously hard-errored (regression introduced by commit 1's original, over-broad BEL guard); now packs/places/routes cleanly.
• BEL-constrained LUT6_2 driving both O5 and O6: confirms this still correctly errors (genuinely ambiguous case).

All cases place and route cleanly except the two intentional log_error cases, which fail with the expected, specific message.

Out of scope (documented, not fixed here)

• A non-constant-I5 LUT6_2 still costs 2 LUT bels instead of 1 — the placer's fracturable-LUT legality check (arch_place.cc) refuses a 5LUT companion whenever the 6LUT genuinely uses all 6 inputs. Closing this needs an arch-level change, not a packer-level one, and doesn't affect correctness — a pre-existing, accepted limitation.
• A constant-I5-pairable LUT6_2 whose O6 half single-fanout-drives a CARRY4 input can have that half adopted directly into the carry chain by pack_carries() before constrain_lut6_2_pairs() gets a chance to re-fuse the pair, permanently costing 2 LUT bels instead of 1. Not a correctness bug (no crash, no wrong output), just a LUT-utilization nit in a narrow corner case — deliberately left out of this PR.
• A pre-existing, unrelated bug in constrain_muxf_tree(): its own reentrancy guard (curr->type == id_SLICE_LUTX && ...) can never fire for a plain LUT, because SLICE_LUTX cells don't exist yet at pack_muxfs() time. A LUT with fanout into two different mux trees can therefore still get double-constrained. This is unchanged by, and unrelated to, LUT6_2 — flagged by review for future work, not part of this PR.

Review

An independent Opus-model review pass was run against commit 1 specifically. It found the BEL-guard regression above (fixed in commit 2) and confirmed the reordering itself is sound: no pass between split_lut6_2() and pack_luts() deletes a LUT5/LUT6 half, and constrain_lut6_2_pairs()'s guard is symmetric and can't leave a half half-fixed.

[hansfbaier]

Please extract the condition to a bool with a self documenting name

[hansfbaier]

Please extract the condition to a bool with a self documenting name

[hansfbaier]

Please extract the condition to a bool with a self documenting name

[hansfbaier]

Please extract the condition to two bools with a self documenting names

[jasonzeng124]

should be addressed...