Local variables on inaccessible lines considered global
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Mars |
Fix Released
|
Critical
|
Matt Giuca |
Bug Description
A variable on an inaccessible line (e.g., after a return) will be considered a global variable in the generated code. e.g.:
def shadowed_return(x :: Int) :: Int:
return 1
return x
Produces assembly code:
def shadowed_return(x :: Int) :: Int:
# XXX CFG representation (not valid Mars code)
# Return variable: $RET:1
Block 0:
# Preds: []
$RET:0 = 1
goto 1
Block 1:
# Preds: [2, 0]
PHI $RET:1 [0 - "$RET:0", 2 - "$RET:2"]
# Null block terminator.
Block 2:
# Preds: []
$RET:2 = @x # Note the "@" sign, indicating a global
goto 1
This has no effect on execution (as the line is inaccessible), but causes havoc if any code analysis is performed (as you have a global variable reference to a variable which doesn't exist).
Related branches
Changed in mars: | |
status: | Fix Committed → Fix Released |
The cause of this bug is very tricky. It is caused by ast_cfg. stmt_to_ cfg, which generates code for an individual statement, and then returns a DefMap for the following statement.
For compound statements (including returns), this will generate a new block, and subsequent statements will be compiled into this block. stmt_to_cfg computes the DefMap for this new block. If the statement branches elsewhere (ie., returns) on ALL code paths, then the DefMap for the new block will be empty, because it has no predecessors.
DefMaps are required (by definition in ast_cfg) to have ALL local variables for the function, even if they are marked as unbound. Therefore, this creates an illegal DefMap (assuming the function has local variables), and as a consequence, local variable names fail to lookup, so they are considered globals, even though they aren't.
Numerous fixes could be applied, but all very tricky. Suggest merely not generating the code if it has no predecessors. Then, the variables won't be looked up at all, and the illegal def map will not be used. (Be sure to comment this thoroughly, as it's a bit of a hack.)