Skip to content

Commit

Permalink
Fix nothrow behavior for intrinsics (#34539)
Browse files Browse the repository at this point in the history
This fixes the case in #34482 and adds some more robustness for similar
cases, though I wouldn't be surprised if there were more dragons hiding
here. Intrinsics are a bit special and from the start, we sort of expected
them to only ever be called correctly under pain of segfaults or other
undefined behavior. We've been gradually making these more robust,
but fundamentally, they were never intended to be used by users directly,
only through the type-validating wrappers in Base. What has changed
to cause the recent influx of issues in this area is that people now
like to do compiler transforms that happily recurse through these wrappers
and perform transforms that are not always legal on Intrinsics. This should
help catch a number of them, but this interface is still not very thoroughly
validated, and I would be surprised to see crashes or other errors stemming
from incorrect usage here.
  • Loading branch information
Keno authored Feb 28, 2020
1 parent 012b270 commit 3f05b0d
Show file tree
Hide file tree
Showing 5 changed files with 94 additions and 27 deletions.
3 changes: 1 addition & 2 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1169,8 +1169,7 @@ function early_inline_special_case(ir::IRCode, s::Signature, e::Expr, params::Pa
val = etype.val
is_inlineable_constant(val) || return nothing
if isa(f, IntrinsicFunction)
if is_pure_intrinsic_infer(f) &&
(intrinsic_nothrow(f) || intrinsic_nothrow(f, atypes[2:end]))
if is_pure_intrinsic_infer(f) && intrinsic_nothrow(f, atypes[2:end])
return quoted(val)
end
elseif ispuretopfunction(f) || contains_is(_PURE_BUILTINS, f)
Expand Down
3 changes: 1 addition & 2 deletions base/compiler/ssair/queries.jl
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ function stmt_effect_free(@nospecialize(stmt), @nospecialize(rt), src, sptypes::
is_return_type(f) && return true
if isa(f, IntrinsicFunction)
intrinsic_effect_free_if_nothrow(f) || return false
return intrinsic_nothrow(f) ||
intrinsic_nothrow(f,
return intrinsic_nothrow(f,
Any[argextype(ea[i], src, sptypes) for i = 2:length(ea)])
end
contains_is(_PURE_BUILTINS, f) && return true
Expand Down
89 changes: 68 additions & 21 deletions base/compiler/tfuncs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -403,20 +403,23 @@ add_tfunc(Core._typevar, 3, 3, typevar_tfunc, 100)
add_tfunc(applicable, 1, INT_INF, (@nospecialize(f), args...)->Bool, 100)
add_tfunc(Core.Intrinsics.arraylen, 1, 1, @nospecialize(x)->Int, 4)
add_tfunc(arraysize, 2, 2, (@nospecialize(a), @nospecialize(d))->Int, 4)
function pointer_eltype(@nospecialize(ptr))
a = widenconst(ptr)
if a <: Ptr
if isa(a,DataType) && isa(a.parameters[1],Type)
return a.parameters[1]
elseif isa(a,UnionAll) && !has_free_typevars(a)
unw = unwrap_unionall(a)
if isa(unw,DataType)
return rewrap_unionall(unw.parameters[1], a)
end
end
end
return Any
end
add_tfunc(pointerref, 3, 3,
function (@nospecialize(a), @nospecialize(i), @nospecialize(align))
a = widenconst(a)
if a <: Ptr
if isa(a,DataType) && isa(a.parameters[1],Type)
return a.parameters[1]
elseif isa(a,UnionAll) && !has_free_typevars(a)
unw = unwrap_unionall(a)
if isa(unw,DataType)
return rewrap_unionall(unw.parameters[1], a)
end
end
end
return Any
return pointer_eltype(a)
end, 4)
add_tfunc(pointerset, 4, 4, (@nospecialize(a), @nospecialize(v), @nospecialize(i), @nospecialize(align)) -> a, 5)

Expand Down Expand Up @@ -1393,24 +1396,35 @@ function builtin_tfunction(@nospecialize(f), argtypes::Array{Any,1},
end

# Query whether the given intrinsic is nothrow
intrinsic_nothrow(f::IntrinsicFunction) = !(
f === Intrinsics.checked_sdiv_int ||
f === Intrinsics.checked_udiv_int ||
f === Intrinsics.checked_srem_int ||
f === Intrinsics.checked_urem_int ||
f === Intrinsics.cglobal
)

function intrinsic_nothrow(f::IntrinsicFunction, argtypes::Array{Any, 1})
# First check that we have the correct number of arguments
iidx = Int(reinterpret(Int32, f::IntrinsicFunction)) + 1
if iidx < 1 || iidx > length(T_IFUNC)
# invalid intrinsic
return false
end
tf = T_IFUNC[iidx]
tf = tf::Tuple{Int, Int, Any}
if !(tf[1] <= length(argtypes) <= tf[2])
# wrong # of args
return false
end
# TODO: We could do better for cglobal
f === Intrinsics.cglobal && return false
# TODO: We can't know for sure, but the user should have a way to assert
# that it won't
f === Intrinsics.llvmcall && return false
if f === Intrinsics.checked_udiv_int || f === Intrinsics.checked_urem_int || f === Intrinsics.checked_srem_int || f === Intrinsics.checked_sdiv_int
# Nothrow as long as the second argument is guaranteed not to be zero
isa(argtypes[2], Const) || return false
if !isprimitivetype(widenconst(argtypes[1])) ||
(widenconst(argtypes[1]) !== widenconst(argtypes[2]))
return false
end
den_val = argtypes[2].val
den_val !== zero(typeof(den_val)) || return false
end
if f === Intrinsics.checked_sdiv_int
f !== Intrinsics.checked_sdiv_int && return true
# Nothrow as long as we additionally don't do typemin(T)/-1
return den_val !== -1 || (isa(argtypes[1], Const) &&
argtypes[1].val !== typemin(typeof(den_val)))
Expand All @@ -1422,6 +1436,39 @@ function intrinsic_nothrow(f::IntrinsicFunction, argtypes::Array{Any, 1})
length(argtypes) == 3 || return false
return argtypes[1] Ptr && argtypes[2] Int && argtypes[3] Int
end
if f === Intrinsics.pointerset
eT = pointer_eltype(argtypes[1])
isprimitivetype(eT) || return false
return argtypes[2] eT && argtypes[3] Int && argtypes[4] Int
end
if f === Intrinsics.arraylen
return argtypes[1] Array
end
if f === Intrinsics.bitcast
ty = instanceof_tfunc(argtypes[1])[1]
xty = widenconst(argtypes[2])
return isprimitivetype(ty) && isprimitivetype(xty) && ty.size === xty.size
end
if f in (Intrinsics.sext_int, Intrinsics.zext_int, Intrinsics.trunc_int,
Intrinsics.fptoui, Intrinsics.fptosi, Intrinsics.uitofp,
Intrinsics.sitofp, Intrinsics.fptrunc, Intrinsics.fpext)
# If !isexact, `ty` may be Union{} at runtime even if we have
# isprimitivetype(ty).
ty, isexact = instanceof_tfunc(argtypes[1])
xty = widenconst(argtypes[2])
return isexact && isprimitivetype(ty) && isprimitivetype(xty)
end
# The remaining intrinsics are math/bits/comparison intrinsics. They work on all
# primitive types of the same type.
isshift = f == shl_int || f == lshr_int || f == ashr_int
argtype1 = widenconst(argtypes[1])
isprimitivetype(argtype1) || return false
for i = 2:length(argtypes)
argtype = widenconst(argtypes[i])
if isshift ? !isprimitivetype(argtype) : argtype !== argtype1
return false
end
end
return true
end

Expand Down
9 changes: 7 additions & 2 deletions src/intrinsics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -895,8 +895,13 @@ static jl_cgval_t emit_intrinsic(jl_codectx_t &ctx, intrinsic f, jl_value_t **ar
// return emit_runtime_call(ctx, f, argv, nargs);

switch (f) {
case arraylen:
return mark_julia_type(ctx, emit_arraylen(ctx, argv[0]), false, jl_long_type);
case arraylen: {
const jl_cgval_t &x = argv[0];
jl_value_t *typ = jl_unwrap_unionall(x.typ);
if (!jl_is_datatype(typ) || ((jl_datatype_t*)typ)->name != jl_array_typename)
return emit_runtime_call(ctx, f, argv, nargs);
return mark_julia_type(ctx, emit_arraylen(ctx, x), false, jl_long_type);
}
case pointerref:
return emit_pointerref(ctx, argv);
case pointerset:
Expand Down
17 changes: 17 additions & 0 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7173,3 +7173,20 @@ function f34247(a)
true
end
@test f34247("")

# Issue #34482
function f34482()
Base.not_int("ABC")
1
end
function g34482()
Core.Intrinsics.arraylen(1)
1
end
function h34482()
Core.Intrinsics.bitcast(1, 1)
1
end
@test_throws ErrorException f34482()
@test_throws TypeError g34482()
@test_throws TypeError h34482()

0 comments on commit 3f05b0d

Please sign in to comment.