search

bepu / bepuphysics2

Pure C# 3D real time physics simulation library, now with a higher version number.
Apache License 2.0
2.25k stars 261 forks source link

Risky stackallocs #303

Open RossNordby opened 5 months ago

RossNordby commented 5 months ago

Just found and fixed an unguarded stackalloc in CompoundPairOverlapHandler, so quickly scanned for other sketchy instances.

CompoundBuilder.ComputeInertia: https://github.com/bepu/bepuphysics2/blob/master/BepuPhysics/Collidables/CompoundBuilder.cs#L380 https://github.com/bepu/bepuphysics2/blob/master/BepuPhysics/Collidables/CompoundBuilder.cs#L402 Helper function; no strict need to allocate a temporary array in the first place. Could fail for extremely large compounds.

Lower risk: Tree.RayCast: https://github.com/bepu/bepuphysics2/blob/563773c8a7f0dc695358b4e131cf7abdc6ac49f3/BepuPhysics/Trees/Tree_RayCast.cs#L109 With the tree revamp, hitting this profoundly unlikely, but we do still expose the old sometimes-pathological insertion in the API. It's not impossible for this to fail. This was always a hack.

https://github.com/bepu/bepuphysics2/blob/master/BepuPhysics/CollisionDetection/CollisionTasks/ConvexHullPairTester.cs#L90 In order for this to fail, the ConvexHull would need to be nigh adversarially pathological, but it's not impossible. Could be worth just wrapping it in a buffer just to have some catch.

On the fence: TaskStack usages: https://github.com/bepu/bepuphysics2/blob/master/BepuUtilities/TaskScheduling/TaskStack.cs The TaskStack is a type I don't expect external users to ever even know about, and the nature of the failure is direct: if a specific function call is provided too many tasks at once, it fails immediately. I'm treating it as a user error condition given that I'm realistically the only user.