From 05a2d36d0e4c5365ff98279c0f2f69d671e570b0 Mon Sep 17 00:00:00 2001 From: Daniel Gibson Date: Thu, 5 Jan 2023 06:21:07 +0100 Subject: [PATCH] Fix GCC -W(maybe-)uninitialized warnings that at least kinda had a point --- idlib/geometry/Surface_Patch.cpp | 7 ++++++- idlib/geometry/Winding.cpp | 15 +++++++++++++-- idlib/geometry/Winding2D.cpp | 11 +++++++++++ idlib/math/Simd_SSE.cpp | 8 ++++++++ 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/idlib/geometry/Surface_Patch.cpp b/idlib/geometry/Surface_Patch.cpp index 6f6f744..f898c50 100644 --- a/idlib/geometry/Surface_Patch.cpp +++ b/idlib/geometry/Surface_Patch.cpp @@ -224,6 +224,7 @@ idSurface_Patch::LerpVert ============ */ void idSurface_Patch::LerpVert( const idDrawVert &a, const idDrawVert &b, idDrawVert &out ) const { + // DG: TODO: what about out.tangent and out.color ? out.xyz[0] = 0.5f * ( a.xyz[0] + b.xyz[0] ); out.xyz[1] = 0.5f * ( a.xyz[1] + b.xyz[1] ); out.xyz[2] = 0.5f * ( a.xyz[2] + b.xyz[2] ); @@ -554,7 +555,11 @@ idSurface_Patch::Subdivide */ void idSurface_Patch::Subdivide( float maxHorizontalError, float maxVerticalError, float maxLength, bool genNormals ) { int i, j, k, l; - idDrawVert prev, next, mid; + // DG: to shut up GCC (maybe-)uninitialized warnings, initialize prev, next and mid + // (maybe the warnings were at least partly correct, because .tangent and .color aren't set by idSurface_Patch::LerpVert()) + idDrawVert prev; + prev.Clear(); + idDrawVert next = prev, mid = prev; idVec3 prevxyz, nextxyz, midxyz; idVec3 delta; float maxHorizontalErrorSqr, maxVerticalErrorSqr, maxLengthSqr; diff --git a/idlib/geometry/Winding.cpp b/idlib/geometry/Winding.cpp index 5f38423..2def85f 100644 --- a/idlib/geometry/Winding.cpp +++ b/idlib/geometry/Winding.cpp @@ -102,7 +102,12 @@ int idWinding::Split( const idPlane &plane, const float epsilon, idWinding **fro idWinding * f, *b; int maxpts; - assert( this ); + assert( this && numPoints > 0); + + // DG: unlikely, but makes sure we don't use uninitialized memory below + if ( numPoints == 0 ) { + return 0; // it's not like the callers check the return value anyway.. + } dists = (float *) _alloca( (numPoints+4) * sizeof( float ) ); sides = (byte *) _alloca( (numPoints+4) * sizeof( byte ) ); @@ -245,7 +250,13 @@ idWinding *idWinding::Clip( const idPlane &plane, const float epsilon, const boo idVec5 mid; int maxpts; - assert( this ); + assert( this && numPoints > 0 ); + + // DG: this shouldn't happen, probably, but if it does we'd use uninitialized memory below + if ( numPoints == 0 ) { + delete this; + return NULL; + } dists = (float *) _alloca( (numPoints+4) * sizeof( float ) ); sides = (byte *) _alloca( (numPoints+4) * sizeof( byte ) ); diff --git a/idlib/geometry/Winding2D.cpp b/idlib/geometry/Winding2D.cpp index 8978f1d..bb02e6d 100644 --- a/idlib/geometry/Winding2D.cpp +++ b/idlib/geometry/Winding2D.cpp @@ -92,6 +92,12 @@ void idWinding2D::ExpandForAxialBox( const idVec2 bounds[2] ) { assert( numPlanes < MAX_POINTS_ON_WINDING_2D ); planes[numPlanes++] = plane; } + + // DG: make sure planes[] isn't used uninitialized and with index -1 below + if ( numPlanes == 0 ) { + return; + } + if ( GetAxialBevel( planes[numPlanes-1], planes[0], p[0], bevel ) ) { planes[numPlanes++] = bevel; } @@ -259,6 +265,11 @@ bool idWinding2D::ClipInPlace( const idVec3 &plane, const float epsilon, const b float dot, dists[MAX_POINTS_ON_WINDING_2D+1]; idVec2 *p1, *p2, mid, newPoints[MAX_POINTS_ON_WINDING_2D+4]; + // DG: avoid all kinds of unitialized usages below + if ( numPoints == 0 ) { + return false; + } + counts[SIDE_FRONT] = counts[SIDE_BACK] = counts[SIDE_ON] = 0; for ( i = 0; i < numPoints; i++ ) { diff --git a/idlib/math/Simd_SSE.cpp b/idlib/math/Simd_SSE.cpp index 609cb59..a9dad8b 100644 --- a/idlib/math/Simd_SSE.cpp +++ b/idlib/math/Simd_SSE.cpp @@ -456,6 +456,14 @@ void VPCALL idSIMD_SSE::Dot( float *dst, const idVec3 &constant, const idPlane * char *dst_p; __m128 xmm0, xmm1, xmm2, xmm3, xmm4, xmm5, xmm6, xmm7; + // DG: GCC and clang warn about xmm1-4 maybe being used uninitialized below. + // according to https://stackoverflow.com/a/18749079 the initialization + // code is generated anyway, so make it explicit to shut up the warning + xmm1 = _mm_setzero_ps(); + xmm2 = _mm_setzero_ps(); + xmm3 = _mm_setzero_ps(); + xmm4 = _mm_setzero_ps(); + /* mov eax, count mov edi, constant