Skip to content

Fix potential out-of-bounds memory in IdealClusterBuilder#2079

Merged
tvami merged 2 commits into
trunkfrom
iss1707-memory-bug-clustbuilder
Jun 17, 2026
Merged

Fix potential out-of-bounds memory in IdealClusterBuilder#2079
tvami merged 2 commits into
trunkfrom
iss1707-memory-bug-clustbuilder

Conversation

@tvami

@tvami tvami commented Jun 16, 2026

Copy link
Copy Markdown
Member

I am updating ldmx-sw, here are the details.

Replace initializer-list vector assignments (= {val}) with push_back to avoid a compiler/ASAN warning about potential out-of-bounds memmove; this works because std::map::operator[] default-constructs an empty vector when the key doesn't exist, so push_back on it is safe without an explicit count check.

What are the issues that this addresses?

Resolves #1707

Check List

  • I successfully compiled ldmx-sw with my developments.
  • I read, understood and follow the coding rules.
  • I ran my developments and the following shows that they are successful.

@tvami tvami marked this pull request as draft June 16, 2026 20:58
@tvami tvami requested review from therwig and tomeichlersmith June 16, 2026 20:58
@tvami

tvami commented Jun 16, 2026

Copy link
Copy Markdown
Member Author

This is ready for review, but the gold creation is in progress so I kept it draft until that finishes.

@LDMX-Software LDMX-Software deleted a comment from github-actions Bot Jun 16, 2026
@tvami tvami marked this pull request as ready for review June 16, 2026 22:00
@github-actions

Copy link
Copy Markdown
Contributor

Validation Results

Some validation samples failed! ❌

Sample Status
deep_ecal_gun ✅ PASS
eat_signal ✅ PASS
ecal_pn ✅ PASS
hcal ✅ PASS
inclusive ✅ PASS
it_pileup ✅ PASS
kaon_enhanced ✅ PASS
reduced_ldmx ✅ PASS
signal ✅ PASS
signal_target_al ✅ PASS
target_genie ✅ PASS
target_pn_lyso ✅ PASS
target_ti_en ✅ PASS
wab_lhe ✅ PASS
cascade_history ❌ FAIL (82 histograms failed KS test) (artifact)

cascade_history:

  • 82 plots failed the KS test against gold.
  • Text Differences Between Logs (69763 lines differ)
  • Log character count differs by 2%: gold=4450203, new=4355143
  • Timing for cascade_history: gold=5968s, new=6244s
  • Timing within 4% of gold (tolerance 10%)

deep_ecal_gun:

  • Text Differences Between Logs (20 lines differ)
  • Timing anomaly for deep_ecal_gun: new=3197s vs gold=4091s (-21%, tolerance 10%)
  • Timing for deep_ecal_gun: gold=4091s, new=3197s

eat_signal:

  • Timing for eat_signal: gold=1602s, new=1548s
  • Timing within 3% of gold (tolerance 10%)

ecal_pn:

  • Text Differences Between Logs (44 lines differ)
  • Timing for ecal_pn: gold=6232s, new=6511s
  • Timing within 4% of gold (tolerance 10%)

hcal:

  • Text Differences Between Logs (2 lines differ)
  • Timing for hcal: gold=829s, new=764s
  • Timing within 7% of gold (tolerance 10%)

inclusive:

  • Text Differences Between Logs (20 lines differ)
  • Timing for inclusive: gold=6805s, new=6784s
  • Timing within 0% of gold (tolerance 10%)

it_pileup:

  • Text Differences Between Logs (44 lines differ)
  • Timing for it_pileup: gold=577s, new=567s
  • Timing within 1% of gold (tolerance 10%)

kaon_enhanced:

  • Text Differences Between Logs (22 lines differ)
  • Timing for kaon_enhanced: gold=3122s, new=3090s
  • Timing within 1% of gold (tolerance 10%)

reduced_ldmx:

  • Text Differences Between Logs (22 lines differ)
  • Timing for reduced_ldmx: gold=211s, new=213s
  • Timing within 0% of gold (tolerance 10%)

signal:

  • Text Differences Between Logs (38 lines differ)
  • Timing for signal: gold=1184s, new=1203s
  • Timing within 1% of gold (tolerance 10%)

signal_target_al:

  • Text Differences Between Logs (28 lines differ)
  • Timing for signal_target_al: gold=1047s, new=1078s
  • Timing within 2% of gold (tolerance 10%)

target_genie:

  • Text Differences Between Logs (4480 lines differ)
  • Timing for target_genie: gold=6488s, new=6545s
  • Timing within 0% of gold (tolerance 10%)

target_pn_lyso:

  • Text Differences Between Logs (28 lines differ)
  • Timing for target_pn_lyso: gold=6455s, new=6380s
  • Timing within 1% of gold (tolerance 10%)

target_ti_en:

  • Text Differences Between Logs (38 lines differ)
  • Timing for target_ti_en: gold=3226s, new=3126s
  • Timing within 3% of gold (tolerance 10%)

wab_lhe:

  • Text Differences Between Logs (30 lines differ)
  • Timing for wab_lhe: gold=6770s, new=6503s
  • Timing within 3% of gold (tolerance 10%)

@tvami

tvami commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

/run-validation

@github-actions

Copy link
Copy Markdown
Contributor

The validation workflow is running here: https://github.com/LDMX-Software/ldmx-sw/actions/runs/27662256723.

@tvami

tvami commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

cascade_history does not run the IdealClusterBuilder... it actually just runs the PN, which has been stable so far. Let's see the next test

@github-actions

Copy link
Copy Markdown
Contributor

Validation Results

Some validation samples failed! ❌

Sample Status
cascade_history ✅ PASS
deep_ecal_gun ✅ PASS
eat_signal ✅ PASS
ecal_pn ✅ PASS
hcal ✅ PASS
inclusive ✅ PASS
it_pileup ✅ PASS
kaon_enhanced ✅ PASS
reduced_ldmx ✅ PASS
signal ✅ PASS
signal_target_al ✅ PASS
target_pn_lyso ✅ PASS
target_ti_en ✅ PASS
wab_lhe ✅ PASS
target_genie ❌ FAIL (11 histograms failed KS test) (artifact)

target_genie:

  • 11 plots failed the KS test against gold.
  • Text Differences Between Logs (6077 lines differ)
  • Log character count differs by 0% (gold=7408794, new=7366567); within tolerance
  • Timing for target_genie: gold=6488s, new=6522s
  • Timing within 0% of gold (tolerance 10%)

cascade_history:

  • Text Differences Between Logs (2 lines differ)
  • Timing for cascade_history: gold=5968s, new=6208s
  • Timing within 4% of gold (tolerance 10%)

deep_ecal_gun:

  • Text Differences Between Logs (24 lines differ)
  • Timing for deep_ecal_gun: gold=4091s, new=4083s
  • Timing within 0% of gold (tolerance 10%)

eat_signal:

  • Timing for eat_signal: gold=1602s, new=1516s
  • Timing within 5% of gold (tolerance 10%)

ecal_pn:

  • Text Differences Between Logs (42 lines differ)
  • Timing for ecal_pn: gold=6232s, new=6646s
  • Timing within 6% of gold (tolerance 10%)

hcal:

  • Text Differences Between Logs (2 lines differ)
  • Timing for hcal: gold=829s, new=819s
  • Timing within 1% of gold (tolerance 10%)

inclusive:

  • Text Differences Between Logs (50 lines differ)
  • Log character count differs by 0% (gold=21025815, new=21025813); within tolerance
  • Timing anomaly for inclusive: new=5140s vs gold=6805s (-24%, tolerance 10%)
  • Timing for inclusive: gold=6805s, new=5140s

it_pileup:

  • Text Differences Between Logs (46 lines differ)
  • Timing for it_pileup: gold=577s, new=593s
  • Timing within 2% of gold (tolerance 10%)

kaon_enhanced:

  • Text Differences Between Logs (30 lines differ)
  • Timing for kaon_enhanced: gold=3122s, new=3063s
  • Timing within 1% of gold (tolerance 10%)

reduced_ldmx:

  • Text Differences Between Logs (20 lines differ)
  • Timing for reduced_ldmx: gold=211s, new=211s
  • Timing within 0% of gold (tolerance 10%)

signal:

  • Text Differences Between Logs (40 lines differ)
  • Timing for signal: gold=1184s, new=1239s
  • Timing within 4% of gold (tolerance 10%)

signal_target_al:

  • Text Differences Between Logs (36 lines differ)
  • Timing for signal_target_al: gold=1047s, new=1029s
  • Timing within 1% of gold (tolerance 10%)

target_pn_lyso:

  • Text Differences Between Logs (40 lines differ)
  • Timing for target_pn_lyso: gold=6455s, new=6529s
  • Timing within 1% of gold (tolerance 10%)

target_ti_en:

  • Text Differences Between Logs (28 lines differ)
  • Timing for target_ti_en: gold=3226s, new=3166s
  • Timing within 1% of gold (tolerance 10%)

wab_lhe:

  • Text Differences Between Logs (22 lines differ)
  • Timing for wab_lhe: gold=6770s, new=6708s
  • Timing within 0% of gold (tolerance 10%)

@tvami

tvami commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

/run-validation

@github-actions

Copy link
Copy Markdown
Contributor

The validation workflow is running here: https://github.com/LDMX-Software/ldmx-sw/actions/runs/27695331269.

@tomeichlersmith tomeichlersmith left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small, logical change that I'm worried about affecting the algorithm

Comment thread Trigger/Algo/src/Trigger/IdealClusterBuilder.cxx
@tvami tvami requested a review from tomeichlersmith June 17, 2026 14:34
@tvami

tvami commented Jun 17, 2026

Copy link
Copy Markdown
Member Author

/run-validation

@github-actions

Copy link
Copy Markdown
Contributor

The validation workflow is running here: https://github.com/LDMX-Software/ldmx-sw/actions/runs/27696786470.

@github-actions

Copy link
Copy Markdown
Contributor

Validation Results

All validation samples passed! ✅

Sample Status
cascade_history ✅ PASS
deep_ecal_gun ✅ PASS
eat_signal ✅ PASS
ecal_pn ✅ PASS
hcal ✅ PASS
inclusive ✅ PASS
it_pileup ✅ PASS
kaon_enhanced ✅ PASS
reduced_ldmx ✅ PASS
signal ✅ PASS
signal_target_al ✅ PASS
target_genie ✅ PASS
target_pn_lyso ✅ PASS
target_ti_en ✅ PASS
wab_lhe ✅ PASS

cascade_history:

  • Text Differences Between Logs (2 lines differ)
  • Timing for cascade_history: gold=5968s, new=6374s
  • Timing within 6% of gold (tolerance 10%)

deep_ecal_gun:

  • Text Differences Between Logs (18 lines differ)
  • Timing for deep_ecal_gun: gold=4091s, new=3701s
  • Timing within 9% of gold (tolerance 10%)

eat_signal:

  • Timing for eat_signal: gold=1602s, new=1563s
  • Timing within 2% of gold (tolerance 10%)

ecal_pn:

  • Text Differences Between Logs (50 lines differ)
  • Timing anomaly for ecal_pn: new=5248s vs gold=6232s (-15%, tolerance 10%)
  • Timing for ecal_pn: gold=6232s, new=5248s

hcal:

  • Text Differences Between Logs (2 lines differ)
  • Timing for hcal: gold=829s, new=826s
  • Timing within 0% of gold (tolerance 10%)

inclusive:

  • Text Differences Between Logs (36 lines differ)
  • Timing for inclusive: gold=6805s, new=6976s
  • Timing within 2% of gold (tolerance 10%)

it_pileup:

  • Text Differences Between Logs (42 lines differ)
  • Timing for it_pileup: gold=577s, new=602s
  • Timing within 4% of gold (tolerance 10%)

kaon_enhanced:

  • Text Differences Between Logs (40 lines differ)
  • Timing anomaly for kaon_enhanced: new=2744s vs gold=3122s (-12%, tolerance 10%)
  • Timing for kaon_enhanced: gold=3122s, new=2744s

reduced_ldmx:

  • Text Differences Between Logs (20 lines differ)
  • Timing for reduced_ldmx: gold=211s, new=210s
  • Timing within 0% of gold (tolerance 10%)

signal:

  • Text Differences Between Logs (28 lines differ)
  • Timing for signal: gold=1184s, new=1183s
  • Timing within 0% of gold (tolerance 10%)

signal_target_al:

  • Text Differences Between Logs (40 lines differ)
  • Timing anomaly for signal_target_al: new=886s vs gold=1047s (-15%, tolerance 10%)
  • Timing for signal_target_al: gold=1047s, new=886s

target_genie:

  • Text Differences Between Logs (4480 lines differ)
  • Timing for target_genie: gold=6488s, new=6556s
  • Timing within 1% of gold (tolerance 10%)

target_pn_lyso:

  • Text Differences Between Logs (44 lines differ)
  • Timing anomaly for target_pn_lyso: new=3689s vs gold=6455s (-42%, tolerance 10%)
  • Timing for target_pn_lyso: gold=6455s, new=3689s

target_ti_en:

  • Text Differences Between Logs (28 lines differ)
  • Timing for target_ti_en: gold=3226s, new=3163s
  • Timing within 1% of gold (tolerance 10%)

wab_lhe:

  • Text Differences Between Logs (24 lines differ)
  • Timing for wab_lhe: gold=6770s, new=6862s
  • Timing within 1% of gold (tolerance 10%)

@tvami tvami merged commit d5171bd into trunk Jun 17, 2026
2 checks passed
@tvami tvami deleted the iss1707-memory-bug-clustbuilder branch June 17, 2026 18:12
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.

Potential out-of-bounds memory in IdealClusterBuilder

3 participants