odin-lang / Odin

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

Using condensed (range index) syntax program crashes (runtime) when allocating literals for a dynamic array of over 1mb #4178

Open galt-john opened 2 weeks ago

galt-john commented 2 weeks ago

Also, see https://github.com/odin-lang/Odin/issues/4179 for a similar issue but compiler segfault crash with slices

Windows and latest build of Odin 08-2024 and also on 09-2024 More than one person confirmed, both on Windows 11 both 24H2 One with Intel and one with AMD Both with LLVM 18.1.8

arr2 below works up to 1mb allocations but crashes at > 1mb allocations (commented out arr3), tested int, u8, etc. they all crash at runtime if > 1mb arr1 using make is fine up to huge allocations well beyond 1mb

  arr1 := make([dynamic]u8, 2_000_000, 2_000_000) // Tracking alloc of context.allocator: leaked 1.91mib
  arr2 := [dynamic]u8 {0 ..= 1_000_000 = 1}       // Tracking alloc of context.allocator: leaked 976.56kib
  //arr3 := [dynamic]u8 {0 ..= 1_200_000 = 1}     // Crashes program when > 1mb allocated

  fmt.println(len(arr1), cap(arr1)) // 2000000 2000000
  fmt.println(len(arr2), cap(arr2)) // 1000001 1000001

Checked that the context.allocator and the arr2 allocator were the same and they are so it is using the OS Heap default context.allocator

On discussions with Chamberlain and Rickard Andersson it appears that maybe the range operator unwinds the literals on the stack as a holding spot before they are allocated on the heap and we get a stack overflow crash on Windows when over 1mb.

Expected Behavior is a warning/error and/or logic that prevents runtime crash - since we did not find the source code this is as far as we got.

gingerBill commented 2 weeks ago

So the reason crashes is that IIRC the data for the dynamic array is initially stored on the stack AND then copied to the dynamic array. So the kind of crash is just the stack overflow I guess.

galt-john commented 2 weeks ago

That is what we thought too based on the type of shutdown. Thanks for confirming. Not sure there is much to do about this other than being aware and always sticking to using make( ) and then allocate by setting the len = cap = desired size and then using the index and loop to fill with desired values if preloading it. The linked bug report https://github.com/odin-lang/Odin/issues/4179 is related but it is a compiler (vs runtime) crash so that might lend one to believe it a different root cause - possibly.

Chamberlain91 commented 2 weeks ago

In this case (and the linked case) would it possible for the compiler to know at compile time about the potential stack overflow and emit a warning? Since 0 ..= 1_200_000 is compile-time known size/range?

galt-john commented 1 week ago

That would be nice Chamberlain. I know the stack size on windows is 1mb per thread and iirc it is larger, maybe 8mb on modern linux - that may really vary a lot. I think it is user variable too, I mean there is a way to change it I think. So if the compiler sees you are going to load the stack with >= something large (~ 900kb or more) it could warn you that you may induce a stack overflow. That is beyond my knowledge on how to implement that with cpp.