google / starlark-go

Starlark in Go: the Starlark configuration language, implemented in Go
BSD 3-Clause "New" or "Revised" License
2.34k stars 212 forks source link

starlark: "panic: rangeLen: zero step" on absurd program #168

Open josharian opened 5 years ago

josharian commented 5 years ago
$ starlark -c "range(1)[::8192][::8192][::8192][::8192][::8192]"
panic: rangeLen: zero step

goroutine 1 [running]:
go.starlark.net/starlark.rangeLen(...)
    /Users/josh/src/go.starlark.net/starlark/library.go:910
go.starlark.net/starlark.rangeValue.Slice(0x0, 0x8000000000, 0x10000000000000, 0x1, 0x0, 0x1, 0x2000, 0x1249201, 0x17000e8)
    /Users/josh/src/go.starlark.net/starlark/library.go:923 +0x134
go.starlark.net/starlark.slice(0x12492a0, 0xc000016200, 0x12491a0, 0x1395220, 0x12491a0, 0x1395220, 0x1249160, 0xc00005c180, 0x12492a0, 0xc000016200, ...)
    /Users/josh/src/go.starlark.net/starlark/eval.go:1064 +0x1a0
go.starlark.net/starlark.(*Function).CallInternal(0xc0000fc230, 0xc0000137d0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x1, 0x2, ...)
    /Users/josh/src/go.starlark.net/starlark/interp.go:400 +0x1770
go.starlark.net/starlark.Call(0xc0000137d0, 0x1249020, 0xc0000fc230, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0xc000010028, ...)
    /Users/josh/src/go.starlark.net/starlark/eval.go:993 +0x145
go.starlark.net/starlark.(*Program).Init(0xc000010028, 0xc0000137d0, 0x0, 0xc00005c0e0, 0xc00005c0f0, 0xc000086240)
    /Users/josh/src/go.starlark.net/starlark/eval.go:338 +0x9a
go.starlark.net/starlark.ExecFile(0xc0000137d0, 0x1207e3d, 0x7, 0x11bd7e0, 0xc00005c0e0, 0x0, 0xc, 0x1, 0x120b2a4)
    /Users/josh/src/go.starlark.net/starlark/eval.go:267 +0xed
main.main()
    /Users/josh/src/go.starlark.net/cmd/starlark/starlark.go:74 +0x21a

This is a ridiculous program. I do plan to see whether there is a reasonable fix, though.

alandonovan commented 5 years ago

Unfortunately Sliceable has no way to report an error such as overflow. (Oops.) I see four choices:

  1. we could add an error result (and break the interface). I suspect very few clients are using it so far.
  2. we could "clip" the multiplication at maxint. This way the stride remains "very large", even if not exact.
  3. we could use big.Int for index calculations. This avoids the error but seems more trouble than it's worth.
  4. do nothing, as we'll never be able to fix the last panic induced by absurd and malicious input.

I prefer 1 > 4 > 2 > 3. Thoughts?

josharian commented 5 years ago

1 > 4 > 3 > 2. Panic is better than being incorrect.