[ODE] problems with trimesh and capsules

[Jaroslav Sinecky]

Well, this is still rather strange to me. How assigning NULL to 
gLocalContacts before exiting would influence something if we don't test 
for NULL before reallocating it again (and at any other place in the code).
It's not a problem to put it in, if it solves the problem for you, but I 
would prefer to understand what's going on. What's your 
platform/compiler, Paul?
Someone else has a clue or experience similar problem?

Jaroslav


Paul MacKenzie wrote:
> 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
>
>
>
> _______________________________________________
> ODE mailing list
> ODE at q12.org
> http://q12.org/mailman/listinfo/ode
>
>