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
firedancer-io · Jan 9, 2024 · aeb2e4e · aeb2e4e
1 parent 9f376fc
commit aeb2e4e
Show file tree

Hide file tree

Showing 9 changed files with 102 additions and 66 deletions.
diff --git 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
@@ -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
@@ -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

diff --git 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
@@ -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
@@ -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,<report_objdir>,<profdata_objdirs>)
 # 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`" ]]  \
+    && $(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
@@ -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
@@ -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,7 +42,7 @@ for MACHINE in "${MACHINES[*]}"; do
       make "${OBJDIR}/cov/cov.profdata"
     fi
   fi
-  export -n EXTRAS
+  export -n MACHINE
 done
 
 # Export coverage report

diff --git 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]]