optick icon indicating copy to clipboard operation
optick copied to clipboard

MinGW Support

Open nathanvoglsam opened this issue 4 years ago • 2 comments

Currently OptickCore only supports compiling for MSVC for Windows. This pull request extends support of the current code and CMake build system to allow compiling OptickCore with the MinGW toolchain as well.

My motivation for this pull request is to enable using the x86_64-pc-windows-gnu target with rustc and the rust bindings. Debugger support for x86_64-pc-windows-gnu is immensely stronger than on the x86_64-pc-windows-msvc target so it would be ideal for me to still be able to use the gnu target.

Very little changes were needed. Almost every change was related to correctly handling conditional compilation as OPTICK_MSVC as used in many places where a more appropriate name would be OPTICK_WINDOWS as it wasn't guarding MSVC features but windows features.

I have tested the code using the CMake build system with MSVC, MinGW 6.0, MinGW 8.0 (Trunk, from MSYS2) and Linux. To provide a breakdown

  • MSVC: Build unchanged, all changes are behind #if guards so the behavior of the MSVC code should be unchanged.
  • MinGW 8.0: Compiles seamlessly and tested as functional with ConsoleApp
  • Linux: Same as MSVC, all changes are guarded by platform checks
  • MinGW 6.0: Can work but will fail to compile unless WINVER and _WIN32_WINNT are defined globally to _WIN32_WINNT_WIN7 or higher. Some of the ETW functions are only supported on Windows 7 or newer, and older MinGW builds have their headers default to _WIN32_WINNT_WINXP which is for Windows XP. In this case the functions are left undefined and the build will fail. In my preprocessor check I explicitly raise WINVER being too low to be an error with an #error directive. The solution, depending on how old your MinGW is either update the MinGW version being used or define WINVER and _WIN32_WINNT to 0x0601 (the value for _WIN32_WINNT_WIN7) with a compiler flag. Manually defining WINVER and _WIN32_WINNT is the intended path for targeting a newer windows version than the default.

The only change which I could see as contentious is for commit 3777e24. The SymLoadModule64 is defined to take char* for the module path parameter on MinGW. I believe this to be an error in the MinGW headers as the official MSVC Windows SDK headers define the parameter to take a const char*. The value for that paramater is taken from std::string.c_str() which always returns a const char* so will cause a compilation error on MinGW.

Under the assumption the MinGW header is wrong and the underlying function will never write through the pointer I cast the const away, which is super unsafe but I believe my assumption to be correct. This could be marked as a workaround and investigated further in the future if needed.

I also fixed a missing include in TestEngine

nathanvoglsam avatar Sep 08 '20 14:09 nathanvoglsam

Solves issue #102

nathanvoglsam avatar Sep 08 '20 14:09 nathanvoglsam

Any updates on this?

MattyGroGy avatar Apr 29 '21 16:04 MattyGroGy