diff --git a/Project.toml b/Project.toml index 31d136e..b27980a 100644 --- a/Project.toml +++ b/Project.toml @@ -1,6 +1,6 @@ name = "LoweredCodeUtils" uuid = "6f1432cf-f94c-5a45-995e-cdbf5db27b0b" -version = "3.5.3" +version = "3.6.0" authors = ["Tim Holy "] [deps] diff --git a/docs/src/api.md b/docs/src/api.md index a4bdbb6..ea5a0bf 100644 --- a/docs/src/api.md +++ b/docs/src/api.md @@ -17,6 +17,7 @@ lines_required lines_required! selective_eval! selective_eval_fromstart! +SelectiveEvalController ``` ## Interpreter diff --git a/src/codeedges.jl b/src/codeedges.jl index 10181c8..451a614 100644 --- a/src/codeedges.jl +++ b/src/codeedges.jl @@ -182,6 +182,60 @@ function postprint_linelinks(io::IO, idx::Int, src::CodeInfo, cl::CodeLinks, bbc return nothing end +struct CFGShortCut + from::Int # pc of GotoIfNot with inactive 𝑰𝑵𝑭𝑳 blocks + to::Int # pc of the entry of the nearest common post-dominator of the GotoIfNot's successors +end + +""" + controller::SelectiveEvalController + +When this object is passed as the `recurse` argument of `selective_eval!`, +the selective execution is adjusted as follows: + +- **Implicit return**: In Julia's IR representation (`CodeInfo`), the final block does not + necessarily return and may `goto` another block. In such cases the actual `return` is an + explicit statement earlier in the code, and if the slice does not include it, + `selective_eval!` must still terminate when execution reaches it. `controller.implicit_returns` + records the PCs of such `return` statements, and `selective_eval!` will return when reaching them. + +- **CFG short-cut**: When the successors of a conditional branch are inactive, and it is + safe to move the program counter from the conditional branch to the nearest common + post-dominator of those successors, this short-cut is taken. + This short-cut is not merely an optimization but is actually essential for the correctness + of the selective execution. This is because, in `CodeInfo`, even if we simply fall-through + dead blocks (i.e., increment the program counter without executing the statements of those + blocks), it does not necessarily lead to the nearest common post-dominator block. + +These adjustments are necessary for performing selective execution correctly. +[`lines_required`](@ref) or [`lines_required!`](@ref) will update the `SelectiveEvalController` +passed as an argument to be appropriate for the program slice generated. +""" +struct SelectiveEvalController + implicit_returns::BitSet # pc where selective execution should terminate even if they're inactive + shortcuts::Vector{CFGShortCut} +end +SelectiveEvalController() = SelectiveEvalController(BitSet(), CFGShortCut[]) + +""" + struct SelectiveInterpreter{S<:Interpreter,T<:AbstractVector{Bool}} <: Interpreter + inner::S + isrequired::T + controller::SelectiveEvalController + end + +An `JuliaInterpreter.Interpreter` that executes only the statements marked `true` in `isrequired`. +Note that this interpreter does not recurse into callee frames. +That is, when `JuliaInterpreter.finish!(interp::SelectiveInterpreter, frame, ...)` is +performed, the `frame` will be executed selectively according to `interp.isrequired`, but +any callee frames within it will be executed by `interp.inner::Interpreter`, not by `interp`. +""" +struct SelectiveInterpreter{S<:Interpreter,T<:AbstractVector{Bool}} <: Interpreter + inner::S + isrequired::T + controller::SelectiveEvalController +end + function namedkeys(cl::CodeLinks) ukeys = Set{GlobalRef}() for c in (cl.namepreds, cl.namesuccs, cl.nameassigns) @@ -606,8 +660,8 @@ function terminal_preds!(s, j, edges, covered) # can't be an inner function bec end """ - isrequired = lines_required(obj::GlobalRef, src::CodeInfo, edges::CodeEdges) - isrequired = lines_required(idx::Int, src::CodeInfo, edges::CodeEdges) + isrequired = lines_required(obj::GlobalRef, src::CodeInfo, edges::CodeEdges, [controller::SelectiveEvalController]) + isrequired = lines_required(idx::Int, src::CodeInfo, edges::CodeEdges, [controller::SelectiveEvalController]) Determine which lines might need to be executed to evaluate `obj` or the statement indexed by `idx`. If `isrequired[i]` is `false`, the `i`th statement is *not* required. @@ -616,21 +670,26 @@ will end up skipping a subset of such statements, perhaps while repeating others See also [`lines_required!`](@ref) and [`selective_eval!`](@ref). """ -function lines_required(obj::GlobalRef, src::CodeInfo, edges::CodeEdges; kwargs...) +function lines_required(obj::GlobalRef, src::CodeInfo, edges::CodeEdges, + controller::SelectiveEvalController=SelectiveEvalController(); + kwargs...) isrequired = falses(length(edges.preds)) objs = Set{GlobalRef}([obj]) - return lines_required!(isrequired, objs, src, edges; kwargs...) + return lines_required!(isrequired, objs, src, edges, controller; kwargs...) end -function lines_required(idx::Int, src::CodeInfo, edges::CodeEdges; kwargs...) +function lines_required(idx::Int, src::CodeInfo, edges::CodeEdges, + controller::SelectiveEvalController=SelectiveEvalController(); + kwargs...) isrequired = falses(length(edges.preds)) isrequired[idx] = true objs = Set{GlobalRef}() - return lines_required!(isrequired, objs, src, edges; kwargs...) + return lines_required!(isrequired, objs, src, edges, controller; kwargs...) end """ - lines_required!(isrequired::AbstractVector{Bool}, src::CodeInfo, edges::CodeEdges; + lines_required!(isrequired::AbstractVector{Bool}, src::CodeInfo, edges::CodeEdges, + [controller::SelectiveEvalController]; norequire = ()) Like `lines_required`, but where `isrequired[idx]` has already been set to `true` for all statements @@ -642,9 +701,11 @@ should _not_ be marked as a requirement. For example, use `norequire = LoweredCodeUtils.exclude_named_typedefs(src, edges)` if you're extracting method signatures and not evaluating new definitions. """ -function lines_required!(isrequired::AbstractVector{Bool}, src::CodeInfo, edges::CodeEdges; kwargs...) +function lines_required!(isrequired::AbstractVector{Bool}, src::CodeInfo, edges::CodeEdges, + controller::SelectiveEvalController=SelectiveEvalController(); + kwargs...) objs = Set{GlobalRef}() - return lines_required!(isrequired, objs, src, edges; kwargs...) + return lines_required!(isrequired, objs, src, edges, controller; kwargs...) end function exclude_named_typedefs(src::CodeInfo, edges::CodeEdges) @@ -664,7 +725,9 @@ function exclude_named_typedefs(src::CodeInfo, edges::CodeEdges) return norequire end -function lines_required!(isrequired::AbstractVector{Bool}, objs, src::CodeInfo, edges::CodeEdges; norequire = ()) +function lines_required!(isrequired::AbstractVector{Bool}, objs, src::CodeInfo, edges::CodeEdges, + controller::SelectiveEvalController=SelectiveEvalController(); + norequire = ()) # Mark any requested objects (their lines of assignment) objs = add_requests!(isrequired, objs, edges, norequire) @@ -699,7 +762,10 @@ function lines_required!(isrequired::AbstractVector{Bool}, objs, src::CodeInfo, end # now mark the active goto nodes - add_active_gotos!(isrequired, src, cfg, postdomtree) + add_active_gotos!(isrequired, src, cfg, postdomtree, controller) + + # check if there are any implicit return blocks + record_implicit_return!(controller, isrequired, cfg) return isrequired end @@ -777,19 +843,19 @@ end ## Add control-flow - # The goal of this function is to request concretization of the minimal necessary control # flow to evaluate statements whose concretization have already been requested. # The basic algorithm is based on what was proposed in [^Wei84]. If there is even one active # block in the blocks reachable from a conditional branch up to its successors' nearest # common post-dominator (referred to as 𝑰𝑵𝑭𝑳 in the paper), it is necessary to follow -# that conditional branch and execute the code. Otherwise, execution can be short-circuited +# that conditional branch and execute the code. Otherwise, execution can be short-cut # from the conditional branch to the nearest common post-dominator. # -# COMBAK: It is important to note that in Julia's intermediate code representation (`CodeInfo`), -# "short-circuiting" a specific code region is not a simple task. Simply ignoring the path -# to the post-dominator does not guarantee fall-through to the post-dominator. Therefore, -# a more careful implementation is required for this aspect. +# It is important to note that in Julia's intermediate code representation (`CodeInfo`), +# "short-cutting" a specific code region is not a simple task. Simply incrementing the +# program counter without executing the statements of 𝑰𝑵𝑭𝑳 blocks does not guarantee that +# the program counter fall-throughs to the post-dominator. +# To handle such cases, `selective_eval!` needs to use `SelectiveInterpreter`. # # [Wei84]: M. Weiser, "Program Slicing," IEEE Transactions on Software Engineering, 10, pages 352-357, July 1984. function add_control_flow!(isrequired, src::CodeInfo, cfg::CFG, postdomtree) @@ -864,8 +930,8 @@ function reachable_blocks(cfg, from_bb::Int, to_bb::Int) return visited end -function add_active_gotos!(isrequired, src::CodeInfo, cfg::CFG, postdomtree) - dead_blocks = compute_dead_blocks(isrequired, src, cfg, postdomtree) +function add_active_gotos!(isrequired, src::CodeInfo, cfg::CFG, postdomtree, controller::SelectiveEvalController) + dead_blocks = compute_dead_blocks!(isrequired, src, cfg, postdomtree, controller) changed = false for bbidx = 1:length(cfg.blocks) if bbidx ∉ dead_blocks @@ -883,7 +949,7 @@ function add_active_gotos!(isrequired, src::CodeInfo, cfg::CFG, postdomtree) end # find dead blocks using the same approach as `add_control_flow!`, for the converged `isrequired` -function compute_dead_blocks(isrequired, src::CodeInfo, cfg::CFG, postdomtree) +function compute_dead_blocks!(isrequired, src::CodeInfo, cfg::CFG, postdomtree, controller::SelectiveEvalController) dead_blocks = BitSet() for bbidx = 1:length(cfg.blocks) bb = cfg.blocks[bbidx] @@ -904,6 +970,11 @@ function compute_dead_blocks(isrequired, src::CodeInfo, cfg::CFG, postdomtree) end if !is_𝑰𝑵𝑭𝑳_active union!(dead_blocks, delete!(𝑰𝑵𝑭𝑳, postdominator)) + if postdominator ≠ 0 + postdominator_bb = cfg.blocks[postdominator] + postdominator_entryidx = postdominator_bb.stmts[begin] + push!(controller.shortcuts, CFGShortCut(termidx, postdominator_entryidx)) + end end end end @@ -911,6 +982,19 @@ function compute_dead_blocks(isrequired, src::CodeInfo, cfg::CFG, postdomtree) return dead_blocks end +function record_implicit_return!(controller::SelectiveEvalController, isrequired, cfg::CFG) + for bbidx = 1:length(cfg.blocks) + bb = cfg.blocks[bbidx] + if isempty(bb.succs) + i = findfirst(idx::Int->!isrequired[idx], bb.stmts) + if !isnothing(i) + push!(controller.implicit_returns, bb.stmts[i]) + end + end + end + nothing +end + # Do a traveral of "numbered" predecessors and find statement ranges and names of type definitions function find_typedefs(src::CodeInfo) typedef_blocks, typedef_names = UnitRange{Int}[], Symbol[] @@ -1033,26 +1117,19 @@ function add_inplace!(isrequired, src, edges, norequire) return changed end -""" - struct SelectiveInterpreter{S<:Interpreter,T<:AbstractVector{Bool}} <: Interpreter - inner::S - isrequired::T - end - -An `JuliaInterpreter.Interpreter` that executes only the statements marked `true` in `isrequired`. -Note that this interpreter does not recurse into callee frames. -That is, when `JuliaInterpreter.finish!(interp::SelectiveInterpreter, frame, ...)` is -performed, the `frame` will be executed selectively according to `interp.isrequired`, but -any callee frames within it will be executed by `interp.inner::Interpreter`, not by `interp`. -""" -struct SelectiveInterpreter{S<:Interpreter,T<:AbstractVector{Bool}} <: Interpreter - inner::S - isrequired::T -end function JuliaInterpreter.step_expr!(interp::SelectiveInterpreter, frame::Frame, istoplevel::Bool) pc = frame.pc + if pc in interp.controller.implicit_returns + return nothing + elseif pc_expr(frame) isa GotoIfNot + for shortcut in interp.controller.shortcuts + if shortcut.from == pc + return frame.pc = shortcut.to + end + end + end if interp.isrequired[pc] - step_expr!(interp.inner, frame::Frame, istoplevel::Bool) + step_expr!(interp.inner, frame, istoplevel) else next_or_nothing!(interp, frame) end @@ -1083,12 +1160,28 @@ See [`selective_eval_fromstart!`](@ref) to have that performed automatically. `isrequired` pertains only to `frame` itself, not any of its callees. +By default `selective_eval!` runs with an empty [`SelectiveEvalController`](@ref), which +reproduces the historical behavior. That is correct for most top-level code, but not for +every possible Julia program: when the slice omits a `return` reached by fall-through, or +omits a branch whose dead region does not simply fall through to its post-dominator, the +interpreter can execute the wrong statements (see +https://github.com/JuliaDebug/LoweredCodeUtils.jl/pull/99 for details). For full correctness, +pass a [`SelectiveEvalController`](@ref) to [`lines_required`](@ref)/[`lines_required!`](@ref) +and reuse that same controller, together with the `isrequired` it produced, to construct a +[`SelectiveInterpreter`](@ref) that you run with `JuliaInterpreter.finish_and_return!`. +Drawing both from the same call is what keeps `isrequired` and the controller synchronized. + +Note that the interpreter does not recurse into callees, so there is currently no +interprocedural selective evaluation. + This will return either a `BreakpointRef`, the value obtained from the last executed statement (if stored to `frame.framedata.ssavlues`), or `nothing`. Typically, assignment to a variable binding does not result in an ssa store by JuliaInterpreter. """ -selective_eval!(interp::Interpreter, frame::Frame, isrequired::AbstractVector{Bool}, istoplevel::Bool=false) = - JuliaInterpreter.finish_and_return!(SelectiveInterpreter(interp, isrequired), frame, istoplevel) +function selective_eval!(interp::Interpreter, frame::Frame, isrequired::AbstractVector{Bool}, istoplevel::Bool=false) + interp = SelectiveInterpreter(interp, isrequired, SelectiveEvalController()) + JuliaInterpreter.finish_and_return!(interp, frame, istoplevel) +end selective_eval!(args...) = selective_eval!(RecursiveInterpreter(), args...) """ diff --git a/src/packagedef.jl b/src/packagedef.jl index 21c7d21..0df64e0 100644 --- a/src/packagedef.jl +++ b/src/packagedef.jl @@ -24,7 +24,8 @@ const trackedheads = (:method,) # Revise uses this (for now), don't delete; a const structdecls = (:_structtype, :_abstracttype, :_primitivetype) export signature, rename_framemethods!, methoddef!, methoddefs!, bodymethod -export CodeEdges, lines_required, lines_required!, selective_eval!, selective_eval_fromstart! +export CodeEdges, SelectiveEvalController, SelectiveInterpreter, + lines_required, lines_required!, selective_eval!, selective_eval_fromstart! include("utils.jl") include("signatures.jl") diff --git a/test/codeedges.jl b/test/codeedges.jl index 3a097c2..ed54ee3 100644 --- a/test/codeedges.jl +++ b/test/codeedges.jl @@ -223,6 +223,25 @@ module ModSelective end @test ModSelective.k11 == 0 @test 3 <= ModSelective.s11 <= 15 + # Final block is not a `return`: Need to use `controller::SelectiveEvalController` explicitly + ex = quote + x = 1 + yy = 7 + @label loop + x += 1 + x < 5 || return yy + @goto loop + end + frame = Frame(ModSelective, ex) + src = frame.framecode.src + edges = CodeEdges(ModSelective, src) + controller = SelectiveEvalController() + isrequired = lines_required(GlobalRef(ModSelective, :x), src, edges, controller) + interp = LoweredCodeUtils.SelectiveInterpreter(LoweredCodeUtils.RecursiveInterpreter(), isrequired, controller) + JuliaInterpreter.finish_and_return!(interp, frame, true) + @test ModSelective.x == 5 + @test !isdefined(ModSelective, :yy) + # Control-flow in an abstract type definition ex = :(abstract type StructParent{T, N} <: AbstractArray{T, N} end) frame = Frame(ModSelective, ex) diff --git a/test/signatures.jl b/test/signatures.jl index 91a450d..092737d 100644 --- a/test/signatures.jl +++ b/test/signatures.jl @@ -436,9 +436,10 @@ bodymethtest5(x, y=Dict(1=>2)) = 5 oldenv = Pkg.project().path try # we test with the old version of CBinding, let's do it in an isolated environment + # so we don't cause package conflicts with everything else Pkg.activate(; temp=true, io=devnull) - @info "Adding CBinding to the environment for test purposes" + @info "Adding CBinding v0.9.4 to the environment for test purposes" Pkg.add(; name="CBinding", version="0.9.4", io=devnull) # `@cstruct` isn't defined for v1.0 and above m = Module()