From 7fe2e25e739d19cdebbed300882bb20034235fc2 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Fri, 26 Jun 2020 18:46:24 -0400 Subject: [PATCH] Canonicalize IR to disallow throwing GlobalRef in value position In anticipation of making the SSA IR more of an interface that packages can use to implement custom compiler transformation, I'd like to do some cleanup first. The is the first of the items on my list: Disallowing GlobalRef in value position if it could potentially throw (because the binding doesn't exist yet). This is done as part of SSA conversion, because we don't know whether a binding will exist during parsing/lowering and we don't modify the IR at all between lowering and the end of type inference, so doing it during SSA conversion is the first possible opportunity. The reason to do this is to simplify transformation passes that want to replace calls with sequences of other instructions. By moving those GlobalRef that could potentially throw into statement position, the order of argument evaluation does not matter (this isn't quite true yet due to static parameters, but I'd like to fix that in a separate commit). I think that's a desirable property to simplify the life os pass authors. --- base/compiler/ssair/slot2ssa.jl | 2 ++ base/compiler/ssair/verify.jl | 18 ++++++++++++------ test/compiler/ssair.jl | 28 ++++++++++++++++++++++------ 3 files changed, 36 insertions(+), 12 deletions(-) diff --git a/base/compiler/ssair/slot2ssa.jl b/base/compiler/ssair/slot2ssa.jl index 0de11fd01f084..a721dd4f0160d 100644 --- a/base/compiler/ssair/slot2ssa.jl +++ b/base/compiler/ssair/slot2ssa.jl @@ -168,6 +168,8 @@ function fixemup!(cond, rename, ir::IRCode, ci::CodeInfo, idx::Int, @nospecializ return nothing end op[] = x + elseif isa(val, GlobalRef) && !isdefined(val.mod, val.name) + op[] = NewSSAValue(insert_node!(ir, idx, Any, val).id - length(ir.stmts)) end end return urs[] diff --git a/base/compiler/ssair/verify.jl b/base/compiler/ssair/verify.jl index 3ba32a85c5aa7..67c03184d4ad5 100644 --- a/base/compiler/ssair/verify.jl +++ b/base/compiler/ssair/verify.jl @@ -2,7 +2,7 @@ if !isdefined(@__MODULE__, Symbol("@verify_error")) macro verify_error(arg) - arg isa String && return esc(:(println(stderr, $arg))) + arg isa String && return esc(:(print && println(stderr, $arg))) (arg isa Expr && arg.head === :string) || error("verify_error macro expected a string expression") pushfirst!(arg.args, GlobalRef(Core, :stderr)) pushfirst!(arg.args, :println) @@ -11,7 +11,7 @@ if !isdefined(@__MODULE__, Symbol("@verify_error")) end end -function check_op(ir::IRCode, domtree::DomTree, @nospecialize(op), use_bb::Int, use_idx::Int) +function check_op(ir::IRCode, domtree::DomTree, @nospecialize(op), use_bb::Int, use_idx::Int, print::Bool) if isa(op, SSAValue) if op.id > length(ir.stmts) def_bb = block_for_inst(ir.cfg, ir.new_nodes[op.id - length(ir.stmts)].pos) @@ -35,6 +35,11 @@ function check_op(ir::IRCode, domtree::DomTree, @nospecialize(op), use_bb::Int, error() end end + elseif isa(op, GlobalRef) + if !isdefined(op.mod, op.name) + @verify_error "Unbound GlobalRef not allowed in value position" + error() + end elseif isa(op, Union{OldSSAValue, NewSSAValue}) #@Base.show ir @verify_error "Left over SSA marker" @@ -55,7 +60,7 @@ function count_int(val::Int, arr::Vector{Int}) n end -function verify_ir(ir::IRCode) +function verify_ir(ir::IRCode, print::Bool=true) # For now require compact IR # @assert isempty(ir.new_nodes) # Verify CFG @@ -169,7 +174,7 @@ function verify_ir(ir::IRCode) @verify_error "GlobalRefs and Exprs are not allowed as PhiNode values" error() end - check_op(ir, domtree, val, edge, last(ir.cfg.blocks[stmt.edges[i]].stmts)+1) + check_op(ir, domtree, val, edge, last(ir.cfg.blocks[stmt.edges[i]].stmts)+1, print) end elseif isa(stmt, PhiCNode) for i = 1:length(stmt.values) @@ -206,17 +211,18 @@ function verify_ir(ir::IRCode) end for op in userefs(stmt) op = op[] - check_op(ir, domtree, op, bb, idx) + check_op(ir, domtree, op, bb, idx, print) end end end end -function verify_linetable(linetable::Vector{LineInfoNode}) +function verify_linetable(linetable::Vector{LineInfoNode}, print::Bool=true) for i in 1:length(linetable) line = linetable[i] if i <= line.inlined_at @verify_error "Misordered linetable" + error() end end end diff --git a/test/compiler/ssair.jl b/test/compiler/ssair.jl index 97a6d3f95e060..7be9fb044ae3e 100644 --- a/test/compiler/ssair.jl +++ b/test/compiler/ssair.jl @@ -147,14 +147,14 @@ let ci = (Meta.@lower 1 + 1).args[1] Core.PhiNode(), Core.Compiler.ReturnNode(), # block 4 - Expr(:call, - GlobalRef(Main, :something), - GlobalRef(Main, :somethingelse)), - Core.Compiler.GotoIfNot(Core.SSAValue(6), 9), + GlobalRef(Main, :something), + GlobalRef(Main, :somethingelse), + Expr(:call, Core.SSAValue(6), Core.SSAValue(7)), + Core.Compiler.GotoIfNot(Core.SSAValue(8), 11), # block 5 - Core.Compiler.ReturnNode(Core.SSAValue(6)), + Core.Compiler.ReturnNode(Core.SSAValue(8)), # block 6 - Core.Compiler.ReturnNode(Core.SSAValue(6)) + Core.Compiler.ReturnNode(Core.SSAValue(8)) ] nstmts = length(ci.code) ci.ssavaluetypes = nstmts @@ -164,3 +164,19 @@ let ci = (Meta.@lower 1 + 1).args[1] ir = Core.Compiler.compact!(ir, true) @test Core.Compiler.verify_ir(ir) == nothing end + +# Test that GlobalRef in value position is non-canonical +using Base.Meta +let ci = (Meta.@lower 1 + 1).args[1] + ci.code = [ + Expr(:call, GlobalRef(Main, :something_not_defined_please)) + ReturnNode(SSAValue(1)) + ] + nstmts = length(ci.code) + ci.ssavaluetypes = nstmts + ci.codelocs = fill(Int32(1), nstmts) + ci.ssaflags = fill(Int32(0), nstmts) + ir = Core.Compiler.inflate_ir(ci) + ir = Core.Compiler.compact!(ir, true) + @test_throws ErrorException Core.Compiler.verify_ir(ir, false) +end