Check interval start to avoid overflowing bin numbers #1774
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.
As noted on samtools/samtools#2032 (which includes test queries exhibiting the problem):
This PR checks start positions of query intervals against the maximum position representable in the index's geometry, to avoid negative bin numbers and the resulting infinite loops in the
do...while
loop. It's effectively thebeg
equivalent of #1595.I considered implementing the “massively beyond” bit by e.g. adding 256 as various parts of the code do for somewhat different reasons. However tracing the guarded code in
hts_itr_query()
andhts_itr_multi_bam()
(and looking at the checks for data getting into the index at all) suggests that the result would be a finished / unchanged iterator forbeg
immediately pastmaxpos
anyway — so having the bound exactly atmaxpos
is fine.Introduces
hts_bin_maxpos()
andhts_idx_maxpos()
, and uses them wherever themaxpos
calculation appears. I've left the latter private, at least for now.This also changes the existing
end
checks to<=
asend
is exclusive -- note it is used asend-1
in the code guarded by the checks. In practice this probably won't make much difference to anything.