stevemk14ebr / PolyHook_2_0

C++20, x86/x64 Hooking Libary v2.0
MIT License
1.58k stars 222 forks source link

fail on link when use vcpkg with x64-windows-static #167

Closed polelf closed 1 year ago

polelf commented 1 year ago

fail on link asmtk when use vcpkg with x64-windows-static

fix with this patch

fix_static_link.patch

stevemk14ebr commented 1 year ago

Can you document the commands you run that generate the linking error?

polelf commented 1 year ago

you can try this zip use vs2022 open folder. test_polyhook2.zip

stevemk14ebr commented 1 year ago

Using your example project with the latest vcpkg and ports (be sure to use latest polyhook vcpkg) I am able to link successfully if I change your CMakeLists.txt to:

cmake_minimum_required(VERSION 3.3.0)
cmake_policy(SET CMP0074 NEW)
if(POLICY CMP0091)
  cmake_policy(SET CMP0091 NEW) 
endif()

project(test_polyhook2)

find_package(PolyHook_2 CONFIG REQUIRED)

set(CMAKE_CXX_FLAGS "/std:c++latest ${CMAKE_CXX_FLAGS}")

add_executable(test_polyhook2 main.cpp)
set_property(TARGET test_polyhook2 PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
target_link_libraries(test_polyhook2 PRIVATE PolyHook_2::PolyHook_2)

This generates the test executable with no imports, as you'd expect for static linking:

image

If I remove the line:

set_property(TARGET test_polyhook2 PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")

Then the test project compiles with the /MD flag and linking for asmtk, as well as ASMJIT fail

Can you try this and let me know if that works?

FeelsAmazingMan commented 1 year ago

I ran into the same issue with vcpkg. It looks like this is only an issue for 64bit. Compiling the vcpkg static example in 64bit and changing x86Detour to x64Detour in the main.cpp file triggers linker errors for asmtk:

[build] lld-link: error: undefined symbol: declspec(dllimport) public: cdecl asmtk::AsmParser::AsmParser(class asmjit::_abi_1_9::BaseEmitter ) [build] >>> referenced by E:_vcpkg\buildtrees\polyhook2\src\b85ea773fb-315c072967.clean\sources\x64Detour.cpp:561 [build] >>> PolyHook_2.lib(x64Detour.cpp.obj):($LN52) [build] [build] lld-link: error: undefined symbol: declspec(dllimport) public: cdecl asmtk::AsmParser::~AsmParser(void) [build] >>> referenced by E:_vcpkg\buildtrees\polyhook2\src\b85ea773fb-315c072967.clean\sources\x64Detour.cpp:565 [build] >>> PolyHook_2.lib(x64Detour.cpp.obj):($LN52) [build] >>> referenced by E:_vcpkg\buildtrees\polyhook2\src\b85ea773fb-315c072967.clean\sources\x64Detour.cpp:620 [build] >>> PolyHook_2.lib(x64Detour.cpp.obj):($LN52) [build] >>> referenced by E:_vcpkg\buildtrees\polyhook2\src\b85ea773fb-315c072967.clean\sources\x64Detour.cpp:627 [build] >>> PolyHook_2.lib(x64Detour.cpp.obj):($LN52) [build] >>> referenced 2 more times [build] [build] lld-link: error: undefined symbol: declspec(dllimport) public: unsigned int cdecl asmtk::AsmParser::parse(char const , unsigned __int64) [build] >>> referenced by E:_vcpkg\buildtrees\polyhook2\src\b85ea773fb-315c072967.clean\sources\x64Detour.cpp:618 [build] >>> PolyHook_2.lib(x64Detour.cpp.obj):($LN52) [build] ninja: build stopped: subcommand failed.

It looks like asmtk is trying to do dllimports on a static build, the polyhook2 cmake file is probably missing a define somewhere?

Vcpkg triplet: x64-windows-static Vcpkg commit: 501db0f17ef6df184fcdbfbe0f87cde2313b6ab1 (which is the latest release tag 2023.04.15) also tried latest master 22b3afdab7098e4dffa884de56c3bd9c2b0636e0 Compiler: Tried with clang and msvc

stevemk14ebr commented 1 year ago

does the suggested patch fix in the original post for this issue fix the issue for you

stevemk14ebr commented 1 year ago

I tried applying the suggested patch, it ends up leading to additional link errors, maybe related to https://github.com/stevemk14ebr/PolyHook_2_0/issues/176

the new errors are:

error LNK2019: unresolved external symbol "__declspec(dllimport) public: __cdecl asmtk::AsmParser::AsmParser(class asmjit::_abi_1_10::BaseEmitter *)" (__imp_??0AsmParser@asmtk@@QEAA@PEAVBaseEmitter@_abi_1_10@asmjit@@@Z) referenced in function "protected: class std::optional<unsigned __int64> __cdecl PLH::x64Detour::generateTranslationRoutine(class PLH::Instruction const &,unsigned __int64)" (?generateTranslationRoutine@x64Detour@PLH@@IEAA?AV?$optional@_K@std@@AEBVInstruction@2@_K@Z)
error LNK2019: unresolved external symbol "__declspec(dllimport) public: __cdecl asmtk::AsmParser::~AsmParser(void)" (__imp_??1AsmParser@asmtk@@QEAA@XZ) referenced in function "protected: class std::optional<unsigned __int64> __cdecl PLH::x64Detour::generateTranslationRoutine(class PLH::Instruction const &,unsigned __int64)" (?generateTranslationRoutine@x64Detour@PLH@@IEAA?AV?$optional@_K@std@@AEBVInstruction@2@_K@Z)
error LNK2019: unresolved external symbol "__declspec(dllimport) public: unsigned int __cdecl asmtk::AsmParser::parse(char const *,unsigned __int64)" (__imp_?parse@AsmParser@asmtk@@QEAAIPEBD_K@Z) referenced in function "protected: class std::optional<unsigned __int64> __cdecl PLH::x64Detour::generateTranslationRoutine(class PLH::Instruction const &,unsigned __int64)" (?generateTranslationRoutine@x64Detour@PLH@@IEAA?AV?$optional@_K@std@@AEBVInstruction@2@_K@Z)
error LNK2019: unresolved external symbol __imp_ZydisRegisterGetClass referenced in function "class std::optional<struct PLH::TranslationResult> __cdecl PLH::translate_instruction(class PLH::Instruction const &)" (?translate_instruction@PLH@@YA?AV?$optional@UTranslationResult@PLH@@@std@@AEBVInstruction@1@@Z)
error LNK2019: unresolved external symbol __imp_ZydisRegisterGetString referenced in function "class std::optional<struct PLH::TranslationResult> __cdecl PLH::translate_instruction(class PLH::Instruction const &)" (?translate_instruction@PLH@@YA?AV?$optional@UTranslationResult@PLH@@@std@@AEBVInstruction@1@@Z)
error LNK2019: unresolved external symbol __imp_ZydisMnemonicGetString referenced in function "public: class std::vector<class PLH::Instruction,class std::allocator<class PLH::Instruction> > __cdecl PLH::ZydisDisassembler::disassemble(unsigned __int64,unsigned __int64,unsigned __int64,class PLH::MemAccessor const &)" (?disassemble@ZydisDisassembler@PLH@@QEAA?AV?$vector@VInstruction@PLH@@V?$allocator@VInstruction@PLH@@@std@@@std@@_K00AEBVMemAccessor@2@@Z)
error LNK2019: unresolved external symbol __imp_ZydisDecoderInit referenced in function "public: __cdecl PLH::ZydisDisassembler::ZydisDisassembler(enum PLH::Mode)" (??0ZydisDisassembler@PLH@@QEAA@W4Mode@1@@Z)
error LNK2019: unresolved external symbol __imp_ZydisDecoderDecodeFull referenced in function "public: class std::vector<class PLH::Instruction,class std::allocator<class PLH::Instruction> > __cdecl PLH::ZydisDisassembler::disassemble(unsigned __int64,unsigned __int64,unsigned __int64,class PLH::MemAccessor const &)" (?disassemble@ZydisDisassembler@PLH@@QEAA?AV?$vector@VInstruction@PLH@@V?$allocator@VInstruction@PLH@@@std@@@std@@_K00AEBVMemAccessor@2@@Z)
error LNK2019: unresolved external symbol __imp_ZydisFormatterInit referenced in function "public: __cdecl PLH::ZydisDisassembler::ZydisDisassembler(enum PLH::Mode)" (??0ZydisDisassembler@PLH@@QEAA@W4Mode@1@@Z)
error LNK2019: unresolved external symbol __imp_ZydisFormatterSetProperty referenced in function "public: __cdecl PLH::ZydisDisassembler::ZydisDisassembler(enum PLH::Mode)" (??0ZydisDisassembler@PLH@@QEAA@W4Mode@1@@Z)
error LNK2019: unresolved external symbol __imp_ZydisFormatterFormatInstruction referenced in function "protected: bool __cdecl PLH::ZydisDisassembler::getOpStr(struct ZydisDecodedInstruction_ *,struct ZydisDecodedOperand_ const *,unsigned __int64,class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > *)" (?getOpStr@ZydisDisassembler@PLH@@IEAA_NPEAUZydisDecodedInstruction_@@PEBUZydisDecodedOperand_@@_KPEAV?$basic_string@DU?$char_traits@D@std@@V?$allocator@D@2@@std@@@Z)

I'm debating just removing vcpkg support, i'm not sure the recurring linking issues and updates of deps is worth all this

stevemk14ebr commented 1 year ago

I've set a $200 USD bounty on this https://twitter.com/stevemk14ebr/status/1677383404969689088?s=20

veteri commented 1 year ago

Removing vcpkg support would be really sad.

I've set a $200 USD bounty on this https://twitter.com/stevemk14ebr/status/1677383404969689088?s=20

I tried building latest master that includes your attempted fix in https://github.com/stevemk14ebr/PolyHook_2_0/blob/a433f35e6370a8e9bb649d19e713c88f67bb9029/CMakeLists.txt#L79

The following error appears:

1> [CMake] CMake Error at C:\Users\Admin\source\repos\PolyHook_2_0\CMakeLists.txt:79 (target_compile_definitions):
1> [CMake]   Cannot specify compile definitions for target "PolyHook_2" which is not
1> [CMake]   built by this project.

I have no idea about CMAKE but i did check the comment from @FeelsAmazingMan and from what i understood your attempt was to set a define for asmtk that would not set the dllimports macros, refering to this:

// API (Export / Import).
#if !defined(ASMTK_STATIC)
  #if defined(_WIN32) && (defined(_MSC_VER) || defined(__MINGW32__))
    #if defined(ASMTK_EXPORTS)
      #define ASMTK_API __declspec(dllexport)
    #else
      #define ASMTK_API __declspec(dllimport)
    #endif
  #elif defined(_WIN32) && defined(__GNUC__)
    #if defined(ASMTK_EXPORTS)
      #define ASMTK_API __attribute__((__dllexport__))
    #else
      #define ASMTK_API __attribute__((__dllimport__))
    #endif
  #elif defined(__GNUC__)
    #define ASMTK_API __attribute__((__visibility__("default")))
  #endif
#endif

#if !defined(ASMTK_API)
  #define ASMTK_API
#endif

Anyway if you change your line to:

target_compile_definitions(asmtk PRIVATE ASMTK_STATIC)

It builds just fine and there is no unresolved external symbols when actually linking against the lib. Again i have no idea about CMAKE and i despise it but it seems logical that instead of ${PROJECT_NAME} which would be PolyHook that you need to define asmtk for the target. I hope this helps.

veteri commented 1 year ago

My previous comment doesn't seem to be related to the unresolved symbols with zydis. I have tried manually building as per your instructions, taking care of submodules etc. When i build like that, all i need to do is remove your line for a successful build: https://github.com/stevemk14ebr/PolyHook_2_0/blob/a433f35e6370a8e9bb649d19e713c88f67bb9029/CMakeLists.txt#L79 (or replace it with the one i posted in my previous comment)

It creates a build32 folder and if i open the solution with VS22 it builds just fine and i can paste the resulting .lib into my vcpkg/installed/libs directory and it works fine and i can use Polyhook2 fine in other projects that are using vcpkg.

Now i tried to reproduce the exact error that the normal vcpkg build gives (zydis unresolved symbols). I suspected that it builds zydis as a shared library for some reason, so i tried to intentionally mess up the settings.

Going from option(POLYHOOK_BUILD_SHARED_ZYDIS "Build zydis as shared libary" OFF) to option(POLYHOOK_BUILD_SHARED_ZYDIS "Build zydis as shared libary" ON)

and indeed that will result in the exact same errors of unresolved symbols for a static polyhook build (duh).

So my current conclusion is that the define ZYDIS_STATIC_BUILD (zydis v4) that was previously named ZYDIS_STATIC_DEFINE is somehow missing when vcpkg builds it. And indeed if you just force the define through a: target_compile_definitions(${PROJECT_NAME} PRIVATE ZYDIS_STATIC_BUILD) then it'll work fine again and no unresolved symbols. I'm trying to find out why it does not add the define during the vcpkg build next, but i thought i share my thoughts if they're helpful for anyone else trying to figure it out.

Neumann-A commented 1 year ago

https://github.com/stevemk14ebr/PolyHook_2_0/compare/master...Neumann-A:PolyHook_2_0:make_static_builds_work for full solution. I'll comment on all the issues if i get more time.

stevemk14ebr commented 1 year ago

@Neumann-A thank you for looking at this, your solution relies on many patch files and the 'overlay' feature which is not something I'm totally familiar with, is there a technique that would result in less patch files being applied than in your solution? I am mildly concerned about maintenance when multiple patches like this are in use, it's part of how I got myself into this linking mess.

@veteri your findings make a lot of sense! I'm excited to see how we can incorporate those into fixing the port files and cmakelist here

Neumann-A commented 1 year ago

The problem is actual more than one problem:

So what did I do to make it work?

In the end it has nothing to do with vcpkg but it is a user error on the consumer side not setting the required _STATIC defines if static linkage is done.

All the applied overlays will be submitted to vcpkg, as soon as my normal inet is back.

stevemk14ebr commented 1 year ago

Awesome, it sounds like you understand the issue better than anyone else at this point 😄

Once all your PRs are submitted and merged to upstream then I will be able to test everything, and if all goes well then I'd say that clearly would make you the receiver of the bounty! Thanks so much for looking at the problem, I'm really happy progress is happening 🚀

Neumann-A commented 1 year ago

PRs are: https://github.com/microsoft/vcpkg/pull/32453 https://github.com/microsoft/vcpkg/pull/32451 https://github.com/microsoft/vcpkg/pull/32455

thank you for looking at this, your solution relies on many patch files and the 'overlay' feature which is not something I'm totally familiar with,

If you want a quick and dirty solution simply add the following to downstream CMakeLists.txt (However this is far from the general solution and is just a solution to make it work):

if(VCPKG_TARGET_TRIPLET MATCHES "-static")
  add_compile_definitions(ZYDIS_STATIC_BUILD ASMTK_STATIC)
endif()

Also the patches aren't that large if you just look at the vcpkg PRs.

'overlay' feature which is not something I'm totally familiar with

Overlay ports/triplets are there to supply/overwrite ports/triplets vcpkg does not currently have or needs some fixes to make work. For ports there are also custom registry but that takes longer to setup. In general an overlay allows to provide a port without modifying vcpkg ports or triplet directory directly. For example I have: https://github.com/Neumann-A/my-vcpkg-triplets and https://github.com/Neumann-A/my-vcpkg-ports.

Neumann-A commented 1 year ago

since https://github.com/microsoft/vcpkg/pull/32453 https://github.com/microsoft/vcpkg/pull/32451 Are merged you should be able to test it now.

stevemk14ebr commented 1 year ago

I can confirm it works, thanks so much! For anyone following along, it's easiest to just delete the vcpkg folder, pull the latest and re-run setup. Then projects should build fine.

@Neumann-A can you provide a way for me to pay you? Venmo is the easiest I can think of, if you would like to send another way privately, you can DM me on twitter @stevemk14ebr

stevemk14ebr commented 1 year ago

@Neumann-A I'm still very interested in making good on my promise for compensation. Please reach out when possible