Skip to content

Commit

Permalink
fix #479: buggy handling of ccall
Browse files Browse the repository at this point in the history
This was quite annoying to track down. The issue here was that
`replace_ssa!` modifies the passed expression, but it is possible for
there to be multiple references to the same copy of an expression in
`stmts`, so the replacement would be accidentally applied twice
generating wrong code.
  • Loading branch information
simeonschaub committed May 5, 2021
1 parent 0375ee2 commit 8315ec5
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 11 deletions.
21 changes: 11 additions & 10 deletions src/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -25,18 +25,18 @@ function extract_inner_call!(stmt::Expr, idx, once::Bool=false)
return nothing
end

function replace_ssa!(@nospecialize(stmt), ssalookup)
isa(stmt, Expr) || return nothing
for (i, a) in enumerate(stmt.args)
function replace_ssa(@nospecialize(stmt), ssalookup)
isa(stmt, Expr) || return stmt
return Expr(stmt.head, Any[
if isa(a, SSAValue)
stmt.args[i] = SSAValue(ssalookup[a.id])
SSAValue(ssalookup[a.id])
elseif isa(a, NewSSAValue)
stmt.args[i] = SSAValue(a.id)
SSAValue(a.id)
else
replace_ssa!(a, ssalookup)
replace_ssa(a, ssalookup)
end
end
return nothing
for a in stmt.args
]...)
end

function renumber_ssa!(stmts::Vector{Any}, ssalookup)
Expand All @@ -54,10 +54,11 @@ function renumber_ssa!(stmts::Vector{Any}, ssalookup)
elseif isa(stmt, NewSSAValue)
stmts[i] = SSAValue(stmt.id)
elseif isa(stmt, Expr)
replace_ssa!(stmt, ssalookup)
stmt = replace_ssa(stmt, ssalookup)
if (stmt.head === :gotoifnot || stmt.head === :enter) && isa(stmt.args[end], Int)
stmt.args[end] = jumplookup(ssalookup, stmt.args[end])
end
stmts[i] = stmt
elseif is_GotoIfNot(stmt)
cond = (stmt::Core.GotoIfNot).cond
if isa(cond, SSAValue)
Expand Down Expand Up @@ -287,7 +288,7 @@ function build_compiled_call!(stmt::Expr, fcall, code, idx, nargs::Int, sparams:
push!(args, cconvert_expr.args[3])
elseif arg isa SlotNumber
index = findfirst(code.code) do expr
expr isa Expr || return false
Meta.isexpr(expr, :(=)) || return false
lhs = expr.args[1]
return lhs isa SlotNumber && lhs.id === arg.id
end
Expand Down
10 changes: 9 additions & 1 deletion test/interpret.jl
Original file line number Diff line number Diff line change
Expand Up @@ -782,4 +782,12 @@ end
@unreachable foobar() = "nope"

@test @interpret(foobar()) == foobar()
end
end

@testset "issue #479" begin
function f()
ptr = @cfunction(+, Int, (Int, Int))
ccall(ptr::Ptr{Cvoid}, Int, (Int, Int), 1, 2)
end
@test @interpret(f()) === 3
end

0 comments on commit 8315ec5

Please sign in to comment.