ponylang / ponyc

Pony is an open-source, actor-model, capabilities-secure, high performance programming language
http://www.ponylang.io
BSD 2-Clause "Simplified" License
5.69k stars 410 forks source link

Wrong code generation/memory errors in generic lambda parameters #4168

Open stefandd opened 2 years ago

stefandd commented 2 years ago

In the code below, when I define the lambda argument in filter() as box->A!, it works, but when I define it as this->A! it compiles and runs, but the lambda surprisingly does not get called at all.

use @printf[I32](fmt: Pointer[U8] tag, ...)

class ArrayLike[A: Any #read] // Array-like class with .filter() function
  embed _data: Array[A] = []

  new fromArray(a: Array[A]) =>
    var i = USize(0)
    while i < a.size() do
      try _data.push(a(i = i + 1)?) end
    end

  fun apply(i: USize): this->A ? => _data(i)?

  // works correctly | with box->A!, but is buggy with this->A!
  //                 v
  fun filter(f: {(this->A!): Bool}): FilterClass[A, this->ArrayLike[A]]^ =>
    FilterClass[A, this->ArrayLike[A]](this, f)

class FilterClass[A: Any #read, B: ArrayLike[A] #read]

  new create(array: B, f: {(B->A!): Bool}) =>
    try @printf((f(array(0)?).string() + "\n").cstring()) end

class Thingy

actor Main

  new create(env: Env) =>
    let a: ArrayLike[I32] = ArrayLike[I32].fromArray([0])
    let b: ArrayLike[Thingy] = ArrayLike[Thingy].fromArray([Thingy])
    let f1 = a.filter({(x) => true})
    let f2 = b.filter({(x) => true})

with fun filter(f: {(box->A!): Bool}) .. this outputs

true
true

with fun filter(f: {(this->A!): Bool}) .. this outputs

true
false

The false is likely bug: it means that the lambda is NOT called! It is worth pointing out that in a slightly different version of this, where the lambda was supposed to actually filter, most of the time no output was generated with the this->A! version but sometimes random characters appeared (memory corruption?!)

stefandd commented 2 years ago

The relevant Zulip discussion is here: https://ponylang.zulipchat.com/#narrow/stream/189985-beginner-help/topic/Behavior.20of.20lambda.20in.20generic.20function.3A.20this-.3E.20vs.20box-.3E/near/291723270

ergl commented 2 years ago

Here's another example that reproduces the problem and that also triggers a segfault:

class Container[A: Any #read]
  let _data: A

  new create(a: A) =>
    _data = a

  fun apply(): this->A =>
    _data

  // works correctly with box->A!, but is buggy with this->A!
  //               |
  //               v
  fun update(f: {(this->A!)}) =>
    Applier[A, this->Container[A]](this, f)

primitive Applier[A: Any #read, B: Container[A] #read]
  fun apply(c: B, f: {(B->A!)}) =>
    f(c.apply())

class Counter
  var value: I32 = 0
  fun string(): String iso^ => value.string()

actor Main
  new create(env: Env) =>
    Container[Counter](Counter)
      .update({(x)(env) => env.out.print(x.value.string())})

Running the above (should print 0) leads to:

# ~/dev/ponylang/ponyc/build/debug/ponyc --debug --bin-name 4168 test_4168/ && ./4168
Building builtin -> ~/dev/ponylang/ponyc/packages/builtin
Building test_4168/ -> ~/test_4168
Generating
 Reachability
 Selector painting
 Data prototypes
 Data types
 Function prototypes
 Functions
 Descriptors
Writing ./4168.o
Linking ./4168
Segmentation fault: 11

Backtrace:

* thread #5, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x0000000000000000
error: memory read failed for 0x0
Target 0: (4168) stopped.
(lldb) bt
* thread #5, stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x0000000000000000
    frame #1: 0x0000000100004f28 4168`Applier_Counter_ref_Container_Counter_ref_ref_val_apply_ooo(this=0x0000000100025ba8, c=0x0000000108fd3a20, f=0x0000000108fd39e0) at main.pony:18:6
    frame #2: 0x0000000100003908 4168`Container_Counter_ref_ref_update_oo(this=0x0000000108fd3a20, f=0x0000000108fd39e0) at main.pony:14:35
    frame #3: 0x0000000100003d68 4168`Main_tag_create_oo(this=0x0000000108ffe000, env=0x0000000108ffdc00) at main.pony:27:14
    frame #4: 0x0000000100003524 4168`Main_Dispatch + 72
    frame #5: 0x00000001000069d4 4168`handle_message(ctx=0x0000000108ffefc8, actor=0x0000000108ffe000, msg=0x0000000108fd5040) at actor.c:400:7
    frame #6: 0x0000000100005fc8 4168`ponyint_actor_run(ctx=0x0000000108ffefc8, actor=0x0000000108ffe000, polling=false) at actor.c:486:8
    frame #7: 0x000000010001e418 4168`run(sched=0x0000000108ffef80) at scheduler.c:984:23
    frame #8: 0x000000010001d96c 4168`run_thread(arg=0x0000000108ffef80) at scheduler.c:1035:3
    frame #9: 0x000000018f67026c libsystem_pthread.dylib`_pthread_start + 148
ergl commented 2 years ago

If one removes the Applier indirection from the previous example:

fun update(f: {(this->A!)}) =>
  f(_data)
  // Applier[A, this->Container[A]](this, f)

Then the program fails to compile:

Error:
/Users/ryan/Downloads/test_4168/main.pony:14:7: argument not assignable to parameter
    f(_data)
      ^
    Info:
    /Users/ryan/Downloads/test_4168/main.pony:14:7: argument type is this->A #read
        f(_data)
          ^
    /Users/ryan/Downloads/test_4168/main.pony:13:23: parameter type requires A #read ^
      fun update(f: {(this->A!)}) =>
                          ^
    /Users/ryan/Downloads/test_4168/main.pony:1:20: Any box is not a subtype of A ref ^: the type parameter has no lower bounds
    class Container[A: Any #read]
                       ^
    /Users/ryan/Downloads/test_4168/main.pony:14:7: (this->A ref, this->A val, this->A box) is not a pairwise subtype of (A ref ^, A val ^, A box ^)
        f(_data)
          ^
stefandd commented 2 years ago

Then the program fails to compile:

Isn't that the compile error that should be generated?

ergl commented 2 years ago

@stefandd Yes, that's what should happen with the original program.

SeanTAllen commented 2 years ago

@ergl i'm intrigued by how this happens. do you know what pass that error is coming from? I assume it is coming from a pass after the pass where we turn the lambda into an object literal. I'm guessing this is mostly a "we are doing a check too early to catch for this case" error. But I'm definitely guessing on that.

stefandd commented 2 years ago
fun update(f: {(this->A!)}) =>
  f(_data)
  // Applier[A, this->Container[A]](this, f)

@SeanTAllen @ergl It seems that the correct error is produced when f is directly called in the function to which it is passed as a parameter, but that somehow when it is passed on (to Applier in the example), the correctness of the parameter viewpoint (this->A! vs box->A!) is not properly checked!

ergl commented 2 years ago

@SeanTAllen The error comes from the expr pass, here: https://github.com/ponylang/ponyc/blob/5978d146d5fd6529e262690734d71081357b5f0c/src/libponyc/expr/call.c#L308-L315

jemc commented 2 years ago

This was discussed in the August 2 sync call, and I had the following notes in the Zulip thread:

I haven't proven it, but I think what's happening is memory unsafety in the form of a virtual call to the wrong virtual table index

And hence you're not even calling the right function address, so you don't get the true result you expect - you could get any kind of garbled junk or a segfault as the result