odin-lang / Odin

Odin Programming Language
https://odin-lang.org
BSD 3-Clause "New" or "Revised" License
6.77k stars 589 forks source link

Silent compiler crash #4395

Open flysand7 opened 7 hours ago

flysand7 commented 7 hours ago
    Odin:    dev-2024-10:2141299d2
    OS:      Windows 10 Unknown Edition (00000064) (version: 22H2), build 19045.5011
    CPU:     12th Gen Intel(R) Core(TM) i5-12500H
    RAM:     16088 MiB
    Backend: LLVM 18.1.8

Happens when assigning the result of simd.masked_compress_store, which doesnt return any values. The compiler crashes without printing any error messages.

package bug

import "core:fmt"
import "core:simd"

main :: proc() {
    v := [2] f64 {1, 2};
    mask := #simd [4]bool { true, false, true, false }
    vals := #simd [4]f64 { 0x7f, 0x7f, 0x7f, 0x7f }
    res := simd.masked_compress_store(&v, vals, mask)
    fmt.println(res)
}

Haven't looked into more detail.

tf2spi commented 5 hours ago

I'm looking into the issue right now. So the compiler is supposed to give you an error with simd.masked_compress_store because simd.masked_compress_store does not return a value. It simply puts your stuff in v and nothing more.

However, Odin's not catching the fact that the left-hand side res is being assigned something that doesn't have a value nor a type, so it segfaults when it tries to check the type because it's nullptr.

(gdb) bt
#0  0x000055555564b6cf in is_type_asm_proc (t=0x0) at src/types.cpp:1624
#1  0x00005555556489c7 in check_init_variable (ctx=0x7fffdf0c57e0, e=0x7fffc49590f0, operand=0x7fffc4959290, context_name=...) at src/check_decl.cpp:86
tf2spi commented 5 hours ago

Oh, now I see. I found something quite strange indeed... This is the operand for simd_masked_compress_store.

(gdb) p *operand
$1 = {mode = Addressing_Value, type = 0x0, value = {kind = ExactValue_Invalid, {value_bool = false, value_string = {text = 0x0, len = 0},
      value_integer = {used = 0, alloc = 0, sign = MP_ZPOS, dp = 0x0}, value_float = 0, value_pointer = 0, value_complex = 0x0,
      value_quaternion = 0x0, value_compound = 0x0, value_procedure = 0x0, value_typeid = 0x0}}, expr = 0x7fff9c7f7d60,
  builtin_id = BuiltinProc_simd_masked_compress_store, proc_group = 0x0}
tf2spi commented 5 hours ago

I think I'm getting closer to the actual root of the problem after putting a watchpoint on operand->mode.

Thread 25 "odin" hit Breakpoint 1, check_builtin_simd_operation (c=0x7fffe11cc710, operand=0x7fffe11cc6c0, call=0x7fffacff9d60, id=155,
    type_hint=0x0) at src/check_builtin.cpp:751
751                                     operand->type = nullptr;
(gdb) watch operand->mode
Hardware watchpoint 2: operand->mode
(gdb) r
The program being debugged has been started already.
Start it from the beginning? (y or n) n
Program not restarted.
(gdb) c
Continuing.

Watchpoint 2 deleted because the program has left the block in
which its expression is valid.
0x00005555556c3a95 in check_builtin_procedure (c=0x7fffe11cc710, operand=0x7fffe11cc6c0, call=0x7fffacff9d60, id=155, type_hint=0x0)
    at src/check_builtin.cpp:2060
2060                    bool ok = check_builtin_simd_operation(c, operand, call, id, type_hint);
(gdb) n
2061                    if (!ok) {
(gdb) p ok
$1 = true
(gdb) n
2065                    operand->mode = Addressing_Value;
(gdb)
2066                    operand->value = {};
(gdb)

I did find at around line 2060 that an early return if !ok might have been intended, but putting in the early return is not sufficient because the check is returning true anyways.

tf2spi commented 5 hours ago

Okay, now I got it. This snippet.

    if (BuiltinProc__simd_begin < id && id < BuiltinProc__simd_end) {
        bool ok = check_builtin_simd_operation(c, operand, call, id, type_hint);
        if (!ok) {
            operand->type = t_invalid;
        }
        operand->mode = Addressing_Value;
        operand->value = {};
        operand->expr = call;
        return ok;
    }

operand->mode = Addressing_Value; falsely assumes that the result of all simd operations returns a value. If this line is commented out, it fixes the issue. From what I could also gloss over from check_builtin_simd_operation, if it returns true, the type and mode are already set anyways. If I had to guess, I think the mode was meant to be set if the condition was !ok? Not sure though.