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

Fix segfault in Diagonal * OffsetMatrix #45548

Merged
merged 2 commits into from
Jun 2, 2022

Conversation

jishnub
Copy link
Contributor

@jishnub jishnub commented Jun 1, 2022

This fixes a segfault in Diagonal * OffsetMatrix if the axes of the offset matrix are not 1-based. On master, the check was for sizes of the inputs, and not axes. I've added appropriate require_one_based_indexing checks.

This should be back-ported to 1.8.

@KristofferC KristofferC added backport 1.6 Change should be backported to release-1.6 backport 1.8 Change should be backported to release-1.8 labels Jun 1, 2022
@oscardssmith oscardssmith added the bugfix This change fixes an existing bug label Jun 1, 2022
@KristofferC KristofferC merged commit 172bddc into JuliaLang:master Jun 2, 2022
@jishnub jishnub deleted the diagmulsegfault branch June 2, 2022 14:25
KristofferC pushed a commit that referenced this pull request Jun 7, 2022
* diagonal*offset matrix should throw

(cherry picked from commit 172bddc)
@KristofferC KristofferC mentioned this pull request Jun 7, 2022
36 tasks
@KristofferC
Copy link
Member

I don't think this is applicable to 1.6 since the dispatch there seems to include this check:

(*)(A::AbstractMatrix, D::Diagonal) =
rmul!(copyto!(similar(A, promote_op(*, eltype(A), eltype(D.diag)), size(A)), A), D)
(*)(D::Diagonal, A::AbstractMatrix) =
lmul!(D, copyto!(similar(A, promote_op(*, eltype(A), eltype(D.diag)), size(A)), A))
function rmul!(A::AbstractMatrix, D::Diagonal)
require_one_based_indexing(A)
A .= A .* permutedims(D.diag)
return A
end
function lmul!(D::Diagonal, B::AbstractVecOrMat)
require_one_based_indexing(B)
B .= D.diag .* B
return B
end

@KristofferC KristofferC removed the backport 1.6 Change should be backported to release-1.6 label Jun 28, 2022
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants