Fixes va_list in strbuf being left uninitialized (causes MSVC to crash)

[Naios]

• Ref Implement a world-local component id caching API and make use of it in cpp components flecs#1036

[SanderMertens]

Thanks for the PR!

[SanderMertens]

I reverted the change as it caused the build to fail on gcc: https://github.com/SanderMertens/bake/actions/runs/6228764312

Are you sure this is necessary? The same code in Flecs works on msvc: https://github.com/SanderMertens/flecs/blob/master/src/datastructures/strbuf.c#L491

[Naios]

@SanderMertens sorry for the inconvenience, but I stil think that va_list needs proper initialization, since if it is implemented through a struct, the struct itself will be uninitialized and this is what MSVC was complaining about.

if it does not works this way I suggest that you make the function itself with variadic arguments (add an ellipsis) and just never add arguments and initialize the va_list correctly based on the ellipsis.

I googled it quickly and it seems there is no proper way of doing it, also you cannot just return it from inside another function since after creation the va_list must be correctly destroyed in the same scope.

See https://stackoverflow.com/questions/15321493/how-should-i-pass-null-to-the-va-list-function-parameter

Btw. is also seems like clang has an analysis pass for detecting uninitialized va_args so this is definitly not valid not to initialize it: https://chromium.googlesource.com/external/github.com/llvm-mirror/clang/+/refs/heads/master/test/Analysis/valist-uninitialized.c#18

[SanderMertens]

Do you remember the scenario in which the code crashed? The uninitialized va_list shouldn't actually be used by the function it's passed into for that specific set of arguments (fmt_string is false).

[Naios]

It crashes deterministically on this revision so I can provide you a stacktrace:

Run-Time Check Failure 3 - The variable 'args' is being used without being initialized.

ut_strbuf_appendstrn(ut_strbuf * 0x000000a32f93eca0, const char * 0x000000a32f93f624, __int64 1) Line 281
	at bake\util\src\strbuf.c(281)
ut_venvparse(const char * 0x00007ff95754fb94, char * 0x000000a32f93f6e0) Line 286
	at bake\util\src\env.c(286)
ut_setenv(const char * 0x00007ff957550140, const char * 0x00007ff95754fb94, ...) Line 36
	at bake\util\src\env.c(36)
ut_log_verbositySet(ut_log_verbosity UT_OK) Line 1859
	at bake\util\src\log.c(1859)
ut_init(const char * 0x00007ff76b4877f0) Line 71
	at bake\util\src\util.c(71)
bake_test_run(const char * 0x00007ff76b4877f0, int 3, char * * 0x000002123b32ae40, bake_test_suite * 0x00007ff76b54bbf0, unsigned int 28) Line 479
	at bake\drivers\test\src\test.c(479)
main(int 3, char * * 0x000002123b32ae40) Line 6428
	at cpp_api\src\main.cpp(6428)

Independently of its use-case it is very clear to me that the va_list should be initialized, also from the sources I have found on the internet.

[SanderMertens]

Independently of its use-case it is very clear to me that the va_list should be initialized

As long as the variable isn't used, it doesn't have to be initialized, and in this case it's not used. The line it's crashing on in the stack trace is not reading the va_list, it's just passing it as argument.

How are you getting this to crash? I run bake in CI on Windows on every flecs commit and have never seen a crash.

[Naios]

Did you compile with Debug maybe it's only a run-time check that is active when the debugger is attached. But does the function called in ut_strbuf_appendstrn not use the va_list?

[Naios]

ut_strbuf_append_intern accesses the list passed by ut_strbuf_vappend at least due to va_copy(arg_cpy, args).

[SanderMertens]

for that specific set of arguments (fmt_string is false).

If fmt_string is false, the va_list isn't used.

All bake installations use debug mode.