Fix crash in netlist verify/ripup/cull from stale DBSrConnect callback signatures - #554
Open
ananthchellappa wants to merge 1 commit into
Open
Conversation
DBSrConnect() passes four arguments to its client function (tile, dinfo, plane, clientData), the "dinfo" split-tile argument having been added when non-Manhattan geometry handling was extended. Four client functions were never updated and still declared the original three arguments, so each one received "dinfo" where it expected the plane number and the plane number where it expected its ClientData, which it then dereferenced. In netmenu, this crashed the "verify", "ripup" and "cull" commands as soon as any tile was traced, making the netlist window's connectivity checking unusable. In lef, defBlockageGeometryFunc() has the same defect and would misbehave when writing DEF blockages. The affected functions are nmwRipTileFunc(), nmwNetTileFunc() and nmwVerifyTileFunc() in netmenu/NMwiring.c, plus defBlockageGeometryFunc() and its forward declaration in lef/defWrite.c. None of them needs the split-tile information, so the argument is accepted and ignored, matching defNetGeometryFunc() and defSimpleBlockageFunc() which already have the correct signature. Verified against a two-terminal test netlist: before the change, "verify" and "ripup" both dumped core; after it, "verify" correctly reports 'Terminal "B" not connected.' on a disconnected layout and "No wiring errors found." once the terminals are joined, and "ripup" completes normally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vdvntfrih4TA8gjnKZ7fD
Owner
|
Looks reasonable. I will do a complete check and review, but I won't be able to get to it for a couple of days. If I haven't reviewed it within a week, please feel free to post a reminder. |
Author
|
No need to review 🙂 It's from the almighty Claude 🙂 I had no hand in it except to say fix and submit PR.
…________________________________
From: R. Timothy Edwards ***@***.***>
Sent: Friday, August 21, 2026 8:03 AM
To: RTimothyEdwards/magic ***@***.***>
Cc: ananthchellappa ***@***.***>; Author ***@***.***>
Subject: Re: [RTimothyEdwards/magic] Fix crash in netlist verify/ripup/cull from stale DBSrConnect callback signatures (PR #554)
[https://avatars.githubusercontent.com/u/12645599?s=20&v=4]RTimothyEdwards left a comment (RTimothyEdwards/magic#554)<#554 (comment)>
Looks reasonable. I will do a complete check and review, but I won't be able to get to it for a couple of days. If I haven't reviewed it within a week, please feel free to post a reminder.
—
Reply to this email directly, view it on GitHub<#554?email_source=notifications&email_token=AONIOU7L2R7Y6WBXRJEMOCL5LBQCPA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMZXGE3DAOJTGI2KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-5371609324>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AONIOUZPHFROXPFKR7GBORT5LBQCPAVCNFSNUABFKJSXA33TNF2G64TZHMYTONZWHA3DINRVHNEXG43VMU5TKMRRGUZDMMJYGMYKC5QC>.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS<https://github.com/notifications/mobile/ios/AONIOU2RXTXS7COMDLAYMWT5LBQCPA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMZXGE3DAOJTGI2KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KUZTPN52GK4S7NFXXG> and Android<https://github.com/notifications/mobile/android/AONIOU2FULR5IIBYXV4DIZD5LBQCPA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMZXGE3DAOJTGI2KM4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2K4ZTPN52GK4S7MFXGI4TPNFSA>. Download it today!
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Owner
|
@ananthchellappa : I've caught Claude being wrong before (not Fable 5, yet, but Opus, definitely). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
verify,ripup, andcullsegfault — staleDBSrConnectcallback signaturesVersion: 8.3.681 (commit
4432d7ec)Component:
netmenu/NMwiring.c,lef/defWrite.cSeverity: High — reproducible crash, takes the whole session down
Summary
Three netlist-window commands —
verify,ripup,cull— segfault as soon as they trace any tile. This makes the netlist window's connectivity-checking functionality unusable.A survey of all
DBSrConnectclients found four stale callbacks, not three: the same defect is present indefBlockageGeometryFuncinlef/defWrite.c, which affects DEF blockage output.nmwRipTileFuncnetmenu/NMwiring.c:126ripup,cullnmwNetTileFuncnetmenu/NMwiring.c:481extractnmwVerifyTileFuncnetmenu/NMwiring.c:719verifydefBlockageGeometryFunclef/defWrite.c:2642(+ forward decl at:2613)def writewith blockagesDBSrConnecthas only two callers outside itself —netmenu/NMwiring.c(4 sites) andlef/defWrite.c(3 sites) — so this list is complete. The correct signature is already used bydefNetGeometryFunc(lef/defWrite.c:935) anddefSimpleBlockageFunc(lef/defWrite.c:2701); the latter actually usesdinfo, asTiGetTypeExact(tile) | dinfo.Each of the four is passed only to
DBSrConnectand to nothing else, so correcting the arity affects no other caller.Root cause
DBSrConnectinvokes its client callback with four arguments (database/DBconnect.c:456):but netmenu's callbacks still have the three-argument signature from before
dinfowas added:The callbacks therefore receive
dinfowhere they expect the plane number, and the plane number where they expect theirClientData— which they then dereference. Under gdb the crash shows a jump to a small integer address:and for
ripup:listHead=0x9is the plane number arriving in theClientDataslot.Reproduction
Any layout and netlist will do — the crash occurs on the first traced tile, on both correctly wired and unwired designs.
Build a cell with two disconnected labelled rectangles:
with
nmcell.netcontaining:then:
load nmcell specialopen netlist netlist nmcell verify # <-- dumps core hereObserved on 8.3.681: the process dies immediately after the netlist is read.
ripup(with a box set beforespecialopen) crashes identically.After the fix, the same script reports
Terminal "B" not connected.and generates one feedback area; joining the two rectangles withbox 0 0 50 2 ; paint metal1then givesNo wiring errors found.Expected behaviour
verifyreports opens and shorts against the netlist;ripupremoves routing;cullprunes. None of them crash.Fix
Add the missing
TileType dinfoparameter to all four callbacks and to the one forward declaration, matchingdefNetGeometryFunc. None of the four needs the split-tile information, so the argument is accepted and ignored:Total change: 8 insertions, 3 deletions across two files. Builds clean with no new warnings.
A separate question, deliberately not addressed here: three of these callbacks call
TiGetType(tile)rather thanTiGetTypeExact(tile) | dinfo, so they will still take the wrong type for split (non-Manhattan) tiles. That is a pre-existing correctness issue independent of the crash, and it deserves its own change.Impact note
This silently removes the only connectivity-verification path Magic has for hand-routed layout. Users hitting it are likely to conclude the netlist window is abandoned rather than that it has a two-line bug.