From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 401B76BEC2A; Fri, 27 Oct 2023 15:58:37 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 401B76BEC2A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1698411517; bh=/VbZib+Nynizda5+dbIWkWON64149XPs1BX3dj9shhM=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=a0hHppGkVZxDwypLvebKIXfjp+s7ibFiY7kaVON3EYbWKRIo1ZS4NqYpz2SGwlPV2 kWO1E7zOrPokY00g9LHM/Vzqy/1vEFnJwvLPdF29XuSobHWBcqV0im8nDuAsqvK9re QEK43fvsoRPgUxgIz19XlMkRtjy1BVcI+aw8Xoro= Received: from mail-lf1-f51.google.com (mail-lf1-f51.google.com [209.85.167.51]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 514A36BEC2D for ; Fri, 27 Oct 2023 15:57:47 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 514A36BEC2D Received: by mail-lf1-f51.google.com with SMTP id 2adb3069b0e04-507f1c29f25so2861602e87.1 for ; Fri, 27 Oct 2023 05:57:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698411466; x=1699016266; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=KUOUbF5k4ahYyuVHrIQbURewjxtyp2QZ8NQ1Lta99kI=; b=IvuDnXNuiGdeOe+tivQOBgHpzczEUBMqlBYKqkUk/FJ9WR3w4DsbSoPbUK4Z2lUQ99 gY0j+mrcG0y8yOHEPAkULbGOrJ3zqsjO70Es1nJsx7rRmLjq/k5FSkkCSZGRteLm12Oo RlPGB/Vc0nLDB0r2/EELBGLmzkeWjxYmKE+/G7lQSp0csMtouZGSM/o7YTl4M9FLqdsu cq66SXgoXblko3YQL3UM/h45gwoH3yGo+dnJHeq3UUhHofFBvikjLMx7IO4ZrDrgWTdS gsc3DG+7MJW3bZXELeexb5cAOVHE/T+nJuE0tIox7wfvU2mGyKq6MJXdeKC4QY++COES jp/Q== X-Gm-Message-State: AOJu0Yz0rxbYAQA/eD8zFtDS6zYh4njeQeq/46is2ydjIBhjMd2zXHh9 hKBZm6t6Z/uiJHUQcO+6jg91PekxJuRG4A== X-Google-Smtp-Source: AGHT+IG6O7e3y3qOICOPSvwuSKFEJ+sM+aB3uapMp6LG/ZuEdhUgaoNfha/konTowRGOoaeRBAveqA== X-Received: by 2002:ac2:4571:0:b0:507:a00a:262b with SMTP id k17-20020ac24571000000b00507a00a262bmr1730337lfm.68.1698411466087; Fri, 27 Oct 2023 05:57:46 -0700 (PDT) Received: from localhost.localdomain ([185.205.79.42]) by smtp.gmail.com with ESMTPSA id g3-20020a19e043000000b004ff8cd27a61sm270510lfj.213.2023.10.27.05.57.45 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Fri, 27 Oct 2023 05:57:45 -0700 (PDT) To: tarantool-patches@dev.tarantool.org, sergeyb@tarantool.org, skaplun@tarantool.org, m.kokryashkin@tarantool.org Date: Fri, 27 Oct 2023 15:57:37 +0300 Message-Id: <20231027125738.29527-2-max.kokryashkin@gmail.com> X-Mailer: git-send-email 2.39.3 (Apple Git-145) In-Reply-To: <20231027125738.29527-1-max.kokryashkin@gmail.com> References: <20231027125738.29527-1-max.kokryashkin@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit 1/2] Print errors from __gc finalizers instead of rethrowing them. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Maksim Kokryashkin via Tarantool-patches Reply-To: Maksim Kokryashkin Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Mike Pall Finalizers are not supposed to throw errors -- this is undefined behavior. Lua 5.1 - 5.3 and (previously) LuaJIT rethrow the error. This randomly breaks some unrelated code that just happens to do an allocation. Bad. Lua 5.4 catches the error and emits a warning instead. But warnings are not enabled by default, so it fails silently. Even worse. LuaJIT (now) catches the error and emits a VM event. The default event handler function prints "ERROR in finalizer: ...". Set a custom handler function with: jit.attach(handler, "errfin") (cherry-picked from commit 1c279127050e86e99970100e9c42e0f09cd54ab7) The test for an error in a finalizer from the PUC-Rio suite is disabled since these errors are now handled with VM event. Sergey Kaplun: * added the test for the problem Part of tarantool/tarantool#9145 --- src/Makefile.dep.original | 14 +++---- src/lib_aux.c | 33 ++++++++++++++- src/lj_gc.c | 10 ++++- src/lj_vmevent.h | 7 ++-- test/PUC-Rio-Lua-5.1-tests/gc.lua | 12 +++--- .../lj-946-print-errors-from-gc-fin.test.lua | 42 +++++++++++++++++++ 6 files changed, 98 insertions(+), 20 deletions(-) create mode 100644 test/tarantool-tests/lj-946-print-errors-from-gc-fin.test.lua diff --git a/src/Makefile.dep.original b/src/Makefile.dep.original index d35b6d9a..53426a7e 100644 --- a/src/Makefile.dep.original +++ b/src/Makefile.dep.original @@ -1,6 +1,6 @@ lib_aux.o: lib_aux.c lua.h luaconf.h lauxlib.h lj_obj.h lj_def.h \ lj_arch.h lj_err.h lj_errmsg.h lj_state.h lj_trace.h lj_jit.h lj_ir.h \ - lj_dispatch.h lj_bc.h lj_traceerr.h lj_lib.h lj_alloc.h + lj_dispatch.h lj_bc.h lj_traceerr.h lj_lib.h lj_alloc.h lj_vmevent.h lib_base.o: lib_base.c lua.h luaconf.h lauxlib.h lualib.h lj_obj.h \ lj_def.h lj_arch.h lj_gc.h lj_err.h lj_errmsg.h lj_debug.h lj_str.h \ lj_tab.h lj_meta.h lj_state.h lj_frame.h lj_bc.h lj_ctype.h lj_cconv.h \ @@ -123,7 +123,7 @@ lj_func.o: lj_func.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h lj_gc.h \ lj_gc.o: lj_gc.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h lj_gc.h \ lj_err.h lj_errmsg.h lj_buf.h lj_str.h lj_tab.h lj_func.h lj_udata.h \ lj_meta.h lj_state.h lj_frame.h lj_bc.h lj_ctype.h lj_cdata.h lj_trace.h \ - lj_jit.h lj_ir.h lj_dispatch.h lj_traceerr.h lj_vm.h + lj_jit.h lj_ir.h lj_dispatch.h lj_traceerr.h lj_vm.h lj_vmevent.h lj_gdbjit.o: lj_gdbjit.c lj_obj.h lua.h luaconf.h lj_def.h lj_arch.h \ lj_gc.h lj_err.h lj_errmsg.h lj_debug.h lj_frame.h lj_bc.h lj_buf.h \ lj_str.h lj_strfmt.h lj_jit.h lj_ir.h lj_dispatch.h @@ -238,11 +238,11 @@ ljamalg.o: ljamalg.c lua.h luaconf.h lauxlib.h lj_assert.c lj_obj.h lj_def.h \ lj_arch.h lj_gc.c lj_gc.h lj_err.h lj_errmsg.h lj_buf.h lj_str.h lj_tab.h \ lj_func.h lj_udata.h lj_meta.h lj_state.h lj_frame.h lj_bc.h lj_ctype.h \ lj_cdata.h lj_trace.h lj_jit.h lj_ir.h lj_dispatch.h lj_traceerr.h \ - lj_vm.h lj_err.c lj_debug.h lj_ff.h lj_ffdef.h lj_strfmt.h lj_char.c \ - lj_char.h lj_bc.c lj_bcdef.h lj_obj.c lj_buf.c lj_wbuf.c lj_wbuf.h lj_utils.h \ - lj_str.c lj_tab.c lj_func.c lj_udata.c lj_meta.c lj_strscan.h lj_lib.h \ - lj_debug.c lj_state.c lj_lex.h lj_alloc.h luajit.h lj_dispatch.c \ - lj_ccallback.h lj_profile.h lj_memprof.h lj_vmevent.c lj_vmevent.h \ + lj_vm.h lj_vmevent.h lj_err.c lj_debug.h lj_ff.h lj_ffdef.h lj_strfmt.h \ + lj_char.c lj_char.h lj_bc.c lj_bcdef.h lj_obj.c lj_buf.c lj_wbuf.c lj_wbuf.h \ + lj_utils.h lj_str.c lj_tab.c lj_func.c lj_udata.c lj_meta.c lj_strscan.h \ + lj_lib.h lj_debug.c lj_state.c lj_lex.h lj_alloc.h luajit.h lj_dispatch.c \ + lj_ccallback.h lj_profile.h lj_memprof.h lj_vmevent.c \ lj_vmmath.c lj_strscan.c lj_strfmt.c lj_strfmt_num.c lj_api.c lj_mapi.c \ lmisclib.h lj_profile.c lj_profile_timer.h lj_profile_timer.c \ lj_memprof.c lj_lex.c lualib.h lj_parse.h lj_parse.c lj_bcread.c \ diff --git a/src/lib_aux.c b/src/lib_aux.c index c40565c3..d532a12f 100644 --- a/src/lib_aux.c +++ b/src/lib_aux.c @@ -21,6 +21,7 @@ #include "lj_state.h" #include "lj_trace.h" #include "lj_lib.h" +#include "lj_vmevent.h" #if LJ_TARGET_POSIX #include @@ -311,6 +312,18 @@ static int panic(lua_State *L) return 0; } +#ifndef LUAJIT_DISABLE_VMEVENT +static int error_finalizer(lua_State *L) +{ + const char *s = lua_tostring(L, -1); + fputs("ERROR in finalizer: ", stderr); + fputs(s ? s : "?", stderr); + fputc('\n', stderr); + fflush(stderr); + return 0; +} +#endif + #ifdef LUAJIT_USE_SYSMALLOC #if LJ_64 && !LJ_GC64 && !defined(LUAJIT_USE_VALGRIND) @@ -332,7 +345,15 @@ static void *mem_alloc(void *ud, void *ptr, size_t osize, size_t nsize) LUALIB_API lua_State *luaL_newstate(void) { lua_State *L = lua_newstate(mem_alloc, NULL); - if (L) G(L)->panic = panic; + if (L) { + G(L)->panic = panic; +#ifndef LUAJIT_DISABLE_VMEVENT + luaL_findtable(L, LUA_REGISTRYINDEX, LJ_VMEVENTS_REGKEY, LJ_VMEVENTS_HSIZE); + lua_pushcfunction(L, error_finalizer); + lua_rawseti(L, -2, VMEVENT_HASH(LJ_VMEVENT_ERRFIN)); + G(L)->vmevmask = VMEVENT_MASK(LJ_VMEVENT_ERRFIN); +#endif + } return L; } @@ -350,7 +371,15 @@ LUALIB_API lua_State *luaL_newstate(void) #else L = lua_newstate(lj_alloc_f, ud); #endif - if (L) G(L)->panic = panic; + if (L) { + G(L)->panic = panic; +#ifndef LUAJIT_DISABLE_VMEVENT + luaL_findtable(L, LUA_REGISTRYINDEX, LJ_VMEVENTS_REGKEY, LJ_VMEVENTS_HSIZE); + lua_pushcfunction(L, error_finalizer); + lua_rawseti(L, -2, VMEVENT_HASH(LJ_VMEVENT_ERRFIN)); + G(L)->vmevmask = VMEVENT_MASK(LJ_VMEVENT_ERRFIN); +#endif + } return L; } diff --git a/src/lj_gc.c b/src/lj_gc.c index 19d4c963..591862b3 100644 --- a/src/lj_gc.c +++ b/src/lj_gc.c @@ -27,6 +27,7 @@ #include "lj_trace.h" #include "lj_dispatch.h" #include "lj_vm.h" +#include "lj_vmevent.h" #define GCSTEPSIZE 1024u #define GCSWEEPMAX 40 @@ -515,8 +516,13 @@ static void gc_call_finalizer(global_State *g, lua_State *L, hook_restore(g, oldh); if (LJ_HASPROFILE && (oldh & HOOK_PROFILE)) lj_dispatch_update(g); g->gc.threshold = oldt; /* Restore GC threshold. */ - if (errcode) - lj_err_throw(L, errcode); /* Propagate errors. */ + if (errcode) { + ptrdiff_t errobj = savestack(L, L->top-1); /* Stack may be resized. */ + lj_vmevent_send(L, ERRFIN, + copyTV(L, L->top++, restorestack(L, errobj)); + ); + L->top--; + } } /* Finalize one userdata or cdata object from the mmudata list. */ diff --git a/src/lj_vmevent.h b/src/lj_vmevent.h index 050fb4dd..56c0d798 100644 --- a/src/lj_vmevent.h +++ b/src/lj_vmevent.h @@ -24,9 +24,10 @@ /* VM event IDs. */ typedef enum { VMEVENT_DEF(BC, 0x00003883), - VMEVENT_DEF(TRACE, 0xb2d91467), - VMEVENT_DEF(RECORD, 0x9284bf4f), - VMEVENT_DEF(TEXIT, 0xb29df2b0), + VMEVENT_DEF(TRACE, 0x12d91467), + VMEVENT_DEF(RECORD, 0x1284bf4f), + VMEVENT_DEF(TEXIT, 0x129df2b0), + VMEVENT_DEF(ERRFIN, 0x12d93888), LJ_VMEVENT__MAX } VMEvent; diff --git a/test/PUC-Rio-Lua-5.1-tests/gc.lua b/test/PUC-Rio-Lua-5.1-tests/gc.lua index 10e1f2dd..05ac0d11 100644 --- a/test/PUC-Rio-Lua-5.1-tests/gc.lua +++ b/test/PUC-Rio-Lua-5.1-tests/gc.lua @@ -260,13 +260,13 @@ u, m = nil collectgarbage() assert(m==10) - +-- The test below is disabled for LuaJIT, since it handles errors from +-- GC finalizers via VM event. -- errors during collection -u = newproxy(true) -getmetatable(u).__gc = function () error "!!!" end -u = nil -assert(not pcall(collectgarbage)) - +-- u = newproxy(true) +-- getmetatable(u).__gc = function () error "!!!" end +-- u = nil +-- assert(not pcall(collectgarbage)) if not rawget(_G, "_soft") then print("deep structures") diff --git a/test/tarantool-tests/lj-946-print-errors-from-gc-fin.test.lua b/test/tarantool-tests/lj-946-print-errors-from-gc-fin.test.lua new file mode 100644 index 00000000..cce417d2 --- /dev/null +++ b/test/tarantool-tests/lj-946-print-errors-from-gc-fin.test.lua @@ -0,0 +1,42 @@ +local tap = require('tap') +local test = tap.test('lj-946-print-errors-from-gc-fin'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(2) + +local ffi = require('ffi') +local error_in_finalizer = false + +local function errfin_handler() + error_in_finalizer = true +end + +local function new_bad_cdata() + return ffi.gc(ffi.new('char [?]', 1024), 'uncallable string') +end + +local function test_f() + collectgarbage('collect') + -- Make GC aggressive enough to end atomic phase before + -- exit from the trace. + collectgarbage('setstepmul', 400) + -- Number of iterations is empirical, just big enough for the + -- issue to strike. + for _ = 1, 4000 do + new_bad_cdata() + end +end + +jit.opt.start('hotloop=1') +-- Handler is registered, but never called before the patch, +-- but should be called after the patch. +jit.attach(errfin_handler, 'errfin') +local status = pcall(test_f) +-- We have to stop GC now because any step raises the error due to +-- cursed cdata objects. +collectgarbage('stop') +test:ok(status, 'test function completed successfully') +test:ok(error_in_finalizer, 'error handler called') + +test:done(true) -- 2.39.3 (Apple Git-145)