fix(auto): stop false-positive generators in effect summarization

README.md

@@ -56,7 +56,7 @@ In BorrowChecker.jl, we demonstrate an implementation of some of these ideas. Th
 
 ## Automatic Checking: `BorrowChecker.@safe`
 
-`BorrowChecker.@safe` automatically instruments a function by analyzing the compiler IR and runs a best-effort borrow check at runtime. This requires Julia 1.12.x (on 1.13+ the checker falls back to warn-and-pass-through stubs until support lands).
+`BorrowChecker.@safe` automatically instruments a function by analyzing the compiler IR and runs a best-effort borrow check at runtime. This requires Julia 1.12 or 1.13 (on newer versions the checker falls back to warn-and-pass-through stubs until support lands).
 
 > [!WARNING]
 > This macro is highly experimental and compiler-dependent. There are likely bugs and false positives. It is intended for development and testing, and does not guarantee memory safety.

src/BorrowChecker.jl

@@ -9,7 +9,7 @@ using DispatchDoctor: @unstable
 # check at runtime. This is the entire library.
 export @safe, @unsafe, disable_by_default!
 
-@static if isdefined(Base, :code_ircode_by_type) && v"1.12.0-" <= VERSION < v"1.13.0-"
+@static if isdefined(Base, :code_ircode_by_type) && v"1.12.0-" <= VERSION < v"1.14.0-"
     @unstable include("safe/auto_ir.jl")
     # `BorrowCheckError` and friends are defined by safe/auto_ir.jl.
     export BorrowCheckError

src/safe/checker.jl

@@ -112,7 +112,9 @@ function _compute_liveness(
     return live_in, live_out
 end
 
-function check_ir(ir::CC.IRCode, cfg::Config)::Vector{BorrowViolation}
+function check_ir(
+    ir::CC.IRCode, cfg::Config; budget_state::Union{Nothing,BudgetTracker}=nothing
+)::Vector{BorrowViolation}
     nargs = length(ir.argtypes)
     nstmts = length(ir.stmts)
 
@@ -153,6 +155,10 @@ function check_ir(ir::CC.IRCode, cfg::Config)::Vector{BorrowViolation}
             union!(live_during, uses)
 
             if !in_unsafe
+                # Each top-level call site gets an isolated depth budget: one deep
+                # call chain must not change how unrelated statements are treated
+                # (the walk is in reverse source order, so sharing a tracker would
+                # let a later-deeper statement affect an earlier shallower one).
                 _check_stmt!(
                     viols,
                     ir,
@@ -166,6 +172,7 @@ function check_ir(ir::CC.IRCode, cfg::Config)::Vector{BorrowViolation}
                     track_ssa,
                     live,
                     live_during,
+                    BudgetTracker(false),
                 )
             end
 
@@ -249,6 +256,7 @@ function _check_stmt!(
     track_ssa,
     live_after::BitSet,
     live_during::BitSet,
+    budget_state::Union{Nothing,BudgetTracker},
 )
     if stmt isa Expr && stmt.head === :foreigncall
         name_sym, ccall_args, _gc_roots, _nccallargs = _foreigncall_parts(stmt)
@@ -331,7 +339,9 @@ function _check_stmt!(
     kw_vals = (f === Core.kwcall) ? _kwcall_value_exprs(stmt, ir) : nothing
     (kw_vals === nothing || isempty(kw_vals)) && (kw_vals = nothing)
 
-    eff = _effects_for_call(stmt, ir, cfg, track_arg, track_ssa, nargs; idx=idx)
+    eff = _effects_for_call(
+        stmt, ir, cfg, track_arg, track_ssa, nargs; idx=idx, budget_state=budget_state
+    )
     moved_positions = _moved_positions_for_eval_order_check(f, raw_args, eff, ir)
     _check_call_eval_order_moves!(
         viols, ir, idx, stmt, uf, moved_positions, raw_args, nargs, track_arg, track_ssa

src/safe/defs.jl

@@ -18,7 +18,16 @@ Base.@kwdef struct Config
     optimize_until::String = _default_optimize_until()
 
     "Max depth for recursive effect summarization."
-    max_summary_depth::Int = 12
+    max_summary_depth::Int = 24
+
+    """
+    How to treat calls whose summary computation hit the depth budget:
+    `:consume` (default) conservatively assumes they may move/consume their
+    owned arguments (sound for escape detection, but can flag unrelated code
+    in deep third-party call chains); `:write` assumes only mutation, which
+    requires aliasing evidence to violate.
+    """
+    budget_fallback::Symbol = :consume
 
     "Recursively borrow-check callees (call graph) within this scope."
     scope::Symbol = :function
@@ -232,6 +241,18 @@ function _populate_registry!()
         # Misc:
         # `Task(f)` needs special handling because it relies on unsafe operations internally.
         (Base, :Task, (), (), (2,)),
+
+        # Locking primitives. Locks serialize access without consuming or writing
+        # through their arguments' contents, so the conservative unknown-call
+        # fallback misflags locked regions as escapes/consumes; register them
+        # explicitly. This entry covers the bare `lock(l)` / `unlock(l)` forms;
+        # the callback-taking `lock(f, l)` form is modeled as a transparent
+        # higher-order call in `_effects_for_call` (see summaries.jl), which
+        # propagates the callback's effects while granting payload writes.
+        (Base, :lock, (), (), ()),
+        (Base, :unlock, (), (), ()),
+        (Base, :trylock, (), (), ()),
+        (Base, :islocked, (), (), ()),
     ]
 
     for (mod, nm, ret_aliases, writes, consumes) in specs

src/safe/frontend.jl

@@ -3,12 +3,13 @@ Run BorrowCheck on a concrete specialization `tt::Type{<:Tuple}`.
 
 Returns `true` on success; throws `BorrowCheckError` on failure.
 """
-const CheckedCacheSig = Tuple{String,Int,Symbol,Module,Bool,Int}
+const CheckedCacheSig = Tuple{String,Int,Symbol,Symbol,Module,Bool,Int}
 
 @inline function _checked_cache_sig(cfg::Config)
     return (
         cfg.optimize_until,
         cfg.max_summary_depth,
+        cfg.budget_fallback,
         cfg.scope,
         cfg.root_module,
         cfg.debug,
@@ -63,7 +64,7 @@ function _scope_allows_module(m::Module, cfg::Config)::Bool
         # "user" means: only recurse into user code (no Core/Base, including submodules).
         return !(_module_is_under(m, Base) || _module_is_under(m, Core))
     end
-    throw(ArgumentError("unknown scope: $(cfg.scope)"))
+    return throw(ArgumentError("unknown scope: $(cfg.scope)"))
 end
 
 function _scope_allows_tt(tt::Type{<:Tuple}, cfg::Config)::Bool
@@ -561,6 +562,7 @@ function parse_config(options, calling_module)::Config
     cfg0 = Config()
     scope = cfg0.scope
     max_summary_depth = cfg0.max_summary_depth
+    budget_fallback = cfg0.budget_fallback
     optimize_until = cfg0.optimize_until
     debug = cfg0.debug
     debug_callee_depth = cfg0.debug_callee_depth
@@ -576,6 +578,9 @@ function parse_config(options, calling_module)::Config
             elseif k === :max_summary_depth
                 max_summary_depth = _parse_cfg_value(v, calling_module)::Int
                 continue
+            elseif k === :budget_fallback
+                budget_fallback = _parse_cfg_value(v, calling_module)::Symbol
+                continue
             elseif k === :optimize_until
                 optimize_until = _parse_cfg_value(v, calling_module)::String
                 continue
@@ -588,19 +593,31 @@ function parse_config(options, calling_module)::Config
             end
         end
         error(
-            "@safe only supports `scope=...`, `max_summary_depth=...`, `optimize_until=...`, `debug=...`, `debug_callee_depth=...`; got: $option",
+            "@safe only supports `scope=...`, `max_summary_depth=...`, `budget_fallback=...`, `optimize_until=...`, `debug=...`, `debug_callee_depth=...`; got: $option",
         )
     end
 
+    budget_fallback ∈ (:consume, :write) || error(
+        "invalid `budget_fallback` for @safe: $budget_fallback (expected :consume or :write)",
+    )
     scope ∈ (:none, :function, :module, :user, :all) || error(
         "invalid `scope` for @safe: $scope (expected :none, :function, :module, :user, or :all)",
     )
+    budget_fallback ∈ (:consume, :write) || error(
+        "invalid `budget_fallback` for @safe: $budget_fallback (expected :consume or :write)",
+    )
 
     root_module = (scope === :module) ? calling_module : cfg0.root_module
     debug_callee_depth >= 0 ||
         error("`debug_callee_depth` must be >= 0; got: $debug_callee_depth")
     return Config(
-        optimize_until, max_summary_depth, scope, root_module, debug, debug_callee_depth
+        optimize_until,
+        max_summary_depth,
+        budget_fallback,
+        scope,
+        root_module,
+        debug,
+        debug_callee_depth,
     )
 end
 
@@ -623,13 +640,13 @@ function _auto(args...; calling_module, source_info=nothing)
             tag_ref,
             QuoteNode(cfg.scope),
             cfg.max_summary_depth,
+            QuoteNode(cfg.budget_fallback),
             QuoteNode(Symbol(cfg.optimize_until)),
             cfg.debug,
             cfg.debug_callee_depth,
         )
     end
 
-    # Function form
     if ex isa Expr && ex.head === :function
         sig = ex.args[1]
         body = ex.args[2]
@@ -672,7 +689,7 @@ part of the checked-cache key).
   - `:module`: recursively check callees whose defining module matches the module where `@safe` is used.
   - `:user`: recursively check callees, but ignore `Core` and `Base` (including their submodules).
   - `:all`: recursively check callees across all modules (very aggressive).
-- `max_summary_depth` (default: `12`): limits recursive effect summarization depth used
+- `max_summary_depth` (default: `24`): limits recursive effect summarization depth used
   when the checker cannot directly resolve effects.
 - `debug` (default: `false`): enable best-effort debug logging to a JSONL file
   (path controlled by `BORROWCHECKER_AUTO_DEBUG_PATH`).

src/safe/generated.jl

@@ -2,31 +2,131 @@ using Core.Compiler
 using Core.IR
 
 struct BCInterpOwner end
+
+@static if isdefined(Core.Compiler, :InferenceCache)
+    # Julia 1.13+ (post-rc): the local inference cache stores `InferenceCacheEntry`
+    # (`InferenceResult` or `LocalInferenceResult`) and lookup goes through
+    # `get_indices(cache, mi)`.
+    const BCInfCacheEntry =
+        isdefined(Core.Compiler, :InferenceCacheEntry) ?
+        Core.Compiler.InferenceCacheEntry : Core.Compiler.InferenceResult
+    const BCInfCache = Vector{BCInfCacheEntry}
+else
+    const BCInfCache = Vector{Core.Compiler.InferenceResult}
+end
+
 Base.@kwdef struct BCInterp <: Compiler.AbstractInterpreter
     world::UInt = Base.get_world_counter()
     inf_params::Compiler.InferenceParams = Compiler.InferenceParams()
     opt_params::Compiler.OptimizationParams = Compiler.OptimizationParams()
-    inf_cache::Vector{Compiler.InferenceResult} = Compiler.InferenceResult[]
+    inf_cache::BCInfCache = BCInfCache()
     codegen_cache::IdDict{CodeInstance,CodeInfo} = IdDict{CodeInstance,CodeInfo}()
 end
 Base.Experimental.@MethodTable BCMT
 
-struct GeneratedCfgTag{S,MSD,OPT,DBG,DCD} end
+struct GeneratedCfgTag{S,MSD,BF,OPT,DBG,DCD} end
 
 Compiler.InferenceParams(interp::BCInterp) = interp.inf_params
 Compiler.OptimizationParams(interp::BCInterp) = interp.opt_params
 Compiler.get_inference_world(interp::BCInterp) = interp.world
 Compiler.get_inference_cache(interp::BCInterp) = interp.inf_cache
+
+# Julia 1.13+ (post-rc): `lookup_local_inference_result` expects an
+# `InferenceCache` with a `get_indices(cache, mi)` index. Our local cache is a
+# plain vector, so provide the lookup directly.
+@static if isdefined(Core.Compiler, :get_indices)
+    function Compiler.get_indices(cache::BCInfCache, mi::Core.MethodInstance)
+        indices = Int[]
+        for i in eachindex(cache)
+            entry = cache[i]
+            (entry isa Core.Compiler.InferenceResult ?
+                entry.linfo === mi : entry.result.linfo === mi) &&
+            push!(indices, i)
+        end
+        return indices
+    end
+end
+
+@static if isdefined(Core.Compiler, :lookup_local_inference_result) &&
+    isdefined(Core.Compiler, :InferenceCache)
+    # Nightly's `lookup_local_inference_result` indexes `cache.results`, which
+    # only exists on `InferenceCache`. Provide the vector-cache equivalent.
+    function Compiler.lookup_local_inference_result(
+        interp::BCInterp, mi::Core.MethodInstance
+    )
+        cache = Compiler.get_inference_cache(interp)
+        world = Compiler.get_inference_world(interp)
+        for i in length(cache):-1:1
+            cached = cache[i]
+            cached isa Core.Compiler.LocalInferenceResult || continue
+            result = cached.result
+            result.overridden_by_const === nothing || continue
+            result.cache_world == world || continue
+            world in Compiler.proof_worlds(cached.proof) || continue
+            return cached
+        end
+        return nothing
+    end
+end
+
+@static if isdefined(Core.Compiler, :constprop_cache_lookup) &&
+    !hasmethod(
+        Core.Compiler.constprop_cache_lookup,
+        Tuple{Any,Any,Vector{Any},BCInfCache,UInt},
+    )
+    # Nightly narrows `constprop_cache_lookup` to `InferenceCache`; mirror the
+    # upstream logic for our plain-vector cache.
+    function Compiler.constprop_cache_lookup(
+        𝕃::Compiler.AbstractLattice,
+        mi::Core.MethodInstance,
+        given_argtypes::Vector{Any},
+        cache::BCInfCache,
+        world::UInt,
+    )
+        nargtypes = length(given_argtypes)
+        found_tombstone = false
+        for cached in cache
+            cached_result = cached isa Core.Compiler.InferenceResult ? cached :
+                cached.result
+            cached_result.linfo === mi || continue
+            cached_result.cache_world == world || continue
+            valid_worlds = cached isa Core.Compiler.InferenceResult ?
+                cached_result.valid_worlds :
+                Compiler.proof_worlds(cached.proof)
+            cache_argtypes = cached_result.argtypes
+            length(cache_argtypes) == nargtypes || continue
+            cache_overridden_by_const = cached_result.overridden_by_const
+            cache_overridden_by_const === nothing && continue
+            ok = true
+            for i in 1:nargtypes
+                if !Compiler.is_argtype_match(
+                    𝕃, given_argtypes[i], cache_argtypes[i], cache_overridden_by_const[i]
+                )
+                    ok = false
+                    break
+                end
+            end
+            ok || continue
+            if cached_result.tombstone
+                found_tombstone = true
+                continue
+            end
+            return cached
+        end
+        return found_tombstone ? missing : nothing
+    end
+end
 Compiler.cache_owner(::BCInterp) = BCInterpOwner()
 Compiler.codegen_cache(interp::BCInterp) = interp.codegen_cache
 Compiler.method_table(interp::BCInterp) = Compiler.OverlayMethodTable(interp.world, BCMT)
 
 function _cfg_from_tag(
-    ::Type{GeneratedCfgTag{S,MSD,OPT,DBG,DCD}}, tt::Type{<:Tuple}, world::UInt
-) where {S,MSD,OPT,DBG,DCD}
+    ::Type{GeneratedCfgTag{S,MSD,BF,OPT,DBG,DCD}}, tt::Type{<:Tuple}, world::UInt
+) where {S,MSD,BF,OPT,DBG,DCD}
     @nospecialize tt
     scope = S::Symbol
     max_summary_depth = MSD::Int
+    budget_fallback = BF::Symbol
     optimize_until = String(OPT::Symbol)
     debug = DBG::Bool
     debug_callee_depth = DCD::Int
@@ -43,7 +143,13 @@ function _cfg_from_tag(
     end
 
     return Config(
-        optimize_until, max_summary_depth, scope, root_module, debug, debug_callee_depth
+        optimize_until,
+        max_summary_depth,
+        budget_fallback,
+        scope,
+        root_module,
+        debug,
+        debug_callee_depth,
     )
 end
 
@@ -92,7 +198,13 @@ function _expr_to_codeinfo(m::Module, argnames, spnames, e::Expr, isva)
     else
         Expr(Symbol("with-static-parameters"), lambda, spnames...)
     end
-    ci = Base.generated_body_to_codeinfo(ex, @__MODULE__(), isva)
+    # Nightly requires an explicit source location; `nothing` is rejected.
+    loc = LineNumberNode(0, Symbol(@__FILE__))
+    ci = if applicable(Base.generated_body_to_codeinfo, ex, @__MODULE__(), isva, loc)
+        Base.generated_body_to_codeinfo(ex, @__MODULE__(), isva, loc)
+    else
+        Base.generated_body_to_codeinfo(ex, @__MODULE__(), isva)
+    end
     @assert ci isa Core.CodeInfo "Failed to create a CodeInfo from the given expression. This might mean it contains a closure or comprehension?\n Offending expression: $e"
     return ci
 end

src/safe/ir_primitives.jl

@@ -45,6 +45,13 @@ function (tt::TypeTracker)(@nospecialize(T))::Bool
     end
     T === Symbol && return false
 
+    # `String` and `SubString` have value semantics (no user-facing in-place
+    # mutation API). Note that on Julia 1.12+, `ismutabletype(String)` is `true`
+    # (memory-based layout), so the generic mutable-type rule below would
+    # misclassify them. Keep the exemption narrow: `AbstractString` is
+    # extensible, and a user-defined mutable string subtype should stay tracked.
+    (T === String || T <: SubString) && return false
+
     # Modules and type objects are globally-shareable handles.
     # Treat them as *not tracked* so they don't participate in move/consume rules.
     (T <: Module) && return false
@@ -130,6 +137,13 @@ function (tt::OwnedTypeTracker)(@nospecialize(T))::Bool
     end
     T === Symbol && return false
 
+    # `String` and `SubString` have value semantics (no user-facing in-place
+    # mutation API). Note that on Julia 1.12+, `ismutabletype(String)` is `true`
+    # (memory-based layout), so the generic mutable-type rule below would
+    # misclassify them. Keep the exemption narrow: `AbstractString` is
+    # extensible, and a user-defined mutable string subtype should stay tracked.
+    (T === String || T <: SubString) && return false
+
     # Modules and type objects are globally-shareable handles.
     # Treat them as *not owned* so unknown/dynamic calls don't spuriously consume them.
     (T <: Module) && return false