[BUG] ffi.metatype(): C struct as __index: read throws 'stack overflow'

[Tomash Brechko]

Hello,

The following code throws 'stack overflow' error:

-- code
ffi = require("ffi")
s = ffi.new("struct { int i; }")
mt = {
__index = s,
__newindex = function(t, k, v)
print(k..": "..s[k].." -> "..v)
s[k] = v
end
}

print("Lua table:")
t = setmetatable({}, mt)
t.i = 21 -- prints "i: 0 -> 21"
print("s.i = "..s.i) -- prints "s.i = 21"
print("t.i = "..t.i) -- prints "t.i = 21"

print("LuaJIT C struct:")
v = ffi.metatype("struct {}", mt)()
v.i = 42 -- prints "i: 21 -> 42"
print("s.i = "..s.i) -- prints "s.i = 42"
print("v.i = "..v.i) -- read throws 'stack overflow'
-- end

Tried with official LuaJIT 2.0.3, git master and git v2.1. I think this is
a LuaJIT bug (sorry if not :)).

What I'm trying to achieve is a LuaJIT C structs with efficient field read
operation and arbitrary callback on field write operation. I believe that
using C struct for __index together with ffi.metatype() requirement that
metatable and its __index must not change are the sufficient preconditions
for read access to be efficiently compiled as if no metatable is there.
Please correct me if I'm wrong, in that case any suggestion how to achieve
the desired are welcome.


Thanks in advance,

[Coda Highland]

On Tue, Aug 5, 2014 at 10:32 AM, Tomash Brechko

Post by Tomash Brechko
Hello,
-- code
ffi = require("ffi")
s = ffi.new("struct { int i; }")
mt = {
__index = s,
__newindex = function(t, k, v)
print(k..": "..s[k].." -> "..v)
s[k] = v
end
}
print("Lua table:")
t = setmetatable({}, mt)
t.i = 21 -- prints "i: 0 -> 21"
print("s.i = "..s.i) -- prints "s.i = 21"
print("t.i = "..t.i) -- prints "t.i = 21"
print("LuaJIT C struct:")
v = ffi.metatype("struct {}", mt)()
v.i = 42 -- prints "i: 21 -> 42"
print("s.i = "..s.i) -- prints "s.i = 42"
print("v.i = "..v.i) -- read throws 'stack overflow'
-- end
Tried with official LuaJIT 2.0.3, git master and git v2.1. I think this is
a LuaJIT bug (sorry if not :)).
What I'm trying to achieve is a LuaJIT C structs with efficient field read
operation and arbitrary callback on field write operation. I believe that
using C struct for __index together with ffi.metatype() requirement that
metatable and its __index must not change are the sufficient preconditions
for read access to be efficiently compiled as if no metatable is there.
Please correct me if I'm wrong, in that case any suggestion how to achieve
the desired are welcome.
Thanks in advance,
--
Tomash Brechko

Quoting http://luajit.org/ext_ffi_api.html's section on ffi.metatype():

"The association with a metatable is permanent and cannot be changed
afterwards. Neither the contents of the metatable nor the contents of
an __index table (if any) may be modified afterwards."

The call to s[k] = v in __newindex is a violation of this rule. I
can't say that "stack overflow" is necessarily the most intuitive
error message, but it's at least a documented thing.

Using "__index = function(t, k) return s[k] end" makes the problem go away.

/s/ Adam

[Tomash Brechko]

Post by Coda Highland
"The association with a metatable is permanent and cannot be changed
afterwards. Neither the contents of the metatable nor the contents of
an __index table (if any) may be modified afterwards."
The call to s[k] = v in __newindex is a violation of this rule. I
can't say that "stack overflow" is necessarily the most intuitive
error message, but it's at least a documented thing.

Hmm, I referenced the same text, but to my understanding the _table_ in
__index should not change, i.e. keys should neither be added nor deleted
(which is not possible with C struct anyway). "s[k] = v" is the same as
"s.i = v" here, and it's not clear how such assignment can break anything.

Post by Coda Highland
Using "__index = function(t, k) return s[k] end" makes the problem go away.

Thanks! I kinda had doubts about using a function here, but can't think of
any reason for it not to be inlined.

[Coda Highland]

On Tue, Aug 5, 2014 at 11:12 AM, Tomash Brechko

Post by Tomash Brechko

Post by Coda Highland
"The association with a metatable is permanent and cannot be changed
afterwards. Neither the contents of the metatable nor the contents of
an __index table (if any) may be modified afterwards."
The call to s[k] = v in __newindex is a violation of this rule. I
can't say that "stack overflow" is necessarily the most intuitive
error message, but it's at least a documented thing.

Hmm, I referenced the same text, but to my understanding the _table_ in
__index should not change, i.e. keys should neither be added nor deleted
(which is not possible with C struct anyway). "s[k] = v" is the same as
"s.i = v" here, and it's not clear how such assignment can break anything.

Nowhere does it say that modifying keys is the only way to modify a
table. Modifying values is still modifying the table. And apparently,
a struct is equivalent to a table in this context.

Strictly speaking it's not documented anywhere that a struct works in
this context, but apparently it does work as a read-only backing store
(perhaps useful for default values?).

Post by Tomash Brechko

Post by Coda Highland
Using "__index = function(t, k) return s[k] end" makes the problem go away.

Thanks! I kinda had doubts about using a function here, but can't think of
any reason for it not to be inlined.

I can think of a few good reasons it couldn't FAIL to be inlined. :P

/s/ Adam

[Tomash Brechko]

Post by Tomash Brechko
Hmm, I referenced the same text, but to my understanding the _table_ in
__index should not change, i.e. keys should neither be added nor deleted
(which is not possible with C struct anyway). "s[k] = v" is the same as
"s.i = v" here, and it's not clear how such assignment can break anything.

And to support that, I get 'stack overflow' even when I remove 'v.i = 42',
i.e. when s[k] = v is never run. Short version:

-- code
ffi = require("ffi")
s = ffi.new("struct { int i; }")
mt = {
__index = s
}

v = ffi.metatype("struct {}", mt)()
print("v.i = "..v.i) -- read throws 'stack overflow'
-- end

So I still think it's a bug (or undocumented misfeature). But I'm fine
with using the function as you suggested, thanks again!

[Mike Pall]

Post by Tomash Brechko
-- code
ffi = require("ffi")
s = ffi.new("struct { int i; }")
mt = {
__index = s,

Thank you for the bug report and the test case! Fixed in the git
repository.

Post by Tomash Brechko
What I'm trying to achieve is a LuaJIT C structs with efficient field read
operation [...]

Well, if you want this construct to be JIT-compiled, then better
use a wrapper __index function.

--Mike