From 36e82b2a49dc6e7096f12d5722a3597e20d75f33 Mon Sep 17 00:00:00 2001 From: Peter Colberg Date: Thu, 3 Nov 2022 22:19:26 -0400 Subject: [PATCH 1/3] acl_test: remove undefined behaviour from acl_test_unsetenv() acl_getenv() always returns NULL, hence the undefined behaviour was never triggered. This looks like a debugging leftover, though assert() would have been the safe alternative to dereferencing a NULL pointer. Signed-off-by: Peter Colberg --- test/acl_test.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/acl_test.cpp b/test/acl_test.cpp index 890735fc..081b878f 100644 --- a/test/acl_test.cpp +++ b/test/acl_test.cpp @@ -198,9 +198,6 @@ void acl_test_unsetenv(const char *var) { #else unsetenv(var); #endif - if (acl_getenv(var)) { - *(char *)0 = 0; - } } void acl_test_setenv(const char *var, const char *value) { #ifdef _WIN32 From 561e89cea5f8563147325bf99cfafb2f4bf13dbc Mon Sep 17 00:00:00 2001 From: Peter Colberg Date: Thu, 3 Nov 2022 22:27:00 -0400 Subject: [PATCH 2/3] acl_test: use _putenv_s() instead of _putenv() on Windows _putenv_s() is recommended over _putenv() as a more secure variant, though the exact benefits are not detailed. Unlike the unsafe putenv() on Linux, _putenv() on Windows does appear to copy its argument. Use _putenv_s() anyway since it provides the same interface as setenv(). Signed-off-by: Peter Colberg --- test/acl_test.cpp | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/test/acl_test.cpp b/test/acl_test.cpp index 081b878f..bc8664b1 100644 --- a/test/acl_test.cpp +++ b/test/acl_test.cpp @@ -188,24 +188,16 @@ SimpleString StringFrom(size_t x) { #endif void acl_test_unsetenv(const char *var) { - #ifdef _WIN32 - { - char buf[1000]; - sprintf(buf, "%s=", var); - _putenv(buf); - } + _putenv_s(var, ""); #else unsetenv(var); #endif } + void acl_test_setenv(const char *var, const char *value) { #ifdef _WIN32 - { - char buf[1000]; - sprintf(buf, "%s=%s", var, value); - _putenv(buf); - } + _putenv_s(var, value); #else setenv(var, value, 1); #endif From bc754bbfb06982439ea40ce756418553dd6a2a33 Mon Sep 17 00:00:00 2001 From: Peter Colberg Date: Fri, 4 Nov 2022 22:51:44 -0400 Subject: [PATCH 3/3] acl_profiler_test: fix undefined behaviour with putenv() The buffer passed to putenv() becomes part of the environment. It is the caller's responsibility to keep the buffer alive as long as the environment variable is not modified by a subsequent invocation. Signed-off-by: Peter Colberg --- test/acl_profiler_test.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/acl_profiler_test.cpp b/test/acl_profiler_test.cpp index e6370966..03b8adb9 100644 --- a/test/acl_profiler_test.cpp +++ b/test/acl_profiler_test.cpp @@ -50,8 +50,8 @@ MT_TEST_GROUP(acl_profile) { enum { MAX_DEVICES = 100, m_num_devices_in_context = 3 }; void setup() { if (threadNum() == 0) { - char profile_runtime_env_var[] = "ACL_RUNTIME_PROFILING_ACTIVE=1"; - char profiler_timer_env_var[] = "ACL_PROFILE_TIMER=1"; + static char profile_runtime_env_var[] = "ACL_RUNTIME_PROFILING_ACTIVE=1"; + static char profiler_timer_env_var[] = "ACL_PROFILE_TIMER=1"; #ifdef _WIN32 _putenv(profile_runtime_env_var); _putenv(profiler_timer_env_var);