Canonicalize IR to disallow throwing GlobalRef in value position (#36450

)

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

@@ -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[]

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

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