artiq
artiq copied to clipboard
Unsound parameter lifetime handling
Bug Report
One-Line Summary
The current lifetime check for function parameters is unsound and can lead to memory corruption.
Issue Details
Even if #1497 (compiler: Function return values (?) not lifetime-tracked) is fixed appropriately, the current escape analysis for function parameters is still unsound and can lead to memory corruption. The reason is that the lifetime for function parameters are treated as the same, but they should be treated as incomparable for correct analysis.
For example, consider the following code:
@kernel
def bar(ls):
a = [1, 2, 3]
ls[0] = a
This would cause a compilation error, as the compiler correctly identified that the variable a
is mutable, and does not outlive the variable ls
which is passed from the caller. However, if we change the function a bit:
@kernel
def foo(ls, a):
ls[0] = a
@kernel
def bar(ls):
a = [1, 2, 3]
foo(ls, a)
Instead of assigning it directly, we pass to another function foo
which does the assignment. As the current escape analysis treats each parameter to have the same scope, this passes the lifetime check, but the behavior is exactly the same as the previous one, and should be rejected.
Steps to Reproduce
Run the following code:
from artiq.experiment import *
@kernel
def foo(ls, a):
ls[0] = a
@kernel
def bar(ls):
a = [1, 2, 3]
foo(ls, a)
class Lifetime(EnvExperiment):
def build(self):
self.setattr_device("core")
@kernel
def run(self):
ls = [[1]]
bar(ls)
ls[0][0] = 0
a = ls[0]
print(ls[0])
bar(ls)
print(ls[0])
print(a)
Expected Behavior
Either reject the code or print
[0, 2, 3]
[1, 2, 3]
[0, 2, 3]
This is because we saved a = ls[0]
. The function bar
should assign a new list [1, 2, 3]
to the first element in ls
, so it should not affect our variable a
.
Actual (undesired) Behavior
Prints
[0, 2, 3]
[1, 2, 3]
[1, 2, 3]
Your System (omit irrelevant parts)
- ARTIQ version: Current master
I think I might have just seen another version of this. We had some code that did something similar to:
class Escape2(EnvExperiment):
def build(self):
self.setattr_device("core")
self.s = ""
def run(self):
self.run1("base")
@kernel
def run1(self, base):
tmp = base + ".extend"
self.run2(tmp)
print("self.s", self.s)
@kernel
def run2(self, s):
self.s = s
Which crashes in the UTF-8 validation of the RPC call to write back the value of s
(the crash looks similar to https://github.com/m-labs/artiq/issues/1379, but I'm convinced that this is the underlying cause). The original code involved more nesting and an explicit RPC that tried to use a reference that had outlived the lifetime of the value. But the upshot was the same.