Bug: Fails to build with GCC 11 because warnings treated as errors

[turol]

Debian testing with GCC 11.2.0:

assimp/code/AssetLib/M3D/m3d.h:4267:46: error: array subscript ‘uint64_t {aka long unsigned int}[0]’ is partly outside array bounds of ‘unsigned char [4]’ [-Werror=array-bounds]
assimp/code/AssetLib/M3D/m3d.h:3097:43: note: while referencing ‘weights’

assimp/code/AssetLib/M3D/m3d.h

Line 4267 in a83db61

*((uint64_t *)out) = *((uint64_t *)&weights[0]);

assimp/code/AssetLib/glTF/glTFAsset.inl:1185:29: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]

assimp/code/AssetLib/glTF/glTFAsset.inl

Line 1185 in a83db61

sceneData[mSceneLength] = '\0';

This should be a release blocker for 5.1.0. These need to either be fixed or warnings as errors mode disabled.

[RichardTea]

The glTF "bug" is semi-bogus. The only way sceneData can ever be of size zero is if mSceneLength is equal to std::numeric_limits<size_t>::max(). That can only happen if ftell (or equivalent) came back with that value, as the GLB format is a uint32_t.

It'd crash either way though, no way can you allocate the many terabytes that implies...

The array is explicitly created at a size mSceneLength + 1, then the char at index mSceneLength is assigned the value zero to ensure it will always be null-terminated for rapidjson.

Eg in the test utglTFImportExport.importglTFFromFileTest, the sceneData is created at length 6855 bytes, then the value at index 6854 is assigned zero.

That said, the next line could assert as some of the IOStream classes will assert for zero count - I guess that is a bug, as fread returns zero for zero item size or zero item count so all the IOStreams should too.

[RichardTea]

What would you suggest is a "sane" maximum size?

I'd guess at ~4GB (max. uint32_t)

[RichardTea]

The m3d format is 3rd party code- https://gitlab.com/bztsrc/model3d
There's quite a few issues with it that some toolchains will warn about.

I've had a look at upstream and it seems bzt has made quite a few changes since they parachuted their code into assimp.

I would recommend disabling m3d by default for the 5.1.0 release to get it out of the door, then start work on syncing to upstream.

[kimkulling]

I will disable it for 5.1 until we have a fix.

[turol]

You should add a way to disable warnings as errors and then disable it in releases by default. Otherwise the next new compiler will probably also fail to build because of new warnings.