DGA-MI-SSI / YaCo

YaCo is an Hex-Rays IDA plugin. When enabled, multiple users can work simultaneously on the same binary. Any modification done by any user is synchronized through git version control.
GNU General Public License v3.0
313 stars 36 forks source link

Fix compilation issues on Ubuntu #16

Closed kamino closed 6 years ago

goulou commented 7 years ago

The first commit seems OK. About the 2nd one I'm skeptical though : beware that assert might be not compiled in Release mode (and the chdir might not be called).

bamiaux commented 7 years ago

First commit is done already in https://github.com/bamiaux/YaCo/commit/cde2632ec15ae52c7827589f2eb4dcc9119a1466

bamiaux commented 7 years ago

-Wunused-result is not enabled by default, did you customize CFLAGS ? I think it's a very verbose warning, and potentially invalid, for example, the result of snprintf just a few lines before your patch is unused, is there not a warning for this ? I believe this warning category should be ignored

bamiaux commented 6 years ago

ping

kamino commented 6 years ago

Sorry for the late answer, I did not see that you guys have answered. I tried to build my commit 2106ec8, thus, before fixing -Wunused-result related error on an Ubuntu 16.10:

[ 97%] Building CXX object CMakeFiles/yatools_tests.dir/home/xxx/YaCo/YaLibs/tests/YaToolsLib_test/test_DatabaseModel.cpp.o
In file included from /home/xxx/YaCo/YaLibs/tests/YaToolsLib_test/XML/test_XMLDatabaseModel.cpp:25:0:
/home/xxx/YaCo/YaLibs/tests/test_common.hpp: In member function ‘virtual void TestInTempFolder::TearDown()’:
/home/xxx/YaCo/YaLibs/tests/test_common.hpp:46:40: error: ignoring return value of ‘int chdir(const char*)’, declared with attribute warn_unused_result [-Werror=unused-result]
         chdir(orig_dir.string().data());
                                        ^
/home/xxx/YaCo/YaLibs/tests/test_common.hpp: In member function ‘virtual void TestInTempFolder::SetUp()’:
/home/xxx/YaCo/YaLibs/tests/test_common.hpp:41:43: error: ignoring return value of ‘int chdir(const char*)’, declared with attribute warn_unused_result [-Werror=unused-result]
         chdir(current_dir.string().data());
                                           ^
[ 97%] Building CXX object CMakeFiles/yatools_tests.dir/home/xxx/YaCo/YaLibs/tests/YaToolsLib_test/test_PrototypeParser.cpp.o
[ 97%] Building CXX object CMakeFiles/yatools_tests.dir/home/xxx/YaCo/YaLibs/tests/YaToolsLib_test/test_YaToolObjectVersions.cpp.o
[ 97%] Building CXX object CMakeFiles/yaida32.dir/home/xxx/YaCo/YaLibs/YaToolsIDALib/YaToolsHashProvider.cpp.o
cc1plus: all warnings being treated as errors
CMakeFiles/yatools_tests.dir/build.make:110: recipe for target 'CMakeFiles/yatools_tests.dir/home/xxx/YaCo/YaLibs/tests/YaToolsLib_test/XML/test_XMLDatabaseModel.cpp.o' failed
make[2]: *** [CMakeFiles/yatools_tests.dir/home/xxx/YaCo/YaLibs/tests/YaToolsLib_test/XML/test_XMLDatabaseModel.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
[ 97%] Building CXX object CMakeFiles/yaida32.dir/home/xxx/YaCo/YaLibs/YaToolsIDALib/YaToolsIDANativeLib.cpp.o
In file included from /home/xxx/YaCo/YaLibs/tests/YaToolsLib_test/test_YaToolObjectVersions.cpp:26:0:
/home/xxx/YaCo/YaLibs/tests/test_common.hpp: In member function ‘virtual void TestInTempFolder::TearDown()’:
/home/xxx/YaCo/YaLibs/tests/test_common.hpp:46:40: error: ignoring return value of ‘int chdir(const char*)’, declared with attribute warn_unused_result [-Werror=unused-result]
         chdir(orig_dir.string().data());
                                        ^
/home/xxx/YaCo/YaLibs/tests/test_common.hpp: In member function ‘virtual void TestInTempFolder::SetUp()’:
/home/xxx/YaCo/YaLibs/tests/test_common.hpp:41:43: error: ignoring return value of ‘int chdir(const char*)’, declared with attribute warn_unused_result [-Werror=unused-result]
         chdir(current_dir.string().data());
                                           ^
[ 97%] cleaning up temp directory
Scanning dependencies of target yagit_tests
[ 97%] Building CXX object CMakeFiles/yagit_tests.dir/home/xxx/YaCo/YaLibs/tests/YaGitLib_test/test_YaGitLib.cpp.o
[ 97%] Linking CXX static library libyaida32.a
[ 97%] Built target yaida32
Scanning dependencies of target yafb2xml
[ 97%] Building CXX object CMakeFiles/yafb2xml.dir/home/xxx/YaCo/YaToolsUtils/YaToolsFBToXML/FBToXML.cpp.o
In file included from /home/xxx/YaCo/YaLibs/tests/YaGitLib_test/test_YaGitLib.cpp:22:0:
/home/xxx/YaCo/YaLibs/tests/test_common.hpp: In member function ‘virtual void TestInTempFolder::TearDown()’:
/home/xxx/YaCo/YaLibs/tests/test_common.hpp:46:40: error: ignoring return value of ‘int chdir(const char*)’, declared with attribute warn_unused_result [-Werror=unused-result]
         chdir(orig_dir.string().data());
                                        ^
/home/xxx/YaCo/YaLibs/tests/test_common.hpp: In member function ‘virtual void TestInTempFolder::SetUp()’:
/home/xxx/YaCo/YaLibs/tests/test_common.hpp:41:43: error: ignoring return value of ‘int chdir(const char*)’, declared with attribute warn_unused_result [-Werror=unused-result]
         chdir(current_dir.string().data());
                                           ^
cc1plus: all warnings being treated as errors
CMakeFiles/yatools_tests.dir/build.make:182: recipe for target 'CMakeFiles/yatools_tests.dir/home/xxx/YaCo/YaLibs/tests/YaToolsLib_test/test_YaToolObjectVersions.cpp.o' failed
make[2]: *** [CMakeFiles/yatools_tests.dir/home/xxx/YaCo/YaLibs/tests/YaToolsLib_test/test_YaToolObjectVersions.cpp.o] Error 1

There is no warning for the sprintf you are mentioning before. The command test_YaGitLib.cpp executed to compile is:

/usr/bin/c++   -DARCH_X86 -I/home/xxx/YaCo/YaLibs/tests -I/home/xxx/YaCo/deps/gtest-1.7.0/include -I/home/xxx/YaCo/YaLibs/YaGitLib -I/home/xxx/YaCo/deps/libgit2-0.25.1/src -I/home/xxx/YaCo/deps/libgit2-0.25.1/include -I/home/xxx/YaCo/deps/libgit2-0.25.1/deps/http-parser -I/home/xxx/YaCo/deps/libssh2-1.8.0/include -I/home/xxx/YaCo/deps/mbedtls-2.4.2/include -I/home/xxx/YaCo/deps/libgit2-0.25.1/deps/zlib -I/home/xxx/YaCo/deps/libgit2-0.25.1/deps/regex  -m32 -pg -O3 -DNDEBUG   -Wall -Wextra -Werror -std=gnu++14 -o CMakeFiles/yagit_tests.dir/home/xxx/YaCo/YaLibs/tests/YaGitLib_test/test_YaGitLib.cpp.o -c /home/xxx/YaCo/YaLibs/tests/YaGitLib_test/test_YaGitLib.cpp

The option -Werror comes from build/common.cmake.

As for @goulou's remark, I've looked at gtest's documentation/source code I did not find any mention of ASSERT_EQ not triggering if in release mode. EXPECT_EQ can probably be an alternative or create a dummy variable just to remove that warning.

Please note that I've tested on the latest commit and it still fails.

bamiaux commented 6 years ago

I don't get those warnings with the linux distro i'm using. I'll make an ubuntu vm soon & fix this

bamiaux commented 6 years ago

Sorry about the delay, I've rebased & pushed your commit about chdir (https://github.com/DGA-MI-SSI/YaCo/commit/44345311854437b2a7ac35fe8d37114634ce771e) The other one is not needed anymore. It compiles cleanly on ubuntu now. It looks like chdir have more annotations on ubuntu compared to debian

kamino commented 6 years ago

Thanks a lot 👍