ci: compile Clang and compiler-rt in msan jobs #27737

pull fanquake wants to merge 5 commits into bitcoin:master from fanquake:fix_native_fuzz_with_msan changing 4 files +40 −19
  1. fanquake commented at 10:21 am on May 24, 2023: member

    This reworks the MSAN CIs, to first compile Clang and compiler-rt (using GCC 12), and then, compile an MSAN instrumented libc++ using the just-built Clang 16. This fixes the native_fuzz_with_msan job, working around https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005341, by not using the Debian provided Clang/LLVM.

    Also included are changes to streamline how we use our “custom libc++”, according to upstream: https://releases.llvm.org/16.0.0/projects/libcxx/docs/UsingLibcxx.html#using-a-custom-built-libc, as well as other minor cleanups in the CI configs.

    An example job is currently running in the qa-assets repo: https://github.com/bitcoin-core/qa-assets/pull/129 (https://cirrus-ci.com/task/4632561431871488).

  2. DrahtBot commented at 10:21 am on May 24, 2023: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK dergoegge
    Concept ACK theuni

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #27495 ([WIP] ci: Use DEBUG=1 in depends for MSAN jobs by fanquake)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  3. DrahtBot added the label Tests on May 24, 2023
  4. maflcko commented at 11:02 am on May 24, 2023: member
    No idea about the bash error. Maybe set -ex is missing in the file?
  5. fanquake commented at 11:04 am on May 24, 2023: member

    No idea about the bash error. Maybe set -ex is missing in the file?

    Have a change to just remove cd usage entirely, that should make the linter happy

  6. in ci/test/00_setup_env_native_fuzz_with_msan.sh:9 in 8a66500489 outdated
    6@@ -7,17 +7,17 @@
    7 export LC_ALL=C.UTF-8
    8 
    9 export CI_IMAGE_NAME_TAG="ubuntu:23.04" # Version 23.04 will reach EOL in Jan 2024, and can be replaced by "ubuntu:24.04" (or anything else that ships the wanted clang version).
    


    maflcko commented at 11:06 am on May 24, 2023:
    0export CI_IMAGE_NAME_TAG="ubuntu:22.04"
    

    nit: Can use LTS if you don’t need clang-16?


    fanquake commented at 12:48 pm on May 24, 2023:
    Switched back to Jammy in next push.
  7. maflcko commented at 11:07 am on May 24, 2023: member
    Do you happen to know what the difference to the debian package is? Maybe it can be fixed upstream instead?
  8. DrahtBot added the label CI failed on May 24, 2023
  9. fanquake force-pushed on May 24, 2023
  10. fanquake commented at 2:24 pm on May 24, 2023: member

    Do you happen to know what the difference to the debian package is? Maybe it can be fixed upstream instead?

    I’m not completely sure, the answer will be in here somewhere: https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/snapshot/debian/rules (also taking into account all of the patches they apply). They should be doing a multi stage build, and building everything with the 2nd-built Clang, but clearly something has been broken since LLVM 13. I’ll followup upstream.

    Have a change to just remove cd usage entirely, that should make the linter happy

    Made this change, and switched back to using Jammy. A qa-assets run of the current branch in: https://cirrus-ci.com/task/4632561431871488.

  11. in ci/test/01_base_install.sh:51 in a3be80e521 outdated
    49+
    50+  cmake -G Ninja -B "${BASE_SCRATCH_DIR}"/msan/clang_build/ -DLLVM_ENABLE_PROJECTS="clang" \
    51+                                                            -DCMAKE_BUILD_TYPE=Release \
    52+                                                            -DLLVM_TARGETS_TO_BUILD=X86 \
    53+                                                            -DLLVM_ENABLE_RUNTIMES="compiler-rt;libcxx;libcxxabi;libunwind" \
    54+                                                            -S "${BASE_SCRATCH_DIR}"/msan/llvm-project/llvm
    


    maflcko commented at 2:41 pm on May 24, 2023:

    Some ideas (feel free to ignore):

    • -DLLVM_USE_LINKER=lld and/or -DLLVM_PARALLEL_LINK_JOBS=1 to reduce change of OOM?
    • -DLLVM_INCLUDE_BENCHMARKS=OFF -DLLVM_INCLUDE_TESTS=OFF to reduce CPU?

    fanquake commented at 4:57 pm on May 24, 2023:

    -DLLVM_INCLUDE_BENCHMARKS=OFF -DLLVM_INCLUDE_TESTS=OFF to reduce CPU?

    These two control the generation of build targets, however nothing should be getting compiled, as LLVM_BUILD_TESTS and LLVM_BUILD_BENCHMARKS both default to OFF. However I’ve also added some additional -DLLVM_INCLUDE_* options that should reduce compilation.

    -DLLVM_USE_LINKER=lld and/or -DLLVM_PARALLEL_LINK_JOBS=1 to reduce change of OOM?

    I could follow up with a change to try and optimise this further if we start running into resource issues?


    maflcko commented at 9:00 am on May 25, 2023:
    Looks like memory usage isn’t reported in https://cirrus-ci.com/task/4543816401682432 ?

    fanquake commented at 9:04 am on May 25, 2023:

    Looks like memory usage isn’t reported in https://cirrus-ci.com/task/4543816401682432 ?

    Is that a CIrrus bug? You should be able to see an example memory usage here: https://cirrus-ci.com/task/4632561431871488.


    maflcko commented at 9:10 am on May 25, 2023:

    ah ok, so it is using 12 GB?

    Looks like it is only passing CI because Cirrus seems to be ignoring the 2CPU/8GB limit and just uses 4/15 unconditionally for ci image builds?


    fanquake commented at 3:57 pm on May 29, 2023:

    Looks like it is only passing CI because Cirrus seems to be ignoring the 2CPU/8GB limit and just uses 4/15 unconditionally for ci image builds? @fkorotkov is this what should be happening, and can we rely on this behaviour into the future?

  12. maflcko approved
  13. maflcko commented at 2:42 pm on May 24, 2023: member
    lgtm. Longer term it may be better to try to go back to using the pre-compiled one from debian, or alternatively try a different Linux distro for the msan tasks?
  14. theuni commented at 3:36 pm on May 24, 2023: member

    Concept ACK. I’m not so familiar with the history here, but the changes seem sane to me.

    The libc++ changes here are especially interesting. I assume they work with no issues because there’s no SDK to take into consideration, but (as @fanquake suggested in an offline discussion yesterday) I’m going to see if we can maybe port this over to our depends builds with the SDK included. Unsure if it’ll work 🤷 but it would be much cleaner if it did.

  15. fanquake force-pushed on May 24, 2023
  16. maflcko commented at 6:36 am on May 25, 2023: member
    Looks like the last push bricked everything? Maybe revert to the previous version?
  17. fanquake force-pushed on May 25, 2023
  18. maflcko commented at 8:58 am on May 25, 2023: member

    I’m going to see if we can maybe port this over to our depends

    I am not sure if compiling a compiler for a CI task is a worthwhile thing to maintain in this repo? Ideally we can use one from the Debian/Ubuntu distros, as they are used the most, so that it will be easy to bootstrap the test config outside the CI env. If the Debian/Ubuntu one isn’t working, we could try into using a different distro temporarily? But imo compiling it from scratch should be for the last fallback and not a long term goal.

  19. fanquake commented at 9:01 am on May 25, 2023: member

    I’m going to see if we can maybe port this over to our depends

    I am not sure if compiling a compiler for a CI task is a worthwhile thing to maintain in this repo?

    This point is in regards to the libc++ flag usage. Not compiling a compiler for anything else.

  20. DrahtBot removed the label CI failed on May 25, 2023
  21. ci: standardize custom libc++ usage in MSAN jobs
    Use `-isystem` & `-nostd*` flags, which is the preferred way to use a
    custom libc++ (ours is libc++ build with MSAN) with Clang, as opposed to
    our current ad-hoc flags.
    
    See: https://releases.llvm.org/16.0.0/projects/libcxx/docs/UsingLibcxx.html#using-a-custom-built-libc
    for more info.
    2d4f4b8f29
  22. ci: remove extra CC & CXX from MSAN jobs
    This is passed through from depends.
    883bc9f561
  23. ci: use LLVM 16.0.4 in MSAN jobs 796bd1d0d1
  24. ci: compile clang and compiler-rt in MSAN jobs
    This works around https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005341.
    d3cbcbf626
  25. ci: return to using Ubuntu 22.04 in MSAN jobs
    We no-longer need to use 23.04, now that we aren't installing clang-16
    and friends.
    5763b232e6
  26. fanquake force-pushed on May 29, 2023
  27. maflcko commented at 12:52 pm on June 1, 2023: member

    Side note: I tried centos and it also failed with the same error as debian. Commit:

     0commit 57903b154ec1dc5d86f174d311e9875bbf0c4106
     1Author: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
     2Date:   Thu Jun 1 13:48:41 2023 +0200
     3
     4    ci: Use centos to work around fuzz msan debian bug
     5
     6diff --git a/ci/test/00_setup_env_native_fuzz_with_msan.sh b/ci/test/00_setup_env_native_fuzz_with_msan.sh
     7index dd694f818c..5aca7a9be9 100755
     8--- a/ci/test/00_setup_env_native_fuzz_with_msan.sh
     9+++ b/ci/test/00_setup_env_native_fuzz_with_msan.sh
    10@@ -8,3 +8,3 @@ export LC_ALL=C.UTF-8
    11 
    12-export CI_IMAGE_NAME_TAG="ubuntu:23.04" # Version 23.04 will reach EOL in Jan 2024, and can be replaced by "ubuntu:24.04" (or anything else that ships the wanted clang version).
    13+export CI_IMAGE_NAME_TAG="quay.io/centos/centos:stream9"  # Use centos over debian/ubuntu due to bug https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005341
    14 LIBCXX_DIR="${BASE_SCRATCH_DIR}/msan/build/"
    15@@ -15,3 +15,3 @@ export MSAN_AND_LIBCXX_FLAGS="${MSAN_FLAGS} ${LIBCXX_FLAGS}"
    16 export CONTAINER_NAME="ci_native_fuzz_msan"
    17-export PACKAGES="clang-16 llvm-16 libclang-rt-16-dev cmake"
    18+export CI_BASE_PACKAGES="clang-16* ccache cmake libtool make git python3 which patch lbzip2 xz procps-ng rsync bison util-linux dash"
    19 # BDB generates false-positives and will be removed in future
    20@@ -19,3 +19,3 @@ export DEP_OPTS="NO_BDB=1 NO_QT=1 CC='clang' CXX='clang++' CFLAGS='${MSAN_FLAGS}
    21 export GOAL="install"
    22-export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening --with-asm=no CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'"
    23+export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,memory --disable-hardening --with-asm=no CFLAGS='${MSAN_FLAGS}' CPPFLAGS='-DBOOST_MULTI_INDEX_ENABLE_SAFE_MODE' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'"
    24 export USE_MEMORY_SANITIZER="true"
    25diff --git a/ci/test/00_setup_env_native_msan.sh b/ci/test/00_setup_env_native_msan.sh
    26index bdb9bd7b5d..a58d511c2f 100755
    27--- a/ci/test/00_setup_env_native_msan.sh
    28+++ b/ci/test/00_setup_env_native_msan.sh
    29@@ -8,3 +8,3 @@ export LC_ALL=C.UTF-8
    30 
    31-export CI_IMAGE_NAME_TAG="ubuntu:23.04" # Version 23.04 will reach EOL in Jan 2024, and can be replaced by "ubuntu:24.04" (or anything else that ships the wanted clang version).
    32+export CI_IMAGE_NAME_TAG="quay.io/centos/centos:stream9"  # Use centos over debian/ubuntu due to bug https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1005341
    33 LIBCXX_DIR="${BASE_SCRATCH_DIR}/msan/build/"
    34@@ -15,3 +15,3 @@ export MSAN_AND_LIBCXX_FLAGS="${MSAN_FLAGS} ${LIBCXX_FLAGS}"
    35 export CONTAINER_NAME="ci_native_msan"
    36-export PACKAGES="clang-16 llvm-16 libclang-rt-16-dev cmake"
    37+export CI_BASE_PACKAGES="clang-16* ccache cmake libtool make git python3 which patch lbzip2 xz procps-ng rsync bison util-linux dash"
    38 # BDB generates false-positives and will be removed in future
    39@@ -19,3 +19,3 @@ export DEP_OPTS="NO_BDB=1 NO_QT=1 CC='clang' CXX='clang++' CFLAGS='${MSAN_FLAGS}
    40 export GOAL="install"
    41-export BITCOIN_CONFIG="--with-sanitizers=memory --disable-hardening --with-asm=no CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'"
    42+export BITCOIN_CONFIG="--with-sanitizers=memory --disable-hardening --with-asm=no CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}'"
    43 export USE_MEMORY_SANITIZER="true"
    44diff --git a/ci/test/01_base_install.sh b/ci/test/01_base_install.sh
    45index 98f96f0ece..bfebb29f6d 100755
    46--- a/ci/test/01_base_install.sh
    47+++ b/ci/test/01_base_install.sh
    48@@ -44,4 +44,2 @@ fi
    49 if [[ ${USE_MEMORY_SANITIZER} == "true" ]]; then
    50-  update-alternatives --install /usr/bin/clang++ clang++ "$(which clang++-16)" 100
    51-  update-alternatives --install /usr/bin/clang clang "$(which clang-16)" 100
    52   git clone --depth=1 https://github.com/llvm/llvm-project -b llvmorg-16.0.1 "${BASE_SCRATCH_DIR}"/msan/llvm-project
    

    Result:

     0...
     1/usr/bin/ld: /usr/lib64/clang/16/lib/linux/libclang_rt.fuzzer-x86_64.a(FuzzerUtilPosix.cpp.o): in function `fuzzer::SearchRegexCmd(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)':
     2(.text+0x793): undefined reference to `std::__throw_length_error(char const*)'
     3/usr/bin/ld: (.text+0x79f): undefined reference to `std::__throw_length_error(char const*)'
     4clang-16: error: linker command failed with exit code 1 (use -v to see invocation)
     5configure:20878: $? = 1
     6configure: failed program was:
     7| /* confdefs.h */
     8| #define PACKAGE_NAME "Bitcoin Core"
     9| #define PACKAGE_TARNAME "bitcoin"
    10| #define PACKAGE_VERSION "25.99.0"
    11| #define PACKAGE_STRING "Bitcoin Core 25.99.0"
    12| #define PACKAGE_BUGREPORT "https://github.com/bitcoin/bitcoin/issues"
    13| #define PACKAGE_URL "https://bitcoincore.org/"
    14| #define HAVE_CXX17 1
    15| #define STDC_HEADERS 1
    16| #define HAVE_SYS_TYPES_H 1
    17| #define HAVE_SYS_STAT_H 1
    18| #define HAVE_STDLIB_H 1
    19| #define HAVE_STRING_H 1
    20| #define HAVE_MEMORY_H 1
    21| #define HAVE_STRINGS_H 1
    22| #define HAVE_INTTYPES_H 1
    23| #define HAVE_STDINT_H 1
    24| #define HAVE_UNISTD_H 1
    25| #define HAVE_DLFCN_H 1
    26| #define LT_OBJDIR ".libs/"
    27| #define USE_ASM 1
    28| /* end confdefs.h.  */
    29|
    30|     #include <cstdint>
    31|     #include <cstddef>
    32|     extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { return 0; }
    33|     __attribute__((weak)) // allow for libFuzzer linking
    34|
    35| int
    36| main ()
    37| {
    38|
    39|   ;
    40|   return 0;
    41| }
    42configure:20888: result: no
    43configure:20893: error: linker did not accept requested flags, you are missing required libraries
    
  28. dergoegge commented at 1:21 pm on June 1, 2023: member

    utACK 5763b232e6e6a0f72d046f8aa322b39328be135b

    Looking forward to green check marks on qa-assets

  29. fanquake commented at 9:41 am on June 2, 2023: member
    Using alternate distros, or the pre-compiled LLVM bins do not currently seem to be viable options. Follow up in the qa-assets repo is here: https://github.com/bitcoin-core/qa-assets/pull/129.
  30. fanquake merged this on Jun 2, 2023
  31. fanquake closed this on Jun 2, 2023

  32. fanquake deleted the branch on Jun 2, 2023
  33. sidhujag referenced this in commit be6b42313d on Jun 2, 2023
  34. maflcko commented at 3:22 pm on June 20, 2023: member
    Did this break the llvm symbolizer? At least it seem plausible, looking at the logs from https://cirrus-ci.com/task/6005512018329600?logs=ci#L3245 and https://cirrus-ci.com/task/5624442194231296?logs=ci#L2452
  35. fanquake commented at 4:09 pm on June 20, 2023: member

    Did this break the llvm symbolizer?

    I’ll follow up with this.

  36. fanquake referenced this in commit 8d5b93cf54 on Jun 21, 2023
  37. sidhujag referenced this in commit 609f277bb3 on Jun 21, 2023
  38. fanquake referenced this in commit 0c84a0e484 on Jun 22, 2023
  39. bitcoin locked this on Jun 19, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-07-03 13:13 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me