Skip to content

Commit 2a59c4b

Browse files
KenoKristofferC
authored andcommitted
Fix #41975 - Dropped typecheck in GotoIfNot (#42010)
Recall the reproducer from the issue: ``` julia> f() = (if nothing; end; unsafe_load(Ptr{Int}(0))) f (generic function with 1 method) julia> f() Unreachable reached at 0x7fb33bb50090 signal (4): Illegal instruction in expression starting at REPL[13]:1 unsafe_load at ./pointer.jl:105 [inlined] unsafe_load at ./pointer.jl:105 [inlined] ``` There were actually two places where we were dropping the GotoIfNot, one in type annotation after inference, one in SSA conversion. The one in SSA conversion was structural: When both branches target the same jump destination, the GotoIfNot would be dropped. This was fine in general, except that as shown above, GotoIfNot can actually itself have a side effect, namely throwing a type error if the condition is not a boolean. Thus in order to actually drop the node we need to prove that the error check does not fire. The reason we want to drop the GotoIfNot node here is that IRCode has an invariant that every basic block is in the predecessor list only once (otherwise PhiNodes would have to carry extra state regarding which branch they refer to). To fix this, insert an `Expr(:call, typecheck, _, Bool)` when dropping the GotoIfNot. We do lose the ability to distinguish the GotoIfNot from literal typechecks as a result, but at the moment they generate identical errors. If we ever wanted to dinstinguish them, we could create another typecheck intrinsic that throws a different error or use an approach like #41994. (cherry picked from commit 2445000)
1 parent ae2c047 commit 2a59c4b

File tree

3 files changed

+6
-2
lines changed

3 files changed

+6
-2
lines changed

base/compiler/ssair/slot2ssa.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -817,7 +817,7 @@ function construct_ssa!(ci::CodeInfo, ir::IRCode, domtree::DomTree, defuse, narg
817817
new_dest = block_for_inst(cfg, stmt.dest)
818818
if new_dest == bb+1
819819
# Drop this node - it's a noop
820-
new_code[idx] = stmt.cond
820+
new_code[idx] = Expr(:call, GlobalRef(Core, :typeassert), stmt.cond, GlobalRef(Core, :Bool))
821821
else
822822
new_code[idx] = GotoIfNot(stmt.cond, new_dest)
823823
end

base/compiler/typeinfer.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -626,7 +626,7 @@ function type_annotate!(sv::InferenceState, run_optimizer::Bool)
626626
expr = body[i]
627627
if isa(expr, GotoIfNot)
628628
if !isa(states[expr.dest], VarTable)
629-
body[i] = expr.cond
629+
body[i] = Expr(:call, GlobalRef(Core, :typeassert), expr.cond, GlobalRef(Core, :Bool))
630630
end
631631
end
632632
end

test/compiler/ssair.jl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,3 +310,7 @@ let cfg = CFG(BasicBlock[
310310
Compiler.domtree_insert_edge!(domtree, cfg.blocks, 1, 3)
311311
@test domtree.idoms_bb == Compiler.naive_idoms(cfg.blocks) == [0, 1, 1, 3, 1, 4]
312312
end
313+
314+
# Issue #41975 - SSA conversion drops type check
315+
f_if_typecheck() = (if nothing; end; unsafe_load(Ptr{Int}(0)))
316+
@test_throws TypeError f_if_typecheck()

0 commit comments

Comments
 (0)