From 03d76ffda6e74135e49bc8512d753509bed190f1 Mon Sep 17 00:00:00 2001 From: Richard Patel Date: Mon, 8 Jan 2024 20:23:35 +0000 Subject: [PATCH 1/2] Build system post-merge fixes - Get Cargo workflow to work again - Remove unused LICENSE allow in deny.toml (tool would report error otherwise) - Fix permissioning in "On Main Push" top-level workflow - Fix argument expansion in ci_tests.sh - Fix OBJDIR resolving in ci_tests.sh - Use linux_clang_haswell instead of linux_clang_zen2 for AVX2 tests --- .github/workflows/cargo.yml | 51 ++++++------------------- .github/workflows/clusterfuzz.yml | 7 ++-- .github/workflows/coverage_report.yml | 4 +- .github/workflows/on_main_push.yml | 4 ++ .github/workflows/tests.yml | 4 +- config/everything.mk | 55 ++++++++++++++++++++++----- config/machine/linux_clang_haswell.mk | 31 +++++++++++++++ contrib/test/ci_tests.sh | 12 +++--- ffi/rust/deny.toml | 4 -- 9 files changed, 104 insertions(+), 68 deletions(-) create mode 100644 config/machine/linux_clang_haswell.mk diff --git a/.github/workflows/cargo.yml b/.github/workflows/cargo.yml index 01aff931c2..a2c6b198a1 100644 --- a/.github/workflows/cargo.yml +++ b/.github/workflows/cargo.yml @@ -11,64 +11,37 @@ jobs: if: github.ref == 'refs/heads/main' steps: - uses: actions/checkout@v4 - - uses: actions-rs/toolchain@v1 - with: - profile: minimal - toolchain: stable - override: true + - uses: dtolnay/rust-toolchain@stable - name: stage run: bash ffi/rust/firedancer-sys/stage.sh - - uses: actions-rs/cargo@v1 - with: - command: package - args: --allow-dirty --manifest-path ffi/rust/firedancer-sys/Cargo.toml + - run: cargo package --allow-dirty --manifest-path ffi/rust/firedancer-sys/Cargo.toml test: name: cargo-test runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - uses: actions-rs/toolchain@v1 - with: - profile: minimal - toolchain: stable - override: true - - uses: actions-rs/cargo@v1 - with: - command: test - args: --all + - uses: dtolnay/rust-toolchain@stable + - run: cargo test --all + working-directory: ffi/rust fmt: name: cargo-rustfmt runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - uses: actions-rs/toolchain@v1 - with: - profile: minimal - toolchain: nightly - override: true - components: rustfmt, clippy - - uses: actions-rs/cargo@v1 - with: - command: fmt - args: --all -- --check + - uses: dtolnay/rust-toolchain@stable + - run: cargo fmt --all -- --check + working-directory: ffi/rust clippy: name: cargo-clippy runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - uses: actions-rs/toolchain@v1 - with: - profile: minimal - toolchain: nightly - override: true - components: rustfmt, clippy - - uses: actions-rs/cargo@v1 - with: - command: clippy - args: --all -- -D warnings + - uses: dtolnay/rust-toolchain@stable + - run: cargo clippy --all -- -D warnings + working-directory: ffi/rust cargo-deny: name: cargo-deny @@ -77,4 +50,4 @@ jobs: - uses: actions/checkout@v4 - uses: EmbarkStudios/cargo-deny-action@v1 with: - command: check bans licenses sources + command: --manifest-path ffi/rust/Cargo.toml check bans licenses sources diff --git a/.github/workflows/clusterfuzz.yml b/.github/workflows/clusterfuzz.yml index 397372bb0f..80888783a6 100644 --- a/.github/workflows/clusterfuzz.yml +++ b/.github/workflows/clusterfuzz.yml @@ -4,18 +4,17 @@ on: workflow_dispatch: jobs: clusterfuzz-publish: - name: Build Fuzzers for ${{ matrix.feature_set }} feature set environment: name: clusterfuzz url: 'https://isol-clusterfuzz.appspot.com/' strategy: matrix: machine: - - linux_clang_zen2 + - linux_clang_haswell - linux_clang_icelake include: - - machine: linux_clang_zen2 - artifact_dir: build/linux/clang/zen2 + - machine: linux_clang_haswell + artifact_dir: build/linux/clang/haswell qualifier: modern - machine: linux_clang_icelake artifact_dir: build/linux/clang/icelake diff --git a/.github/workflows/coverage_report.yml b/.github/workflows/coverage_report.yml index 8fd20497df..f504d86d75 100644 --- a/.github/workflows/coverage_report.yml +++ b/.github/workflows/coverage_report.yml @@ -25,7 +25,7 @@ jobs: - name: Generate all coverage run: | sudo prlimit --pid $$ --memlock=-1:-1 - MACHINES="linux_clang_x86_64 linux_clang_zen2 linux_clang_icelake" \ + MACHINES="linux_clang_x86_64 linux_clang_haswell linux_clang_icelake" \ EXTRAS="llvm-cov fuzz" \ COV_REPORT=1 \ contrib/test/ci_tests.sh @@ -40,7 +40,7 @@ jobs: - name: Upload artifact run: | - gcloud storage rm -r ${{ vars.COVERAGE_BUCKET }}/ + gcloud storage rm -r ${{ vars.COVERAGE_BUCKET }}/ || true gcloud storage cp -r ./build/cov/html/* ${{ vars.COVERAGE_BUCKET }}/ - name: Upload coverage report to CodeCov diff --git a/.github/workflows/on_main_push.yml b/.github/workflows/on_main_push.yml index de0c838e90..60ec12379c 100644 --- a/.github/workflows/on_main_push.yml +++ b/.github/workflows/on_main_push.yml @@ -6,6 +6,10 @@ jobs: book: uses: ./.github/workflows/book.yml secrets: inherit + permissions: + contents: read + pages: write + id-token: write clusterfuzz: uses: ./.github/workflows/clusterfuzz.yml diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 5f93557995..5f06b221e5 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -16,7 +16,7 @@ jobs: - linux_gcc_noarch64 # least capable target - linux_gcc_icelake # most capable target - linux_clang_x86_64 - - linux_clang_zen2 + - linux_clang_haswell - linux_clang_icelake # Attach additional params to machine types include: @@ -26,7 +26,7 @@ jobs: group: rhel85-icelake - machine: linux_clang_x86_64 extras: fuzz - - machine: linux_clang_zen2 + - machine: linux_clang_haswell extras: fuzz - machine: linux_clang_icelake group: rhel85-icelake diff --git a/config/everything.mk b/config/everything.mk index 794886d2cc..bfb8666844 100644 --- a/config/everything.mk +++ b/config/everything.mk @@ -402,27 +402,61 @@ seccomp-policies: $(FIND) . -name '*.seccomppolicy' -exec $(PYTHON) contrib/test/generate_filters.py {} \; ############################## -# Coverage - -# Merge and index "raw" profile data from test runs +# LLVM Coverage +# +# Below steps create a report which lines of code have been executed/ +# "covered" by tests. For convenience, below supports merging coverage +# data from multiple machine types. +# +# Enabling the 'llvm-cov' extra has two effects on clang-compiled objects: +# - Coverage instrumentation is inserted, which causes profile data +# to get written out to disk when running code +# - Adds an "__llvm_covmap" section to each object containing "coverage +# mappings". Those tells tooling how to translate profile data to +# source line coverage +# +# Documentation: +# https://clang.llvm.org/docs/SourceBasedCodeCoverage.html +# https://llvm.org/docs/CoverageMappingFormat.html +# +# We thus have these steps: +# 1. Compile with llvm-cov +# 2. Run tests (This Makefile sets $LLVM_PROFILE_DATA appropriately for each kind of test) +# 3. Merge raw profiles from test runs into a single profile per machine using 'llvm-profdata merge' +# 4. Merge coverage mappings from all objects across all machines into an .ar file +# 5. Combine profile data and coverage mapping using 'llvm-cov export' +# 6. Generate HTML report using 'genhtml' + +# llvm-cov step 3: Merge and index "raw" profile data from test runs $(OBJDIR)/cov/cov.profdata: $(wildcard $(OBJDIR)/cov/raw/*.profraw) mkdir -p $(OBJDIR)/cov && $(LLVM_PROFDATA) merge -o $@ $^ -# + # Usage: $(call make-lcov,,) # e.g. $(call make-lcov,build/cov,build/machine1 build/machine2) # will create build/cov/cov.lcov from build/machine{1,2}/cov/cov.profdata define _make-lcov -$(1)/cov/cov.lcov: $$(addsuffix /cov/cov.profdata,$(2)) +# llvm-cov step 4 +# Create a thin archive containing all objects with coverage mappings. +# Sigh ... llvm-cov has a bug that makes it blow up when it encounters +# any object in the archive without an __llvm_covmap section, such as +# objects compiled from assembly code. +.PHONY: $(1)/cov/mappings.ar +$(1)/cov/mappings.ar: + rm -f $(1)/cov/mappings.ar && \ + mkdir -p $$(dir $$@) && \ + find $$(addsuffix /obj,$(2)) -name '*.o' -exec sh -c \ + '[[ -n "`llvm-objdump -h $$$$1 | grep llvm_covmap`" ]] \ + && llvm-ar --thin q $$@ $$$$1' sh {} \; + +# llvm-cov step 5 +$(1)/cov/cov.lcov: $$(addsuffix /cov/cov.profdata,$(2)) $(1)/cov/mappings.ar ifeq ($(2),) echo "No profile data found. Did you set OBJDIRS?" >&2 && exit 1 endif - mkdir -p $$(dir $$@) && \ -$(LLVM_COV) export \ + $(LLVM_COV) export \ -format=lcov \ $$(addprefix -instr-profile=,$$<) \ - $$(foreach dir,$(2),$$(shell find $(2)/obj \ - -name '*.o' \ - -exec printf "-object=%q\n" {} \;)) \ + $(1)/cov/mappings.ar \ --ignore-filename-regex="test_.*\\.c" \ > $$@ endef @@ -431,6 +465,7 @@ make-lcov = $(eval $(call _make-lcov,$(1),$(2))) # Create lcov report for current target $(call make-lcov,$(OBJDIR),$(OBJDIR)) +# llvm-cov step 6 # Create HTML coverage report using lcov genhtml %/cov/html/index.html: %/cov/cov.lcov rm -rf $(dir $@) && $(GENHTML) --output $(dir $@) $< diff --git a/config/machine/linux_clang_haswell.mk b/config/machine/linux_clang_haswell.mk new file mode 100644 index 0000000000..7835e2bccd --- /dev/null +++ b/config/machine/linux_clang_haswell.mk @@ -0,0 +1,31 @@ +BUILDDIR?=linux/clang/haswell + +include config/base.mk +include config/extra/with-security.mk +include config/extra/with-clang.mk +include config/extra/with-x86-64.mk +include config/extra/with-debug.mk +include config/extra/with-brutality.mk +include config/extra/with-optimization.mk +include config/extra/with-threads.mk +include config/extra/with-openssl.mk +include config/extra/with-zstd.mk + +CPPFLAGS+=-march=haswell -mtune=haswell + +CPPFLAGS+=\ + -DFD_HAS_INT128=1 \ + -DFD_HAS_DOUBLE=1 \ + -DFD_HAS_ALLOCA=1 \ + -DFD_HAS_X86=1 \ + -DFD_HAS_SSE=1 \ + -DFD_HAS_AVX=1 \ + -DFD_HAS_AESNI=1 + +FD_HAS_INT128:=1 +FD_HAS_DOUBLE:=1 +FD_HAS_ALLOCA:=1 +FD_HAS_X86:=1 +FD_HAS_SSE:=1 +FD_HAS_AVX:=1 +FD_HAS_AESNI:=1 diff --git a/contrib/test/ci_tests.sh b/contrib/test/ci_tests.sh index fd1cb2319d..91cba983a7 100755 --- a/contrib/test/ci_tests.sh +++ b/contrib/test/ci_tests.sh @@ -26,12 +26,10 @@ set -x # Build and run tests for all machines OBJDIRS=( ) -for MACHINE in "${MACHINES[*]}"; do - # TODO hacky - OBJDIR="build/$(echo "$MACHINE" | tr "_" "/")" - OBJDIRS+=( "${OBJDIR}" ) - +for MACHINE in ${MACHINES[*]}; do export MACHINE + OBJDIR="$(make help | grep OBJDIR | awk '{print $4}')" + OBJDIRS+=( "${OBJDIR}" ) make clean --silent >/dev/null contrib/make-j if [[ "$NOTEST" != 1 ]]; then @@ -44,11 +42,11 @@ for MACHINE in "${MACHINES[*]}"; do make "${OBJDIR}/cov/cov.profdata" fi fi - export -n EXTRAS + export -n MACHINE done # Export coverage report if [[ "$COV_REPORT" == 1 ]]; then make dist-cov-report OBJDIRS="${OBJDIRS[*]}" - contrib/test/find_uncovered_fuzz_canaries.py buld/cov/cov.lcov + contrib/test/find_uncovered_fuzz_canaries.py build/cov/cov.lcov || true fi diff --git a/ffi/rust/deny.toml b/ffi/rust/deny.toml index 3c19e7ce84..863e34c015 100644 --- a/ffi/rust/deny.toml +++ b/ffi/rust/deny.toml @@ -5,13 +5,9 @@ unused-allowed-license = "deny" allow = [ "MIT", "Apache-2.0", - "BSD-2-Clause", "BSD-3-Clause", - "0BSD", "ISC", "Unicode-DFS-2016", - "MPL-2.0", - "OpenSSL", ] [[licenses.clarify]] From 8e5bb22c00b6c80794e76e7c4bd881ee9cd6118f Mon Sep 17 00:00:00 2001 From: Richard Patel Date: Mon, 8 Jan 2024 23:06:41 +0000 Subject: [PATCH 2/2] Never substitute VSQRTPS with VRSQRTPS on Clang --- config/extra/with-clang.mk | 3 +++ 1 file changed, 3 insertions(+) diff --git a/config/extra/with-clang.mk b/config/extra/with-clang.mk index b4159820e2..021c8ab71b 100644 --- a/config/extra/with-clang.mk +++ b/config/extra/with-clang.mk @@ -41,3 +41,6 @@ CPPFLAGS+=-DFD_USING_CLANG=1 -Wno-address-of-packed-member -Wno-unused-command-l FD_USING_CLANG:=1 +# Don't attempt to transform vsprtps into vrsqrtps (not IEEE-compliant) + +CPPFLAGS+=-Xclang -target-feature -Xclang +fast-vector-fsqrt