Assertion failed: (0), function carith_int64, file lj_carith.c, line 211.

[olegrok]

Tarantool version:

Tarantool 2.3.1-2-g9887ca8fe
Target: Darwin-x86_64-Debug
Build options: cmake . -DCMAKE_INSTALL_PREFIX=/usr/local -DENABLE_BACKTRACE=OFF
Compiler: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/cc /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/c++
C_FLAGS: -Wno-unknown-pragmas -fexceptions -funwind-tables -fno-common -msse2 -std=c11 -Wall -Wextra -Wno-strict-aliasing -Wno-unused-value -Wno-char-subscripts -Werror
CXX_FLAGS: -Wno-unknown-pragmas -fexceptions -funwind-tables -fno-common -msse2 -std=c++11 -Wall -Wextra -Wno-strict-aliasing -Wno-unused-value -Wno-char-subscripts -Werror

OS version: MacOS Mojave 10.14.6

Bug description:

Steps to reproduce:

Apply following patch:

diff --git a/src/lua/error.lua b/src/lua/error.lua
index d091ee48b..70d040dbd 100644
--- a/src/lua/error.lua
+++ b/src/lua/error.lua
@@ -160,9 +160,25 @@ local function error_index(err, key)
     return error_methods[key]
 end
 
+local function error_concat(lhs, rhs)
+    local lhs_is_error = ffi.istype('struct error', lhs)
+    local rhs_is_error = ffi.istype('struct error', rhs)
+
+    if lhs_is_error and rhs_is_error then
+        return tostring(lhs) .. tostring(rhs)
+    elseif lhs_is_error then
+        return tostring(lhs) .. rhs
+    elseif rhs_is_error then
+        return lhs .. tostring(rhs)
+    else
+       error('error_mt.__concat(): neither of args is an error')
+    end
+end
+
 local error_mt = {
     __index = error_index;
     __tostring = error_message;
+    __concat = error_concat;
 };
 
 ffi.metatype('struct error', error_mt);
diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index c3e992a48..f7c965249 100644
--- a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -59,6 +59,29 @@ e = box.error.new(box.error.CREATE_SPACE, "space", "error")
 e
 box.error.new()
 
+--
+-- gh-4489: box.error has __concat metamethod
+--
+test_run:cmd("push filter '(.builtin/.*.lua):[0-9]+' to '\\1'")
+e = box.error.new(box.error.UNKNOWN)
+'left side: ' .. e
+e .. ': right side'
+e .. nil
+nil .. e
+e .. box.NULL
+box.NULL .. e
+123 .. e
+e .. 123
+e .. e
+e .. {}
+{} .. e
+-1ULL .. e
+e .. -1ULL
+1LL .. e
+e .. 1LL
+debug.getmetatable(e).__concat(1, 1)
+e = nil
+
 --
 -- System errors expose errno as a field.
 --
@@ -281,7 +304,7 @@ dostring('return abc')
 dostring('return ...', 1, 2, 3)
 --  A test case for Bug#1043804 lua error() -> server crash
 error()
---  A test case for bitwise operations 
+--  A test case for bitwise operations
 bit.lshift(1, 32)
 bit.band(1, 3)
 bit.bor(1, 2)

There's nothing criminal about this patch

Then build cmake . && make

Then run following script:

tarantool> e = box.error.new(box.error.UNKNOWN)
---
...

tarantool> debug.getmetatable(e).__concat(1,1)
Assertion failed: (0), function carith_int64, file lj_carith.c, line 211.
[1]    89440 abort      ./src/tarantool

For strings:

tarantool> debug.getmetatable(e).__concat('1', '1')
Assertion failed: ((((uint32_t)((o2)->it64 >> 47)) == (~10u))), function carith_checkarg, file lj_carith.c, line 65.
[1]    89939 abort      ./src/tarantool

Expected error "error_mt.__concat(): neither of args is an error"

[igormunkin]

There's nothing criminal about this patch

Well, the hunk for src/lua/error.lua is fine (I believe so, I didn't dive into its semantics), but the hacks with debug.* in tests are not. Looks like no one is interested why the plain getmetatable routine yields 'ffi' but no the metatable itself. OK, let's see what we have.

According to this section in Lua Reference Manual there are only two base types with individual metatables: tables and userdata. Here is the exact part clearing this fact:

Tables and full userdata have individual metatables (although multiple tables and userdata can share their metatables). Values of all other types share one single metatable per type; that is, there is one single metatable for all numbers, one for all strings, etc.

Furthermore, if you obtain the metatable for ctype<const struct error &> and ctype<struct tt_uuid> it occurs both ctypes shares the one. Consider the following.

$ ./src/tarantool
Tarantool 2.7.0-42-g374917337
type 'help' for interactive help
tarantool> require('ffi').typeof(box.error.new(box.error.UNKNOWN))
---
- ctype<const struct error &>
...

tarantool> require('ffi').typeof(require('uuid')())
---
- ctype<struct tt_uuid>
...

tarantool> debug.getmetatable(box.error.new(box.error.UNKNOWN)) == debug.getmetatable(require('uuid')())
---
- true
...

tarantool>

Oops... At this place FFI conforms Lua Reference Manual, so, unfortunately, it simply doesn't work the way you want.

Moreover, I guess there is no legal (i.e. using only Lua with no LuaJIT internals) way to check that the function you obtained via debug.getmetatable(box.error.new(box.error.UNKNOWN)).__concat is lj_cf_ffi_meta___concat fast function (although, you can check yourself via luajit-gdb.py), so just trust me (/me making Jedi mind tricks here).

Anyway, how does this magic work? We need to distinguish metatables and metatypes -- the latter one is specific only for GCcdata. The metatype method is chosen underneath lj_cf_ffi_meta___concat considering the ctype passed via the one of the arguments. However, none of the arguments you passed is not GCcdata type, so your metatype method is not used. Ergo I guess the last branch in the error_concat with the error is kinda dead code.

As for the failures you faced, the official position is that debug library is unsafe, so all crashes one has by "prisedanie" with debug.* can be freely classified as "sam sebe zlobniy buratino" or PEBKAC. At the same time I personally believe, there should be no crashes while using pure Lua (though strictly saying FFI is not a pure Lua). I checked the upstream for the fixes and find the one for your first problem.

Then run following script:

tarantool> e = box.error.new(box.error.UNKNOWN)
---
...

tarantool> debug.getmetatable(e).__concat(1,1)
Assertion failed: (0), function carith_int64, file lj_carith.c, line 211.
[1]    89440 abort      ./src/tarantool

This is fixed in scope of this patch. The second crash still presents on the LuaJIT HEAD, so I'll take a look what we can do with it a bit later.

[olegrok]

@igormunkin

Thanks for explanation. Feel free to close this issue and consider it as a part of #5483. If we need only to cherry-pick LuaJIT/LuaJIT@cc4bbec4

[igormunkin]

@olegrok, great, then I close this one. Anyway, this is a good example for enriching our LuaJIT knowledge base, so I hope we'll move our discussion to the docs.