[Core] Bounds checking for vectors

[kolen]

Currently, Vector elements are designed to be accessed via vector.data which returns raw C pointer object that is unsafe:

local types = require('radio.types')
local vec = types.Float32.vector(10)
vec.data[-100000000].value = 1

▶ luaradio test.lua
[1]    53436 segmentation fault  luaradio test5.lua

It feels weird that such low-level unsafe behavior is in effect when using scripting language Lua and seemingly high-level construct Vector. At least it is not mentioned in documentation and was surprise to me. I was getting confusing errors such as:

luajit(52395,0x7fffa10713c0) malloc: *** error for object 0x7fb5eb018000: pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug

Maybe some vector access API with bounds checking will not add much overhead, given that vectors are usually accessed inside regular Lua loops?

[vsergeev]

Sure, I'm very open to implementing bounds checking. I avoided doing it at the outset, because I was concerned by any performance hit -- LuaJIT can be a bit sensitive to the implementation of critical path code. So it will take a little investigation to do it right.

It might be implemented such that vec[i] is the bounds-checked accessor to the element, and vec.data is the raw pointer version to be used primarily for C APIs.

[lukego]

Just an experience report on bounds-checking with LuaJIT FFI:

There is some example code from Mike Pall about how to efficiently put bounds checking on FFI arrays. One gotcha though is that each time you call ffi.typeof(...) to create a wrapper struct you will create a unique FFI type and the JIT will want to generate separate code for each such type, which can have interesting consequences (see https://github.com/snabbco/snabb/pull/612).

I'd recommend finding a solution that only requires creating one FFI type. Could do that by insisting that all vectors are the same size (create the wrapper with typeof() and then reuse it) or e.g. by stashing the length inside the wrapper struct instead of the metatype closure.

Give me a ping with @lukego if you want a second pair of eyes on potential bad cases for any solution you cook up :).