Skip to content

[tree] Fix integer overflow in TEntryList#11028

Merged
vepadulano merged 1 commit intoroot-project:masterfrom
vepadulano:fix-tentrylist-int-overflow
Jul 25, 2022
Merged

[tree] Fix integer overflow in TEntryList#11028
vepadulano merged 1 commit intoroot-project:masterfrom
vepadulano:fix-tentrylist-int-overflow

Conversation

@vepadulano
Copy link
Copy Markdown
Member

@vepadulano vepadulano commented Jul 22, 2022

Change signature of methods in TEntryList and derived class that expect
an entry index. Usually this is stored as Long64_t, but in GetEntry
and GetEntryAndTree methods this was passed as Int_t leading to
integer overflows.

Fixes #11026

With respect to the reproducer in the linked issue, this PR outputs the following

$: ./repro.o
std::numeric_limits<int>::max(): 2147483647
Total entries in chain: 2168958436
Total entries in entrylist: 2168958436
Giving input entry 2147483647
Got input index: 2147483647
Got tree index 100 and entry index 47
Giving input entry 2147483648
Got input index: 2147483648
Got tree index 100 and entry index 48
Got input index: 2147483648
With entryAfterList=2147483648 got loadResult=48
What is the entry status? not kEntryBeyondEnd

@vepadulano
[tree] Fix integer overflow in TEntryList
e8642cd 
Change signature of methods in TEntryList and derived class that expect
an entry index. Usually this is stored as Long64_t, but in `GetEntry`
and `GetEntryAndTree` methods this was passed as `Int_t` leading to
integer overflows.

Fixes root-project#11026
@vepadulano vepadulano requested review from eguiraud and etejedor July 22, 2022 13:51
@vepadulano vepadulano self-assigned this Jul 22, 2022
@vepadulano vepadulano requested a review from pcanal as a code owner July 22, 2022 13:51
@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Jul 22, 2022

Starting build on ROOT-debian10-i386/soversion, ROOT-performance-centos8-multicore/cxx17, ROOT-ubuntu18.04/nortcxxmod, ROOT-ubuntu2004/python3, mac1015/cxx17, mac11/cxx14, windows10/cxx14
How to customize builds

@pcanal
Copy link
Copy Markdown
Member

pcanal commented Jul 22, 2022

Thanks.

@pcanal
Copy link
Copy Markdown
Member

pcanal commented Jul 22, 2022 •
edited
Loading

Please also consider adding the reproducer as a test in roottest.

@phsft-bot
Copy link
Copy Markdown

phsft-bot commented Jul 22, 2022

Build failed on ROOT-performance-centos8-multicore/cxx17.
Running on olbdw-01.cern.ch:/data/sftnight/workspace/root-pullrequests-build
See console output.

Failing tests:

@eguiraud
Copy link
Copy Markdown
Contributor

eguiraud commented Jul 22, 2022 •
edited
Loading

Thank you Vincenzo! This probably also deserves a v6.26 backport. (removing myself from the reviewers as Philippe's review should be enough)

@eguiraud eguiraud removed their request for review July 22, 2022 20:47
This was referenced Jul 23, 2022
Merged
Merged
Closed
@vepadulano vepadulano merged commit bf9d2d6 into root-project:master Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pcanal pcanal pcanal approved these changes

@etejedor etejedor Awaiting requested review from etejedor

Assignees

@vepadulano vepadulano

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Integer overflow in TEntryList

4 participants

@vepadulano @phsft-bot @pcanal @eguiraud