Eliminating allocation of ccall root in simple cases

Also fix and test an invalid `unsafe_convert` definition in Base.
The result of `pointer_from_objref` on isbits types is never valid to use
across safepoint.

base/inference.jl

@@ -5231,6 +5231,27 @@ function occurs_outside_getfield(@nospecialize(e), @nospecialize(sym),
  if head === :(=)
  return occurs_outside_getfield(e.args[2], sym, sv,
  field_count, field_names)
+ elseif head === :foreigncall
+ args = e.args
+ nccallargs = args[5]::Int
+ # Only arguments escape the structure/layout of the object,
+ # GC root arguments do not.
+ # Also note that only being used in the root slot for this ccall itself
+ # does **not** mean that the object is not needed during the ccall.
+ # However, if its address is never taken
+ # and the object is never used in a way that escapes its layout, we can be sure
+ # that there's no way the user code can rely on the heap allocation of this object.
+ for i in 1:length(args)
+ a = args[i]
+ if i > 5 + nccallargs && symequal(a, sym)
+ # No need to verify indices, uninitialized members can be
+ # ignored in root slot.
+ continue
+ end
+ if occurs_outside_getfield(a, sym, sv, field_count, field_names)
+ return true
+ end
+ end
  else
  if (head === :block && isa(sym, Slot) &&
  sv.src.slotflags[slot_id(sym)] & Slot_UsedUndef == 0)
@@ -5814,8 +5835,11 @@ end
 function replace_getfield!(e::Expr, tupname, vals, field_names, sv::InferenceState)
  for i = 1:length(e.args)
  a = e.args[i]
- if isa(a,Expr) && is_known_call(a, getfield, sv.src, sv.mod) &&
- symequal(a.args[2],tupname)
+ if !isa(a, Expr)
+ continue
+ end
+ a = a::Expr
+ if is_known_call(a, getfield, sv.src, sv.mod) && symequal(a.args[2], tupname)
  idx = if isa(a.args[3], Int)
  a.args[3]
  else
@@ -5844,8 +5868,23 @@ function replace_getfield!(e::Expr, tupname, vals, field_names, sv::InferenceSta
  end
  end
  e.args[i] = val
- elseif isa(a, Expr)
- replace_getfield!(a::Expr, tupname, vals, field_names, sv)
+ else
+ if a.head === :foreigncall
+ args = a.args
+ nccallargs = args[5]::Int
+ le = length(args)
+ next_i = 6 + nccallargs
+ while next_i <= le
+ i = next_i
+ next_i += 1
+
+ symequal(args[i], tupname) || continue
+ # Replace the gc root argument with its fields
+ splice!(args, i, vals)
+ next_i += length(vals) - 1
+ end
+ end
+ replace_getfield!(a, tupname, vals, field_names, sv)
  end
  end
 end

base/refpointer.jl

@@ -54,9 +54,16 @@ convert(::Type{Ref{T}}, x) where {T} = RefValue{T}(x)
 
 function unsafe_convert(P::Type{Ptr{T}}, b::RefValue{T}) where T
  if isbits(T)
- return convert(P, data_pointer_from_objref(b))
+ return convert(P, pointer_from_objref(b))
+ elseif isleaftype(T)
+ return convert(P, pointer_from_objref(b.x))
  else
- return convert(P, data_pointer_from_objref(b.x))
+ # If the slot is not leaf type, it could be either isbits or not.
+ # If it is actually an isbits type and the type inference can infer that
+ # it can rebox the `b.x` if we simply call `pointer_from_objref(b.x)` on it.
+ # Instead, explicitly load the pointer from the `RefValue` so that the pointer
+ # is the same as the one rooted in the `RefValue` object.
+ return convert(P, pointerref(Ptr{Ptr{Void}}(pointer_from_objref(b)), 1, 0))
  end
 end
 function unsafe_convert(P::Type{Ptr{Any}}, b::RefValue{Any})

test/ccall.jl

@@ -1285,3 +1285,21 @@ evalf_callback_c_19805{FUNC_FT}(ci::callinfos_19805{FUNC_FT}) = cfunction(
 
 @test_throws(ErrorException("ccall: the type of argument 1 doesn't correspond to a C type"),
  evalf_callback_c_19805( callinfos_19805(sin) ))
+
+# test Ref{abstract_type} calling parameter passes a heap box
+abstract type Abstract22734 end
+struct Bits22734 <: Abstract22734
+ x::Int
+ y::Float64
+end
+function cb22734(ptr::Ptr{Void})
+ gc()
+ obj = unsafe_pointer_to_objref(ptr)::Bits22734
+ obj.x + obj.y
+end
+ptr22734 = cfunction(cb22734, Float64, Tuple{Ptr{Void}})
+function caller22734(ptr)
+ obj = Bits22734(12, 20)
+ ccall(ptr, Float64, (Ref{Abstract22734},), obj)
+end
+@test caller22734(ptr22734) === 32.0

test/codegen.jl

@@ -128,10 +128,37 @@ function compare_large_struct(a)
  end
 end
 
+mutable struct MutableStruct
+ a::Int
+ MutableStruct() = new()
+end
+
+breakpoint_mutable(a::MutableStruct) = ccall(:jl_breakpoint, Void, (Ref{MutableStruct},), a)
+
+# Allocation with uninitialized field as gcroot
+mutable struct BadRef
+ x::MutableStruct
+ y::MutableStruct
+ BadRef(x) = new(x)
+end
+Base.cconvert(::Type{Ptr{BadRef}}, a::MutableStruct) = BadRef(a)
+Base.unsafe_convert(::Type{Ptr{BadRef}}, ar::BadRef) = Ptr{BadRef}(pointer_from_objref(ar.x))
+
+breakpoint_badref(a::MutableStruct) = ccall(:jl_breakpoint, Void, (Ptr{BadRef},), a)
+
 if opt_level > 0
  @test !contains(get_llvm(isequal, Tuple{Nullable{BigFloat}, Nullable{BigFloat}}), "%gcframe")
  @test !contains(get_llvm(pointer_not_safepoint, Tuple{}), "%gcframe")
  compare_large_struct_ir = get_llvm(compare_large_struct, Tuple{typeof(create_ref_struct())})
  @test contains(compare_large_struct_ir, "call i32 @memcmp")
  @test !contains(compare_large_struct_ir, "%gcframe")
+
+ @test contains(get_llvm(MutableStruct, Tuple{}), "jl_gc_pool_alloc")
+ breakpoint_mutable_ir = get_llvm(breakpoint_mutable, Tuple{MutableStruct})
+ @test !contains(breakpoint_mutable_ir, "%gcframe")
+ @test !contains(breakpoint_mutable_ir, "jl_gc_pool_alloc")
+
+ breakpoint_badref_ir = get_llvm(breakpoint_badref, Tuple{MutableStruct})
+ @test !contains(breakpoint_badref_ir, "%gcframe")
+ @test !contains(breakpoint_badref_ir, "jl_gc_pool_alloc")
 end