Skip to content

Commit 8aed4d7

Browse files
committed
ccall: make distinction of pointer vs name a syntatic distinction
We have long expected users to be explicit about the library name for `ccall`, and the `@ccall` macro has even always enforced that. That means users should have already been using explicit syntax. And indeed, other syntax forms weren't handled reliably (since doing so would require linearizing IR if and only if the runtime values required it, which is not something that is computable, and thus was often done wrong). This now aligns the runtime and compiler to expect only syntax forms that we could reliably handle before without errors, and adds explicit errors for cases that we previously knew would be unreliable because they relied upon inference making particular decisions for the semantics. The `ccall` function is already very special since it is more like a actual macro (it does not exist as a binding or value), so we can make unusual syntax decisions like this, mirroring `@ccall`. This drops support for #37123, since we were going to use that for LazyLibraries, be we decided that approach was quite buggy and that PR would make static compilation quite impossible to support, so we instead actually implemented LazyLibraries with a different approach. It could be re-enabled, but we never had correct lowering or inference support for it, so it is presumably still unused. The goal is to cause breakage only where the package authors really failed to express intent with syntax, and otherwise to explicitly maintain support by adding cases to normalize the given syntax into one of the supported cases. All existing functionality (and more) can be accessed by explicit management of a pointer or by a LazyLibrary-like type, so this shouldn't cause any reduction in possible functionality, just possibly altered syntax.
1 parent 0d4cdfc commit 8aed4d7

18 files changed

+498
-291
lines changed

Compiler/src/abstractinterpretation.jl

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3488,6 +3488,13 @@ end
34883488
function abstract_eval_foreigncall(interp::AbstractInterpreter, e::Expr, sstate::StatementState, sv::AbsIntState)
34893489
mi = frame_instance(sv)
34903490
t = sp_type_rewrap(e.args[2], mi, true)
3491+
let fptr = e.args[1]
3492+
if !isexpr(fptr, :tuple)
3493+
if !hasintersect(widenconst(abstract_eval_value(interp, fptr, sstate, sv)), Ptr)
3494+
return RTEffects(Bottom, Any, EFFECTS_THROWS)
3495+
end
3496+
end
3497+
end
34913498
for i = 3:length(e.args)
34923499
if abstract_eval_value(interp, e.args[i], sstate, sv) === Bottom
34933500
return RTEffects(Bottom, Any, EFFECTS_THROWS)

Compiler/src/optimize.jl

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1429,8 +1429,11 @@ function statement_cost(ex::Expr, line::Int, src::Union{CodeInfo, IRCode}, sptyp
14291429
return params.inline_nonleaf_penalty
14301430
elseif head === :foreigncall
14311431
foreigncall = ex.args[1]
1432-
if foreigncall isa QuoteNode && foreigncall.value === :jl_string_ptr
1433-
return 1
1432+
if isexpr(foreigncall, :tuple, 1)
1433+
foreigncall = foreigncall.args[1]
1434+
if foreigncall isa QuoteNode && foreigncall.value === :jl_string_ptr
1435+
return 1
1436+
end
14341437
end
14351438
return 20
14361439
elseif head === :invoke || head === :invoke_modify

Compiler/src/ssair/EscapeAnalysis.jl

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1037,8 +1037,10 @@ function escape_foreigncall!(astate::AnalysisState, pc::Int, args::Vector{Any})
10371037
# NOTE array allocations might have been proven as nothrow (https://github.com/JuliaLang/julia/pull/43565)
10381038
nothrow = is_nothrow(astate.ir, pc)
10391039
name_info = nothrow ?: ThrownEscape(pc)
1040-
add_escape_change!(astate, name, name_info)
1041-
add_liveness_change!(astate, name, pc)
1040+
if !isexpr(name, :tuple)
1041+
add_escape_change!(astate, name, name_info)
1042+
add_liveness_change!(astate, name, pc)
1043+
end
10421044
for i = 1:nargs
10431045
# we should escape this argument if it is directly called,
10441046
# otherwise just impose ThrownEscape if not nothrow

Compiler/src/ssair/inlining.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1755,7 +1755,7 @@ function late_inline_special_case!(ir::IRCode, idx::Int, stmt::Expr, flag::UInt3
17551755
length(stmt.args) == 2 ? Any : stmt.args[end])
17561756
return SomeCase(typevar_call)
17571757
elseif f === UnionAll && length(argtypes) == 3 && (optimizer_lattice(state.interp), argtypes[2], TypeVar)
1758-
unionall_call = Expr(:foreigncall, QuoteNode(:jl_type_unionall), Any, svec(Any, Any),
1758+
unionall_call = Expr(:foreigncall, Expr(:tuple, QuoteNode(:jl_type_unionall)), Any, svec(Any, Any),
17591759
0, QuoteNode(:ccall), stmt.args[2], stmt.args[3])
17601760
return SomeCase(unionall_call)
17611761
elseif is_return_type(f)

Compiler/src/ssair/verify.jl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,10 @@ function check_op(ir::IRCode, domtree::DomTree, @nospecialize(op), use_bb::Int,
7777
end
7878
elseif isa(op, Expr)
7979
# Only Expr(:boundscheck) is allowed in value position
80-
if isforeigncall && arg_idx == 1 && op.head === :call
81-
# Allow a tuple in symbol position for foreigncall - this isn't actually
82-
# a real call - it's interpreted in global scope by codegen. However,
83-
# we do need to keep this a real use, because it could also be a pointer.
80+
if isforeigncall && arg_idx == 1 && op.head === :tuple
81+
# Allow a tuple literal in symbol position for foreigncall - this
82+
# is syntax for a literal value or globalref - it is interpreted in
83+
# global scope by codegen.
8484
elseif !is_value_pos_expr_head(op.head)
8585
if !allow_frontend_forms || op.head !== :opaque_closure_method
8686
@verify_error "Expr not allowed in value position"

Compiler/src/verifytrim.jl

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -279,9 +279,17 @@ function verify_codeinstance!(interp::NativeInterpreter, codeinst::CodeInstance,
279279
error = "unresolved cfunction"
280280
elseif isexpr(stmt, :foreigncall)
281281
foreigncall = stmt.args[1]
282-
if foreigncall isa QuoteNode
283-
if foreigncall.value in runtime_functions
284-
error = "disallowed ccall into a runtime function"
282+
if isexpr(foreigncall, :tuple, 1)
283+
foreigncall = foreigncall.args[1]
284+
if foreigncall isa String
285+
foreigncall = QuoteNode(Symbol(foreigncall))
286+
end
287+
if foreigncall isa QuoteNode
288+
if foreigncall.value in runtime_functions
289+
error = "disallowed ccall into a runtime function"
290+
end
291+
else
292+
error = "disallowed ccall with non-constant name and no library"
285293
end
286294
end
287295
elseif isexpr(stmt, :new_opaque_closure)

Compiler/test/irpasses.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1071,7 +1071,7 @@ let # Test for https://github.com/JuliaLang/julia/issues/43402
10711071
end
10721072

10731073
refs = map(Core.SSAValue, findall(@nospecialize(x)->Meta.isexpr(x, :new), src.code))
1074-
some_ccall = findfirst(@nospecialize(x) -> Meta.isexpr(x, :foreigncall) && x.args[1] == :(:some_ccall), src.code)
1074+
some_ccall = findfirst(@nospecialize(x) -> Meta.isexpr(x, :foreigncall) && x.args[1] == Expr(:tuple, :(:some_ccall)), src.code)
10751075
@assert some_ccall !== nothing
10761076
stmt = src.code[some_ccall]
10771077
nccallargs = length(stmt.args[3]::Core.SimpleVector)

base/c.jl

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -313,11 +313,15 @@ function ccall_macro_parse(exprs)
313313
# get the function symbols
314314
func = let f = call.args[1]
315315
if isexpr(f, :.)
316-
:(($(f.args[2]), $(f.args[1])))
316+
Expr(:tuple, f.args[2], f.args[1])
317317
elseif isexpr(f, :$)
318-
f
318+
func = f.args[1]
319+
if isa(func, String) || (isa(func, QuoteNode) && !isa(func.value, Ptr)) || isa(func, Tuple) || isexpr(func, :tuple)
320+
throw(ArgumentError("interpolated value should be a variable or expression, not a literal name or tuple"))
321+
end
322+
func
319323
elseif f isa Symbol
320-
QuoteNode(f)
324+
Expr(:tuple, QuoteNode(f))
321325
else
322326
throw(ArgumentError("@ccall function name must be a symbol, a `.` node (e.g. `libc.printf`) or an interpolated function pointer (with `\$`)"))
323327
end
@@ -363,33 +367,13 @@ end
363367

364368

365369
function ccall_macro_lower(convention, func, rettype, types, args, gc_safe, nreq)
366-
statements = []
367-
368-
# if interpolation was used, ensure the value is a function pointer at runtime.
369-
if isexpr(func, :$)
370-
push!(statements, Expr(:(=), :func, esc(func.args[1])))
371-
name = QuoteNode(func.args[1])
372-
func = :func
373-
check = quote
374-
if !isa(func, Ptr{Cvoid})
375-
name = $name
376-
throw(ArgumentError(LazyString("interpolated function `", name, "` was not a Ptr{Cvoid}, but ", typeof(func))))
377-
end
378-
end
379-
push!(statements, check)
380-
else
381-
func = esc(func)
382-
end
383-
cconv = nothing
384370
if convention isa Tuple
385371
cconv = Expr(:cconv, (convention..., gc_safe), nreq)
386372
else
387373
cconv = Expr(:cconv, (convention, UInt16(0), gc_safe), nreq)
388374
end
389-
390-
return Expr(:block, statements...,
391-
Expr(:call, :ccall, func, cconv, esc(rettype),
392-
Expr(:tuple, map(esc, types)...), map(esc, args)...))
375+
return Expr(:call, :ccall, esc(func), cconv, esc(rettype),
376+
Expr(:tuple, map(esc, types)...), map(esc, args)...)
393377
end
394378

395379
"""

base/gmp.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ using ..GMP: BigInt, Limb, BITS_PER_LIMB, libgmp
151151
const mpz_t = Ref{BigInt}
152152
const bitcnt_t = Culong
153153

154-
gmpz(op::Symbol) = (Symbol(:__gmpz_, op), libgmp)
154+
gmpz(op::Symbol) = Expr(:tuple, QuoteNode(Symbol(:__gmpz_, op)), GlobalRef(MPZ, :libgmp))
155155

156156
init!(x::BigInt) = (ccall((:__gmpz_init, libgmp), Cvoid, (mpz_t,), x); x)
157157
init2!(x::BigInt, a) = (ccall((:__gmpz_init2, libgmp), Cvoid, (mpz_t, bitcnt_t), x, a); x)
@@ -917,7 +917,7 @@ module MPQ
917917
import .Base: unsafe_rational, __throw_rational_argerror_zero
918918
import ..GMP: BigInt, MPZ, Limb, libgmp
919919

920-
gmpq(op::Symbol) = (Symbol(:__gmpq_, op), libgmp)
920+
gmpq(op::Symbol) = Expr(:tuple, QuoteNode(Symbol(:__gmpq_, op)), GlobalRef(MPZ, :libgmp))
921921

922922
mutable struct _MPQ
923923
num_alloc::Cint

doc/src/devdocs/llvm.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ need to make sure that the array does stay alive while we're doing the
346346
[`ccall`](@ref). To understand how this is done, lets look at a hypothetical
347347
approximate possible lowering of the above code:
348348
```julia
349-
return $(Expr(:foreigncall, :(:foo), Cvoid, svec(Ptr{Float64}), 0, :(:ccall), Expr(:foreigncall, :(:jl_array_ptr), Ptr{Float64}, svec(Any), 0, :(:ccall), :(A)), :(A)))
349+
return $(Expr(:foreigncall, Expr(:tuple, :(:foo)), Cvoid, svec(Ptr{Float64}), 0, :(:ccall), Expr(:foreigncall, Expr(:tuple, :(:jl_array_ptr)), Ptr{Float64}, svec(Any), 0, :(:ccall), :(A)), :(A)))
350350
```
351351
The last `:(A)`, is an extra argument list inserted during lowering that informs
352352
the code generator which Julia level values need to be kept alive for the

0 commit comments

Comments
 (0)