Allocated structures are not always properly freed

Hi,

I’ve been looking into some memory related issues in using Acados and I believe I’ve spotted a bug.
The interface offers create functions and matching destroy functions. The create functions allocate memory (using malloc/calloc). However, the pointer returned by malloc/calloc is not retained. E.g. for ocp_nlp_in_create, the pointer can be changed for alignment in ocp_nlp_in_assign_self, by the call to

align_char_to(8, &c_ptr);

When the address returned by malloc/calloc was already on an 8-byte boundary, this line won’t change c_ptr, but on my target system (a Simulink Real-Time 6.7 xPC target, using acados via an sfunction) it appears the pointers are only aligned on 4-byte boundaries. When this statement changes the pointer, this changed pointer is then returned by ocp_nlp_in_create. This will also be the pointer that will eventually be passed to ocp_nlp_in_destroy, but this function just passes the pointer directly to free. But this fails if the pointer was modified (it won’t correspond to something the malloc/calloc returns). It looks like my target recognizes this (it prints a message “HeapFree: Invalid parameter”), so I expect this will only result in a memory leak for me, but other C-library implementations might crash on this, or cause data corruption.

Of course, ocp_nlp_in is just one case and the same applies to the other structures managed in the same way.

A fix that seems to work at first glance is to also store the original pointer inside the returned struct and use that pointer in the destroy function for the free call. I’ve tried this for ocp_nlp_in and seems to indeed solve the issue. If it helps, I can do it for the other structs as well and provide a patch, but I’m still totally new to acados, so I might overlook some relevant details…

Regards,
Tom.

Hi Tom,

thanks for reporting this bug!

I agree with the solution you proposed.
If you can open a pull request with this fix, that would be great!

Let me know if you have any questions doing so.

Best,
Jonathan

1 Like

Thanks for the PR!
Further communication is happening here: