Skip to content

[hist] prevent TFormula out of bounds access#21110

Merged
vepadulano merged 6 commits intoroot-project:masterfrom
ferdymercury:patch-15
Feb 25, 2026
Merged

[hist] prevent TFormula out of bounds access#21110
vepadulano merged 6 commits intoroot-project:masterfrom
ferdymercury:patch-15

Conversation

@ferdymercury
Copy link
Copy Markdown
Collaborator

This Pull request:

Changes or fixes:

Fixes #21104

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

Comment thread hist/hist/test/test_TFormula.cxx
@ferdymercury ferdymercury marked this pull request as ready for review February 2, 2026 17:12
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 3, 2026

Test Results

    22 files      22 suites   3d 4h 1m 45s ⏱️
 3 795 tests  3 791 ✅ 0 💤 4 ❌
75 448 runs  75 444 ✅ 0 💤 4 ❌

For more details on these failures, see this check.

Results for commit 678b360.

♻️ This comment has been updated with latest results.

@ferdymercury

This comment was marked as outdated.

Comment thread hist/hist/src/TFormula.cxx Outdated
Comment thread hist/hist/src/TFormula.cxx Outdated
Comment thread hist/hist/src/TFormula.cxx Outdated
Comment thread hist/hist/test/test_TFormula.cxx Outdated
Comment thread hist/hist/test/test_TFormula.cxx Outdated
Comment thread hist/hist/src/TFormula.cxx Outdated
Comment thread hist/hist/src/TFormula.cxx Outdated
@ferdymercury
Copy link
Copy Markdown
Collaborator Author

Could someone restart alma9 glitch?
Thks

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

Thanks, looking green now.

@dpiparo
Copy link
Copy Markdown
Member

dpiparo commented Feb 14, 2026

@vepadulano , do you think we can merge this PR?

@ferdymercury
Copy link
Copy Markdown
Collaborator Author

gentle ping also to @lmoneta

Copy link
Copy Markdown
Member

@vepadulano vepadulano left a comment

Choose a reason for hiding this comment

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

LGTM for the changes, I just noticed a small typo. Also, I think the commit history could be squashed and adjusted a bit, I will let you decide what's the best way.

Comment thread hist/hist/src/TFormula.cxx Outdated
Comment thread hist/hist/src/TFormula.cxx Outdated
ferdymercury and others added 6 commits February 24, 2026 13:12
Fixes root-project#21104

[hist] fix access when opening bracket pos exactly at length

[hist] additional out of bounds access safety guards
set IsValid to false otherwise while constructing TFormula
[test] singleQuote printed string

Co-authored-by: Vincenzo Eduardo Padulano <v.e.padulano@gmail.com>
[nfc][hist] clarify docu

[nfc] typo

vepadulano and lmoureaux thks

Co-authored-by: Louis Moureaux <m_louis30@yahoo.com>
Amends root-project@dcf9209#r176468371

see also 1f1cb91

Should make TFormulaTest39 pass again

[hist] fix out of bounds access at N-th term
@ferdymercury
Copy link
Copy Markdown
Collaborator Author

LGTM for the changes, I just noticed a small typo. Also, I think the commit history could be squashed and adjusted a bit, I will let you decide what's the best way.

Thanks.
I fixed commit history. But it's fine for me if you decide it's better to squash all on merge.
Mac26 failure seems unrelated.
So ready to merge from my side

@vepadulano vepadulano merged commit d3d1a47 into root-project:master Feb 25, 2026
27 of 30 checks passed
@ferdymercury ferdymercury deleted the patch-15 branch February 25, 2026 10:47
@dpiparo
Copy link
Copy Markdown
Member

dpiparo commented Feb 25, 2026

/backport to 6.38

@root-project-bot
Copy link
Copy Markdown

Something went wrong with the backport to 6.38: @dpiparo please see the logs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Out-of-bounds access in TFormula constructor

6 participants