GCC and -Werror=alloc-size-larger-than error

[cwoffenden]

trafficstars

Building the latest (c4cfc35) with GCC 10.2.1-6 on Debian Bullseye fails with multiples of this error:

In member function ‘allocate’,
    inlined from ‘meshopt_optimizeOverdraw’ at /home/carl/Work/Native/Obj2Buf/src/overdrawoptimizer.cpp:294:67,
    inlined from ‘main’ at /home/carl/Work/Native/Obj2Buf/src/main.cpp:195:29:
/home/carl/Work/Native/Obj2Buf/inc/meshoptimizer.h:739:48: error: argument 1 value ‘18446744073709551615’ exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
  739 |   T* result = static_cast<T*>(Storage::allocate(size > size_t(-1) / sizeof(T) ? size_t(-1) : size * sizeof(T)));

This is on a Power9 CPU, and size_t is 64-bit as expected.

The fix was to use PTRDIFF_MAX as the upper limit (which matches the signed 9223372036854775807 maximum object size). Whilst it seems extreme to be limiting to half the size of size_t, even on a 32-bit system this is surely larger that any workable buffer size?

Thoughts? Besides no-one has a Power9 and real men have Power10 these days...

[zeux]

This warning is a little unfortunate. The goal of this code is to pass the size that can't possibly be allocated on any system to convert a numeric overflow during multiplication into an allocation error (and do this in a way that isn't relying on the specifics of the error handling during allocation failure, eg without actually spelling out throw std::bad_alloc in case the allocation handler uses a different strategy).

PTRDIFF_MAX is technically possible to allocate... I doubt any system actually allows this, but still. Let me look into this a little bit since I'm wondering if there's some other way to silence this warning.

[zeux]

Curiously I can't reproduce this on g++-10 (Ubuntu 10.3.0-15ubuntu1) 10.3.0 with the following command line:

g++-10 src/overdrawoptimizer.cpp -g -Wall -Wextra -Wshadow -Wno-missing-field-initializers -Werror -std=c++98 -Walloc-size-larger-than=9223372036854775807 -O3 -DNDEBUG -c -MMD -MP -o build/release/src/overdrawoptimizer.cpp.o

and I'm not sure where the difference is coming from - this is on x64, but I would think the warning behavior would be the same.

[cwoffenden]

Testing your exact command line doesn't produce the error. A little further testing: GCC 10 and 11 error as above, GCC 12 doesn't but creates a new related error:

/home/carl/Work/Native/Obj2Buf/src/overdrawoptimizer.cpp:288:23: error: ‘memcpy’ specified bound between 9223372036854775808 and 18446744073709551612 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
  288 |                 memcpy(indices_copy, indices, index_count * sizeof(unsigned int));
      |    

Passing index_count as an unsigned int (or simply casting) keeps the compiler happy.

Clang 11 on the same system is fine.

I will investigate further.

[zeux]

Does the warning for memcpy also get traced to main.cpp and mention inlined functions? I'm wondering if the root of the issue is the calling program, where for example the index/vertex count are signed and GCC thinks they might be negative.

[cwoffenden]

Yes, the error goes back to the inline from here:

In function ‘meshopt_optimizeOverdraw’,
    inlined from ‘process’ at /home/carl/Work/Native/Obj2Buf/src/main.cpp:195:29,
    inlined from ‘main’ at /home/carl/Work/Native/Obj2Buf/src/main.cpp:219:10:
/home/carl/Work/Native/Obj2Buf/src/overdrawoptimizer.cpp:288:23: error: ‘memcpy’ specified size between 9223372036854775808 and 18446744073709551612 exceeds maximum object size 9223372036854775807 [-Werror=stringop-overflow=]
  288 |                 memcpy(indices_copy, indices, index_count * sizeof(unsigned int));
      |                       ^

The calling line being:

meshopt_optimizeOverdraw(mesh.index.data(), mesh.index.data(), mesh.index.size(), mesh.verts[0].posn, mesh.verts.size(), sizeof(ObjVertex), 1.01f);

And mesh.index being a std::vector<unsigned>.

Now, if I limit how big the index buffer can get, either by only passing in a uint32_t or by limiting the size otherwise, then it works fine.

index.resize(std::max(numIndices, (size_t)UINT32_MAX));

I'll need to look at GCC on Intel to compare whether this is a Power9 particularity or not. The project will be public on GitHub shortly (it's not very interesting, it's just a tool I made to dump out buffers for quick demos), but this part differs little from what your demo/main.cpp is doing, and if I change your code to mirror mine it oddly doesn't fail (as in: the vector resize is done in a single function, which is about the only difference).

I'll come back to this next week with more data when I have more time.

[cwoffenden]

Closing this. It's not reproducible standalone in meshopt, and looks to be over eager static analysis which isn't triggered in newer GCC versions.