lcompilers / lpython

Python compiler
https://lpython.org/
Other
1.51k stars 164 forks source link

Failing symbolic case example #2614

Open anutosh491 opened 7 months ago

anutosh491 commented 7 months ago

What doesn't work

from lpython import S
from sympy import Symbol, log, E, Pow, exp, pi

def mmrv(e: S, x: S) -> list[S]:
    if e.args[0] != E:
        e1: S = E * pi
    else:
        list4: list[S] = [x]
        return list4

def test_mrv():
    x: S = Symbol("x")
    ans4: list[S] = mmrv(exp(log(x)), x)
    print(ans4[0])

test_mrv()
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine integration_tests/test_gruntz.py 
Segmentation fault
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine --backend=c integration_tests/test_gruntz.py 
Segmentation fault
anutosh491 commented 7 months ago

What works

from lpython import S
from sympy import Symbol, log, E, Pow, exp, pi

def mmrv(e: S, x: S) -> list[S]:
    if e.args[0] != E:
        e1: S = E
    else:
        list4: list[S] = [x]
        return list4

def test_mrv():
    x: S = Symbol("x")
    ans4: list[S] = mmrv(exp(log(x)), x)
    print(ans4[0])

test_mrv()
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine integration_tests/test_gruntz.py 
x
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine --backend=c integration_tests/test_gruntz.py 
x
anutosh491 commented 7 months ago

Also something similar which works

def mmrv(e: S, x: S) -> list[S]:
    if e.args[0] != E:
        a: S = E
        b: S = pi
        e1: S = a * b
    else:
        list4: list[S] = [x]
        return list4

def test_mrv():
    x: S = Symbol("x")
    ans4: list[S] = mmrv(exp(log(x)), x)
    print(ans4[0])

test_mrv()
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine --backend=c integration_tests/test_gruntz.py 
x
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine integration_tests/test_gruntz.py 
x

This hints that it might be something related to declaring or freeing of variables.

anutosh491 commented 7 months ago

Smallest failing example

from lpython import S
from sympy import Symbol, E

def mmrv(e: S) -> S:
    if False:
        print(E)
    else:
        return e

def test_mrv():
    x: S = Symbol("x")
    ans: S = mmrv(x)
    print(ans)

test_mrv()

The C code for this tells me that we are trying to free the variable (using basic_free_stack) even before creating a stack for it (using basic_new_stack), Hence the error I think

void mmrv(void* e, void* _lpython_return_variable)
{
    int64_t _stack0;
    void* stack0;
    if (false) {
        _stack0 = 0;
        stack0 = NULL;
        stack0 = (void*) &_stack0;
        basic_new_stack(stack0);
        basic_const_E(stack0);
        printf("%s\n", basic_str(stack0));
    } else {
        basic_assign(_lpython_return_variable, e);
        basic_free_stack(stack0);
        return;
    }
    basic_free_stack(stack0);
}
anutosh491 commented 7 months ago

One approach that comes to my mind is the following, i) We can maybe introduce a function in symengine's cwrapper.h (and it's corresponding implementation in cwrapper.cpp)

int basic_has_allocated_stack(basic s);

int basic_has_allocated_stack(basic s) {
    return s != nullptr;
}

And then check everytime before freeing if it was allocated on the stack beforehand. Not sure if this is go how we should proceed.

ii) Or maybe something like basic_free_stack_if_allocated that first checks if the basic variable is NULL and if it's not then frees it !

iii) And even if we don't plan to make any changes through SymEngine just adding a statement like this

        if (stack0 != NULL) {
            basic_free_stack(stack0);
        }

instead of just basic_free_stack(stack0) would work

certik commented 7 months ago

The bug is in the else branch:

    } else {
        basic_assign(_lpython_return_variable, e);
        basic_free_stack(stack0);
        return;
    }

Where we are freeing the stack0 that has not been allocated, so it fails.

certik commented 7 months ago

I can see two approaches forward:

  1. We initialize stack0 as NULL at the beginning, and we only free it if it is not null
  2. We always call basic_new_stack(stack0) every time we create a local variable stack0 and then we always deallocate it.
  3. Keep the stack0 inside the "if" statement where it is needed (the "then" branch of the if statement only)

Here is the Python code:

def mmrv(e: S) -> S:
    if False:
        print(E)
    else:
        return e

Here is how 1. would be applied to the this code:

void mmrv(void* e, void* _lpython_return_variable)
{
    int64_t _stack0;
    void* stack0;
    stack0 = NULL;
    if (false) {
        _stack0 = 0;
        stack0 = (void*) &_stack0;
        basic_new_stack(stack0);
        basic_const_E(stack0);
        printf("%s\n", basic_str(stack0));
    } else {
        basic_assign(_lpython_return_variable, e);
        basic_free_stack_if_not_null(stack0);
        return;
    }
    basic_free_stack_if_not_null(stack0);
}

Here is how 2. would be applied to the above:

void mmrv(void* e, void* _lpython_return_variable)
{
    int64_t _stack0;
    void* stack0;
    _stack0 = 0;
    stack0 = NULL;
    stack0 = (void*) &_stack0;
    basic_new_stack(stack0);
    if (false) {
        basic_const_E(stack0);
        printf("%s\n", basic_str(stack0));
    } else {
        basic_assign(_lpython_return_variable, e);
        basic_free_stack(stack0);
        return;
    }
    basic_free_stack(stack0);
}

Here is how 3. would be applied to the above:

void mmrv(void* e, void* _lpython_return_variable)
{
    if (false) {
        int64_t _stack0;
        void* stack0;
        _stack0 = 0;
        stack0 = NULL;
        stack0 = (void*) &_stack0;
        basic_new_stack(stack0);
        basic_const_E(stack0);
        printf("%s\n", basic_str(stack0));
        basic_free_stack(stack0);
    } else {
        basic_assign(_lpython_return_variable, e);
        return;
    }
}
anutosh491 commented 7 months ago

I think we can go for the first approach, I've raise a pr to symengine (https://github.com/symengine/symengine/pull/2011) for the same.

certik commented 7 months ago

Let's go with 1. for now. Let's write a function in the symbolic pass that will insert the equivalent of:

    if (stack0 != NULL) {
        basic_free_stack(stack0);
    }

into ASR.