Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Risky stackallocs #303

Open
RossNordby opened this issue Jan 13, 2024 · 0 comments
Open

Risky stackallocs #303

RossNordby opened this issue Jan 13, 2024 · 0 comments

Comments

@RossNordby
Copy link
Member

RossNordby commented Jan 13, 2024

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:

var stack = stackalloc int[TraversalStackCapacity];

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant