Fixed issues found by PVS-Studio (#729)

* Correctly compute SoftBodyShape memory size in bytes
* Remove empty loop with no side effects
* Drop already tru condition guaranteed above
* Ensure no out of bounds read/write to triangles in production JPH_ASSERT is in Debug only. Release builds can use UINT_MAX as result.  It leads to out of bounds reads and sometimes not crash but hides bugs. Suggest to hard stop in this case instead of failing silently.
* At least check in Debug mode we do not overflow during first vertex computation
* Correctly print size_t. Unfortunately MinGW doesn't support %zu C11 specifier. So decided to cast size_t -> uint.
* Correct format specifiers for printf-like API
* Create objects in place to speedup containers filling
* Speedup reading pointer data

Jolt/AABBTree/AABBTreeBuilder.cpp

@@ -192,7 +192,7 @@ AABBTreeBuilder::Node *AABBTreeBuilder::BuildInternal(const TriangleSplitter::Ra
 		TriangleSplitter::Range left, right;
 		if (!mTriangleSplitter.Split(inTriangles, left, right))
 		{
-			JPH_IF_DEBUG(Trace("AABBTreeBuilder: Doing random split for %d triangles (max per node: %d)!", (int)inTriangles.Count(), mMaxTrianglesPerLeaf);)
+			JPH_IF_DEBUG(Trace("AABBTreeBuilder: Doing random split for %d triangles (max per node: %u)!", (int)inTriangles.Count(), mMaxTrianglesPerLeaf);)
 			int half = inTriangles.Count() / 2;
 			JPH_ASSERT(half > 0);
 			left = TriangleSplitter::Range(inTriangles.mBegin, inTriangles.mBegin + half);

Jolt/Core/LockFreeHashMap.inl

@@ -339,7 +339,7 @@ void LockFreeHashMap<Key, Value>::TraceStats() const
 		histogram[min(objects_in_bucket, cMaxPerBucket - 1)]++;
 	}
 
-	Trace("max_objects_per_bucket = %d, num_buckets = %d, num_objects = %d", max_objects_per_bucket, mNumBuckets, num_objects);
+	Trace("max_objects_per_bucket = %d, num_buckets = %u, num_objects = %d", max_objects_per_bucket, mNumBuckets, num_objects);
 
 	for (int i = 0; i < cMaxPerBucket; ++i)
 		if (histogram[i] != 0)

Jolt/Geometry/ConvexHullBuilder.cpp

@@ -408,7 +408,7 @@ ConvexHullBuilder::EResult ConvexHullBuilder::Initialize(int inMaxVertices, floa
 		Array<Vec3> positions_2d;
 		positions_2d.reserve(mPositions.size());
 		for (Vec3 v : mPositions)
-			positions_2d.push_back(Vec3(base1.Dot(v), base2.Dot(v), 0));
+			positions_2d.emplace_back(base1.Dot(v), base2.Dot(v), 0.0f);
 
 		// Build hull
 		Array<int> edges_2d;
@@ -1290,7 +1290,7 @@ void ConvexHullBuilder::GetCenterOfMassAndVolume(Vec3 &outCenterOfMass, float &o
 
 			// Update v2 for next triangle
 			v2 = v3;
-		} while (e != f->mFirstEdge);
+		}
 	}
 
 	// Calculate center of mass, fall back to average point in case there is no volume (everything is on a plane in this case)

Jolt/Geometry/EPAPenetrationDepth.h

@@ -201,7 +201,7 @@ public:
 		hull.DrawLabel("Build initial hull");
 #endif
 #ifdef JPH_EPA_PENETRATION_DEPTH_DEBUG
-		Trace("Init: num_points = %d", support_points.mY.size());
+		Trace("Init: num_points = %u", (uint)support_points.mY.size());
 #endif
 		hull.Initialize(0, 1, 2);
 		for (typename Points::size_type i = 3; i < support_points.mY.size(); ++i)

Jolt/ObjectStream/ObjectStreamIn.cpp

@@ -413,11 +413,11 @@ bool ObjectStreamIn::ReadPointerData(const RTTI *inRTTI, void **inPointer, int i
 		else
 		{
 			// Put pointer on the list to be resolved later on
-			mUnresolvedLinks.emplace_back();
-			mUnresolvedLinks.back().mPointer = inPointer;
-			mUnresolvedLinks.back().mRefCountOffset = inRefCountOffset;
-			mUnresolvedLinks.back().mIdentifier = identifier;
-			mUnresolvedLinks.back().mRTTI = inRTTI;
+			Link &link = mUnresolvedLinks.emplace_back();
+			link.mPointer = inPointer;
+			link.mRefCountOffset = inRefCountOffset;
+			link.mIdentifier = identifier;
+			link.mRTTI = inRTTI;
 		}
 
 		return true;

Jolt/Physics/Collision/Shape/ConvexHullShape.cpp

@@ -129,6 +129,7 @@ ConvexHullShape::ConvexHullShape(const ConvexHullShapeSettings &inSettings, Shap
 	for (BuilderFace *builder_face : builder_faces)
 	{
 		// Determine where the vertices go
+		JPH_ASSERT(mVertexIdx.size() <= 0xFFFF);
 		uint16 first_vertex = (uint16)mVertexIdx.size();
 		uint16 num_vertices = 0;
 
@@ -180,7 +181,7 @@ ConvexHullShape::ConvexHullShape(const ConvexHullShapeSettings &inSettings, Shap
 	// Test if GetSupportFunction can support this many points
 	if (mPoints.size() > cMaxPointsInHull)
 	{
-		outResult.SetError(StringFormat("Internal error: Too many points in hull (%d), max allowed %d", mPoints.size(), cMaxPointsInHull));
+		outResult.SetError(StringFormat("Internal error: Too many points in hull (%u), max allowed %d", (uint)mPoints.size(), cMaxPointsInHull));
 		return;
 	}
 
@@ -205,61 +206,58 @@ ConvexHullShape::ConvexHullShape(const ConvexHullShapeSettings &inSettings, Shap
 			return;
 		}
 
-		if (faces.size() > 1)
+		// Find the 3 normals that form the largest tetrahedron
+		// The largest tetrahedron we can get is ((1, 0, 0) x (0, 1, 0)) . (0, 0, 1) = 1, if the volume is only 5% of that,
+		// the three vectors are too coplanar and we fall back to using only 2 plane normals
+		float biggest_volume = 0.05f;
+		int best3[3] = { -1, -1, -1 };
+
+		// When using 2 normals, we get the two with the biggest angle between them with a minimal difference of 1 degree
+		// otherwise we fall back to just using 1 plane normal
+		float smallest_dot = Cos(DegreesToRadians(1.0f));
+		int best2[2] = { -1, -1 };
+
+		for (int face1 = 0; face1 < (int)faces.size(); ++face1)
 		{
-			// Find the 3 normals that form the largest tetrahedron
-			// The largest tetrahedron we can get is ((1, 0, 0) x (0, 1, 0)) . (0, 0, 1) = 1, if the volume is only 5% of that,
-			// the three vectors are too coplanar and we fall back to using only 2 plane normals
-			float biggest_volume = 0.05f;
-			int best3[3] = { -1, -1, -1 };
-
-			// When using 2 normals, we get the two with the biggest angle between them with a minimal difference of 1 degree
-			// otherwise we fall back to just using 1 plane normal
-			float smallest_dot = Cos(DegreesToRadians(1.0f));
-			int best2[2] = { -1, -1 };
-
-			for (int face1 = 0; face1 < (int)faces.size(); ++face1)
+			Vec3 normal1 = mPlanes[faces[face1]].GetNormal();
+			for (int face2 = face1 + 1; face2 < (int)faces.size(); ++face2)
 			{
-				Vec3 normal1 = mPlanes[faces[face1]].GetNormal();
-				for (int face2 = face1 + 1; face2 < (int)faces.size(); ++face2)
-				{
-					Vec3 normal2 = mPlanes[faces[face2]].GetNormal();
-					Vec3 cross = normal1.Cross(normal2);
+				Vec3 normal2 = mPlanes[faces[face2]].GetNormal();
+				Vec3 cross = normal1.Cross(normal2);
 
-					// Determine the 2 face normals that are most apart
-					float dot = normal1.Dot(normal2);
-					if (dot < smallest_dot)
-					{
-						smallest_dot = dot;
-						best2[0] = faces[face1];
-						best2[1] = faces[face2];
-					}
+				// Determine the 2 face normals that are most apart
+				float dot = normal1.Dot(normal2);
+				if (dot < smallest_dot)
+				{
+					smallest_dot = dot;
+					best2[0] = faces[face1];
+					best2[1] = faces[face2];
+				}
 
-					// Determine the 3 face normals that form the largest tetrahedron
-					for (int face3 = face2 + 1; face3 < (int)faces.size(); ++face3)
+				// Determine the 3 face normals that form the largest tetrahedron
+				for (int face3 = face2 + 1; face3 < (int)faces.size(); ++face3)
+				{
+					Vec3 normal3 = mPlanes[faces[face3]].GetNormal();
+					float volume = abs(cross.Dot(normal3));
+					if (volume > biggest_volume)
 					{
-						Vec3 normal3 = mPlanes[faces[face3]].GetNormal();
-						float volume = abs(cross.Dot(normal3));
-						if (volume > biggest_volume)
-						{
-							biggest_volume = volume;
-							best3[0] = faces[face1];
-							best3[1] = faces[face2];
-							best3[2] = faces[face3];
-						}
+						biggest_volume = volume;
+						best3[0] = faces[face1];
+						best3[1] = faces[face2];
+						best3[2] = faces[face3];
 					}
 				}
 			}
-
-			// If we didn't find 3 planes, use 2, if we didn't find 2 use 1
-			if (best3[0] != -1)
-				faces = { best3[0], best3[1], best3[2] };
-			else if (best2[0] != -1)
-				faces = { best2[0], best2[1] };
-			else
-				faces = { faces[0] };
 		}
 
+		// If we didn't find 3 planes, use 2, if we didn't find 2 use 1
+		if (best3[0] != -1)
+			faces = { best3[0], best3[1], best3[2] };
+		else if (best2[0] != -1)
+			faces = { best2[0], best2[1] };
+		else
+			faces = { faces[0] };
+
 		// Copy the faces to the points buffer
 		Point &point = mPoints[p];
 		point.mNumFaces = (int)faces.size();

Jolt/Physics/Collision/Shape/CylinderShape.cpp

@@ -59,12 +59,12 @@ static const std::vector<Vec3> sUnitCylinderTriangles = []() {
 		Vec3 b2 = cTopFace[(i + 1) % num_verts] + bottom_offset;
 
 		// Top
-		verts.push_back(Vec3(0.0f, 1.0f, 0.0f));
+		verts.emplace_back(0.0f, 1.0f, 0.0f);
 		verts.push_back(t1);
 		verts.push_back(t2);
 
 		// Bottom
-		verts.push_back(Vec3(0.0f, -1.0f, 0.0f));
+		verts.emplace_back(0.0f, -1.0f, 0.0f);
 		verts.push_back(b2);
 		verts.push_back(b1);
 

Jolt/Physics/Collision/Shape/MeshShape.cpp

@@ -234,6 +234,7 @@ void MeshShape::sFindActiveEdges(const MeshShapeSettings &inSettings, IndexedTri
 			}
 
 			JPH_ASSERT(false);
+			JPH_CRASH;
 			return ~uint(0);
 		}
 

Jolt/Physics/IslandBuilder.cpp

@@ -184,20 +184,20 @@ void IslandBuilder::ValidateIslands(uint32 inNumActiveBodies) const
 		// If the bodies in this link ended up in different groups we have a problem
 		if (mBodyLinks[mLinkValidation[i].mFirst].mIslandIndex != mBodyLinks[mLinkValidation[i].mSecond].mIslandIndex)
 		{
-			Trace("Fail: %d, %d", mLinkValidation[i].mFirst, mLinkValidation[i].mSecond);
-			Trace("Num Active: %d", inNumActiveBodies);
+			Trace("Fail: %u, %u", mLinkValidation[i].mFirst, mLinkValidation[i].mSecond);
+			Trace("Num Active: %u", inNumActiveBodies);
 
 			for (uint32 j = 0; j < mNumLinkValidation; ++j)
-				Trace("builder.Link(%d, %d);", mLinkValidation[j].mFirst, mLinkValidation[j].mSecond);
+				Trace("builder.Link(%u, %u);", mLinkValidation[j].mFirst, mLinkValidation[j].mSecond);
 
 			IslandBuilder tmp;
 			tmp.Init(inNumActiveBodies);
 			for (uint32 j = 0; j < mNumLinkValidation; ++j)
 			{
-				Trace("Link %d -> %d", mLinkValidation[j].mFirst, mLinkValidation[j].mSecond);
+				Trace("Link %u -> %u", mLinkValidation[j].mFirst, mLinkValidation[j].mSecond);
 				tmp.LinkBodies(mLinkValidation[j].mFirst, mLinkValidation[j].mSecond);
 				for (uint32 t = 0; t < inNumActiveBodies; ++t)
-					Trace("%d -> %d", t, (uint32)tmp.mBodyLinks[t].mLinkedTo);
+					Trace("%u -> %u", t, (uint32)tmp.mBodyLinks[t].mLinkedTo);
 			}
 
 			JPH_ASSERT(false, "IslandBuilder validation failed");

Jolt/Physics/Ragdoll/Ragdoll.cpp

@@ -449,7 +449,7 @@ void RagdollSettings::CalculateConstraintIndexToBodyIdxPair()
 		if (p.mToParent != nullptr)
 		{
 			int parent_joint_idx = mSkeleton->GetJoint(joint_idx).mParentJointIndex;
-			mConstraintIndexToBodyIdxPair.push_back(BodyIdxPair(parent_joint_idx, joint_idx));
+			mConstraintIndexToBodyIdxPair.emplace_back(parent_joint_idx, joint_idx);
 		}
 
 		++joint_idx;
@@ -457,7 +457,7 @@ void RagdollSettings::CalculateConstraintIndexToBodyIdxPair()
 
 	// Add additional constraints
 	for (const AdditionalConstraint &c : mAdditionalConstraints)
-		mConstraintIndexToBodyIdxPair.push_back(BodyIdxPair(c.mBodyIdx[0], c.mBodyIdx[1]));
+		mConstraintIndexToBodyIdxPair.emplace_back(c.mBodyIdx[0], c.mBodyIdx[1]);
 }
 
 Ragdoll::~Ragdoll()

Jolt/Physics/SoftBody/SoftBodyShape.cpp

@@ -218,7 +218,7 @@ int SoftBodyShape::GetTrianglesNext(GetTrianglesContext &ioContext, int inMaxTri
 
 Shape::Stats SoftBodyShape::GetStats() const
 {
-	return Stats(sizeof(this), (uint)mSoftBodyMotionProperties->GetFaces().size());
+	return Stats(sizeof(*this), (uint)mSoftBodyMotionProperties->GetFaces().size());
 }
 
 float SoftBodyShape::GetVolume() const

Jolt/Physics/StateRecorderImpl.cpp

@@ -34,7 +34,7 @@ void StateRecorderImpl::ReadBytes(void *outData, size_t inNumBytes)
 		if (memcmp(data, outData, inNumBytes) != 0)
 		{
 			// Mismatch, print error
-			Trace("Mismatch reading %d bytes", inNumBytes);
+			Trace("Mismatch reading %u bytes", (uint)inNumBytes);
 			for (size_t i = 0; i < inNumBytes; ++i)
 			{
 				int b1 = reinterpret_cast<uint8 *>(outData)[i];
@@ -79,7 +79,7 @@ bool StateRecorderImpl::IsEqual(StateRecorderImpl &inReference)
 		fail = inReference.mStream.get() != mStream.get();
 		if (fail)
 		{
-			Trace("Failed to properly recover state, different at offset %d!", i);
+			Trace("Failed to properly recover state, different at offset %d!", (int)i);
 			return false;
 		}
 	}

Samples/SamplesApp.cpp

@@ -2364,10 +2364,10 @@ void SamplesApp::StepPhysics(JobSystem *inJobSystem)
 	mStepNumber++;
 
 	// Print timing information
-	constexpr int cNumSteps = 60;
+	constexpr uint cNumSteps = 60;
 	if (mStepNumber % cNumSteps == 0)
 	{
-		Trace("Timing: %d, %d", mStepNumber / cNumSteps, mTotalTime.count() / cNumSteps);
+		Trace("Timing: %u, %llu", mStepNumber / cNumSteps, static_cast<unsigned long long>(mTotalTime.count()) / cNumSteps);
 		mTotalTime = chrono::microseconds(0);
 	}
 

Samples/Utils/ContactListenerImpl.cpp

@@ -25,7 +25,7 @@ ValidateResult ContactListenerImpl::OnContactValidate(const Body &inBody1, const
 	RVec3 contact_point = inBaseOffset + inCollisionResult.mContactPointOn1;
 	DebugRenderer::sInstance->DrawArrow(contact_point, contact_point - inCollisionResult.mPenetrationAxis.NormalizedOr(Vec3::sZero()), Color::sBlue, 0.05f);
 
-	Trace("Validate %d and %d result %d", inBody1.GetID().GetIndex(), inBody2.GetID().GetIndex(), (int)result);
+	Trace("Validate %u and %u result %d", inBody1.GetID().GetIndex(), inBody2.GetID().GetIndex(), (int)result);
 
 	return result;
 }
@@ -36,7 +36,7 @@ void ContactListenerImpl::OnContactAdded(const Body &inBody1, const Body &inBody
 	if (!(inBody1.GetID() < inBody2.GetID()))
 		JPH_BREAKPOINT;
 
-	Trace("Contact added %d (%08x) and %d (%08x)", inBody1.GetID().GetIndex(), inManifold.mSubShapeID1.GetValue(), inBody2.GetID().GetIndex(), inManifold.mSubShapeID2.GetValue());
+	Trace("Contact added %u (%08x) and %u (%08x)", inBody1.GetID().GetIndex(), inManifold.mSubShapeID1.GetValue(), inBody2.GetID().GetIndex(), inManifold.mSubShapeID2.GetValue());
 
 	DebugRenderer::sInstance->DrawWirePolygon(RMat44::sTranslation(inManifold.mBaseOffset), inManifold.mRelativeContactPointsOn1, Color::sGreen, 0.05f);
 	DebugRenderer::sInstance->DrawWirePolygon(RMat44::sTranslation(inManifold.mBaseOffset), inManifold.mRelativeContactPointsOn2, Color::sGreen, 0.05f);
@@ -61,7 +61,7 @@ void ContactListenerImpl::OnContactPersisted(const Body &inBody1, const Body &in
 	if (!(inBody1.GetID() < inBody2.GetID()))
 		JPH_BREAKPOINT;
 
-	Trace("Contact persisted %d (%08x) and %d (%08x)", inBody1.GetID().GetIndex(), inManifold.mSubShapeID1.GetValue(), inBody2.GetID().GetIndex(), inManifold.mSubShapeID2.GetValue());
+	Trace("Contact persisted %u (%08x) and %u (%08x)", inBody1.GetID().GetIndex(), inManifold.mSubShapeID1.GetValue(), inBody2.GetID().GetIndex(), inManifold.mSubShapeID2.GetValue());
 
 	DebugRenderer::sInstance->DrawWirePolygon(RMat44::sTranslation(inManifold.mBaseOffset), inManifold.mRelativeContactPointsOn1, Color::sYellow, 0.05f);
 	DebugRenderer::sInstance->DrawWirePolygon(RMat44::sTranslation(inManifold.mBaseOffset), inManifold.mRelativeContactPointsOn2, Color::sYellow, 0.05f);
@@ -88,7 +88,7 @@ void ContactListenerImpl::OnContactRemoved(const SubShapeIDPair &inSubShapePair)
 	if (!(inSubShapePair.GetBody1ID() < inSubShapePair.GetBody2ID()))
 		JPH_BREAKPOINT;
 
-	Trace("Contact removed %d (%08x) and %d (%08x)", inSubShapePair.GetBody1ID().GetIndex(), inSubShapePair.GetSubShapeID1().GetValue(), inSubShapePair.GetBody2ID().GetIndex(), inSubShapePair.GetSubShapeID2().GetValue());
+	Trace("Contact removed %u (%08x) and %u (%08x)", inSubShapePair.GetBody1ID().GetIndex(), inSubShapePair.GetSubShapeID1().GetValue(), inSubShapePair.GetBody2ID().GetIndex(), inSubShapePair.GetSubShapeID2().GetValue());
 
 	// Update existing manifold in state map
 	{