Skip to content

[Core] Bounds checking for vectors #41

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kolen opened this issue Feb 20, 2017 · 2 comments
Open

[Core] Bounds checking for vectors #41

kolen opened this issue Feb 20, 2017 · 2 comments

Comments

@kolen
Copy link

kolen commented Feb 20, 2017

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
Copy link
Owner

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.

@vsergeev vsergeev changed the title Bounds checking for vectors [Core] Bounds checking for vectors Mar 23, 2017
@vsergeev vsergeev assigned vsergeev and unassigned vsergeev Mar 23, 2017
@lukego
Copy link

lukego commented Mar 29, 2017

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 snabbco/snabb#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 :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants