gjkPenetration() crash about physx HOT 9 CLOSED

I tried to add this code you suggested, and it completely broke PxGjkQuery::proximityInfo in my tests. Suddenly no longer returning overlap for shapes that are overlapping.

from physx.

Interesting, I would have thought that NaN output from the ScaleInv function would always cause issues but I guess you found a case where it doesn't (perhaps size < 4 when the vector is zero and in the next iteration of the loop the vector is not zero).

I think a more foolproof condition could be to check if size >= 4 at the beginning of the while loop and return GJK_DEGENERATE if so....because if it is >= 4 then stack corruption is guaranteed because 'aInd' (accessed at the start of the loop) is an array variable initialized with a size of 4 (so 0-3 are the only valid indices).

from physx.

Hi @Tysoni. Thanks for the report.
It's hard to say without debugging your exact situation, but looking into the code it seems that a zero vector (0,0,0,0) it valid output for 'GJKCPairDoSimplex', and it means that the shapes are penetrating. In this case it's perhaps just some zero division check that's missing in line 265.
Something like
v = V3Sel(FIsGrtr(dist, eps), V3ScaleInv(closest, dist), closest);
Could you try it?
Thanks.

from physx.

Hi @vreutskyy,

That may alleviate the division by zero issue but in my case the zero vector is being returned on the fourth iteration of the loop, and it's not satisfying the loop exit condition, which means the stack still gets corrupted in the next iteration when the out-of-bounds array access occurs (ie: I swapped in your code and the crash still happens).

I apologize for not being able to provide a snippet here...the crash is occurring in a complex setup. Either way, the fact that the loop can still continue iterating even when the "size" index value is invalid is a problem.

from physx.

Hm. Why is there next iteration? Shouldn't 'dist' equal 0 set 'bNotTerminated' to false?
bNotTerminated = BAnd(FIsGrtr(dist, eps), bNotDegenerated);

from physx.

@vreutskyy

Actually, I slotted your snippet in but didn't step through the result...just saw that the same crash was happening due to stack corruption.

Looking closer, I'm not sure I understand the logic of your snippet....looking at my sim results, eps is negative...so dist will always be greater than eps regardless of whether or not it's zero or a larger number (dist will always be positive because it's just a vector length)...thus the FIsGrtr condition will always be true, and V3ScaleInv will always be assigned to v.

That would explain why the loop is not being broken...v is still NaN.

from physx.

@Tysoni
As I see eps comes from here
const FloatV minMargin = FMin(a.ConvexA::getMinMargin(), b.ConvexB::getMinMargin()); const FloatV eps = FMul(minMargin, FLoad(0.1f));
Do you have negative margin on one of you shape? Margin should never be negative if I'm not wrong.

from physx.

Ah...interesting. The margin calculation is indeed negative, but it's not something I ever assign. However, it's dependent on the convex scale value, which is negative in my case due to a negative scale value on the geometry in my scene that I'm assigning to the convex mesh geometry. So I think that may actually be the root cause of all these issues!

What's interesting is that the negative scale doesn't seem to have an impact on the sim outside of these edge cases preventing the loop exit (and it doesn't trigger any asserts in PhysX code), so that's why I never noticed these issues before....in another scene I tested with negative scale on colliding convex objects, I get no errors...and collisions are all normal, hull is normal, etc.

I've now ensured the convex mesh scaling values I assign are always positive, and no longer get any crashing issues. So, this ultimately appears to be user error. Now that I've looked closer I do see the following line in the PxMeshScale header:

Only positive scale values are supported for PxConvexMeshGeometry

...but it might also be good to get a PX_ASSERT somewhere warning users that negative scale values on convex hulls are invalid, since they may not immediately cause any issues in the sim so it's harder to track down the cause when they do occur.

Thanks for your help in getting to the bottom of this!

from physx.

Yes, it definitely had to show an error when the invalid value was set. Thanks again for the report.

from physx.