galilei: fix out-of-bounds access for duplicate ComFixed types (bug-sweep #27)#5361
Draft
RudolfWeeber wants to merge 1 commit into
Draft
Conversation
…weep #27) ComFixed::set_fixed_types assigned m_type_index[p_type] = i++ over the raw input list. Because m_type_index is an unordered_map keyed on the type, a duplicated type collapsed onto a single key while the index counter kept growing, leaving the stored index >= m_type_index.size() (e.g. types == [1, 1] gave m_type_index == {1: 1}, size() == 1). Consumers (local_type_forces, local_type_masses, apply) size their vectors by m_type_index.size() and index them by it->second, so any particle of the duplicated type triggered an out-of-bounds heap read+write on every force evaluation: an OOB write in local_type_forces and an OOB read of masses[]/forces[] in apply(), corrupting forces (huge-force RuntimeError) and the heap. Assign dense indices 0..n_distinct-1 via try_emplace using the current map size as the candidate index, so duplicate types no longer inflate the counter and every stored index stays < m_type_index.size(). Duplicates are now handled safely (deduplicated) at the core, fixing all entry points including checkpoint round-trips. Extends testsuite/python/comfixed.py with a regression case that fixes the COM of types [1, 1] and asserts COM conservation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
Alternatveily, albeit at higher effort, we could support the set type in the script interface |
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.
ComFixed::set_fixed_types assigned m_type_index[p_type] = i++ over the raw
input list. Because m_type_index is an unordered_map keyed on the type, a
duplicated type collapsed onto a single key while the index counter kept
growing, leaving the stored index >= m_type_index.size() (e.g. types == [1, 1]
gave m_type_index == {1: 1}, size() == 1). Consumers (local_type_forces,
local_type_masses, apply) size their vectors by m_type_index.size() and index
them by it->second, so any particle of the duplicated type triggered an
out-of-bounds heap read+write on every force evaluation: an OOB write in
local_type_forces and an OOB read of masses[]/forces[] in apply(), corrupting
forces (huge-force RuntimeError) and the heap.
Assign dense indices 0..n_distinct-1 via try_emplace using the current map size
as the candidate index, so duplicate types no longer inflate the counter and
every stored index stays < m_type_index.size(). Duplicates are now handled
safely (deduplicated) at the core, fixing all entry points including checkpoint
round-trips. Extends testsuite/python/comfixed.py with a regression case that
fixes the COM of types [1, 1] and asserts COM conservation.
Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
🤖 Generated with Claude Code