Skip to content

Commit

Permalink
Merge pull request #34326 from JuliaLang/tb/cfunction_closure_error
Browse files Browse the repository at this point in the history
Generate an error for closure cfunctions on unsupported platforms.
  • Loading branch information
maleadt authored Jan 14, 2020
2 parents dd79e77 + f53e144 commit 756891d
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 5 deletions.
2 changes: 1 addition & 1 deletion base/c.jl
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ Note that the argument type tuple must be a literal tuple, and not a tuple-value
(although it can include a splat expression). And that these arguments will be evaluated in global scope
during compile-time (not deferred until runtime).
Adding a '\\\$' in front of the function argument changes this to instead create a runtime closure
over the local variable `callable`.
over the local variable `callable` (this is not supported on all architectures).
See [manual section on ccall and cfunction usage](@ref Calling-C-and-Fortran-Code).
Expand Down
4 changes: 4 additions & 0 deletions doc/src/manual/calling-c-and-fortran-code.md
Original file line number Diff line number Diff line change
Expand Up @@ -945,6 +945,10 @@ function qsort(a::Vector{T}, cmp) where T
end
```

!!! note
Closure [`@cfunction`](@ref) rely on LLVM trampolines, which are not available on all
platforms (for example ARM and PowerPC).


## Closing a Library

Expand Down
6 changes: 6 additions & 0 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5084,6 +5084,12 @@ static jl_cgval_t emit_cfunction(jl_codectx_t &ctx, jl_value_t *output_type, con
}

bool nest = (!fexpr_rt.constant || unionall_env);
#if defined(_CPU_AARCH64_) || defined(_CPU_ARM_) || defined(_CPU_PPC64_)
if (nest) {
emit_error(ctx, "cfunction: closures are not supported on this platform");
return jl_cgval_t();
}
#endif
Value *F = gen_cfun_wrapper(
jl_Module,
sig, fexpr_rt.constant,
Expand Down
1 change: 1 addition & 0 deletions src/support/platform.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
* CPU/Architecture:
* _CPU_X86_
* _CPU_X86_64_
* _CPU_AARCH64_
* _CPU_ARM_
* _CPU_WASM_
*/
Expand Down
11 changes: 11 additions & 0 deletions test/ccall.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ using InteractiveUtils: code_llvm

import Libdl

# for cfunction_closure
include("testenv.jl")

const libccalltest = "libccalltest"

const verbose = false
Expand Down Expand Up @@ -791,6 +794,8 @@ end
## cfunction roundtrip

verbose && Libc.flush_cstdio()

if cfunction_closure
verbose && println("Testing cfunction closures: ")

# helper Type for testing that constructors work
Expand Down Expand Up @@ -974,6 +979,12 @@ for (t, v) in ((Complex{Int32}, :ci32), (Complex{Int64}, :ci64),
end
end

else

@test_broken "cfunction: no support for closures on this platform"

end

# issue 13031
foo13031(x) = Cint(1)
foo13031p = @cfunction(foo13031, Cint, (Ref{Tuple{}},))
Expand Down
4 changes: 4 additions & 0 deletions test/testenv.jl
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,8 @@ if !@isdefined(testenv_defined)
const curmod_name = fullname(curmod)
const curmod_str = curmod === Main ? "Main" : join(curmod_name, ".")
const curmod_prefix = "$(["$m." for m in curmod_name]...)"

# platforms that support cfunction with closures
# (requires LLVM back-end support for trampoline intrinsics)
const cfunction_closure = Sys.ARCH === :x86_64 || Sys.ARCH === :i686
end
13 changes: 9 additions & 4 deletions test/threads_exec.jl
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ using Test
using Base.Threads
using Base.Threads: SpinLock

# for cfunction_closure
include("testenv.jl")

# threading constructs

let a = zeros(Int, 2 * nthreads())
Expand Down Expand Up @@ -460,10 +463,12 @@ function test_thread_cfunction()
end
@test sum(ok) == 10000
end
if nthreads() == 1
test_thread_cfunction()
else
@test_broken "cfunction trampoline code not thread-safe"
if cfunction_closure
if nthreads() == 1
test_thread_cfunction()
else
@test_broken "cfunction trampoline code not thread-safe"
end
end

# Compare the two ways of checking if threading is enabled.
Expand Down

2 comments on commit 756891d

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your test job has completed - possible new issues were detected. A full report can be found here. cc @maleadt

Please sign in to comment.