Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

fsanitize emits memory leaks errors #76

Closed
omalaspinas opened this issue Dec 18, 2023 · 1 comment
Closed

fsanitize emits memory leaks errors #76

omalaspinas opened this issue Dec 18, 2023 · 1 comment
Assignees
Labels
improvement Improve an already implemented feature
Milestone

Comments

@omalaspinas
Copy link

omalaspinas commented Dec 18, 2023

Hello,

when compiling with sanitizers (I used memory, leaks, and undefined), I get a memory leak during the call to the following macro call (I guess it returns early and some temporary malloc is not freed)

__CLOVE_VECTOR_FOREACH(&cmd_handlers, __clove_cmdline_handler_f, handler, {
        __clove_cmdline_errno_t result = (*handler)(&cmdline);
        if (result != __CLOVE_CMD_ERRNO_UNMANAGED) {
            cmd_result =  result;
            break;
        }
    });

I tried to fix it by using various free functions but did not manage to make it pass the CI tests. I'm probably missing some understanding in here. In particular I tried adding

__clove_vector_free(&cmd_handlers);
__clove_cmdline_free(&cmdline);

on the other branch of the if but I guess it frees too much (it indeed removes the memory leak but..... then no test is executed anymore in the CI...).

To add the sanitizers you can add the following lines in the CMakeLists.txt files:

add_compile_options(-fsanitize=address,leak,undefined -g)
add_link_options(-fsanitize=address,leak,undefined)

Edit: formatting.

@fdefelici
Copy link
Owner

I made some verification and the memory leak is not due to the foreach macro itself, but it's from the "polymorophic" function call __clove_cmdline_errno_t result = (*handler)(&cmdline);

There are some vectors stored in map that that are not properly freed. Now, I remember that I did it on purpose to avoid to make complex memory management in that case (considering the fact we are in a test library and leaks are negligible related to data related to library itself and that data are just stored once and then freed at the end of test execution)

Anyhow, this could be improved

@fdefelici fdefelici added this to the v2.4.2 milestone Dec 19, 2023
@fdefelici fdefelici added the improvement Improve an already implemented feature label Dec 19, 2023
@fdefelici fdefelici self-assigned this Dec 22, 2023
fdefelici added a commit that referenced this issue Dec 25, 2023
fdefelici added a commit that referenced this issue Dec 25, 2023
fdefelici added a commit that referenced this issue Dec 26, 2023
fdefelici added a commit that referenced this issue Dec 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improve an already implemented feature
Projects
None yet
Development

When branches are created from issues, their pull requests are automatically linked.

2 participants