wren-lang / wren

The Wren Programming Language. Wren is a small, fast, class-based concurrent scripting language.
http://wren.io
MIT License
6.92k stars 554 forks source link

[0.3.0] Wren does not check for stack overflow #939

Open Techcable opened 3 years ago

Techcable commented 3 years ago

The following code triggers the Linux out of memory killer on my system:

class Vec {
  construct new(x, y, z) {
    _x = x
    _y = y
    _z = z
  }
  x { x }
  y { y }
  z { z }
}

Vec.new(0, 0, 0).x

The problem is in wrenMethodBufferFill -> wrenReallocate -> defaultReallocate

You can debug this in more detail with valgrind --tool=massif. Hopefully it should be relatively straightforward to add a check for this. A SIGKILL on stack overflow is extremely unintuitive 😢

ChayimFriedman2 commented 3 years ago

The problem is wider: Wren does not check for any allocation failure. Stack is growing using allocation, and this is what happening.

You can watch this using the following code:

var l = []
while (true) l.add({})
fermian commented 3 years ago

Wren also doesn't check for parser-related stack overflows. Using Python to generate a file containing deeply nested lists:

x=""
iters=500_000
for i in range(iters):
    x+="["
print(x)

Then running that file using wren_test causes it to segfault.

joshgoebel commented 2 years ago

I think a counter inside void expression(Compiler* compiler) would help here (though it would likely slightly slow down parsing expressions), but I'm not sure what the correct behavior is when you hit the limit, just throw an error and crash?

Techcable commented 2 years ago

Lua's parser also uses recursive decent. It has tests for this scenario.

The lua 5.3 parser has special checks for each type of possible overflow.

On Lua 5.4 it looks like it uses the internal C stack for temporary storage.

$ python3 -c 'print("foo = " + "{" * 5_000_000 + str(6) + "}" * 5_000_00
0)' > foo.lua
$ lua5.3 foo.lua
lua5.3: foo.lua:1: too many C levels (limit is 200) in main function near '{'
$ lua5.4 foo.lua
lua5.4: C stack overflow

I suggest we use the lua 5.3 style solution :)

Techcable commented 2 years ago

On my Macbook Pro M1 and Wren 0.4.0, this is still an issue.

It manages to use up all 32 GB of my system memory before swapping to disk and stalling 😉

joshgoebel commented 2 years ago

I'd have no idea how to pick a limit. I just started with 1000 locally and that solved all the crashes.

It manages to use up all 32 GB of my system memory before swapping to disk and stalling 😉

Are you sure that's the same issue? We're talking about the stack overflowing here, not an exhaustion of all available RAM...

Which code sample above are you using to see that behavior?