-
Notifications
You must be signed in to change notification settings - Fork 20
Fix: Check non-exhaustible conditional returns using a goto mechanism
#75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Fix: Check non-exhaustible conditional returns using a goto mechanism
#75
Conversation
…lation Also uncomments the inline_bug_mre test
|
I tried a few other tests from #65. Starting with the one @TomWambsgans recommended: [divya@archdivya leanMultisig]$ RUST_BACKTRACE=1 cargo test -p lean_compiler test_doesnt_work -- --nocapture
Compiling lean_compiler v0.1.0 (/home/divya/src/leanMultisig/crates/lean_compiler)
Finished `test` profile [unoptimized + debuginfo] target(s) in 0.19s
Running unittests src/lib.rs (target/debug/deps/lean_compiler-1a2d7a131c613260)
running 0 tests
test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 3 filtered out; finished in 0.00s
Running tests/test_compiler.rs (target/debug/deps/test_compiler-c47d8cf7d224f8c7)
running 1 test
Parsed program: fn doesnt_work(x) -> 1 {
if x == 1 {
if x == 0 {
return 100
}
if x == 1 {
return 200
}
}
return 300
}
fn main() -> 0 {
b = doesnt_work(1)
print(b)
return
}
Simplified program: fn main() -> 0 {
@diff_0 = 1 - 1
if @diff_0 != 0 {
} else {
@diff_1 = 1 - 0
if @diff_1 != 0 {
} else {
b = 100 + 0
goto_@inline_epilogue_0
}
@diff_2 = 1 - 1
if @diff_2 != 0 {
} else {
b = 200 + 0
goto_@inline_epilogue_0
}
}
b = 300 + 0
goto_@inline_epilogue_0
print(b)
return
}
Intermediate Bytecode:
@function_main:
1 = m[fp + 2] + 1
m[fp + 6] = inverse(m[fp + 2])
m[fp + 7] = m[fp + 2] x m[fp + 6]
1 = m[fp + 8] + m[fp + 7]
0 = m[fp + 8] x m[fp + 2]
jump_if_not_zero m[fp + 7] to @if_0
jump @else_0
@if_0:
jump @if_else_end_0
@else_0:
1 = m[fp + 3] + 0
m[fp + 9] = inverse(m[fp + 3])
m[fp + 10] = m[fp + 3] x m[fp + 9]
1 = m[fp + 11] + m[fp + 10]
0 = m[fp + 11] x m[fp + 3]
jump_if_not_zero m[fp + 10] to @if_1
jump @else_1
@if_else_end_0:
m[fp + 5] = 300 + 0
jump @inline_epilogue_0
@if_1:
jump @if_else_end_1
@else_1:
m[fp + 5] = 100 + 0
jump @inline_epilogue_0
@if_else_end_1:
1 = m[fp + 4] + 1
m[fp + 12] = inverse(m[fp + 4])
m[fp + 13] = m[fp + 4] x m[fp + 12]
1 = m[fp + 14] + m[fp + 13]
0 = m[fp + 14] x m[fp + 4]
jump_if_not_zero m[fp + 13] to @if_2
jump @else_2
@if_2:
jump @if_else_end_2
@else_2:
m[fp + 5] = 200 + 0
jump @inline_epilogue_0
@if_else_end_2:
jump @if_else_end_0
@inline_epilogue_0:
print print: m[fp + 5]
m[fp + 15] = 0 + 0
jump @end_program with fp = m[fp + 15]
Memory size per function:
main: 16
Function Locations:
main: 2
doesnt_work: 8
Compiled Program:
hint: label: @function_main
hint: source line number: 3
hint: source line number: 9
0: 1 = 1 + m[fp + 2]
hint: m[fp + 6] = inverse(m[fp + 2])
1: m[fp + 7] = m[fp + 2] x m[fp + 6]
2: 1 = m[fp + 8] + m[fp + 7]
3: 0 = m[fp + 8] x m[fp + 2]
4: if m[fp + 7] != 0 jump to @if_0 = 7 with next(fp) = fp
5: if 1 != 0 jump to @else_0 = 8 with next(fp) = fp
hint: label: @end_program
6: if 1 != 0 jump to @end_program = 6 with next(fp) = fp
hint: label: @if_0
7: if 1 != 0 jump to @if_else_end_0 = 14 with next(fp) = fp
hint: label: @else_0
hint: source line number: 10
8: 1 = 0 + m[fp + 3]
hint: m[fp + 9] = inverse(m[fp + 3])
9: m[fp + 10] = m[fp + 3] x m[fp + 9]
10: 1 = m[fp + 11] + m[fp + 10]
11: 0 = m[fp + 11] x m[fp + 3]
12: if m[fp + 10] != 0 jump to @if_1 = 16 with next(fp) = fp
13: if 1 != 0 jump to @else_1 = 17 with next(fp) = fp
hint: label: @if_else_end_0
hint: source line number: 17
14: 300 = 0 + m[fp + 5]
15: if 1 != 0 jump to @inline_epilogue_0 = 29 with next(fp) = fp
hint: label: @if_1
16: if 1 != 0 jump to @if_else_end_1 = 19 with next(fp) = fp
hint: label: @else_1
hint: source line number: 11
17: 100 = 0 + m[fp + 5]
18: if 1 != 0 jump to @inline_epilogue_0 = 29 with next(fp) = fp
hint: label: @if_else_end_1
hint: source line number: 13
19: 1 = 1 + m[fp + 4]
hint: m[fp + 12] = inverse(m[fp + 4])
20: m[fp + 13] = m[fp + 4] x m[fp + 12]
21: 1 = m[fp + 14] + m[fp + 13]
22: 0 = m[fp + 14] x m[fp + 4]
23: if m[fp + 13] != 0 jump to @if_2 = 25 with next(fp) = fp
24: if 1 != 0 jump to @else_2 = 26 with next(fp) = fp
hint: label: @if_2
25: if 1 != 0 jump to @if_else_end_2 = 28 with next(fp) = fp
hint: label: @else_2
hint: source line number: 14
26: 200 = 0 + m[fp + 5]
27: if 1 != 0 jump to @inline_epilogue_0 = 29 with next(fp) = fp
hint: label: @if_else_end_2
28: if 1 != 0 jump to @if_else_end_0 = 14 with next(fp) = fp
hint: label: @inline_epilogue_0
hint: source line number: 4
hint: print(m[fp + 5]) for "print"
hint: source line number: 5
29: 0 = 0 + m[fp + 15]
30: if 1 != 0 jump to @end_program = 6 with next(fp) = m[fp + 15]
╔═════════════════════════════════════════════════════════════════════════╗
║ STD-OUT ║
╚═════════════════════════════════════════════════════════════════════════╝
"print" -> 200
──────────────────────────────────────────────────────────────────────────
╔═════════════════════════════════════════════════════════════════════════╗
║ STATS ║
╚═════════════════════════════════════════════════════════════════════════╝
CYCLES: 22
MEMORY: 81
Bytecode size: 31
Public input size: 0
Private input size: 0
Runtime memory: 33 (0.00% vec) (no vec mem: 0)
Memory usage: 90.9%
──────────────────────────────────────────────────────────────────────────
test test_doesnt_work ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 21 filtered out; finished in 0.00s
[divya@archdivya leanMultisig]$ |
TomWambsgans
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I have added 2 comments on the code. Also, some tests do not pass (for instance test_match). But I believe this goes in the right direction.
| updated_fp: None, | ||
| }); | ||
|
|
||
| let remaining = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a GOTO intuitively there should not remain any lines to compile? So we could simply assert lines[i + 1..].is_empty() instead of compiling it? Or maybe am I missing some case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think one cannot assert that lines[i + 1..] is empty after a GOTO. There are valid cases where lines exist after a GOTO, and those lines need to be compiled and registered with the epilogue label.
| .zip(&simplified_args) | ||
| .map(|((var, _), expr)| (var.clone(), expr.clone())) | ||
| .collect::<BTreeMap<_, _>>(); | ||
| let epilogue_label = format!("@inline_epilogue_{}", total_inlined_counter.0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This idea of "epilogue_label" looks promising. Currently you always put an epilogue_label when inlining but in some cases, when there is no instructions after the RETURN this is unnecessary (we can call such RETURNs "final"). But detecting it is not trivial. We would somehow need to peek at the following of the code (what comes after the RETURN), to see if it's terminal or not, if that makes sense
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, you don't need to always put the label. But yeah finding where you don't need to is quite difficult. We'll need to have some context of what is final in the remainder of the scope.
Fixes #54.
I've added a
gotomechanism, that adds explicit jump instructions to handle early returns in inlined functions. When an inline function has areturnstatement inside a conditional branch that doesn't cover all code paths, the mechanism ensures that execution jumps to the end of the inlined function body (the "epilogue") instead of continuing to execute subsequent code.The mechanism operates at two levels:
inline_lines()encounters aLine::FunctionRetstatement, it now:Line::Goto instructionthat jumps to a unique epilogue label (e.g.,@inline_epilogue_0)At the
SimpleLinelevel (during simplification): TheLine::Gotois converted toSimpleLine::Gotowhen the program is simplified.At the intermediate bytecode level:
The SimpleLine::Gotocompiles to an unconditionalIntermediateInstruction::Jumpto the epilogue label, and the remaining code after thegoto is registered in the bytecode map with that label.Running the previously buggy
inline_bug_mretest withprintstatements enabled inlib.rs, we get: