Windows debug builds modify global CRT error modes

[glebov-andrey]

Hi,

On Windows, using a debug build of vulkan-1.dll changes global CRT error reporting modes.
Here's the code in question:

Vulkan-Loader/loader/loader_windows.c

Lines 91 to 95 in 1a337fe

	#if !defined(NDEBUG)
	_set_error_mode(_OUT_TO_STDERR);
	_CrtSetReportMode(_CRT_ERROR, _CRTDBG_MODE_FILE);
	_CrtSetReportFile(_CRT_ERROR, _CRTDBG_FILE_STDERR);
	#endif

While this is probably useful for Vulkan-Loader's own tests in CI, it's not expected behavior for users who might want to set their own error report modes.
I would suggest moving the error mode code to the test runner instead.

[charles-lunarg]

The Vulkan-Loader statically links the CRT into itself, so this code should not affect user code. Because the loader statically links the CRT, it is necessary for the loader to do it instead of the test runner as the test runner's CRT is dynamically linked, and the test runner cannot modify the settings of the CRT for the loader.

[glebov-andrey]

Ah, I see - we actually do build the loader with DLL runtimes, and so it does affect user code in our case.
And we aren't the only ones - ConanCenter does this too: https://github.com/conan-io/conan-center-index/blob/0c86693f18b14e52d7119b3e225706a002f27a9e/recipes/vulkan-loader/all/conanfile.py#L129-L147

I traced the usage of static runtime libraries back to c8c0057, and it seems like this is more of a packaging/distribution decision.
As long as there isn't a technical reason why the loader won't work correctly with the DLL runtime, I think it might be a good idea not to force this, and to provide an opt-out CMake option - this would help in packaging. At the same time, the error mode changing code could be guarded with #ifndef _DLL.

[charles-lunarg]

That is a good point - there isn't an extremely strong technical reason that the CRT must be statically linked. Because the loader only interfaces with user code, drivers, and layers through the Vulkan API, there is a very strict barrier preventing the 'bad' cases where a statically linked CRT causes issues (namely sharing allocations and sync objects).

Statically linking the CRT should mean that using the vulkan-1.dll doesn't require installing a CRT on the system, which is a general benefit. But more often than not if an application is going to use vulkan they'll have the CRT available regardless (either because they use it or its very widely available). Maybe I'm thinking of the C++ runtime being less readily available, but still that doesn't mean the option can't be made.

It shouldn't be hard to not set the CRT settings if not statically linking the CRT.