JonathanSalwan / Triton

Triton is a dynamic binary analysis library. Build your own program analysis tools, automate your reverse engineering, perform software verification or just emulate code.
https://triton-library.github.io
Apache License 2.0
3.4k stars 524 forks source link

Fixed the way in which windows is build #1283

Closed cctv130 closed 9 months ago

cctv130 commented 10 months ago

1693800008132

cctv130 commented 10 months ago

1693800095384

JonathanSalwan commented 10 months ago

Just to fully understand, what is the problem of compile Triton on windows? Does this step not works?

cctv130 commented 10 months ago

Just to fully understand, what is the problem of compile Triton on windows? Does this step not works?

This piece is modified according to #1248

cctv130 commented 10 months ago

Just to fully understand, what is the problem of compile Triton on windows? Does this step not works?

For building projects on windows, I have full instructions on llvm builds and a full triton installation at WindowsBuild.txt.

cctv130 commented 10 months ago

Just to fully understand, what is the problem of compile Triton on windows? Does this step not works?

I am very sorry that due to my network problem, the local docker cannot access the external network, so I can only use github ci to compile, one compilation failed, and the others succeeded. At present, there is another problem, docker compilation can not find the path of meson, I do not know how to modify this path in docker. Try to change the DOCKERFILE. Build Python Package :the problem is that meson I see is already installed, there is a warning that meson is not added to the path.

cctv130 commented 10 months ago

Just to fully understand, what is the problem of compile Triton on windows? Does this step not works?

All builds completed successfully, please check.

JonathanSalwan commented 10 months ago

Thanks a lot @cctv130! Just some typo like removing empty lines and we are good. I just need the confirmation from @cnheitman regarding the build-wheel-linux.sh file and we are all good :)

cnheitman commented 10 months ago

@JonathanSalwan I'll take a look and test it on my local setup tomorrow.

cctv130 commented 10 months ago

Thanks a lot @cctv130! Just some typo like removing empty lines and we are good. I just need the confirmation from @cnheitman regarding the build-wheel-linux.sh file and we are all good :)

Latest has been repair according to your request to delete the unnecessary files and space, according to my work yesterday, build - wheel - Linux. Sh, can not find a python version of the specified absolute path meson environment variables also can not find, then add the virtual environment, build python package will compile, maybe the image version is updated.

mrexodia commented 10 months ago

I just tried building Triton master on Windows with ClangCL (latest Visual Studio update):

cmake -B build -T ClangCL "-DCMAKE_PREFIX_PATH:FILEPATH=d:\CodeBlocks\z3\install;d:\CodeBlocks\capstone\build\install;d:\CodeBlocks\llvm-12.0.0-rc1-install" -DLLVM_INTERFACE=ON
cmake --build build --config Release

It works without any modifications to the CMakeLists.txt (or passing the CXX_STANDARD_REQUIRED variable). I saw this issue before, it happens in one of the following circumstances:

  1. Using a non-Microsoft version of clang-cl (eg downloaded from the latest LLVM releases, or modifying the VS installation)
  2. Using CLion, where they made a major mistake and pass -m64 to the clang command line, which completely breaks all builds. The issue was closed, but you can see it here: https://youtrack.jetbrains.com/issue/CPP-32063 (I am pretty confident it's not resolved yet, but perhaps it's only fixed in a prerelease).

To confirm everything works I also did a build with Ninja:

vs2022 x64 # open a VS2022 command line
cmake -B build "-DCMAKE_PREFIX_PATH:FILEPATH=d:/CodeBlocks/z3/install;d:/CodeBlocks/capstone/build/install;d:/CodeBlocks/llvm-12.0.0-rc1-install" -DLLVM_INTERFACE=ON "-DCMAKE_C_COMPILER:FILEPATH=c:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/Llvm/x64/bin/clang-cl.exe" "-DCMAKE_CXX_COMPILER:FILEPATH=c:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/Llvm/x64/bin/clang-cl.exe" -G Ninja -DCMAKE_BUILD_TYPE=Release
cmake --build

There are some pitfalls, notably you cannot pass -DCMAKE_CXX_COMPILER=clang-cl.exe, because this will trigger the usage of the clang-cl.exe in your LLVM installation (which causes errors are mentioned in point 1). Additionally forgetting to pass -DCMAKE_BUILD_TYPE=Release will error and you need to use / in the paths because of a CMake bug.

When it comes to modifying CMAKE_CXX_FLAGS, this is generally to be avoided. The reason is that this flag is supposed to be passed to the command line (either manually or by preset). When you are changing the flag you are messing with package managers and downstream consumers because you introduce unnecessary or conflicting flags (or in your case you completely override whatever is being passed to the command line). This means that downstream consumers have to patch the CMakeLists.txt, introducing additional maintenance burden.

If a flag is required for the compilation to work (eg the project doesn't compile without this option), you try to detect the exact compilers using if() statements and then add it to the target with target_compile_options(target PRIVATE /flag) or target_link_options. "Common" examples of this include /bigobj and linker flags like /STACK.

JonathanSalwan commented 10 months ago

I agreed with that we should avoid as much as possible touching the CMake file and let the user provide his arguments according to his setup.

cctv130 commented 10 months ago

I just tried building Triton master on Windows with ClangCL (latest Visual Studio update):

cmake -B build -T ClangCL "-DCMAKE_PREFIX_PATH:FILEPATH=d:\CodeBlocks\z3\install;d:\CodeBlocks\capstone\build\install;d:\CodeBlocks\llvm-12.0.0-rc1-install" -DLLVM_INTERFACE=ON
cmake --build build --config Release

It works without any modifications to the CMakeLists.txt (or passing the CXX_STANDARD_REQUIRED variable). I saw this issue before, it happens in one of the following circumstances:

  1. Using a non-Microsoft version of clang-cl (eg downloaded from the latest LLVM releases, or modifying the VS installation)
  2. Using CLion, where they made a major mistake and pass -m64 to the clang command line, which completely breaks all builds. The issue was closed, but you can see it here: https://youtrack.jetbrains.com/issue/CPP-32063 (I am pretty confident it's not resolved yet, but perhaps it's only fixed in a prerelease).

To confirm everything works I also did a build with Ninja:

vs2022 x64 # open a VS2022 command line
cmake -B build "-DCMAKE_PREFIX_PATH:FILEPATH=d:/CodeBlocks/z3/install;d:/CodeBlocks/capstone/build/install;d:/CodeBlocks/llvm-12.0.0-rc1-install" -DLLVM_INTERFACE=ON "-DCMAKE_C_COMPILER:FILEPATH=c:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/Llvm/x64/bin/clang-cl.exe" "-DCMAKE_CXX_COMPILER:FILEPATH=c:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/Llvm/x64/bin/clang-cl.exe" -G Ninja -DCMAKE_BUILD_TYPE=Release
cmake --build

There are some pitfalls, notably you cannot pass -DCMAKE_CXX_COMPILER=clang-cl.exe, because this will trigger the usage of the clang-cl.exe in your LLVM installation (which causes errors are mentioned in point 1). Additionally forgetting to pass -DCMAKE_BUILD_TYPE=Release will error and you need to use / in the paths because of a CMake bug.

When it comes to modifying CMAKE_CXX_FLAGS, this is generally to be avoided. The reason is that this flag is supposed to be passed to the command line (either manually or by preset). When you are changing the flag you are messing with package managers and downstream consumers because you introduce unnecessary or conflicting flags (or in your case you completely override whatever is being passed to the command line). This means that downstream consumers have to patch the CMakeLists.txt, introducing additional maintenance burden.

If a flag is required for the compilation to work (eg the project doesn't compile without this option), you try to detect the exact compilers using if() statements and then add it to the target with target_compile_options(target PRIVATE /flag) or target_link_options. "Common" examples of this include /bigobj and linker flags like /STACK.

Yes, agree with your opinion

cctv130 commented 10 months ago

@mrexodia One thing I need to explain is that I have no idea what compilation options are provided by llvm provided by Microsoft. And whether Microsoft provides the latest version in a timely manner is indeed a challenge for people who are new to llvm or who want to customize llvm.

cnheitman commented 10 months ago

I agreed with that we should avoid as much as possible touching the CMake file and let the user provide his arguments according to his setup.

I agree with this too. There seems to be no reason to modify the CMake file.

cctv130 commented 10 months ago

I agreed with that we should avoid as much as possible touching the CMake file and let the user provide his arguments according to his setup.

The redundant cmake configuration has been removed, please check what else is wrong?

cctv130 commented 10 months ago

@JonathanSalwan And I found in your project, for this deobfuscation_process.png, help me get through the way of thinking, there is a big help to me, thank you very much. image

cnheitman commented 10 months ago

The redundant cmake configuration has been removed, please check what else is wrong?

@cctv130 Why do you need to modify the files under scripts/docker?

cctv130 commented 9 months ago

@cnheitman The absolute path used for the environment variable related to py in this configuration is incorrect, and Meson cannot find it. It was stuck here for a long time during ci construction

cnheitman commented 9 months ago

@cctv130 Ok, thanks. I will check that. It shouldn't be necessary to use conda in that script.

cctv130 commented 9 months ago

@cnheitman This mini conda and conda are different. Mini has a very small size. If you don't think it's necessary to add this configuration, I think this PR can closed. As for the script issue and the previous update of Bitwuzla to the latest version, you should submit the perfect PR. After all, I have a short time with Triton and need a lot of time to learn this framework. Sorry.

cnheitman commented 9 months ago

@cctv130 Commit 357f372d953ab1aedca193f93ddd2165379ffd08 should fix the issue that you were having with the docker image. The problem was that the base image was updated (new python versions) and it broke the script. I'll rework the script so this doesn't happen again. For now the commit I mentioned should suffice.

cctv130 commented 9 months ago

@cnheitman thank you very much ^.^