Fix segfault in pipe ele:floor for girder elements#2050
Closed
ChristopherMayes wants to merge 3 commits into
Closed
Fix segfault in pipe ele:floor for girder elements#2050ChristopherMayes wants to merge 3 commits into
pipe ele:floor for girder elements#2050ChristopherMayes wants to merge 3 commits into
Conversation
pipe ele:floor for girder elements
Member
|
@ChristopherMayes I put in a fix (PR #2052). The fix includes a regression test but this is different from the tests here. If you want to include your tests ,please open a new PR. |
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.
Fixes #2049.
Problem
Running Tao with a lattice containing a
girderelement and querying the floor coordinates of that girder crashed with a segmentation fault:Minimal reproducer:
The same crash also occurred for the more common case where a girder's slaves are
multipass_lordelements (e.g. a cryomodule girder over a multipass linac line).Cause
In
write_this_ele_floor(tao/code/tao_pipe_cmd.f90), the code special-casedsuper_lord$andgirder_lord$to skip the slave recursion, then unconditionally called:pointer_to_next_elereturnsnull()for any lord that is not asuper_lord, soele0%flooron the next line dereferenced a null pointer for every girder.A naive fix that descends one level with
pointer_to_slave(girder, 1)is not enough either: when the girder's slaves are themselves multipass_lord elements, the result still lives outside the tracking part of the lattice andpointer_to_next_elestill returns null.Fix
Added a dedicated
girder_lordbranch inwrite_this_ele_floorthat usesfind_element_ends, which already walks through nested lord chains (multipass_lord,super_lord, nested girders) down to actual tracking elements:beginning→ upstream floor of the first tracking slave (fromele1returned byfind_element_ends)center→ the girder's ownele%floor(midpoint of the slave span)end→ downstream floor of the last tracking slave (ele2)If
find_element_endscannot resolve the ends (e.g. an ill-defined girder), the routine emits aninvalidmessage instead of dereferencing null.Verification
Before the fix, both the minimal reproducer above and a real linac model with a multipass cryomodule girder (
L1_CM1) segfaulted onpipe ele:floor <girder> beginning. After the fix, both return sensible floor coordinates:Tests
Added
regression_tests/pytao/girder_floor/with two minimal lattices and a pytest-based regression test (test_girder_floor.py):lat.bmad— simple girder over two pipeslat_multipass.bmad— girder whose slaves are multipass_lord elementsTest cases:
test_girder_floor_beginning_does_not_crash— confirmspipe ele:floor g1 beginningno longer crashes the subprocesstest_girder_floor_beginning_matches_first_slave— confirmsbeginningmatches the upstream floor of the first slavetest_girder_floor_end_matches_last_slave— confirmsendmatches the downstream floor of the last slavetest_girder_floor_center— confirmscentermatches the midpoint of the slave spantest_girder_floor_multipass_does_not_crash— confirms the girder-over-multipass case (the original real-world crash) returns sensible coordinates without crashingAll five cases pass against the patched build.
Files changed
tao/code/tao_pipe_cmd.f90— replaced the brokenpointer_to_next_elepath with agirder_lordbranch backed byfind_element_ends.regression_tests/pytao/girder_floor/lat.bmad— minimal girder lattice.regression_tests/pytao/girder_floor/lat_multipass.bmad— girder over multipass lattice.regression_tests/pytao/girder_floor/test_girder_floor.py— pytest regression tests (5 cases).Acknowledgement
This PR was prepared with assistance from GitHub Copilot (Claude Opus 4.7, Anthropic) running in the VS Code agent surface. The model performed the codebase exploration, diagnosis, edits, build, and test authoring; all changes were reviewed and verified locally.