Bug in execute_from_str

C programming, build, interpreter/VM.
Target audience: MicroPython Developers.
BramPeeters
Posts: 54
Joined: Wed Jan 31, 2018 3:10 pm

Bug in execute_from_str

Post by BramPeeters » Wed Sep 12, 2018 1:37 pm

Code: Select all

mp_obj_t execute_from_str(const char *str) {
    nlr_buf_t nlr;
    if (nlr_push(&nlr) == 0) {
        mp_lexer_t *lex = mp_lexer_new_from_str_len(0/*MP_QSTR_*/, str, strlen(str), false);
        mp_parse_tree_t pt = mp_parse(lex, MP_PARSE_FILE_INPUT);
        mp_obj_t module_fun = mp_compile(&pt, lex->source_name, MP_EMIT_OPT_NONE, false);
        mp_call_function_0(module_fun);
        nlr_pop();
        return 0;
    } 


Since mp_parse will free the lexer at the end, the lex->source_name argument of mp_compile is using memory that is not valid anymore.

jickster
Posts: 629
Joined: Thu Sep 07, 2017 8:57 pm

Re: Bug in execute_from_str

Post by jickster » Wed Sep 12, 2018 2:02 pm

No.

lex->source_name is not a char * but a qstring.

Qstrings never get collected even if they’re only used ONCE. Qstrings are accumulated in RAM throughout the lifetime of the VM+heap (though there’s discussion to change that).


Sent from my iPhone using Tapatalk Pro

BramPeeters
Posts: 54
Joined: Wed Jan 31, 2018 3:10 pm

Re: Bug in execute_from_str

Post by BramPeeters » Wed Sep 12, 2018 2:12 pm

But 'lex' itself is collected, so you are dereferencing a memory address that no longer contains a valid value
You do not even get to the qstring anymore....

jickster
Posts: 629
Joined: Thu Sep 07, 2017 8:57 pm

Re: Bug in execute_from_str

Post by jickster » Wed Sep 12, 2018 2:13 pm

BramPeeters wrote:But 'lex' itself is collected, so you are dereferencing a memory address that no longer contains a valid value
You do not even get to the qstring anymore....
Where do you see that lex is collected?


Sent from my iPhone using Tapatalk Pro

BramPeeters
Posts: 54
Joined: Wed Jan 31, 2018 3:10 pm

Re: Bug in execute_from_str

Post by BramPeeters » Wed Sep 12, 2018 2:16 pm

Because mp_parse 'frees' it. So 'collected' is the wrong word, it is explicitly freed by the code, not by the GC.

Code: Select all

mp_parse_tree_t mp_parse(mp_lexer_t *lex, mp_parse_input_kind_t input_kind) 
{
    ....
    mp_lexer_free(lex); <====== 

    return parser.tree;
}

jickster
Posts: 629
Joined: Thu Sep 07, 2017 8:57 pm

Re: Bug in execute_from_str

Post by jickster » Wed Sep 12, 2018 2:18 pm

Look into that function. It doesn’t free lex itself.


Sent from my iPhone using Tapatalk Pro

BramPeeters
Posts: 54
Joined: Wed Jan 31, 2018 3:10 pm

Re: Bug in execute_from_str

Post by BramPeeters » Wed Sep 12, 2018 2:22 pm

In my codebase it sure does ...

Code: Select all

void mp_lexer_free(mp_lexer_t *lex) {
    if (lex) {
        lex->reader.close(lex->reader.data);
        vstr_clear(&lex->vstr);
        m_del(uint16_t, lex->indent_level, lex->alloc_indent_level);
        m_del_obj(mp_lexer_t, lex);   <=============
    }
}


#define m_del_obj(type, ptr) (m_del(type, ptr, 1))


#define m_del(type, ptr, num) ((void)(num), m_free(ptr))


void m_free(void *ptr) {
    free(ptr);
...
}

jickster
Posts: 629
Joined: Thu Sep 07, 2017 8:57 pm

Re: Bug in execute_from_str

Post by jickster » Wed Sep 12, 2018 4:40 pm

BramPeeters wrote:
Wed Sep 12, 2018 2:22 pm
In my codebase it sure does ...

Code: Select all

void mp_lexer_free(mp_lexer_t *lex) {
    if (lex) {
        lex->reader.close(lex->reader.data);
        vstr_clear(&lex->vstr);
        m_del(uint16_t, lex->indent_level, lex->alloc_indent_level);
        m_del_obj(mp_lexer_t, lex);   <=============
    }
}


#define m_del_obj(type, ptr) (m_del(type, ptr, 1))


#define m_del(type, ptr, num) ((void)(num), m_free(ptr))


void m_free(void *ptr) {
    free(ptr);
...
}
Ok you're right but it's in the example API usage, not the API itself.


BramPeeters
Posts: 54
Joined: Wed Jan 31, 2018 3:10 pm

Re: Bug in execute_from_str

Post by BramPeeters » Wed Sep 12, 2018 6:24 pm

Looks way better than the direct 0 I replaced lex->source_name with ;p
Thanks

Post Reply