GCC Wstringop-overflow warnings

[JPeterMugaas]

I compiled UVAtlas with GNU 13.2.0 and I got some Wstringop-overflow warnings:

C:/msys64/home/jpmugaas/uvatlas-PKGBUILD/src/uvatlas-oct2023/UVAtlas/isochart/meshapplyisomap.cpp: In member function 'HRESULT Isochart::CIsochartMesh::CalculateGeodesicDistanceToVertexKS98(uint32_t, bool, uint32_t*) const':
C:/msys64/home/jpmugaas/uvatlas-PKGBUILD/src/uvatlas-oct2023/UVAtlas/isochart/meshapplyisomap.cpp:565:11: warning: 'void* memset(void*, int, size_t)' specified size between 9223372036854775809 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
  565 |     memset(pbVertProcessed.get(), 0, sizeof(bool) * m_dwVertNumber);
      |     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[20/25] Building CXX object CMakeFiles/UVAtlas.dir/UVAtlas/isochart/isochartmesh.cpp.obj
C:/msys64/home/jpmugaas/uvatlas-PKGBUILD/src/uvatlas-oct2023/UVAtlas/isochart/isochartmesh.cpp: In member function 'HRESULT Isochart::CIsochartMesh::CalculateDijkstraPathToVertex(uint32_t, uint32_t*) const':
C:/msys64/home/jpmugaas/uvatlas-PKGBUILD/src/uvatlas-oct2023/UVAtlas/isochart/isochartmesh.cpp:3350:11: warning: 'void* memset(void*, int, size_t)' specified size between 9223372036854775809 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
 3350 |     memset(pbVertProcessed, 0, sizeof(bool) * m_dwVertNumber);
      |     ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

From the GCC manual:

Warn for calls to string manipulation functions such as memcpy and strcpy that are determined to overflow the destination buffer. The optional argument is one greater than the type of Object Size Checking to perform to determine the size of the destination. See Object Size Checking. The argument is meaningful only for functions that operate on character arrays but not for raw memory functions like memcpy which always make use of Object Size type-0. The option also warns for calls that specify a size in excess of the largest possible object or at most SIZE_MAX / 2 bytes. The option produces the best results with optimization enabled but can detect a small subset of simple buffer overflows even without optimization in calls to the GCC built-in functions like __builtin_memcpy that correspond to the standard functions. In any case, the option warns about just a subset of buffer overflows detected by the corresponding overflow checking built-ins.

[walbourn]

I've done a lot of recent updates for CodeQL warnings, so please retry with the latest version (I.e. the main branch) to see if this one is fixed.

[JPeterMugaas]

I tried the main branch but still got the warnings.

[walbourn]

If I understand correctly here, it's complaining about the difference between a uint64_t and an int64_t.

I don't know how it got there. m_dwVertNumber is size_t. memset takes a size_t per the standard. I thought size_t was always unsigned and ptrdiff_t was always signed. Per the standard, I thought sizeof returns size_t, but I wonder if GNU is assuming it's int?

What's strange is the same pattern is used elsewhere and it doesn't complain elsewhere...

[walbourn]

I just tried building with MinGW-w64 13.2.0 r4 and I'm not seeing these warnings.

[walbourn]

OK, so they only show up in release builds. Not debug builds.

lto-wrapper.exe: warning: using serial compilation of 9 LTRANS jobs
lto-wrapper.exe: note: see the '-flto' option documentation for more information
In member function 'CalculateDijkstraPathToVertex',
    inlined from 'CaculateDistanceToExtremeVertex' at D:/Microsoft/UVAtlas/UVAtlas/isochart/meshpartitionchart.cpp:1840:9,
    inlined from 'CheckCylinderLonghornShape' at D:/Microsoft/UVAtlas/UVAtlas/isochart/meshpartitionchart.cpp:1740:5,
    inlined from 'ProcessSpecialShape' at D:/Microsoft/UVAtlas/UVAtlas/isochart/meshpartitionchart.cpp:1640:5,
    inlined from 'Partition' at D:/Microsoft/UVAtlas/UVAtlas/isochart/isochartmesh.cpp:797:29,
    inlined from '_ZN8Isochart15CIsochartEngine36ParameterizeChartsInHeapParallelizedEby._omp_fn.0' at D:/Microsoft/UVAtlas/UVAtlas/isochart/isochartengine.cpp:322:47:
D:/Microsoft/UVAtlas/UVAtlas/isochart/isochartmesh.cpp:3350:11: warning: '__builtin_memset' specified size between 9223372036854775809 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
 3350 |     memset(pbVertProcessed, 0, sizeof(bool) * m_dwVertNumber);
      |           ^
In member function 'CalculateGeodesicDistanceToVertexKS98',
    inlined from 'CalculateGeodesicDistanceToVertex' at D:/Microsoft/UVAtlas/UVAtlas/isochart/meshapplyisomap.cpp:475:46,
    inlined from 'CalculateGeodesicDistance' at D:/Microsoft/UVAtlas/UVAtlas/isochart/meshapplyisomap.cpp:291:13:
D:/Microsoft/UVAtlas/UVAtlas/isochart/meshapplyisomap.cpp:565:11: warning: '__builtin_memset' specified size between 9223372036854775809 and 18446744073709551615 exceeds maximum object size 9223372036854775807 [-Wstringop-overflow=]
  565 |     memset(pbVertProcessed.get(), 0, sizeof(bool) * m_dwVertNumber);
      |           ^
D:/Microsoft/UVAtlas/UVAtlas/isochart/meshpartitionchart.cpp: In member function 'GenerateAllSubCharts':
D:/Microsoft/UVAtlas/UVAtlas/isochart/meshpartitionchart.cpp:32:119: warning: argument 1 value '18446744073709551615' exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
   32 |     std::unique_ptr<std::vector<uint32_t>[]> chartFaceList(new (std::nothrow) std::vector<uint32_t>[dwMaxSubchartCount]);
      |
      ^
C:/mingw64/include/c++/13.2.0/new:142:26: note: in a call to allocation function 'operator new []' declared here
  142 | _GLIBCXX_NODISCARD void* operator new[](std::size_t, const std::nothrow_t&) _GLIBCXX_USE_NOEXCEPT

[walbourn]

According to some research here, GCC is actually complaining if it exceeds PTRDIFF_MAX - 1 which is the signed int vs. size_t which is an unsigned int.

It looks like a number of people feel this warning is spurious. Still, I'll see if I can make it happy. What I really don't understand is why it only complains in SOME of the cases which are all basically identical.

[JPeterMugaas]

There is a page in the GCC manual about warning options at: https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html .

I have to be honest in saying that I do feel that this is a spurious warning and from a practical point of view, it might be best to use a "hacky" but somewhat elegant fix for it that will work. The for loop I tied does work and I think SecureZeroMemory will also work if it's ifdefed out for Linux.