vnmakarov / mir

A lightweight JIT compiler based on MIR (Medium Internal Representation) and C11 JIT compiler and interpreter based on MIR
MIT License
2.24k stars 147 forks source link

possible bug in process_inlines #316

Closed Itay2805 closed 1 year ago

Itay2805 commented 1 year ago

I have noticed that I get a strange assert (wrong insert_before for MIR_insn_t) in the following code in mir.c inside the process_inlines function:

        if (head_func_insn->code != MIR_LABEL)
          MIR_insert_insn_before (ctx, func_item, head_func_insn, func_top_alloca);
        else
          MIR_insert_insn_after (ctx, func_item, head_func_insn, func_top_alloca);

In my test code I have a function whose first instruction is a call, which from what I can tell gets inlined into the first function, that causes the head_func_insn to no longer actually point to the first instruction (func->insns->head), which makes for the assert to eventually fire.

The first function:

int32 [Corelib-v1]System.Random+ThreadSafeRandom::Next():   func    i32, p:this
    local   i64:exception, i64:si0, i64:ti0, i64:ti1
# 1 arg, 4 locals
L32680:
    call    [Corelib-v1]System.Random+XoshiroImpl [Corelib-v1]System.Random+ThreadSafeRandom::get_LocalRandom()$proto, [Corelib-v1]System.Random+XoshiroImpl [Corelib-v1]System.Random+ThreadSafeRandom::get_LocalRandom(), si0
L32681:
    mov ti0, si0
    mov ti1, u32:(ti0)
    call    int32 [Corelib-v1]System.Random+ImplBase::Next()$proto, int32 [Corelib-v1]System.Random+XoshiroImpl::Next(), si0, ti0
L32682:
    mov ti0, si0
    ret ti0
    endfunc

The second function:

[Corelib-v1]System.Random+XoshiroImpl [Corelib-v1]System.Random+ThreadSafeRandom::get_LocalRandom():    func    p
    local   i64:exception, i64:si0, i64:ti0, i64:si1
# 0 args, 4 locals
L32164:
    mov ti0, [Corelib-v1]System.Random+ThreadSafeRandom::_random
    mov si0, p:(ti0)
L32165:
    mov ti0, si0
    mov si0, ti0
    mov si1, ti0
L32166:
    mov ti0, si1
    bt  L32167, ti0
L32168:
L32169:
    call    [Corelib-v1]System.Random+XoshiroImpl [Corelib-v1]System.Random+ThreadSafeRandom::Create()$proto, [Corelib-v1]System.Random+XoshiroImpl [Corelib-v1]System.Random+ThreadSafeRandom::Create(), si0
L32167:
    mov ti0, si0
    ret ti0
    endfunc
Itay2805 commented 1 year ago

After more inspection I think I understand more, the first instruction is a call that gets inlined, that causes the first instruction to then get removed from the insn list (since the call itself is no longer needed), then at a later stage the head_func_insn (which now has its prev and next being NULL) is used to insert another thing at the start, which fails because the instruction is no longer linked.

vnmakarov commented 1 year ago

Thank you for reporting this and provide the test cases. I'll try to fix the problem on this week.

vnmakarov commented 1 year ago

Your analysis is right. After some tries, I create a code which reproduces the bug.

I've pushed the fix https://github.com/vnmakarov/mir/commit/c476b61dbe86b1d9ea36d142b978b28c0b2ec21d into master and v_0_x branch.

Thank you again for reporting this.

Itay2805 commented 1 year ago

Tested and it works great! Thanks!