[ODE] problems with trimesh and capsules

[Paul MacKenzie]

Jaroslav Sinecky wrote:
> Memory allocated by dALLOCA16 should be freed automatically when leaving 
> the function, see documentation for CRT alloc function. I did this 
> change recently so it's not 100% guaranteed to be bug free, but I tried 
> to run test_trimesh and test_moving_trimesh with lot of capsules with no 
> problem. And revising the code can't see any error neither.

I think this is causing a problem -- gLocalContacts in 
collision_trimesh_ccylinder.cpp is a global variable, and is allocated 
once inside the dCollideCCTL() function and does not seem to be reallocated.

However, when using the dALLOCA16() function, the stack memory allocated 
only survives within the context of that call to dCollideCCTL(), and is 
released afterwards.  However, since gLocalContacts is global, it 
remembers that same memory space for the life of the program.  So later 
on when ODE tries to allocate new stack space elsewhere, it reuses the 
memory previously owned by gLocalContacts, and now we have a stack 
integrity problem.

I tried using malloc() instead of dALLOCA16(), and there is no stack 
integrity problem.  However, this leaks gLocalContacts, and besides, if 
I later call dCollide() with a larger maximum number of contacts, there 
could be a buffer overrun.

I followed through collision_trimesh_ccylinder.cpp and it looks like the 
only place that gLocalContacts is accessed is within the context of 
dCollideCCTL() and the functions that it calls.  So, at the end of this 
function, we can set gLocalContacts to NULL.  In this way it will be 
reallocated at every call, so that we are sure we have a properly sized 
array.

One way to do this would be to use malloc() to allocate the memory, and 
call free() at the end of the function followed by a set to NULL. 
Alternatively, we can continue to use dALLOCA16() and just set 
gLocalContacts to NULL at the end, forgetting about the free().  Both 
work for me in my tests.  We would have to modify the end of 
dCollideCCTL() with a temporary variable because _ProcessLocalContacts() 
needs gLocalContacts.

Here's a proposed modification, at the end of dCollideCCTL() in 
collision_trimesh_ccylinder.cpp, assuming the continued use of 
dALLOCA16() to allocate gLocalContacts if it is NULL:

----------------OLD CODE------------------

return _ProcessLocalContacts();

----------------NEW CODE------------------

int plc = _ProcessLocalContacts();
gLocalContacts = NULL;
return plc;

------------------------------------------

There should be no memory leaked as dALLOCA16() uses alloca() which is 
supposed to free up all stack-allocated memory automatically.

I have not submitted a patch for this yet, as I would like someone to 
review my logic, in case I missed something and my stack problem 
actually goes away for other reasons.

Thanks!
Paul