Skip to content

Commit

Permalink
don't capture arrays/tuples in BoundsError
Browse files Browse the repository at this point in the history
This commit changes the semantics of `BoundsError` so that it doesn't
capture arrays/tuples passed to its constructor.

The change wouldn't improve any performance by itself because the
`BoundsError` constructor is mostly called on error paths and thus
it is opaque to analyses/optimizers when compiling a caller context.
Rather it's supposed to be a building block for broadening the possibility
of certain memory optimizations in the future such as copy elision for
`ImmutableArray` construction and stack allocation of `Array`s, by
allowing us to assume the invariant that primitive indexing operations
into array/tuple indexing don't escape it so that our escape analyses
can achieve further accuracy.

Specifically, when `BoundsError` constructor will now compute the "summary"
of its `Array`/`Tuple` argument, rather than capturing it for the later
inspection. Assuming `Base.summary(x::Array)` or `Base.summary(x::Tuple)`
don't escape `x`, we can assume that `BoundsError` doesn't escape `x`.
This change won't appear as breaking in most cases since `showerror` will
print the exact same error message as before, but obviously this is
technically breaking since we can no longer access to the original
arrays/tuples by catching `BoundsError`.
I'd say this breaking semantic change would be still acceptable, since
I think it's enough if we can know size/type of arrays/tuples from
`BoundsError` in most cases.

As a last note, I didn't change the semantics for arbitrary user objects,
since we still don't have a good infrastructure to tell the compiler some
primitive assumptions/rules about user type objects anyway.
  • Loading branch information
aviatesk committed Jan 10, 2022
1 parent dbd10e0 commit 4929b0a
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 11 deletions.
3 changes: 3 additions & 0 deletions base/Base.jl
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,9 @@ end_base_include = time_ns()
const _sysimage_modules = PkgId[]
in_sysimage(pkgid::PkgId) = pkgid in _sysimage_modules

# used for the hack within BoundsError constructor
const _BASE_DEFINED_AGE = get_world_counter()

# Precompiles for Revise
# TODO: move these to contrib/generate_precompile.jl
# The problem is they don't work there
Expand Down
39 changes: 36 additions & 3 deletions base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -270,13 +270,46 @@ end
macro inline() Expr(:meta, :inline) end
macro noinline() Expr(:meta, :noinline) end

import .Intrinsics: ult_int
struct Summarized
desc::String
size
type
function Summarized(@nospecialize a)
# try to get a summary of `a` here now rather than capturing it for later inspection,
# in order to allow compiler analyses or optimization passes to assume the invariant
# that this `BoundsError` doesn't escape `a` when it is a certain primitive object
# that they can reason about
if isa(a, Array) || isa(a, Tuple)
if isdefined(Main, :Base)
Base = Main.Base
# these `invoke_in_world`s here essentially prohibits users from changing
# the semantics of `BoundsError` by overriding `summary(a)` implementations
# (and potentially allowing `a` to be escaped to somewhere)
desc = Base.invoke_in_world(Base._BASE_DEFINED_AGE, Base.summary, a)::String
if isa(a, Array)
size = Base.invoke_in_world(Base._BASE_DEFINED_AGE, Base.size, a)
else
size = (nfields(a),)
end
else
desc = "`summary` implementation not available"
size = nothing
end
type = typeof(a)
return new(desc, size, type)
end
return a
end
end
struct BoundsError <: Exception
a::Any
a
i::Any
BoundsError() = new()
BoundsError(@nospecialize(a)) = (@noinline; new(a))
BoundsError(@nospecialize(a), i) = (@noinline; new(a,i))
BoundsError(@nospecialize(a)) = (@noinline; new(Summarized(a)))
BoundsError(@nospecialize(a), i) = (@noinline; new(Summarized(a), i))
end

struct DivideError <: Exception end
struct OutOfMemoryError <: Exception end
struct ReadOnlyMemoryError <: Exception end
Expand Down
7 changes: 6 additions & 1 deletion base/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,12 @@ function showerror(io::IO, ex::BoundsError)
print(io, "BoundsError")
if isdefined(ex, :a)
print(io, ": attempt to access ")
summary(io, ex.a)
a = ex.a
if isa(a, Core.Summarized)
print(io, a.desc)
else
summary(io, a)
end
if isdefined(ex, :i)
print(io, " at index [")
if ex.i isa AbstractRange
Expand Down
3 changes: 3 additions & 0 deletions base/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2792,6 +2792,9 @@ function summary(x)
String(take!(io))
end

# NOTE `summary(a)` implementations for `Array` and `Tuple` should never escape `a` anywhere,
# since that invariant can be assumed by some compiler analyses and optimization passes

## `summary` for AbstractArrays
# sizes such as 0-dimensional, 4-dimensional, 2x3
dims2string(d) = isempty(d) ? "0-dimensional" :
Expand Down
30 changes: 25 additions & 5 deletions src/rtutils.c
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,11 @@ JL_DLLEXPORT void JL_NORETURN jl_atomic_error(char *str) // == jl_exceptionf(jl_
JL_DLLEXPORT void JL_NORETURN jl_bounds_error(jl_value_t *v, jl_value_t *t)
{
JL_GC_PUSH2(&v, &t); // root arguments so the caller doesn't need to
jl_throw(jl_new_struct((jl_datatype_t*)jl_boundserror_type, v, t));
jl_value_t **args;
JL_GC_PUSHARGS(args, 2);
args[0] = v;
args[1] = t;
jl_throw(jl_apply_generic((jl_value_t*)jl_boundserror_type, args, 2));
}

JL_DLLEXPORT void JL_NORETURN jl_bounds_error_v(jl_value_t *v, jl_value_t **idxs, size_t nidxs)
Expand All @@ -152,7 +156,11 @@ JL_DLLEXPORT void JL_NORETURN jl_bounds_error_v(jl_value_t *v, jl_value_t **idxs
// items in idxs are assumed to already be rooted
JL_GC_PUSH2(&v, &t); // root v so the caller doesn't need to
t = jl_f_tuple(NULL, idxs, nidxs);
jl_throw(jl_new_struct((jl_datatype_t*)jl_boundserror_type, v, t));
jl_value_t **args;
JL_GC_PUSHARGS(args, 2);
args[0] = v;
args[1] = t;
jl_throw(jl_apply_generic((jl_value_t*)jl_boundserror_type, args, 2));
}

JL_DLLEXPORT void JL_NORETURN jl_bounds_error_tuple_int(jl_value_t **v, size_t nv, size_t i)
Expand All @@ -169,15 +177,23 @@ JL_DLLEXPORT void JL_NORETURN jl_bounds_error_unboxed_int(void *data, jl_value_t
JL_GC_PUSH2(&v, &t);
v = jl_new_bits(vt, data);
t = jl_box_long(i);
jl_throw(jl_new_struct((jl_datatype_t*)jl_boundserror_type, v, t));
jl_value_t **args;
JL_GC_PUSHARGS(args, 2);
args[0] = v;
args[1] = t;
jl_throw(jl_apply_generic((jl_value_t*)jl_boundserror_type, args, 2));
}

JL_DLLEXPORT void JL_NORETURN jl_bounds_error_int(jl_value_t *v JL_MAYBE_UNROOTED, size_t i)
{
jl_value_t *t = NULL;
JL_GC_PUSH2(&v, &t); // root arguments so the caller doesn't need to
t = jl_box_long(i);
jl_throw(jl_new_struct((jl_datatype_t*)jl_boundserror_type, v, t));
jl_value_t **args;
JL_GC_PUSHARGS(args, 2);
args[0] = v;
args[1] = t;
jl_throw(jl_apply_generic((jl_value_t*)jl_boundserror_type, args, 2));
}

JL_DLLEXPORT void JL_NORETURN jl_bounds_error_ints(jl_value_t *v JL_MAYBE_UNROOTED,
Expand All @@ -191,7 +207,11 @@ JL_DLLEXPORT void JL_NORETURN jl_bounds_error_ints(jl_value_t *v JL_MAYBE_UNROOT
jl_svecset(t, i, jl_box_long(idxs[i]));
}
t = jl_f_tuple(NULL, jl_svec_data(t), nidxs);
jl_throw(jl_new_struct((jl_datatype_t*)jl_boundserror_type, v, t));
jl_value_t **args;
JL_GC_PUSHARGS(args, 2);
args[0] = v;
args[1] = t;
jl_throw(jl_apply_generic((jl_value_t*)jl_boundserror_type, args, 2));
}

JL_DLLEXPORT void JL_NORETURN jl_eof_error(void)
Expand Down
32 changes: 30 additions & 2 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2640,8 +2640,16 @@ end

# pull request #9534
@test_throws BoundsError((1, 2), 3) begin; a, b, c = 1, 2; end
let a = []
@test try; a[]; catch ex; (ex::BoundsError).a === a && ex.i == (); end
let a = Any[]
try
a[]
catch ex
x = (ex::BoundsError).a
@test isa(x, Core.Summarized)
@test x.size === (0,)
@test x.type === Vector{Any}
@test ex.i == ()
end
@test_throws BoundsError(a, (1, 2)) a[1, 2]
@test_throws BoundsError(a, (10,)) a[10]
end
Expand Down Expand Up @@ -2681,6 +2689,26 @@ f9534h(a, b, c...) = c[a]
@test f9534h(4, 2, 3, 4, 5, 6) == 6
@test_throws BoundsError((3, 4, 5, 6), 5) f9534h(5, 2, 3, 4, 5, 6)

@test let # https://github.com/JuliaLang/julia/pull/43738
# BoundsError should never capture arrays
code = quote
const THROWN_ARRAY = Ref{Any}()
function Base.summary(io::IO, a::Array)
Main.THROWN_ARRAY[] = a
return "bad override"
end
try
a = []
a[1]
catch err
end
isdefined(THROWN_ARRAY, :x) && println(stderr, "test failed")
end |> string
cmd = `$(Base.julia_cmd()) -e $code`
stderr = IOBuffer()
success(pipeline(Cmd(cmd); stdout=stdout, stderr=stderr)) && isempty(String(take!(stderr)))
end

# issue #7978, comment 332352438
f7978a() = 1
@test_throws BoundsError(1, 2) begin; a, b = f7978a(); end
Expand Down

0 comments on commit 4929b0a

Please sign in to comment.