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.71k stars 415 forks source link

Partial constructor application ponyc segfault #4240

Open Liasain opened 1 year ago

Liasain commented 1 year ago

Minimal example:

actor Main
  new create(env: Env) =>
    Main~create()

Partial application of a constructor (actor, class) segfaults with an LLVM error.

0.52.0-a48b9ddf [release] Compiled with: LLVM 14.0.3 -- Clang-13.0.1-x86_64 Defaults: pic=true PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.

Segmentation fault (core dumped)

SeanTAllen commented 1 year ago

Can you build a debug version of the runtime and compiler and run the program again under a debugger and report back with a backtrace?

SeanTAllen commented 1 year ago

I've added "good first issue" to this as at the moment, the first step is triaging which is a good first issue. Depending on what is found, the label will probably come off.

Liasain commented 1 year ago
Generating
 Reachability
 Selector painting
 Data prototypes
 Data types
 Function prototypes
 Functions
/ponyc/src/libponyc/codegen/genexpr.c:314: gen_assign_cast: Assertion `r_value != GEN_NOTNEEDED` failed.

Backtrace:
  /ponyc/build/debug/ponyc(ponyint_assert_fail+0x96) [0x563f11f383e6]
  /ponyc/build/debug/ponyc(gen_assign_cast+0x67) [0x563f11e55f67]
  /ponyc/build/debug/ponyc(gen_call+0x789) [0x563f11e4eb29]
  /ponyc/build/debug/ponyc(gen_expr+0x1b8) [0x563f11e55b68]
  /ponyc/build/debug/ponyc(gen_seq+0x54) [0x563f11f0a9b4]
  /ponyc/build/debug/ponyc(gen_expr+0x76) [0x563f11e55a26]
  /ponyc/build/debug/ponyc(+0x7090d3) [0x563f11f0f0d3]
  /ponyc/build/debug/ponyc(+0x7082d6) [0x563f11f0e2d6]
  /ponyc/build/debug/ponyc(genfun_method_bodies+0x99) [0x563f11f0e109]
  /ponyc/build/debug/ponyc(gentypes+0x370) [0x563f11e6e000]
  /ponyc/build/debug/ponyc(genexe+0x2c6) [0x563f11e55036]
  /ponyc/build/debug/ponyc(codegen+0xd4) [0x563f11e49b44]
  /ponyc/build/debug/ponyc(generate_passes+0x33) [0x563f11e964f3]
  /ponyc/build/debug/ponyc(+0x637b3b) [0x563f11e3db3b]
  /ponyc/build/debug/ponyc(main+0x261) [0x563f11e3d971]
  /lib/x86_64-linux-gnu/libc.so.6(+0x29d90) [0x7fd46d5ebd90]
  /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x80) [0x7fd46d5ebe40]
  /ponyc/build/debug/ponyc(_start+0x25) [0x563f11e3d645]
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace.
Aborted (core dumped)
Liasain commented 1 year ago

gen_assign_cast in genexpr.c fails when ast is a typeref. The error only happens when using partial application on constructors from a type, but not a variable:

class A
struct B
actor Main
  new create(env: Env) =>
    Main~create() // fails
    this~create() // works
    A~create() // fails
    let a = A
    a~create() // works
    B~create() // fails
    let b = B
    b~create() // works

The types encountered before failing are Main tag, A ref, and B ref respectively.

SeanTAllen commented 1 year ago

Thanks for the initial triaging @Liasain. It was a great help kick starting our discussion during today's sync call.

jemc commented 1 year ago

We discussed this a bit in today's sync call.

The assertion is happening during LLVM code generation, where the code that generates a call site is trying to use the partially applied receiver as a call argument value - the assertion comes when trying to "cast" that value to the param type.

In Pony, a type by itself does not have a value, hence we get the GEN_NOTNEEDED as the value, which cannot be cast.

I believe the correct fix is to add a special case path for constructors in the part of the compiler that sugars the partial application into a lambda, such that we use the type as a receiver rather than the "value" of that type (which again, does not exist in Pony).

There's already a special code path for "bare" functions, which has some relevant code I think we can leverage, so perhaps the fix is as simple as what I've pasted below. However, I have not tested this yet.

Whoever works on this issue could use this as a starting point and reach out in Zulip or on this ticket if they need more guidance.

diff --git a/src/libponyc/expr/call.c b/src/libponyc/expr/call.c
index 6fac89e2..c2862068 100644
--- a/src/libponyc/expr/call.c
+++ b/src/libponyc/expr/call.c
@@ -795,6 +795,7 @@ static bool partial_application(pass_opt_t* opt, ast_t** astp)
   AST_GET_CHILDREN(type, cap, type_params, target_params, result);

   bool bare = ast_id(cap) == TK_AT;
+  bool is_constructor = ast_id(method_ast) == TK_NEW;

   token_id apply_cap = TK_AT;
   if(!bare)
@@ -819,7 +820,10 @@ static bool partial_application(pass_opt_t* opt, ast_t** astp)

       arg = ast_sibling(arg);
     }
+  }

+  if(bare || is_constructor)
+  {
     ast_t* receiver_type = ast_type(receiver);
     if(is_bare(receiver_type))
     {