Build fails with "fatal error: filesystem: No such file or directory #include <filesystem>" with GCC 7

[bannsec]

When building this, I get an error:

[ 12%] Built target retdec-project
[ 15%] Built target annotations
[ 92%] Built target retdec
[ 93%] Building CXX object src/CMakeFiles/core_retdec.dir/core_plugin.cpp.o
/home/angr/opt/retdec-r2plugin/src/core_plugin.cpp:9:10: fatal error: filesystem: No such file or directory
 #include <filesystem>
          ^~~~~~~~~~~~
compilation terminated.
src/CMakeFiles/core_retdec.dir/build.make:79: recipe for target 'src/CMakeFiles/core_retdec.dir/core_plugin.cpp.o' failed
make[2]: *** [src/CMakeFiles/core_retdec.dir/core_plugin.cpp.o] Error 1
CMakeFiles/Makefile2:254: recipe for target 'src/CMakeFiles/core_retdec.dir/all' failed
make[1]: *** [src/CMakeFiles/core_retdec.dir/all] Error 2
Makefile:146: recipe for target 'all' failed
make: *** [all] Error 2

I've tried switching it to experimental/filesystem but that just gives other errors.

My gcc is version 7. It appears that filesystem here requires 8 or some modification to how it's imported.

[bannsec]

Actually, correction. Two changes in this file do allow me to compile it:

src/core_plugin.cpp

#include <filesystem>
-- Change to ---
#include <experimental/filesystem>

namespace fs = std::filesystem
--- change to --
namespace fs = std::experimental::filesystem

[xkubov]

With GCC 7 those changes are mandatory. The reason is that the C++17 implementation is only experimental with this version of GCC. The official filesystem support was introduced in GCC 8 (including the transition to namespace std::). So the result is that GCC 7 requires to include experimental/filesystem and use experimental::filesystem namespace to get the features of the C++17 standard. To compile this plugin we require nonexperimental C++17 implementation as stated in README.md.

But thank you for providing the solution for this in case someone wanted to use this plugin with GCC 7.

[xkubov]

I am closing this issue as we require a compiler supporting C++17.

[xkubov]

But perhaps we should state in README.md that nonexperimental filesystem implementation is required from C++17? What do you think @s3rvac? It's bugging me because the GCC 7 has a full implementation of C++17 but it is experimental.

[s3rvac]

@xkubov This is the reason why we have not used <filesystem> in RetDec (to make it buildable with GCC 7 + there were some issues with <filesystem> on Windows/macOS). Do you think it would be possible to replace <filesystem> with our own FilesystemPath from RetDec? Alternatively, we could check if the compiler supports <filesystem>, and if not, use <experimental/filesystem>.

Let me know what you think. We can then discuss a solution.

[xkubov]

For the path it would be possible to use RetDec's implementation. But <filesystem> also provides some convenient functions like temp_directory_path for getting a temporary directory for output.

If I remember correctly the implementation in RetDec was chosen, besides other reasons, because the older version of clang on macOS with older XCode did not have full C++17 implementation (not even full experimental implementation). Sure the required functions in this plugin can be reimplemented or replaced but I don't think we should make exceptions when it comes to the standard we want to use just to support compilers versions that do not have full C++17 implementation.

But with GCC 7 I think we can provide support for it (as it has implemented full C++17 support). I found out that the <experimental/filesystem> is still the standard library that is part of <filesystem> library (source).

So we can either test the existence of C++17 features with a feature testing macro or include it as <experimental/filesystem> (I've tested it and it is alright).

[bannsec]

For what it's worth, my gcc is the default on Ubuntu Bionic, which is the current LTS for Ubuntu, which is the most commonly used Ubuntu version.

[s3rvac]

I agree with going with the __has_include(<filesystem>) approach in commit 6df724003325f984de94b292fbe0771b7b14c537 :+1: This is the standard way of using std::filesystem when supporting GCC 7 as well as newer versions.

[xkubov]

Conditional include is now solved in commit cf29a76. I am closing this issue. Thank you @bannsec for your input on this matter and if you have any other issue regarding this you can continue commenting here or reopen this issue.