facebookresearch / ocean

Ocean is the in-house framework for Computer Vision (CV) and Augmented Reality (AR) applications at Meta. It is platform independent and is mainly implemented in C/C++.
https://facebookresearch.github.io/ocean/
MIT License
641 stars 58 forks source link

Assimp build error with gcc-14 #12

Closed theartful closed 4 months ago

theartful commented 4 months ago

Instructions to reproduce the problem:

When building the third party libraries I get the error

In member function ‘bool aiString::operator==(const aiString&) const’,
    inlined from ‘void Assimp::ObjFileParser::getMaterialDesc()’ at /home/theartful/build_ocean_thirdparty/third-party_cache/assimp/33d84aa4f2c2ba4a1a48bea47ef7749ccc9b9941/code/AssetLib/Obj/ObjFileParser.cpp:587:80:
/home/theartful/build_ocean_thirdparty/third-party_cache/assimp/33d84aa4f2c2ba4a1a48bea47ef7749ccc9b9941/include/assimp/types.h:347:54: error: ‘<anonymous>’ may be used uninitialized [-Werror=maybe-uninitialized]
  347 |         return (length == other.length && 0 == memcmp(data, other.data, length));
      |                                                ~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
<built-in>: In member function ‘void Assimp::ObjFileParser::getMaterialDesc()’:
<built-in>: note: by argument 2 of type ‘const void*’ to ‘int __builtin_memcmp_eq(const void*, const void*, long unsigned int)’ declared here
/home/theartful/build_ocean_thirdparty/third-party_cache/assimp/33d84aa4f2c2ba4a1a48bea47ef7749ccc9b9941/code/AssetLib/Obj/ObjFileParser.cpp:587:99: note: ‘<anonymous>’ declared here
  587 |     if (m_pModel->mCurrentMaterial && m_pModel->mCurrentMaterial->MaterialName == aiString(strName)) {
      |

Expected behavior:

Should build as expected.

Suggested solution:

The assimp developers fixed the issue in https://github.com/assimp/assimp/pull/5593, so we can either update assimp to 5.4.2, or we can pull their patch into assimp.patch, or maybe disable Werror for assimp (should be disabled for third party libraries in general imo).

enpe commented 4 months ago

Thanks for reporting this! And for suggesting a potential solution! We'll take a closer look at this.

enpe commented 4 months ago

@theartful, on what OS did you run into this problem?

theartful commented 4 months ago

@enpe I'm running archlinux, which is a hassle I know since it's a rolling release, and so I get these problems with newer versions of stuff. It's a compiler issue though with gcc-14.

enpe commented 4 months ago

@theartful, and did you confirm it builds fine again with assimp 5.4.2? You can test it here:

https://github.com/facebookresearch/ocean/blob/8c31cdaa859ad74dae5dee240a2d55679e91bfff/build/cmake/third-party/assimp.cmake#L35

theartful commented 4 months ago

@enpe Yep, I did. I updated assimp version locally, and all dependencies built fine, and as far as I know the newer changes only introduces bug fixes, so it should not be breaking.

I'm still having other build problems, so I couldn't run the test suite.

leapally commented 4 months ago

I have repro'd the bug with gcc-14.

Options "-Wall" and "-Werror" are coming from assimp configuration: https://github.com/assimp/assimp/blob/206839d4f23162fb515010d5d93a21e1bbde5c50/code/CMakeLists.txt#L1274

Haven't found Ocean config that adds these if they exist somewhere. I guess we could override their config, but going to try 5.4.2 first to see if that works.

leapally commented 4 months ago

This commit updates dependency on assimp to version v5.4.2. Build succeeds on x86-64 Linux (GCC 13/14, Clang 18), x86-64 Windows (MSVC 19.39), and Apple Silicon MacOS(Apple Clang 15).