-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
prevent conj! into uninitialized memory #40481
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch! Seems correct to me. Not sure what performance implications might be. Thanks Kristoffer! :)
Yep, seems right to me. We limit this buffer use to bitstypes, and most bitstypes will happily julia> struct BrokenInt <: Number
i::Int
end
julia> Base.:*(::BrokenInt, ::BrokenInt) = BrokenInt(42)
julia> Base.:+(::BrokenInt, ::BrokenInt) = BrokenInt(42)
julia> Base.zero(::BrokenInt) = BrokenInt(0)
julia> Base.conj(b::BrokenInt) = b.i == 42 ? b : error()
julia> fill(BrokenInt(42), 10,100)' * fill(BrokenInt(42), 100,10)'
ERROR:
Stacktrace:
[1] error()
@ Base ./error.jl:42
[2] conj
@ ./REPL[5]:1 [inlined] Your patch fixes it. |
I'll add that as a test! |
054374f
to
32f1f98
Compare
The test caused some ambiguities so removed it for now. I'll open another PR with it. |
(cherry picked from commit aa7d879)
Thanks! |
Oops, totally missed that issue. |
(cherry picked from commit aa7d879)
From my understanding, the current matmul code loads some buffer memory and "casts" it to an array of the element type:
julia/stdlib/LinearAlgebra/src/matmul.jl
Line 833 in 2fed7c3
stores some data into a part of that buffer:
julia/stdlib/LinearAlgebra/src/matmul.jl
Line 840 in 2fed7c3
, but then goes on and
conj!
the full memory:julia/stdlib/LinearAlgebra/src/matmul.jl
Line 686 in 2fed7c3
Since this code runs for user defined types calling
conj
on uninitialized memory can have side effects (like throwing). I think this is what causes the errors in https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_hash/f2bc4d3_vs_9283b6c/NiceNumbers.1.6.1-pre-d190257cce.log. Tests passes with this PR.Im not 100% sure my analysis here is correct so some more eyes on it would be nice.