Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bounds check in step range does not take into account overflow #26623

Closed
haampie opened this issue Mar 26, 2018 · 1 comment · Fixed by #50118
Closed

Bounds check in step range does not take into account overflow #26623

haampie opened this issue Mar 26, 2018 · 1 comment · Fixed by #50118
Labels
bug Indicates an unexpected problem or unintended behavior ranges Everything AbstractRange

Comments

@haampie
Copy link
Contributor

haampie commented Mar 26, 2018

For example

> (1:3:4)[6148914691236517207]
3

This happens because first(v) + (i - 1)*step_hp(v)) in getindex can overflow. A solution is to use checkbounds(1:3:4, 6148914691236517207) which will throw a BoundsError for the above, but is not suitable for step ranges where the length cannot be represented in the integer type: checkbounds(-10:1:typemax(Int), 10) throws an OverflowError.

@mbauman mbauman added bug Indicates an unexpected problem or unintended behavior arrays [a, r, r, a, y, s] labels Mar 26, 2018
@brenhinkeller
Copy link
Contributor

Still overflowing on 1.8 and 1.9, FWIW

julia> (1:3:4)[6148914691236517207]
3

julia> versioninfo()
Julia Version 1.8.2
Commit 36034abf260 (2022-09-29 15:21 UTC)
Platform Info:
  OS: macOS (arm64-apple-darwin21.3.0)
  CPU: 10 × Apple M1 Max
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-13.0.1 (ORCJIT, apple-m1)
  Threads: 1 on 8 virtual cores

@LilithHafner LilithHafner added ranges Everything AbstractRange and removed arrays [a, r, r, a, y, s] labels Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior ranges Everything AbstractRange
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants