Skip to content
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

Unsound capture of mutable variables in closures #1365

Closed
vkuncak opened this issue Dec 16, 2022 · 3 comments · Fixed by #1385
Closed

Unsound capture of mutable variables in closures #1365

vkuncak opened this issue Dec 16, 2022 · 3 comments · Fixed by #1385
Assignees
Labels
aliasing Alias and effect analysis for imperative bug high priority imperative

Comments

@vkuncak
Copy link
Collaborator

vkuncak commented Dec 16, 2022

Stainless freezes the value of a closure incorrectly at creation time, resulting in unsoundness.
For example, it verifies this.

import stainless.annotation.*
object Lambda4 {
  def app[@mutable A](mkA: A, ping: A => Unit, get: A => BigInt): BigInt = {
    val f: A => BigInt = ((a: A) => {ping(a); get(a)+1})
    f(mkA)
  } ensuring(res => true)

  case class Cell[T](var content: T)

  def test = {
    val c = Cell[BigInt](42)
    val d = Cell[BigInt](7)
    val myPing = {
      (c1 : Cell[BigInt]) => c1.content = c1.content + d.content
    }
    d.content = 1000
    val res: BigInt = app(c, myPing, _.content)
    assert(c.content == 49)
    assert(res == 50)
  }
}

edit: of course Stainless needs @mutable parameter on A in app; I had inadvertedly removed it as I was testing it in REPL. The use in REPL confirms that the program crashes at run time due to assertion viaolation, yet it verifies with stainless Version: 0.9.7-7-g99a3f92

@vkuncak vkuncak added bug imperative aliasing Alias and effect analysis for imperative labels Dec 16, 2022
@vkuncak
Copy link
Collaborator Author

vkuncak commented Dec 16, 2022

In my view, we should just disable capturing of mutable objects inside closures.
If you need to capture something, you need to do it through objects of some other class, and then aliasing checks need to apply.

We should also check what happens if we do similar construction using inner classes.

@vkuncak
Copy link
Collaborator Author

vkuncak commented Dec 19, 2022

While one can imagine generalization to allow this, I believe that we should for now use the basic type systems to enforce that functions are only working on their arguments and do not capture anything mutable. For more general case, I propose to use a different type, see #1369

@mario-bucev
Copy link
Collaborator

Note that the assertion is correctly rejected if we were to replace the lambda with an inner function

mario-bucev added a commit to mario-bucev/stainless that referenced this issue Mar 6, 2023
@mario-bucev mario-bucev mentioned this issue Mar 6, 2023
vkuncak pushed a commit that referenced this issue Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aliasing Alias and effect analysis for imperative bug high priority imperative
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants