lifting-bits / remill

Library for lifting machine code to LLVM bitcode
Apache License 2.0
1.31k stars 144 forks source link

Fix for successful build of container image. #692

Open yomaytk opened 10 months ago

yomaytk commented 10 months ago

This pull request enables us to successfully build container images from the Dockerfile on both x86_64 and aarch64. This fixed 3 files as follows.

CLAassistant commented 10 months ago

CLA assistant check
All committers have signed the CLA.

ekilmer commented 10 months ago

Hello! Thank you for pointing out the issue with the ARM images being named differently and working on a fix.

I need to think about this a bit because the initial reason we used the cxx-common base image was to keep the build environment as close as possible to how the dependencies were built, with the same compiler (exact version of LLVM), runtime libraries outside of vcpkg (like glibc), etc.

I'd like to revisit whether we can build multi-platform Docker images in cxx-common before I consider these changes.

mrexodia commented 10 months ago

I have successfully built remill in a multi-arch image without customizing it at all: https://github.com/mrexodia/cxx-common-cmake/

You build it with this command (from an M1 mac in my case):

git submodule update --init
docker buildx build --platform linux/arm64,linux/amd64 -t ghcr.io/mrexodia/cxx-common-cmake:latest .

That being said, I do not use the cxx-common repository because vcpkg is a trash fire and it unnecessarily complicates things. Additionally my pure CMake approach (a superbuild to handle the dependencies) allows for easy customization where vcpkg doesn't.

yomaytk commented 10 months ago

@ekilmer I see. As you say, we should keep the environment as close as possible to how the dependencies were built, and we don't have to expose all dependencies on the Dockerfile if we prepare the image with another name (e.g. trailofbits/cxx-common-vcpkg-builder-ubuntu:22.04_amd64 and trailofbits/cxx-common-vcpkg-builder-ubuntu:22.04_arm64). Do you have a plan to upload to dockerhub a new container image?

@mrexodia Thank you for the interesting proposal. Could you tell me not only the command to build cxx-common-cmake but also the command to build remill with it?

mrexodia commented 10 months ago

Remill is also included in the cxx-common-cmake repository (sorry for the name confusion)

yomaytk commented 10 months ago

@mrexodia Thanks. However, I am concerned that it will be tough to maintain all repositories if the dependencies on third-party projects increase. @ekilmer May I have your thoughts on this?🙏

ekilmer commented 10 months ago

@yomaytk I just opened #695 to test the new way (https://github.com/lifting-bits/cxx-common/pull/1045) we build Docker images in cxx-common.

Can you try my PR and let me know if it works for you or not?

As for your other fixes, I think we can include them, but it would be great if you could update this PR (or make a new one) to better separate concerns and possibly merge faster.

Thank you!


Also, @mrexodia, I agree that vcpkg isn't perfect, and we try to work around or document the issues and confusing aspects.

I am curious about what specific aspects of vcpkg cause you the most headaches for the workflows or issues you have when developing these tools (or others). Maybe we can document those better or potentially smooth out the experience?

For our situation, we use vcpkg primarily for binary caching during our CI runs. Local developers almost always build the vcpkg dependencies from source so that they have the Debug versions as well. We've also had decent success using the ASAN triplets to catch more issues.

Also, your cxx-common-cmake repo is a great reference for managing a superbuild with CMake. Thank you! CMake superbuilds are the best solution for having absolute control over the dependencies and are certainly better if/when you need to fix/add/debug something in your dependencies.

tamaroning commented 8 months ago

I got an error for this. after removing this error, build succeeded. thank you.

lib/Arch/Arch.cpp: When I build with LLVM 16, #include <llvm/IR/AttributeMask.h> caused and error and this is not used and therefore I removed this.