Conditional expression branch may be evaluated before the condition within 'proc' functions

ddjoh commented 4 days ago

Acton Version

0.23.0.20241018.14.24.5

Steps to Reproduce and Observed Behavior

class Foo(object):
    def __init__(self):
        pass

    proc def foo(self) -> int:
        return 42

def bar(f: ?Foo):
    return f.foo() if f is not None else None

actor main(env):
    print(bar(None))
    env.exit(0)

Expected Behavior

The lhs f.foo() of of f.foo() if f is not None else None to be evaluated only if the condition is true.

plajjan commented 3 days ago

I modified the source slightly, adding a print statement to help me find the generated code:

class Foo(object):
    def __init__(self):
        pass

    proc def foo(self) -> int:
        return 42

def bar(f: ?Foo):
    print("FOO") # to help find the right code location
    return f.foo() if f is not None else None

actor main(env):
    print(bar(None))
    env.exit(0)

Running this we get a segfault.

kll@Boxy:~/dt/acton/condbranch$ out/bin/condbranch --rts-wthreads=1 --rts-bt-dbg
FOO

ERROR: This is the automatic debug launcher for out/bin/condbranch

ERROR: illegal instruction
... snip
Thread 19 has target id 'Thread 0x7f11ae9cc6c0 (LWP 2228611)'
(gdb) thread 19
[Switching to thread 19 (Thread 0x7f11ae9cc6c0 (LWP 2228611))]
#0  0x00007f11b644cb57 in __GI___wait4 (pid=2228612, stat_loc=0x0, options=0, usage=0x0)
    at ../sysdeps/unix/sysv/linux/wait4.c:30
30  ../sysdeps/unix/sysv/linux/wait4.c: No such file or directory.
(gdb) bt
#0  0x00007f11b644cb57 in __GI___wait4 (pid=2228612, stat_loc=0x0, options=0, usage=0x0)
    at ../sysdeps/unix/sysv/linux/wait4.c:30
#1  0x00000000011c53ca in launch_debugger (signum=4) at base/rts/rts.c:2213
#2  <signal handler called>
#3  0x00000000015c57da in condbranchQ_bar (C_cont=0x7f11b6263fa0, f=0x0) at condbranch/out/types/condbranch.c:71
#4  0x00000000015c7189 in condbranchQ_mainD___init__ (self=0x7f11b6267f60, C_cont=0x15eb3c0 <$Done$instance>, 
    env=0x7f11b6266f00) at condbranch/out/types/condbranch.c:187
#5  0x000000000119770a in $InitRootD___call__ ($this=0x15eb470 <$InitRoot$cont>, val=0x15eb3c0 <$Done$instance>)
    at base/rts/rts.c:723
#6  0x000000000119b83e in wt_work_cb (ev=0x160d5c8 <work_ev+120>) at base/rts/rts.c:1558
#7  0x00000000012ebec2 in uv__run_check (loop=0x7f11b625f800)
    at /home/kll/dt/acton/condbranch/.build/sys/base/.build/sys/deps/libuv/src/unix/loop-watcher.c:67
#8  0x00000000012beb16 in uv_run (loop=0x7f11b625f800, mode=UV_RUN_DEFAULT)
    at /home/kll/dt/acton/condbranch/.build/sys/base/.build/sys/deps/libuv/src/unix/core.c:462
#9  0x00000000011a0810 in main_loop (idx=0x1) at base/rts/rts.c:1753
#10 0x000000000129b361 in GC_pthread_start_inner (sb=0x7f11ae9cbea0, arg=0x7fff8bd8e780)
    at /home/kll/dt/acton/condbranch/.build/sys/base/.build/sys/deps/libgc/pthread_start.c:55
#11 0x0000000001280c63 in GC_call_with_stack_base (fn=0x129b2a0 <GC_pthread_start_inner>, arg=0x7fff8bd8e780)
    at /home/kll/dt/acton/condbranch/.build/sys/base/.build/sys/deps/libgc/misc.c:2307
#12 0x0000000001299d7b in GC_pthread_start (arg=0x7fff8bd8e780)
    at /home/kll/dt/acton/condbranch/.build/sys/base/.build/sys/deps/libgc/pthread_support.c:2513
#13 0x00007f11b6402144 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:442
#14 0x00007f11b64827dc in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
(gdb) select-frame 3
(gdb) info locals
$tmp = 0x0
(gdb)

That particular bar() function generated code looks like:

$R condbranchQ_bar ($Cont C_cont, condbranchQ_Foo f) {
    ((B_NoneType (*) (B_tuple, B_str, B_str, B_bool, B_bool))B_print)($NEWTUPLE(1, to$str("FOO")), B_None, B_None, B_None, B_None);
    return ({ condbranchQ_Foo $tmp = ((condbranchQ_Foo)f);
              (($R (*) (condbranchQ_Foo, $Cont))$tmp->$class->foo)($tmp, (($Cont)condbranchQ_L_2ContG_new(f, C_cont))); });
}

nordlander commented 3 days ago

This bug is actually well understood, immediately from the example. What happens is that the preprocessing stage of CPS translates bar into

def bar(f: ?Foo):
    tmp = f.foo()
    return tmp if f is not None else None

All with the intent to lift out every function call subject to CPS translation as a single assignment right-hand side. But in this case the result is obviously wrong, since lifting out a call that's part of a conditional expression violates the lazy semantics of the conditional. I cannot explain this by anyting but a lapse of mind.

The correct transformation is of course to lift out the conditional as well in these cases, turning bar into something like

def bar(f: ?Foo):
    if f is not None:
        tmp = f.foo()
    else:
        tmp = None
    return tmp

Should be easy to fix.

actonlang / acton