Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

acl_profiler_test: fix undefined behaviour #196

Merged
merged 3 commits into from
Nov 7, 2022
Merged

Conversation

pcolberg
Copy link
Contributor

@pcolberg pcolberg commented Nov 7, 2022

This resolves the final issue before enabling address sanitizer 🥳

putenv() should be replaced with acl_test_setenv() and acl_test_unsetenv() in a subsequent pull request, but for now this is sufficient to pass address sanitizer.

@pcolberg pcolberg added the bug Something isn't working label Nov 7, 2022
@pcolberg pcolberg added this to the 2023.1 milestone Nov 7, 2022
@pcolberg pcolberg self-assigned this Nov 7, 2022
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 <peter.colberg@intel.com>
_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 <peter.colberg@intel.com>
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 <peter.colberg@intel.com>
Copy link
Contributor

@zibaiwan zibaiwan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pcolberg !

@pcolberg pcolberg merged commit 900e86e into intel:main Nov 7, 2022
@pcolberg pcolberg deleted the putenv branch November 7, 2022 21:58
pcolberg added a commit to pcolberg/fpga-runtime-for-opencl that referenced this pull request Nov 8, 2022
Ensure each setenv() is matched with unsetenv().

This amends intel#196
pcolberg added a commit to pcolberg/fpga-runtime-for-opencl that referenced this pull request Nov 8, 2022
Ensure each setenv() is matched with unsetenv().

This amends intel#196

Signed-off-by: Peter Colberg <peter.colberg@intel.com>
pcolberg added a commit that referenced this pull request Nov 8, 2022
Ensure each setenv() is matched with unsetenv().

This amends #196

Signed-off-by: Peter Colberg <peter.colberg@intel.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AddressSanitizer: stack-buffer-overflow in test (track_object, some_leak)
2 participants