Merge pull request JuliaLang#48158 from JuliaLang/avi/inbounds

effects: taint `:consistent`-cy on `:inbounds` and `:boundscheck` exprs

base/compiler/abstractinterpretation.jl

@@ -2169,22 +2169,35 @@ function abstract_eval_cfunction(interp::AbstractInterpreter, e::Expr, vtypes::V
     nothing
 end
 
-function abstract_eval_value_expr(interp::AbstractInterpreter, e::Expr, sv::Union{InferenceState, IRCode})
+function abstract_eval_value_expr(interp::AbstractInterpreter, e::Expr, vtypes::Union{VarTable, Nothing}, sv::Union{InferenceState, IRCode})
+    rt = Any
     head = e.head
     if head === :static_parameter
         n = e.args[1]::Int
-        t = Any
         if 1 <= n <= length(sv.sptypes)
-            t = sv.sptypes[n]
+            rt = sv.sptypes[n]
         end
-        return t
     elseif head === :boundscheck
-        return Bool
+        if isa(sv, InferenceState)
+            stmt = sv.src.code[sv.currpc]
+            if isexpr(stmt, :call)
+                f = abstract_eval_value(interp, stmt.args[1], vtypes, sv)
+                if f isa Const && f.val === getfield
+                    # boundscheck of `getfield` call is analyzed by tfunc potentially without
+                    # tainting :consistent-cy when it's known to be nothrow
+                    @goto delay_effects_analysis
+                end
+            end
+        end
+        merge_effects!(interp, sv, Effects(EFFECTS_TOTAL; consistent=ALWAYS_FALSE, noinbounds=false))
+        @label delay_effects_analysis
+        rt = Bool
+    elseif head === :inbounds
+        merge_effects!(interp, sv, Effects(EFFECTS_TOTAL; consistent=ALWAYS_FALSE, noinbounds=false))
     elseif head === :the_exception
         merge_effects!(interp, sv, Effects(EFFECTS_TOTAL; consistent=ALWAYS_FALSE))
-        return Any
     end
-    return Any
+    return rt
 end
 
 function abstract_eval_special_value(interp::AbstractInterpreter, @nospecialize(e), vtypes::Union{VarTable, Nothing}, sv::Union{InferenceState, IRCode})
@@ -2214,7 +2227,7 @@ end
 
 function abstract_eval_value(interp::AbstractInterpreter, @nospecialize(e), vtypes::Union{VarTable, Nothing}, sv::Union{InferenceState, IRCode})
     if isa(e, Expr)
-        return abstract_eval_value_expr(interp, e, sv)
+        return abstract_eval_value_expr(interp, e, vtypes, sv)
     else
         typ = abstract_eval_special_value(interp, e, vtypes, sv)
         return collect_limitations!(typ, sv)
@@ -2443,7 +2456,7 @@ function abstract_eval_statement_expr(interp::AbstractInterpreter, e::Expr, vtyp
         effects = EFFECTS_THROWS
         merge_effects!(interp, sv, effects)
     else
-        t = abstract_eval_value_expr(interp, e, sv)
+        t = abstract_eval_value_expr(interp, e, vtypes, sv)
     end
     return RTEffects(t, effects)
 end

base/compiler/inferencestate.jl

@@ -184,7 +184,7 @@ mutable struct InferenceState
         #       are stronger than the inbounds assumptions, since the latter
         #       requires dynamic reachability, while the former is global).
         inbounds = inbounds_option()
-        noinbounds = inbounds === :on || (inbounds === :default && !any_inbounds(code))
+        noinbounds = inbounds === :on || (inbounds === :default && all(flag::UInt8->iszero(flag&IR_FLAG_INBOUNDS), src.ssaflags))
         consistent = noinbounds ? ALWAYS_TRUE : ALWAYS_FALSE
         ipo_effects = Effects(EFFECTS_TOTAL; consistent, noinbounds)
 
@@ -240,16 +240,6 @@ function bail_out_apply(::AbstractInterpreter, @nospecialize(rt), sv::Union{Infe
     return rt === Any
 end
 
-function any_inbounds(code::Vector{Any})
-    for i = 1:length(code)
-        stmt = code[i]
-        if isexpr(stmt, :inbounds)
-            return true
-        end
-    end
-    return false
-end
-
 was_reached(sv::InferenceState, pc::Int) = sv.ssavaluetypes[pc] !== NOT_FOUND
 
 function compute_trycatch(code::Vector{Any}, ip::BitSet)

base/compiler/tfuncs.jl

@@ -2197,7 +2197,7 @@ function getfield_effects(argtypes::Vector{Any}, @nospecialize(rt))
         # If we cannot independently prove inboundsness, taint consistency.
         # The inbounds-ness assertion requires dynamic reachability, while
         # :consistent needs to be true for all input values.
-        # N.B. We do not taint for `--check-bounds=no` here -that happens in
+        # N.B. We do not taint for `--check-bounds=no` here that happens in
         # InferenceState.
         consistent = ALWAYS_FALSE
     end

base/ntuple.jl

@@ -33,13 +33,13 @@ end
 
 function _ntuple(f::F, n) where F
     @noinline
-    (n >= 0) || throw(ArgumentError(string("tuple length should be ≥ 0, got ", n)))
+    (n >= 0) || throw(ArgumentError(LazyString("tuple length should be ≥ 0, got ", n)))
     ([f(i) for i = 1:n]...,)
 end
 
 function ntupleany(f, n)
     @noinline
-    (n >= 0) || throw(ArgumentError(string("tuple length should be ≥ 0, got ", n)))
+    (n >= 0) || throw(ArgumentError(LazyString("tuple length should be ≥ 0, got ", n)))
     (Any[f(i) for i = 1:n]...,)
 end
 
@@ -68,7 +68,7 @@ julia> ntuple(i -> 2*i, Val(4))
 """
 @inline function ntuple(f::F, ::Val{N}) where {F,N}
     N::Int
-    (N >= 0) || throw(ArgumentError(string("tuple length should be ≥ 0, got ", N)))
+    (N >= 0) || throw(ArgumentError(LazyString("tuple length should be ≥ 0, got ", N)))
     if @generated
         :(@ntuple $N i -> f(i))
     else
@@ -79,7 +79,7 @@ end
 @inline function fill_to_length(t::Tuple, val, ::Val{_N}) where {_N}
     M = length(t)
     N = _N::Int
-    M > N && throw(ArgumentError("input tuple of length $M, requested $N"))
+    M > N && throw(ArgumentError(LazyString("input tuple of length ", M, ", requested ", N)))
     if @generated
         quote
             (t..., $(fill(:val, (_N::Int) - length(t.parameters))...))

test/boundscheck.jl

@@ -2,17 +2,14 @@
 
 # run boundscheck tests on separate workers launched with --check-bounds={default,yes,no}
 
-cmd = `$(Base.julia_cmd()) --depwarn=error --startup-file=no boundscheck_exec.jl`
-if !success(pipeline(cmd; stdout=stdout, stderr=stderr))
-    error("boundscheck test failed, cmd : $cmd")
+let cmd = `$(Base.julia_cmd()) --check-bounds=auto --depwarn=error --startup-file=no boundscheck_exec.jl`
+    success(pipeline(cmd; stdout=stdout, stderr=stderr)) || error("boundscheck test failed, cmd : $cmd")
 end
 
-cmd = `$(Base.julia_cmd()) --check-bounds=yes --startup-file=no --depwarn=error boundscheck_exec.jl`
-if !success(pipeline(cmd; stdout=stdout, stderr=stderr))
-    error("boundscheck test failed, cmd : $cmd")
+let cmd = `$(Base.julia_cmd()) --check-bounds=yes --startup-file=no --depwarn=error boundscheck_exec.jl`
+    success(pipeline(cmd; stdout=stdout, stderr=stderr)) || error("boundscheck test failed, cmd : $cmd")
 end
 
-cmd = `$(Base.julia_cmd()) --check-bounds=no --startup-file=no --depwarn=error boundscheck_exec.jl`
-if !success(pipeline(cmd; stdout=stdout, stderr=stderr))
-    error("boundscheck test failed, cmd : $cmd")
+let cmd = `$(Base.julia_cmd()) --check-bounds=no --startup-file=no --depwarn=error boundscheck_exec.jl`
+    success(pipeline(cmd; stdout=stdout, stderr=stderr)) || error("boundscheck test failed, cmd : $cmd")
 end

test/compiler/effects.jl

@@ -323,15 +323,27 @@ invoke44763(x) = @invoke increase_x44763!(x)
 end |> only === Int
 @test x44763 == 0
 
+# `@inbounds`/`@boundscheck` expression should taint :consistent-cy correctly
+# https://github.com/JuliaLang/julia/issues/48099
+function A1_inbounds()
+    r = 0
+    @inbounds begin
+        @boundscheck r += 1
+    end
+    return r
+end
+@test !Core.Compiler.is_consistent(Base.infer_effects(A1_inbounds))
+
 # Test that purity doesn't try to accidentally run unreachable code due to
 # boundscheck elimination
 function f_boundscheck_elim(n)
-    # Inbounds here assumes that this is only ever called with n==0, but of
+    # Inbounds here assumes that this is only ever called with `n==0`, but of
     # course the compiler has no way of knowing that, so it must not attempt
-    # to run the @inbounds `getfield(sin, 1)`` that ntuple generates.
+    # to run the `@inbounds getfield(sin, 1)` that `ntuple` generates.
     ntuple(x->(@inbounds getfield(sin, x)), n)
 end
-@test Tuple{} <: code_typed(f_boundscheck_elim, Tuple{Int})[1][2]
+@test !Core.Compiler.is_consistent(Base.infer_effects(f_boundscheck_elim, (Int,)))
+@test Tuple{} <: only(Base.return_types(f_boundscheck_elim, (Int,)))
 
 # Test that purity modeling doesn't accidentally introduce new world age issues
 f_redefine_me(x) = x+1