Skip to content

Commit

Permalink
Fix condition for unreachable code in IRCode conversion
Browse files Browse the repository at this point in the history
This is a partial back-port of JuliaLang#50924, where we discovered that the
optimizer would ignore:
  1. must-throw `%XX = SlotNumber(_)` statements
  2. must-throw `goto #bb if not %x` statements
when re-building the CFG during IRCode conversion.

This is mostly harmless, except that in the case of (1) we can accidentally
fall through the statically deleted (`Const()`-wrapped) code from inference
and observe a control-flow edge that never should have existed, such as an
edge into a catch block. Such an edge is invalid semantically and breaks our
SSA conversion.

This one-line change fixes (1) but not (2), which is enough for IR validity.
We should follow-up with a tweak to `compute_basic_blocks` to enforce this
requirement.

The test added here is very brittle, but it's better than nothing until we
have utilities to provide hand-written (pre-optimizer) IR and pass it through
part of our pipeline.
  • Loading branch information
topolarity committed Mar 7, 2024
1 parent 72fed47 commit fc7a881
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 1 deletion.
2 changes: 1 addition & 1 deletion base/compiler/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ function convert_to_ircode(ci::CodeInfo, sv::OptimizationState)
idx += 1
prevloc = codeloc
end
if code[idx] isa Expr && ssavaluetypes[idx] === Union{}
if ssavaluetypes[idx] === Union{} && code[idx] !== nothing && !(code[idx] isa Core.Const)
if !(idx < length(code) && isa(code[idx + 1], ReturnNode) && !isdefined((code[idx + 1]::ReturnNode), :val))
# insert unreachable in the same basic block after the current instruction (splitting it)
insert!(code, idx + 1, ReturnNode())
Expand Down
24 changes: 24 additions & 0 deletions test/compiler/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5161,3 +5161,27 @@ let x = 1, _Any = Any
foo27031() = bar27031((x, 1.0), Val{_Any})
@test foo27031() == "OK"
end

# Issue #53366
#
# FIXME: This test is quite brittle, since it relies on lowering making a particular
# (and unnecessary) decision to embed a `SlotNumber` in statement position.
#
# This should be re-written to have the bad IR directly, then run type inference +
# SSA conversion. It should also avoid running `compact!` afterward, since that can
# mask bugs by cleaning up unused ϕ nodes that should never have existed.
function issue53366(sc::Threads.Condition)
@lock sc begin
try
if Core.Compiler.inferencebarrier(true)
return nothing
end
return nothing
finally
end
end
end

let (ir, rt) = only(Base.code_ircode(issue53366, (Threads.Condition,)))
Core.Compiler.verify_ir(ir)
end

0 comments on commit fc7a881

Please sign in to comment.