From 97660307f5ac10d01febd7ffba63dfeb24d10351 Mon Sep 17 00:00:00 2001 From: bfredl Date: Mon, 7 Sep 2026 13:39:56 +0200 Subject: [PATCH] fix(strings): non fatal errors for [NULL] safety in vim_snprintf context: vim_snprintf provides a "portable" replacement for a system vsnprintf(). However we rely on system vsnprintf() in places. This is quite arbitrary. I want to use system vsnprintf() only, and reduce the machinery in string.c for typval printf only, which will delet a lot of duplicated code. The biggest hurdle here is not "portably" (we only support sane c runtimes) but the following discrepancy: standard libc printf() considers NULL args to %s to be undefined behavior strictly while vim_snprintf() _defacto_ allows this by replacement to "[NULL]" IMO, this should be seen as "graceful" error handling (nicer than crashing on the user), not a valid means to have the string "[NULL]" intentionally be shoved in the users face. more robust code should explicitly check for NULL and replace it with a _appropriate_ fallback for the situation, depending on what a NULL string actually means in the context (unnamed buffer? anonymous namespace? global augroup?) soo, this PR provides the most _gently_ incremental nudge towards considering this case again as an error one could possible imagine. If sanitizers are complied in, this minor incursion gets printed into the sanitizers' log as a "report-and-continue" error with a traceback. This doesn't annoy the end user or "stop the world" in the test suite, but the CI will see the reported error and fail the build, just like other non-fatal CI errors. --- src/nvim/lua/executor.c | 17 +++++------------ src/nvim/sign.c | 3 ++- src/nvim/strings.c | 7 +++++++ test/functional/lua/overrides_spec.lua | 6 +++--- test/functional/ui/sign_spec.lua | 2 +- test/old/testdir/test_signs.vim | 4 ++-- 6 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/nvim/lua/executor.c b/src/nvim/lua/executor.c index 5ac5e7203b..29c31a4c82 100644 --- a/src/nvim/lua/executor.c +++ b/src/nvim/lua/executor.c @@ -1131,12 +1131,6 @@ static void nlua_print_event(void **argv) static int nlua_print(lua_State *const lstate) FUNC_ATTR_NONNULL_ALL { -#define PRINT_ERROR(msg) \ - do { \ - errmsg = msg; \ - errmsg_len = sizeof(msg) - 1; \ - goto nlua_print_error; \ - } while (0) const int nargs = lua_gettop(lstate); lua_getglobal(lstate, "tostring"); const char *errmsg = NULL; @@ -1148,22 +1142,21 @@ static int nlua_print(lua_State *const lstate) lua_pushvalue(lstate, -1); // tostring lua_pushvalue(lstate, curargidx); // arg // Do not use nlua_pcall here to avoid duplicate stack trace information - if (lua_pcall(lstate, 1, 1, 0)) { - errmsg = lua_tolstring(lstate, -1, &errmsg_len); + if (lua_pcall(lstate, 1, 1, 0) || lua_type(lstate, -1) != LUA_TSTRING) { + // NB: this might try tostring once more to print a weird error from + // a __tostring methamethod but we stop after that. + errmsg = nlua_get_error(lstate, &errmsg_len); goto nlua_print_error; } + size_t len; const char *const s = lua_tolstring(lstate, -1, &len); - if (s == NULL) { - PRINT_ERROR(""); - } ga_concat_len(&msg_ga, s, len); if (curargidx < nargs) { ga_append(&msg_ga, ' '); } lua_pop(lstate, 1); } -#undef PRINT_ERROR ga_append(&msg_ga, NUL); lua_getfield(lstate, LUA_REGISTRYINDEX, "nvim.thread"); diff --git a/src/nvim/sign.c b/src/nvim/sign.c index 50016e5f04..72b83b2b3a 100644 --- a/src/nvim/sign.c +++ b/src/nvim/sign.c @@ -264,7 +264,8 @@ static void sign_list_placed(buf_T *rbuf, char *group) while (buf != NULL && !got_int) { if (buf_has_signs(buf)) { msg_putchar('\n'); - vim_snprintf(lbuf, MSG_BUF_LEN, _("Signs for %s:"), buf->b_fname); + const char *fname = buf->b_fname ? buf->b_fname : _("Untitled"); + snprintf(lbuf, MSG_BUF_LEN, _("Signs for %s:"), fname); msg_puts_hl(lbuf, HLF_D, false); } diff --git a/src/nvim/strings.c b/src/nvim/strings.c index 22770a9ba5..6b92d75110 100644 --- a/src/nvim/strings.c +++ b/src/nvim/strings.c @@ -1784,6 +1784,13 @@ int vim_vsnprintf_typval(char *str, size_t str_m, const char *fmt, va_list ap_st va_arg(ap, const char *))); if (!str_arg) { +#ifdef ENABLE_ASAN_UBSAN + // this is only for vsnprintf() emulation, v:_null_string handled in tv_str() + if (!tvs) { + __sanitizer_print_stack_trace(); + __sanitizer_report_error_summary("SUMMARY: NULL argument passed to %s"); + } +#endif str_arg = "[NULL]"; str_arg_l = 6; } else if (!precision_specified) { diff --git a/test/functional/lua/overrides_spec.lua b/test/functional/lua/overrides_spec.lua index 5ce5ab7443..0b3505d6ab 100644 --- a/test/functional/lua/overrides_spec.lua +++ b/test/functional/lua/overrides_spec.lua @@ -59,15 +59,15 @@ describe('print', function() eq('', exec_capture('luafile ' .. fname)) -- TODO(bfredl): these look weird, print() should not use "E5114:" style errors.. eq( - 'Vim(lua):E5108: Lua: E5114: Converting print argument #2: [NULL]', + 'Vim(lua):E5108: Lua: E5114: Converting print argument #2: nil', pcall_err(command, 'lua print("foo", v_nilerr, "bar")') ) eq( 'Vim(lua):E5108: Lua: E5114: Converting print argument #2: Xtest-functional-lua-overrides-luafile:2: abc', pcall_err(command, 'lua print("foo", v_abcerr, "bar")') ) - eq( - 'Vim(lua):E5108: Lua: E5114: Converting print argument #2: ', + matches( + '^Vim%(lua%):E5108: Lua: E5114: Converting print argument #2: table: 0x%x+$', pcall_err(command, 'lua print("foo", v_tblout, "bar")') ) end) diff --git a/test/functional/ui/sign_spec.lua b/test/functional/ui/sign_spec.lua index 8d64ecbd6a..c270f9a3a2 100644 --- a/test/functional/ui/sign_spec.lua +++ b/test/functional/ui/sign_spec.lua @@ -451,7 +451,7 @@ describe('Signs', function() {3: }| :sign place | {100:--- Signs ---} | - {18:Signs for [NULL]:} | + {18:Signs for Untitled:} | line=1 id=100000 name=piet priority=10 | {6:Press ENTER or type command to continue}^ | ]]) diff --git a/test/old/testdir/test_signs.vim b/test/old/testdir/test_signs.vim index 8ff14a0371..e7d2089d88 100644 --- a/test/old/testdir/test_signs.vim +++ b/test/old/testdir/test_signs.vim @@ -61,7 +61,7 @@ func Test_sign() " Check placed signs let a=execute('sign place') - call assert_equal("\n--- Signs ---\nSigns for [NULL]:\n" . + call assert_equal("\n--- Signs ---\nSigns for Untitled:\n" . \ " line=3 id=41 name=Sign1 priority=10", a) " Unplace the sign and try jumping to it again should fail. @@ -90,7 +90,7 @@ func Test_sign() sign place 77 line=9 name=Sign2 let a=execute('sign place') " Nvim: sign line clamped to buffer length - call assert_equal("\n--- Signs ---\nSigns for [NULL]:\n" . + call assert_equal("\n--- Signs ---\nSigns for Untitled:\n" . \ " line=4 id=77 name=Sign2 priority=10", a) sign unplace *