mirror of
https://github.com/neovim/neovim.git
synced 2026-09-12 09:01:02 +00:00
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.
This commit is contained in:
@@ -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("<Unknown error: lua_tolstring returned NULL for tostring result>");
|
||||
}
|
||||
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");
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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: <Unknown error: lua_tolstring returned NULL for tostring result>',
|
||||
matches(
|
||||
'^Vim%(lua%):E5108: Lua: E5114: Converting print argument #2: table: 0x%x+$',
|
||||
pcall_err(command, 'lua print("foo", v_tblout, "bar")')
|
||||
)
|
||||
end)
|
||||
|
||||
@@ -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}^ |
|
||||
]])
|
||||
|
||||
@@ -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 *
|
||||
|
||||
|
||||
Reference in New Issue
Block a user