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 2F0176E445; Fri, 6 Nov 2026 14:00:09 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2F0176E445 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1793962809; bh=Py1osXKCX2IoR5md7WSgqR5coSOYkkKLh8EGvA2tb18=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=N6RXnfMRbf6lMT15S9SEo663NK8I3cOx/O3KYn8pYHHIPeTUhSOztbsjM+glE6a3L ttUViT9/9RPwY6NitAgfsDQVvMJM1inWTDvodOT76Y7FTv/nsBgtUBQ42PK5whcQIf lv5paPDVVt5cCbr/P43M8txI71YHntR6gzfr1E2Q= Received: from send241.i.mail.ru (send241.i.mail.ru [95.163.59.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 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 D20046E445 for ; Fri, 6 Nov 2026 14:00:07 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D20046E445 Received: by exim-smtp-canary-10-75cf684ff8-dv4x7 with esmtpa (envelope-from ) id 1xPHfy-000000008BQ-0Q7O; Fri, 06 Nov 2026 14:00:07 +0300 Date: Fri, 6 Nov 2026 13:59:38 +0300 To: Sergey Bronnikov Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9615418BB48D65B56359AB3DCF0B1FCD72B31C70A17D1DA6B182A05F5380850404C228DA9ACA6FE27CD01E172D30D8A163DE06ABAFEAF6705B3054169761C1EDAEBE7F5DC363E3FCF28006A18B89A7667 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7544B1CCE26E01C74EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637AC83A81C8FD4AD23D82A6BABE6F325AC2E85FA5F3EDFCBAA7353EFBB5533756692EBD3885B9961068FBDFF872D1A2C8A29D768F5472D1067110BC756445ECDDC389733CBF5DBD5E9D5E8D9A59859A8B6F459A8243F1D1D44CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB86D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE77DCDFB3399A2F72843847C11F186F3C59DAA53EE0834AAEE X-87b9d050: 1 X-C1DE0DAB: 0D63561A33F958A5BF7703493AB339E95002B1117B3ED6962789F1560C365387E20DC3F561CE41503610D81D389A125CDE35189EBF2DEA28FEA14CD2CD220BB99C5DF10A05D560A9880EC71AF561E0AAD9143641EC25BB390CBC66767E1C2D61 X-C8649E89: 1C3962B70DF3F0AD73CAD6646DEDE1918E10F71CB4DF9F96AB70F9BE574AE9C625B6776AC983F447FC0B9F89525902EE6F57B2FD27647F25E66C117BDB76D6592615AB5A79B17443358E6631FB8CFBBA78DAF280AD582721D0C1604EC09530CF2679E17996C6F44CB8341EE9D5BE9A0AFC311137C9406CF53803ABC8B3F03B81BFB63726664BEFFFC7CEAA0681F5848F4C41F94D744909CECFA6C6B0C050A61A8CAF69B82BA93681CD72808BE417F3B9E0E7457915DAA85F X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu53w8ahmwBjZKM/YPHZyZHvz5uv+WouB9+ObcCpyrx6l7KImUglyhkEat/+ysWwi0gdhEs0JGjl6ggRWTy1haxBpVdbIX1nthFXOcIETfglQORZ0zpDET4Zrk3igikrdHlWDxsUd01w1U6nNR0l9D9uPM= X-Mailru-Sender: 689FA8AB762F73937C9FA53A4753B313A938D1D1570E4D6B114586375C801FAF9B0552CD37225AC2E49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 2/3] sysprof: fix crash on a failed stream flush 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: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Sergey, Thanks for the fixing this issue! Generally LGTM, with a minor nits regarding the tests below. On 02.11.26, Sergey Bronnikov wrote: > From: Sergey Bronnikov > > A writer may release the stream buffer and set its pointer to NULL > on an unrecoverable error, e.g., when the file system with the > profile is full. The write buffer stops the stream in this case, > but a caller of `wbuf_reserve()` still writes data using the > released buffer. This leads to a null pointer dereference inside > `lj_utils_write_uleb128()` when the flush is triggered while > streaming a sample. > > Now `wbuf_reserve()` reports whether the stream was stopped during > the flush. All the write functions skip writing in this case, > including the loop pumping a large payload through the buffer. > > Fixes tarantool/tarantool#11620 Typo: s/Fixes/Resolves/ It will be fixed after submodule bump in the Tarantool. > --- > src/lj_wbuf.c | 20 +- > .../gh-11620-sysprof-enospc-crash.test.c | 180 ++++++++++++++++++ > test/tarantool-c-tests/unit-wbuf.test.c | 102 ++++++++++ > 3 files changed, 298 insertions(+), 4 deletions(-) > create mode 100644 test/tarantool-c-tests/gh-11620-sysprof-enospc-crash.test.c > > diff --git a/src/lj_wbuf.c b/src/lj_wbuf.c > index 0001a02e..0794b186 100644 > --- a/src/lj_wbuf.c > +++ b/src/lj_wbuf.c > diff --git a/test/tarantool-c-tests/gh-11620-sysprof-enospc-crash.test.c b/test/tarantool-c-tests/gh-11620-sysprof-enospc-crash.test.c > new file mode 100644 > index 00000000..0f5ffe53 > --- /dev/null > +++ b/test/tarantool-c-tests/gh-11620-sysprof-enospc-crash.test.c > @@ -0,0 +1,180 @@ > +/* > +** Testing of the sysprof behaviour when a stream write fails. > +** > +** A full file system makes the sysprof writer return an error and > +** release the stream buffer. The profiler must stop the stream > +** gracefully, return an error from luaM_sysprof_stop() and > +** release the resources. > +*/ Minor: It's better to use | /* | * | */ Style of comments in the tests to be consistent with other codebase. The comment style in the is inconsistent by my mistake. > + > +#include "lauxlib.h" > +#include "lua.h" > + > +/* Need for skipcond for OS and ARCH. */ > +#include "lj_arch.h" > + > +#include "test.h" > + > +#if LJ_HASSYSPROF && !defined(LUAJIT_USE_VALGRIND) > + > +#include "lmisclib.h" > +#include "utils.h" > + > +#include > +#include > +#include > +#include > +#include > + > +/* > + * The buffer size is enough for the maximal ULEB128 value. > + * A small buffer makes the profiler flush it right during the > + * prologue or the first sample. > + */ > +#define STREAM_BUFFER_SIZE 10 > + > +/* Do not let the timer interfere with the manually emitted signals. */ Nit: Comment line length is more than 66 symbols. > +static int sysprof_enospc_during_sampling(void *test_state) > +{ > + lua_State *L = test_state; > + static uint8_t buf[STREAM_BUFFER_SIZE]; > + struct luam_Sysprof_Options opt; > + struct luam_Sysprof_Counters counters = {}; > + > + sysprof_test_configure(&opt, buf); > + assert_true(luaM_sysprof_start(L, &opt) == PROFILE_SUCCESS); > + assert_int_equal(ctx.on_stop_calls, 0); > + > + /* Let the next stream flush fail as if the disk is full. */ Nit: Comment line length is more than 66 symbols. > + ctx.fail = 1; > + assert_int_equal(kill(getpid(), SIGPROF), 0); > + > + /* The stream is stopped gracefully, without a crash. */ > + errno = 0; > + assert_int_equal(luaM_sysprof_stop(L), PROFILE_ERRIO); > + assert_int_equal(errno, ENOSPC); > + > + /* Counters are still available. */ > + assert_true(luaM_sysprof_report(&counters) == PROFILE_SUCCESS); > + assert_true(counters.samples >= 1); > + > + /* The profiler is stopped. */ > + assert_int_equal(luaM_sysprof_stop(L), PROFILE_ERRRUN); > + > + return TEST_EXIT_SUCCESS; > +} > + > +static int sysprof_enospc_on_start(void *test_state) > +{ > + lua_State *L = test_state; > + static uint8_t buf[STREAM_BUFFER_SIZE]; > + struct luam_Sysprof_Options opt; > + > + sysprof_test_configure(&opt, buf); > + /* The very first flush fails, so the prologue cannot be streamed. */ Nit: Comment line length is more than 66 symbols. > + ctx.fail = 1; > + errno = 0; > + assert_int_equal(luaM_sysprof_start(L, &opt), PROFILE_ERRIO); > + assert_int_equal(errno, ENOSPC); > + /* The start error path releases the profiler resources. */ Nit: Comment line length is more than 66 symbols. > + assert_int_equal(ctx.on_stop_calls, 1); > + > + /* The profiler is not running. */ > + assert_int_equal(luaM_sysprof_stop(L), PROFILE_ERRRUN); > + > + return TEST_EXIT_SUCCESS; > +} > + > +#endif /* LJ_HASSYSPROF && !defined(LUAJIT_USE_VALGRIND) */ > + > +int main(void) > +{ > +#if LUAJIT_USE_VALGRIND > + return skip_all("Disabled due to #10803"); > +#elif !LJ_HASSYSPROF > + return skip_all("Sysprof is disabled"); > +#else > + if (LUAJIT_OS != LUAJIT_OS_LINUX) > + return skip_all("Sysprof is implemented for Linux only"); > + if (LUAJIT_TARGET != LUAJIT_ARCH_X86 > + && LUAJIT_TARGET != LUAJIT_ARCH_X64) > + return skip_all("Sysprof is implemented for x86_64 only"); > + > + lua_State *L = utils_lua_init(); > + > + const struct test_unit tgroup[] = { > + test_unit_def(sysprof_enospc_during_sampling), > + test_unit_def(sysprof_enospc_on_start) > + }; > + const int test_result = test_run_group(tgroup, L); > + utils_lua_close(L); > + return test_result; > +#endif /* LUAJIT_USE_VALGRIND */ > +} > diff --git a/test/tarantool-c-tests/unit-wbuf.test.c b/test/tarantool-c-tests/unit-wbuf.test.c > index 4bee6243..c1b71ff3 100644 > --- a/test/tarantool-c-tests/unit-wbuf.test.c > +++ b/test/tarantool-c-tests/unit-wbuf.test.c > @@ -367,6 +367,104 @@ static int test_terminate(void *state) > ** this case, so the write functions must not touch the released > ** buffer. > */ > +static int test_addbyte_failed_flush(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + size_t i; > + > + UNUSED(state); > + > + writer_ctx_init(&ctx, WMODE_FAIL); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + > + /* Fill the buffer without flushes. */ > + for (i = 0; i < sizeof(mem); ++i) > + lj_wbuf_addbyte(&buf, (uint8_t)i); > + assert_sizet_equal(ctx.flushes, 0); > + assert_sizet_equal(buf_len(&buf), sizeof(mem)); > + > + /* The buffer is full, so the byte below triggers a failed flush. */ Nit: Comment line length is more than 66 symbols. > + lj_wbuf_addbyte(&buf, 0xaa); > + assert_stream_stopped_with_error(&buf); > + assert_sizet_equal(ctx.flushes, 1); > + > + return TEST_EXIT_SUCCESS; > +} > + > +static int test_addu64_failed_flush(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + > + UNUSED(state); > + > + writer_ctx_init(&ctx, WMODE_FAIL); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + > + /* The maximal value takes exactly BUFFER_SIZE bytes. */ > + lj_wbuf_addu64(&buf, UINT64_MAX); > + assert_sizet_equal(buf_len(&buf), sizeof(mem)); > + > + /* There is no space left, so the next write triggers a failed flush. */ Nit: Comment line length is more than 66 symbols. > + lj_wbuf_addu64(&buf, 0); > + assert_stream_stopped_with_error(&buf); > + assert_sizet_equal(ctx.flushes, 1); > + > + return TEST_EXIT_SUCCESS; > +} > + > +static int test_addn_failed_flush(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + uint8_t payload[BUFFER_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + > + UNUSED(state); > + > + memset(payload, 'a', sizeof(payload)); > + writer_ctx_init(&ctx, WMODE_FAIL); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + > + /* Fill the buffer without flushes. */ > + lj_wbuf_addn(&buf, payload, sizeof(payload)); > + assert_sizet_equal(ctx.flushes, 0); > + > + /* There is no space left, so the next write triggers a failed flush. */ Nit: Comment line length is more than 66 symbols. > + lj_wbuf_addn(&buf, "b", 1); > + assert_stream_stopped_with_error(&buf); > + assert_sizet_equal(ctx.flushes, 1); > + > + return TEST_EXIT_SUCCESS; > +} > + > +static int test_addn_failed_flush_in_loop(void *state) > +{ > + uint8_t mem[BUFFER_SIZE]; > + uint8_t payload[PAYLOAD_SIZE]; > + struct lj_wbuf buf; > + struct writer_ctx ctx; > + > + UNUSED(state); > + > + memset(payload, 'a', sizeof(payload)); > + writer_ctx_init(&ctx, WMODE_FAIL); > + lj_wbuf_init(&buf, test_writer, &ctx, mem, sizeof(mem)); > + > + /* > + * The payload does not fit into the buffer, so it is pumped through > + * it. The flush fails in the middle of the pump loop. > + */ Nit: Comment line length is more than 66 symbols. > + lj_wbuf_addn(&buf, payload, sizeof(payload)); > + assert_stream_stopped_with_error(&buf); > + assert_sizet_equal(ctx.flushes, 1); > + > + return TEST_EXIT_SUCCESS; > +} > + > int main(void) > { > const struct test_unit tgroup[] = { > @@ -381,6 +479,10 @@ int main(void) > test_unit_def(test_flush_stopped_stream), > test_unit_def(test_write_to_stopped_stream), > test_unit_def(test_terminate), > + test_unit_def(test_addbyte_failed_flush), > + test_unit_def(test_addu64_failed_flush), > + test_unit_def(test_addn_failed_flush), I suppose this 3 tests may be placed in one test_group, since they are: 1) Init buffer 2) Fill it until maximum capacity. 3) Write a value with 1 flush and ENOSPC error. Hence, I suppose initialization of the test may be unified. Feel free to ignore, though. > + test_unit_def(test_addn_failed_flush_in_loop), > }; > const int test_result = test_run_group(tgroup, NULL); > > -- > 2.56.0 > -- Best regards, Sergey Kaplun