Tarantool development patches archive
 help / color / mirror / Atom feed
From: Evgeniy Temirgaleev via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: "Sergey Kaplun" <skaplun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches]  [PATCH luajit] test: add tests for the leb128 encode/decode
Date: Tue, 28 Jul 2026 22:52:08 +0300	[thread overview]
Message-ID: <1785268328.931937759@5.f.mail-release.rc.mrucl.ru> (raw)
In-Reply-To: <20260722140036.3606677-1-skaplun@tarantool.org>

[-- Attachment #1: Type: text/plain, Size: 13956 bytes --]

Hi, Sergey! Thanks for the patch!

LGTM

--
Best regards,
Evgeniy Temirgaleev

> 
> From: Sergey Kaplun <skaplun@tarantool.org>
> To: Sergey Bronnikov <sergeyb@tarantool.org>, Evgeniy Temirgaleev <e.temirgaleev@tarantool.org
> >
> Cc: tarantool-patches@dev.tarantool.org, Sergey Kaplun <skaplun@tarantool.org
> >
> Date: Wednesday, July 22, 2026 5:00 PM +03:00
> This patch is a follow-up to the commit
> 7058e9215f8a50d00701302ecbb510503d21148d ("utils: introduce leb128
> reader and writer").
> 
> The leb128 encoding is used inside our profilers (memprof/sysprof). But
> there are no tests for this module, so its coverage is low, since only
> `lj_utils_write_uleb128()` is used. Also, there is no correctness check
> for this implementation.
> 
> This patch adapts the LuaVela tests [1] to cover this module. Since the
> initial version of the module wasn't adapted as is, the guarded version
> of the reader functions has different behaviour for zero-sized requested
> reads. Hence, this case is removed for now. The readers with a requested
> size are unused in our code, so this is not crucial at the moment.
> 
> Also, this patch fixes the UBSan warning in the
> `lj_utils_write_leb128()` on checking the remaining value to be written
> by fixing the cast to uint64_t.
> 
> [1]: https://github.com/luavela/luavela/blob/288e8a95218ed47dad93016259533406e30b109b/tests/impl/uJIT-tests-C/suite/test_leb128.c
> 
> ---
> 
> The main idea of the patch is to increase coverage for the
> lj_utils_leb128.c. The UBSan "bug" (not really, since this part is
> unused, anyway) is fixed as a side effect. You may compare the coverage
> between master [1] and this branch [2] with coveralls.
> 
> Branch: https://github.com/tarantool/luajit/tree/skaplun/gh-noticket-leb128-testsing
> 
> 
> [1]: https://coveralls.io/builds/80394140/source?filename=src%2Flj_utils_leb128.c
> 
> [2]: https://coveralls.io/builds/80728053/source?filename=src%2Flj_utils_leb128.c
> 
> 
> src/lj_utils_leb128.c | 2 +-
> test/tarantool-c-tests/unit-leb128.test.c | 366 ++++++++++++++++++++++
> 2 files changed, 367 insertions(+), 1 deletion(-)
> create mode 100644 test/tarantool-c-tests/unit-leb128.test.c
> 
> diff --git a/src/lj_utils_leb128.c b/src/lj_utils_leb128.c
> index d66961da..5ad2bd3c 100644
> --- a/src/lj_utils_leb128.c
> +++ b/src/lj_utils_leb128.c
> @@ -105,7 +105,7 @@ size_t LJ_FASTCALL lj_utils_write_leb128(uint8_t
> *buffer, int64_t value)
> size_t i = 0;
> 
> /* LEB_SIGN_BIT propagation to check the remaining value. */
> - while ((uint64_t)(value + LEB_SIGN_BIT) >= MIN_TWOBYTE_VALUE) {
> + while ((uint64_t)value + LEB_SIGN_BIT >= MIN_TWOBYTE_VALUE) {
> buffer[i++] = (uint8_t)((value & PAYLOAD_MASK) | LINK_BIT);
> value >>= SHIFT_STEP;
> }
> diff --git a/test/tarantool-c-tests/unit-leb128.test.c
> b/test/tarantool-c-tests/unit-leb128.test.c
> new file mode 100644
> index 00000000..142476fb
> --- /dev/null
> +++ b/test/tarantool-c-tests/unit-leb128.test.c
> @@ -0,0 +1,366 @@
> +/*
> +** Testing of LEB128/ULEB128 encoding.
> +**
> +** Major portions taken verbatim or adapted from the LuaVela.
> +** Copyright (C) 2020-2026 LuaVela Authors.
> +** Copyright (C) 2015-2020 IPONWEB Ltd.
> +*/
> +
> +#include <stdint.h>
> +#include <string.h>
> +
> +#include "test.h"
> +
> +#include "lj_utils.h"
> +
> +#define BUFFER_SIZE 16
> +
> +static int test_write_uleb128(void *state)
> +{
> + size_t bytes_written = 0;
> + uint8_t buffer[BUFFER_SIZE] = {0};
> +
> + UNUSED(state);
> +
> + bytes_written = lj_utils_write_uleb128(buffer, 0);
> + assert_true(bytes_written == 1);
> + assert_true(buffer[0] == 0x0);
> +
> + bytes_written = lj_utils_write_uleb128(buffer, 64);
> + assert_true(bytes_written == 1);
> + assert_true(buffer[0] == 0x40);
> +
> + bytes_written = lj_utils_write_uleb128(buffer, 128);
> + assert_true(bytes_written == 2);
> + assert_true(buffer[0] == 0x80 && buffer[1] == 0x01);
> +
> + bytes_written = lj_utils_write_uleb128(buffer, UINT64_MAX);
> + assert_true(bytes_written == 10);
> + assert_true(buffer[0] == 0xff && buffer[1] == 0xff &&
> + buffer[2] == 0xff && buffer[3] == 0xff &&
> + buffer[4] == 0xff && buffer[5] == 0xff &&
> + buffer[6] == 0xff && buffer[7] == 0xff &&
> + buffer[8] == 0xff && buffer[9] == 0x01);
> +
> + return TEST_EXIT_SUCCESS;
> +}
> +
> +static int test_write_leb128(void *state)
> +{
> + size_t bytes_written = 0;
> + uint8_t buffer[BUFFER_SIZE] = {0};
> +
> + UNUSED(state);
> +
> + bytes_written = lj_utils_write_leb128(buffer, 0);
> + assert_true(bytes_written == 1);
> + assert_true(buffer[0] == 0x0);
> +
> + bytes_written = lj_utils_write_leb128(buffer, -624485);
> + assert_true(bytes_written == 3);
> + assert_true(buffer[0] == 0x9b && buffer[1] == 0xf1 &&
> + buffer[2] == 0x59);
> +
> + bytes_written = lj_utils_write_leb128(buffer, INT64_MIN);
> + assert_true(bytes_written == 10);
> + assert_true(buffer[0] == 0x80 && buffer[1] == 0x80 &&
> + buffer[2] == 0x80 && buffer[3] == 0x80 &&
> + buffer[4] == 0x80 && buffer[5] == 0x80 &&
> + buffer[6] == 0x80 && buffer[7] == 0x80 &&
> + buffer[8] == 0x80 && buffer[9] == 0x7f);
> +
> + bytes_written = lj_utils_write_leb128(buffer, INT64_MAX);
> + assert_true(bytes_written == 10);
> + assert_true(buffer[0] == 0xff && buffer[1] == 0xff &&
> + buffer[2] == 0xff && buffer[3] == 0xff &&
> + buffer[4] == 0xff && buffer[5] == 0xff &&
> + buffer[6] == 0xff && buffer[7] == 0xff &&
> + buffer[8] == 0xff && buffer[9] == 0x00);
> +
> + return TEST_EXIT_SUCCESS;
> +}
> +
> +/* Test miscellaneous writes, both signed and unsigned. */
> +static int test_misc_writes(void *state)
> +{
> + size_t bytes_written = 0;
> + uint8_t buffer[BUFFER_SIZE] = {0};
> +
> + UNUSED(state);
> +
> + bytes_written = lj_utils_write_leb128(buffer, 0x10);
> + assert_true(bytes_written == 1);
> + assert_true(buffer[0] == 0x10);
> +
> + bytes_written = lj_utils_write_uleb128(buffer, 0x10);
> + assert_true(bytes_written == 1);
> + assert_true(buffer[0] == 0x10);
> +
> + bytes_written = lj_utils_write_leb128(buffer, -0x3b);
> + assert_true(bytes_written == 1);
> + assert_true(buffer[0] == 0x45);
> +
> + bytes_written = lj_utils_write_uleb128(buffer, 0x45);
> + assert_true(bytes_written == 1);
> + assert_true(buffer[0] == 0x45);
> +
> + bytes_written = lj_utils_write_leb128(buffer, 0x190e);
> + assert_true(bytes_written == 2);
> + assert_true(buffer[0] == 0x8e && buffer[1] == 0x32);
> +
> + bytes_written = lj_utils_write_uleb128(buffer, 0x190e);
> + assert_true(bytes_written == 2);
> + assert_true(buffer[0] == 0x8e && buffer[1] == 0x32);
> +
> + bytes_written = lj_utils_write_leb128(buffer, -0x143f);
> + assert_true(bytes_written == 2);
> + assert_true(buffer[0] == 0xc1 && buffer[1] == 0x57);
> +
> + bytes_written = lj_utils_write_uleb128(buffer, 0x2bc1);
> + assert_true(bytes_written == 2);
> + assert_true(buffer[0] == 0xc1 && buffer[1] == 0x57);
> +
> + bytes_written = lj_utils_write_leb128(buffer, 0x7e00000);
> + assert_true(bytes_written == 4);
> + assert_true(buffer[0] == 0x80 && buffer[1] == 0x80 &&
> + buffer[2] == 0x80 && buffer[3] == 0x3f);
> +
> + bytes_written = lj_utils_write_uleb128(buffer, 0x7e00000);
> + assert_true(bytes_written == 4);
> + assert_true(buffer[0] == 0x80 && buffer[1] == 0x80 &&
> + buffer[2] == 0x80 && buffer[3] == 0x3f);
> +
> + bytes_written = lj_utils_write_leb128(buffer, -0x6200000);
> + assert_true(bytes_written == 4);
> + assert_true(buffer[0] == 0x80 && buffer[1] == 0x80 &&
> + buffer[2] == 0x80 && buffer[3] == 0x4f);
> +
> + bytes_written = lj_utils_write_uleb128(buffer, 0x9e00000);
> + assert_true(bytes_written == 4);
> + assert_true(buffer[0] == 0x80 && buffer[1] == 0x80 &&
> + buffer[2] == 0x80 && buffer[3] == 0x4f);
> +
> + return TEST_EXIT_SUCCESS;
> +}
> +
> +static int test_read_uleb128(void *state)
> +{
> + uint64_t value = 0;
> + size_t bytes_read = 0;
> + uint8_t buffer[BUFFER_SIZE] = {0};
> +
> + UNUSED(state);
> +
> + bytes_read = lj_utils_read_uleb128(&value, buffer);
> + assert_true(bytes_read == 1);
> + assert_true(value == 0);
> +
> + buffer[0] = 0x40;
> + bytes_read = lj_utils_read_uleb128(&value, buffer);
> + assert_true(bytes_read == 1);
> + assert_true(value == 64);
> +
> + buffer[0] = 0x80;
> + buffer[1] = 0x01;
> + bytes_read = lj_utils_read_uleb128(&value, buffer);
> + assert_true(bytes_read == 2);
> + assert_true(value == 128);
> +
> + memset(buffer, 0xff, 9);
> + buffer[9] = 0x01;
> + bytes_read = lj_utils_read_uleb128(&value, buffer);
> + assert_true(bytes_read == 10);
> + assert_true(value == UINT64_MAX);
> +
> + return TEST_EXIT_SUCCESS;
> +}
> +
> +static int test_read_leb128(void *state)
> +{
> + int64_t value = 0;
> + size_t bytes_read = 0;
> + uint8_t buffer[BUFFER_SIZE] = {0};
> +
> + UNUSED(state);
> +
> + bytes_read = lj_utils_read_leb128(&value, buffer);
> + assert_true(bytes_read == 1);
> + assert_true(value == 0);
> +
> + buffer[0] = 0x9b;
> + buffer[1] = 0xf1;
> + buffer[2] = 0x59;
> + bytes_read = lj_utils_read_leb128(&value, buffer);
> + assert_true(bytes_read == 3);
> + assert_true(value == (int64_t)-624485);
> +
> + memset(buffer, 0x80, 9);
> + buffer[9] = 0x7f;
> + bytes_read = lj_utils_read_leb128(&value, buffer);
> + assert_true(bytes_read == 10);
> + assert_true(value == INT64_MIN);
> +
> + memset(buffer, 0xff, 9);
> + buffer[9] = 0x00;
> + bytes_read = lj_utils_read_leb128(&value, buffer);
> + assert_true(bytes_read == 10);
> + assert_true(value == INT64_MAX);
> +
> + return TEST_EXIT_SUCCESS;
> +}
> +
> +/* Test miscellaneous reads, both signed and unsigned. */
> +static int test_misc_reads(void *state)
> +{
> + int64_t i_value = 0;
> + uint64_t u_value = 0;
> +
> + size_t bytes_read = 0;
> + uint8_t buffer[BUFFER_SIZE] = {0};
> +
> + UNUSED(state);
> +
> + buffer[0] = 0x10;
> +
> + bytes_read = lj_utils_read_leb128(&i_value, buffer);
> + assert_true(bytes_read == 1);
> + assert_true(i_value == (int64_t)0x10);
> +
> + bytes_read = lj_utils_read_uleb128(&u_value, buffer);
> + assert_true(bytes_read == 1);
> + assert_true(u_value == (uint64_t)0x10);
> +
> + buffer[0] = 0x45;
> +
> + bytes_read = lj_utils_read_leb128(&i_value, buffer);
> + assert_true(bytes_read == 1);
> + assert_true(i_value == (int64_t)-0x3b);
> +
> + bytes_read = lj_utils_read_uleb128(&u_value, buffer);
> + assert_true(bytes_read == 1);
> + assert_true(u_value == (uint64_t)0x45);
> +
> + buffer[0] = 0x8e;
> + buffer[1] = 0x32;
> +
> + bytes_read = lj_utils_read_leb128(&i_value, buffer);
> + assert_true(bytes_read == 2);
> + assert_true(i_value == (int64_t)0x190e);
> +
> + bytes_read = lj_utils_read_uleb128(&u_value, buffer);
> + assert_true(bytes_read == 2);
> + assert_true(u_value == (uint64_t)0x190e);
> +
> + buffer[0] = 0xc1;
> + buffer[1] = 0x57;
> +
> + bytes_read = lj_utils_read_leb128(&i_value, buffer);
> + assert_true(bytes_read == 2);
> + assert_true(i_value == (int64_t)-0x143f);
> +
> + bytes_read = lj_utils_read_uleb128(&u_value, buffer);
> + assert_true(bytes_read == 2);
> + assert_true(u_value == (uint64_t)0x2bc1);
> +
> + buffer[0] = 0xc1;
> + buffer[1] = 0x57;
> +
> + bytes_read = lj_utils_read_leb128(&i_value, buffer);
> + assert_true(bytes_read == 2);
> + assert_true(i_value == (int64_t)-0x143f);
> +
> + bytes_read = lj_utils_read_uleb128(&u_value, buffer);
> + assert_true(bytes_read == 2);
> + assert_true(u_value == (uint64_t)0x2bc1);
> +
> + buffer[0] = 0x80;
> + buffer[1] = 0x80;
> + buffer[2] = 0x80;
> + buffer[3] = 0x3f;
> +
> + bytes_read = lj_utils_read_leb128(&i_value, buffer);
> + assert_true(bytes_read == 4);
> + assert_true(i_value == (int64_t)0x7e00000);
> +
> + bytes_read = lj_utils_read_uleb128(&u_value, buffer);
> + assert_true(bytes_read == 4);
> + assert_true(u_value == (uint64_t)0x7e00000);
> +
> + buffer[0] = 0x80;
> + buffer[1] = 0x80;
> + buffer[2] = 0x80;
> + buffer[3] = 0x4f;
> +
> + bytes_read = lj_utils_read_leb128(&i_value, buffer);
> + assert_true(bytes_read == 4);
> + assert_true(i_value == (int64_t)-0x6200000);
> +
> + bytes_read = lj_utils_read_uleb128(&u_value, buffer);
> + assert_true(bytes_read == 4);
> + assert_true(u_value == (uint64_t)0x9e00000);
> +
> + return TEST_EXIT_SUCCESS;
> +}
> +
> +static int test_read_n(void *state)
> +{
> + int64_t i_value = 0;
> + uint64_t u_value = 0;
> +
> + size_t bytes_read = 0;
> + uint8_t buffer[BUFFER_SIZE] = {0};
> +
> + UNUSED(state);
> +
> + bytes_read = lj_utils_read_leb128_n(&i_value, buffer, 1);
> + assert_true(bytes_read == 1);
> + assert_true(i_value == 0);
> +
> + bytes_read = lj_utils_read_uleb128_n(&u_value, buffer, 1);
> + assert_true(bytes_read == 1);
> + assert_true(u_value == 0);
> +
> + buffer[0] = 0x80;
> + buffer[1] = 0x80;
> + buffer[2] = 0x80;
> + buffer[3] = 0x3f;
> +
> + assert_true(lj_utils_read_leb128_n(&i_value, buffer, 3) == 0);
> + assert_true(lj_utils_read_uleb128_n(&u_value, buffer, 3) == 0);
> + /* Values are untouched in case of failure. */
> + assert_true(i_value == 0);
> + assert_true(u_value == 0);
> +
> + bytes_read = lj_utils_read_leb128_n(&i_value, buffer, 4);
> + assert_true(bytes_read == 4);
> + assert_true(i_value == (int64_t)0x7e00000);
> +
> + bytes_read = lj_utils_read_uleb128_n(&u_value, buffer, 4);
> + assert_true(bytes_read == 4);
> + assert_true(u_value == (uint64_t)0x7e00000);
> +
> + bytes_read = lj_utils_read_leb128_n(&i_value, buffer, 5);
> + assert_true(bytes_read == 4);
> + assert_true(i_value == (int64_t)0x7e00000);
> +
> + bytes_read = lj_utils_read_uleb128_n(&u_value, buffer, 5);
> + assert_true(bytes_read == 4);
> + assert_true(u_value == (uint64_t)0x7e00000);
> +
> + return TEST_EXIT_SUCCESS;
> +}
> +
> +int main(void)
> +{
> + const struct test_unit tgroup[] = {
> + test_unit_def(test_write_uleb128),
> + test_unit_def(test_write_leb128),
> + test_unit_def(test_misc_writes),
> + test_unit_def(test_read_uleb128),
> + test_unit_def(test_read_leb128),
> + test_unit_def(test_misc_reads),
> + test_unit_def(test_read_n),
> + };
> + const int test_result = test_run_group(tgroup, NULL);
> + return test_result;
> +}
> --
> 2.55.0
>

[-- Attachment #2: Type: text/html, Size: 16104 bytes --]

      parent reply	other threads:[~2026-07-28 19:52 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22 14:00 Sergey Kaplun via Tarantool-patches
2026-07-24 10:08 ` Sergey Bronnikov via Tarantool-patches
2026-07-28 19:52 ` Evgeniy Temirgaleev via Tarantool-patches [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1785268328.931937759@5.f.mail-release.rc.mrucl.ru \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=e.temirgaleev@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches]  [PATCH luajit] test: add tests for the leb128 encode/decode' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox