lcompilers / lpython

Python compiler
https://lpython.org/
Other
1.5k stars 158 forks source link

Issues with declaring and freeing out symbolic variables #2552

Closed anutosh491 closed 7 months ago

anutosh491 commented 7 months ago

This issue arises out of this PR https://github.com/lcompilers/lpython/pull/2528. Consider this minimalistic example

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

def mmrv(e: S, x: S) -> list[S]:
    empty_list : list[S] = []
    if not e.has(x):
        return empty_list
    elif e.func == log:
        arg0: S = e.args[0]
        return empty_list
    elif e.func == Pow:
        arg1: S = e.args[0]
        if arg1 != E:
            return empty_list
    else:
        raise

def test_mrv():
    # Case 1
    x: S = Symbol("x")
    y: S = Symbol("y")
    ans1: list[S] = mmrv(y, x)
    print(ans1)
    assert len(ans1) == 0

test_mrv()
(lf) anutosh491@spbhat68:~/lpython/lpython$ lpython --enable-symengine --backend=c integration_tests/test_gruntz.py 
test_gruntz__tmp__generated__.c: In function ‘test_mrv’:
test_gruntz__tmp__generated__.c:145:12: warning: zero-length gnu_printf format string [-Wformat-zero-length]
  145 |     printf("");
      |            ^~
Segmentation fault
anutosh491 commented 7 months ago

The llvm_sym backend works but I think it should not. So we shall debug it through the C backend Let's go issue by issue 1) I am not sure if every symbolic variable is declared in the correct scope

// Implementations
void mmrv(void* e, void* x, struct list_CPtr *_lpython_return_variable)
{
    int64_t _arg0;
    int64_t _arg1;
    struct list_CPtr _empty_list;
    void* _lcompilers_symbolic_argument_container;
    void* _lcompilers_symbolic_argument_container1;
    int64_t _stack0;
    void* arg0;
    void* arg1;
    struct list_CPtr empty_list;
    void* stack0;
    int32_t symbolic_list_index;
    _arg0 = 0;
    arg0 = NULL;
    arg0 = (void*) &_arg0;
    basic_new_stack(arg0);
    _arg1 = 0;
    arg1 = NULL;
    arg1 = (void*) &_arg1;
    basic_new_stack(arg1);

Looking at this block I can see the scope being used is not really correct. Shouldn't we just be seeing empty_list being declared in this scope. Talking about arg0, shouldn't we find it being defined in the second if block (first elif block)

if (basic_get_type(e) == 29) {
   int64_t _arg0;
   void* arg0;
   _arg0 = 0;
   arg0 = NULL;
   arg0 = (void*) &_arg0;
   // further operation
anutosh491 commented 7 months ago

2) Freeing out variables.

The reason for the segfault is this in the c code

    if (!basic_has_symbol(e, x)) {
        list_deepcopy_CPtr(&empty_list, _lpython_return_variable);

        basic_free_stack(arg0);
        basic_free_stack(arg1);
    }

As you can see the arg0 & arg1 variables are being freed even before they can be used in the further if blocks.

anutosh491 commented 7 months ago

Freeing is related to this comment (https://github.com/lcompilers/lpython/issues/2533#issuecomment-1946100684) I think

certik commented 7 months ago

I can only reproduce this issue when I am on the branch of #2528. In master I don't get this segfault with neither LLVM nor C backends.