Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for successful build of container image. #692

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yomaytk
Copy link
Contributor

@yomaytk yomaytk commented Jan 4, 2024

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.

  • Dockerfile: Fist, the Dockerfile is dependent on trailofbits/cxx-common-vcpkg-builder-ubuntu:${UBUNTU_VERSION}, but the name of deps image for aarch64 ends with ${UBUNTU_VERSION}_arm64. Thus, we had to specify the argument when building on aarch64 (e.g. --build-arg UBUNTU_VERSION=22.04_arm64). To avoid this, I fixed to install all dependencies on the Dockerfile. Second, I commented out the line CTEST_OUTPUT_ON_FAILURE=1 cmake --build . --verbose --target test -- -j $(nproc) && \ because the test has failed.
  • scritps/docker-lifter-entrypoint.sh: remill should support the version of LLVM 16, but only 17 had been supported and therefore I added the support of LLVM 16.
  • 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.

@yomaytk yomaytk requested a review from pgoodman as a code owner January 4, 2024 13:18
@CLAassistant
Copy link

CLAassistant commented Jan 4, 2024

CLA assistant check
All committers have signed the CLA.

@ekilmer
Copy link
Contributor

ekilmer commented Jan 4, 2024

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
Copy link
Contributor

mrexodia commented Jan 5, 2024

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
Copy link
Contributor Author

yomaytk commented Jan 5, 2024

@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
Copy link
Contributor

mrexodia commented Jan 5, 2024

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

@yomaytk
Copy link
Contributor Author

yomaytk commented Jan 7, 2024

@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?🙏

Dockerfile Show resolved Hide resolved
@ekilmer
Copy link
Contributor

ekilmer commented Jan 8, 2024

@yomaytk I just opened #695 to test the new way (lifting-bits/cxx-common#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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants