Skip to content

Fix #30462, out of bounds access in maximum/minimum for certain-sized arrays #30484

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jan 2, 2019
Merged

Fix #30462, out of bounds access in maximum/minimum for certain-sized arrays #30484

merged 3 commits into from
Jan 2, 2019

Conversation

@jw3126
Copy link
Contributor

@jw3126 jw3126 commented Dec 21, 2018 •
edited
Loading

#30462 This is still fast:

julia> using BenchmarkTools; arr = randn(10^6); @benchmark maximum($arr)
[ Info: Recompiling stale cache file /home/jan/.julia/compiled/v1.2/BenchmarkTools/ZXPQo.ji for BenchmarkTools [6e4b80f9-dd63-53aa-95a3-0cdb28fa8baf]
BenchmarkTools.Trial: 
  memory estimate:  0 bytes
  allocs estimate:  0
  --------------
  minimum time:     557.330 μs (0.00% GC)
  median time:      587.258 μs (0.00% GC)
  mean time:        588.535 μs (0.00% GC)
  maximum time:     855.520 μs (0.00% GC)
  --------------
  samples:          8403
  evals/sample:     1

jw3126
jw3126 commented Dec 21, 2018
View reviewed changes
test/reduce.jl Outdated

# 30462
# test that there is no out of bounds access
maximum(randn(128,128))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like to add more/better tests that there are no out of bounds accesses. Is there a recommended way to do this? Just make a function call and rely on --check-bounds=yes?

Copy link
Member

Choose a reason for hiding this comment

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

You could also test maximum(fill(-Inf, 128, 128)) === -Inf. It's unlikely that uninitialized memory will be a bitpattern that represents -Inf. But we do run all our tests with --check-bounds=yes (excepting in the test/boundscheck*.jl files)

@KristofferC KristofferC added the bugfix This change fixes an existing bug label Dec 21, 2018
@mbauman mbauman changed the title Fix30462 Fix #30462, out of bounds access in maximum/minimum for certain-sized arrays Dec 21, 2018
@mbauman
Copy link
Member

mbauman commented Dec 21, 2018

Applying same backport tag as the original PR (#30320).

@StefanKarpinski
Copy link
Member

Bump. This seems good to go doesn't it?

@KristofferC KristofferC merged commit 9731ee7 into JuliaLang:master Jan 2, 2019
@StefanKarpinski StefanKarpinski added backport 1.1 triage This should be discussed on a triage call and removed backport 1.1 labels Jan 31, 2019
@Keno Keno removed the triage This should be discussed on a triage call label Feb 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mbauman mbauman mbauman left review comments

Assignees

No one assigned

Labels

bugfix This change fixes an existing bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@jw3126 @mbauman @StefanKarpinski @KristofferC @Keno