Skip to content

Commit 9527cad

Browse files
authored
Coverage with codecov.io (#82)
* Coverage with codecov.io * Submit coverage data in lcov format instead * Address coverage review comments
1 parent 02e6606 commit 9527cad

File tree

12 files changed

+226
-64
lines changed

12 files changed

+226
-64
lines changed

.circleci/config.yml

+29-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
---
22
version: 2.1
3+
orbs:
4+
codecov: codecov/[email protected]
35
release_tag_only: &release_tag_only
46
filters:
57
tags:
@@ -107,6 +109,28 @@ jobs:
107109
MAKE_JOB_COUNT: 8
108110
WAF: "<< parameters.waf >>"
109111
NGINX_VERSION: "<< parameters.nginx-version >>"
112+
coverage:
113+
environment:
114+
DOCKER_BUILDKIT: 1
115+
steps:
116+
- checkout
117+
- run: git submodule sync && git submodule update --init --recursive
118+
- run: echo -e "ARCH=amd64\nBASE_IMAGE=nginx:1.26.0\n" > nginx-version-info
119+
- run:
120+
command: 'make coverage'
121+
environment:
122+
ARCH: x86_64
123+
MAKE_JOB_COUNT: 8
124+
BUILD_TYPE: RelWithDebInfo
125+
NGINX_VERSION: 1.26.0
126+
WAF: ON
127+
- codecov/upload:
128+
upload_args: '--disable-search'
129+
file: .musl-build/coverage.lcov
130+
upload_name: circleci
131+
machine:
132+
image: ubuntu-2204:current
133+
resource_class: xlarge
110134
test:
111135
parameters:
112136
base-image:
@@ -133,7 +157,6 @@ jobs:
133157
- run: printf "ARCH=%s\nBASE_IMAGE=%s\n" << parameters.arch >> << parameters.base-image >> > nginx-version-info
134158
- setup_remote_docker:
135159
docker_layer_caching: true
136-
- run: 'env | sort'
137160
- run: test/bin/run --verbose --failfast
138161
- store_artifacts:
139162
path: test/logs/test.log
@@ -195,6 +218,11 @@ workflows:
195218
filters:
196219
tags:
197220
ignore: "/^v[0-9]+\\.[0-9]+\\.[0-9]+/"
221+
- coverage:
222+
name: Coverage on 1.26.0 with WAF ON
223+
filters:
224+
tags:
225+
ignore: "/^v[0-9]+\\.[0-9]+\\.[0-9]+/"
198226
- test:
199227
matrix:
200228
parameters:

CMakeLists.txt

+12
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ if (NGINX_SRC_DIR STREQUAL "" AND NGINX_VERSION STREQUAL "")
2121
message(FATAL_ERROR "Set NGINX_SRC_DIR or, alternatively NGINX_VERSION")
2222
endif()
2323
option(NGINX_PATCH_AWAY_LIBC "Patch away libc dependency" OFF)
24+
option(NGINX_COVERAGE "Add coverage instrumentation" OFF)
2425

2526
# Make curl link against a static zlib (requires cmake 3.24)
2627
set(ZLIB_USE_STATIC_LIBS ON)
@@ -116,6 +117,17 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
116117
target_compile_options(ngx_http_datadog_module PRIVATE -Wall -Werror)
117118
endif()
118119

120+
if(NGINX_COVERAGE)
121+
if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
122+
target_compile_options(ngx_http_datadog_module PRIVATE
123+
-fprofile-instr-generate -fcoverage-mapping -mllvm -runtime-counter-relocation=true)
124+
target_link_options(ngx_http_datadog_module PRIVATE
125+
-fprofile-instr-generate -mllvm -runtime-counter-relocation=true)
126+
target_sources(ngx_http_datadog_module PRIVATE src/coverage_fixup.c)
127+
else()
128+
message(FATAL_ERROR "NGINX_COVERAGE is only supported with clang")
129+
endif()
130+
endif()
119131

120132
target_include_directories(ngx_http_datadog_module
121133
PRIVATE

Makefile

+21-1
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,16 @@ MAKE_JOB_COUNT ?= $(shell nproc)
77
PWD ?= $(shell pwd)
88
NGINX_SRC_DIR ?= $(PWD)/nginx
99
ARCH ?= $(shell arch)
10+
COVERAGE ?= OFF
1011
DOCKER_REPOS ?= public.ecr.aws/b1o7r7e0/nginx_musl_toolchain
1112

1213
SHELL := /bin/bash
1314

1415
.PHONY: build
1516
build: build-deps sources
1617
# -DCMAKE_C_FLAGS=-I/opt/homebrew/Cellar/pcre2/10.42/include/ -DCMAKE_CXX_FLAGS=-I/opt/homebrew/Cellar/pcre2/10.42/include/ -DCMAKE_LDFLAGS=-L/opt/homebrew/Cellar/pcre2/10.42/lib -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang
17-
cmake -B$(BUILD_DIR) -DNGINX_SRC_DIR=$(NGINX_SRC_DIR) -DCMAKE_BUILD_TYPE=$(BUILD_TYPE) -DNGINX_DATADOG_ASM_ENABLED=$(WAF) . \
18+
cmake -B$(BUILD_DIR) -DNGINX_SRC_DIR=$(NGINX_SRC_DIR) \
19+
-DNGINX_COVERAGE=$(COVERAGE) -DCMAKE_BUILD_TYPE=$(BUILD_TYPE) -DNGINX_DATADOG_ASM_ENABLED=$(WAF) . \
1820
&& cmake --build $(BUILD_DIR) -j $(MAKE_JOB_COUNT) -v
1921
chmod 755 $(BUILD_DIR)/ngx_http_datadog_module.so
2022
@echo 'build successful 👍'
@@ -95,6 +97,7 @@ build-musl:
9597
--env BUILD_TYPE=$(BUILD_TYPE) \
9698
--env NGINX_VERSION=$(NGINX_VERSION) \
9799
--env WAF=$(WAF) \
100+
--env COVERAGE=$(COVERAGE) \
98101
--mount "type=bind,source=$(PWD),destination=/mnt/repo" \
99102
$(DOCKER_REPOS):latest \
100103
make -C /mnt/repo build-musl-aux
@@ -108,6 +111,7 @@ build-musl-aux:
108111
-DCMAKE_BUILD_TYPE=$(BUILD_TYPE) \
109112
-DNGINX_VERSION="$(NGINX_VERSION)" \
110113
-DNGINX_DATADOG_ASM_ENABLED="$(WAF)" . \
114+
-DNGINX_COVERAGE=$(COVERAGE) \
111115
&& cmake --build .musl-build -j $(MAKE_JOB_COUNT) -v
112116

113117

@@ -116,6 +120,22 @@ test: build-musl
116120
cp -v .musl-build/ngx_http_datadog_module.so* test/services/nginx/
117121
test/bin/run $(TEST_ARGS)
118122

123+
.PHONY: coverage
124+
coverage:
125+
COVERAGE=ON $(MAKE) build-musl
126+
cp -v .musl-build/ngx_http_datadog_module.so* test/services/nginx/
127+
rm -f test/coverage_data.tar.gz
128+
test/bin/run --verbose --failfast
129+
docker run --init --rm --platform $(DOCKER_PLATFORM) \
130+
--mount "type=bind,source=$(PWD),destination=/mnt/repo" \
131+
$(DOCKER_REPOS):latest \
132+
tar -C /mnt/repo/.musl-build -xzf /mnt/repo/test/coverage_data.tar.gz
133+
docker run --init --rm --platform $(DOCKER_PLATFORM) \
134+
--mount "type=bind,source=$(PWD),destination=/mnt/repo" \
135+
$(DOCKER_REPOS):latest \
136+
bin/sh -c 'cd /mnt/repo/.musl-build; llvm-profdata merge -sparse *.profraw -o default.profdata && llvm-cov export ./ngx_http_datadog_module.so -format=lcov -instr-profile=default.profdata -ignore-filename-regex=/mnt/repo/src/coverage_fixup\.c > coverage.lcov'
137+
138+
119139
.PHONY: test-parallel
120140
test-parallel: build-in-docker
121141
cp -v .musl-build/ngx_http_datadog_module.so* test/services/nginx/

bin/format.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ if ! [ -e .clang-format ]; then
66
exit 1
77
fi
88

9-
find src/ -type f \( -name '*.h' -o -name '*.cpp' \) -print0 | xargs -0 clang-format-14 -i --style=file
9+
find src/ -type f \( -name '*.h' -o -name '*.cpp' -o -name '*.c' \) -print0 | xargs -0 clang-format-14 -i --style=file
1010
find bin/ -type f -name '*.py' -print0 | xargs -0 yapf3 -i
1111

1212
yapf3 --recursive --in-place "$@" "test/"

build_env/Dockerfile

+1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ RUN mkdir -p /sysroot/${ARCH}-none-linux-musl/usr
5252
RUN ln -s /usr/lib /sysroot/${ARCH}-none-linux-musl/usr/
5353
RUN ln -s /usr/include /sysroot/${ARCH}-none-linux-musl/usr/
5454
RUN ln -s /lib /sysroot/${ARCH}-none-linux-musl/
55+
RUN ln -s /usr/lib/llvm16/lib/clang/16/lib /sysroot/${ARCH}-none-linux-musl/usr/lib/resource_dir/lib
5556

5657
COPY Toolchain.cmake.${ARCH} /sysroot/${ARCH}-none-linux-musl/Toolchain.cmake
5758

codecov.yml

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
comment:
2+
layout: "reach, diff, flags, files"
3+
behavior: default
4+
require_changes: no
5+
6+
coverage:
7+
status:
8+
project:
9+
default:
10+
enabled: no

src/coverage_fixup.c

+50
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
#include <pthread.h>
2+
#include <stdio.h>
3+
#include <stdlib.h>
4+
#include <string.h>
5+
6+
// The name of the file to which profile data is written is inherited by the
7+
// forked processes, which never recalculate this, thereby attempting to
8+
// overwrite the file belonging to the parent process. Recalculation can be
9+
// forced by calling __llvm_profile_initialize_file() on the child, but this is
10+
// not enough because nginx will have santized the environment.
11+
12+
void __llvm_profile_initialize_file(void);
13+
14+
static const char *profile_file;
15+
16+
static void datadog_fixup_profile_file_child(void);
17+
18+
__attribute__((constructor)) static void datadog_fixup_profile_file_init() {
19+
profile_file = getenv("LLVM_PROFILE_FILE");
20+
if (!profile_file) {
21+
fprintf(stderr, "Environment variable LLVM_PROFILE_FILE is undefined");
22+
abort();
23+
}
24+
25+
// work around the profile not being updated when the new pattern is the
26+
// same as the new one.
27+
// See
28+
// https://github.com/llvm/llvm-project/blob/82c5d350d200ccc5365d40eac187b9ec967af727/compiler-rt/lib/profile/InstrProfilingFile.c#L870
29+
char *modified_profile_file = malloc(strlen(profile_file) + 3);
30+
if (profile_file[0] == '/') {
31+
modified_profile_file[0] = '/';
32+
strcpy(&modified_profile_file[1], profile_file);
33+
} else {
34+
modified_profile_file[0] = '.';
35+
modified_profile_file[1] = '/';
36+
strcpy(&modified_profile_file[2], profile_file);
37+
}
38+
profile_file = modified_profile_file;
39+
40+
int ret = pthread_atfork(0, 0, datadog_fixup_profile_file_child);
41+
if (ret != 0) {
42+
perror("Calling pthread_atfork");
43+
abort();
44+
}
45+
}
46+
47+
static void datadog_fixup_profile_file_child() {
48+
setenv("LLVM_PROFILE_FILE", profile_file, 1);
49+
__llvm_profile_initialize_file();
50+
}

src/glibc_compat.c

+60-59
Original file line numberDiff line numberDiff line change
@@ -2,54 +2,42 @@
22

33
#ifdef __x86_64__
44
float ceilf(float x) {
5-
float result;
6-
__asm__(
7-
"roundss $0x0A, %[x], %[result]"
8-
: [result] "=x" (result)
9-
: [x] "x" (x)
10-
);
11-
return result;
5+
float result;
6+
__asm__("roundss $0x0A, %[x], %[result]"
7+
: [result] "=x"(result)
8+
: [x] "x"(x));
9+
return result;
1210
}
1311
double ceil(double x) {
14-
double result;
15-
__asm__(
16-
"roundsd $0x0A, %[x], %[result]"
17-
: [result] "=x" (result)
18-
: [x] "x" (x)
19-
);
20-
return result;
12+
double result;
13+
__asm__("roundsd $0x0A, %[x], %[result]"
14+
: [result] "=x"(result)
15+
: [x] "x"(x));
16+
return result;
2117
}
2218
#endif
2319

2420
#ifdef __aarch64__
2521
float ceilf(float x) {
26-
float result;
27-
__asm__(
28-
"frintp %s0, %s1\n"
29-
: "=w" (result)
30-
: "w" (x)
31-
);
32-
return result;
22+
float result;
23+
__asm__("frintp %s0, %s1\n" : "=w"(result) : "w"(x));
24+
return result;
3325
}
3426
double ceil(double x) {
35-
double result;
36-
__asm__(
37-
"frintp %d0, %d1\n"
38-
: "=w" (result)
39-
: "w" (x)
40-
);
41-
return result;
27+
double result;
28+
__asm__("frintp %d0, %d1\n" : "=w"(result) : "w"(x));
29+
return result;
4230
}
4331
#endif
4432

4533
int stat(const char *restrict path, void *restrict buf) {
46-
int __xstat(int, const char *restrict, void *restrict);
47-
return __xstat(3, path, buf);
34+
int __xstat(int, const char *restrict, void *restrict);
35+
return __xstat(3, path, buf);
4836
}
4937

5038
int fstat(int fd, void *buf) {
51-
int __fxstat(int, int, void *);
52-
return __fxstat(3, fd, buf);
39+
int __fxstat(int, int, void *);
40+
return __fxstat(3, fd, buf);
5341
}
5442

5543
// glibc doesn't define pthread_atfork on aarch64. We need to delegate to
@@ -80,45 +68,58 @@ int __register_atfork(void (*prepare)(void), void (*parent)(void),
8068

8169
int pthread_atfork(void (*prepare)(void), void (*parent)(void),
8270
void (*child)(void)) {
83-
// glibc
84-
if (__dso_handle && __register_atfork) {
85-
return __register_atfork(prepare, parent, child, __dso_handle);
86-
}
71+
// glibc
72+
if (__dso_handle && __register_atfork) {
73+
return __register_atfork(prepare, parent, child, __dso_handle);
74+
}
8775

88-
static int (*real_atfork)(void (*)(void), void (*)(void), void (*)(void));
76+
static int (*real_atfork)(void (*)(void), void (*)(void), void (*)(void));
8977

90-
if (!real_atfork) {
91-
// dlopen musl
78+
if (!real_atfork) {
79+
// dlopen musl
9280
#ifdef __aarch64__
93-
void *handle = dlopen("ld-musl-aarch64.so.1", RTLD_LAZY);
94-
if (!handle) {
95-
fprintf(stderr, "dlopen of ld-musl-aarch64.so.1 failed: %s\n",
96-
dlerror());
97-
abort();
98-
}
81+
void *handle = dlopen("ld-musl-aarch64.so.1", RTLD_LAZY);
82+
if (!handle) {
83+
fprintf(stderr, "dlopen of ld-musl-aarch64.so.1 failed: %s\n", dlerror());
84+
abort();
85+
}
9986
#else
100-
void *handle = dlopen("libc.musl-x86_64.so.1", RTLD_LAZY);
101-
if (!handle) {
102-
fprintf(stderr, "dlopen of libc.musl-x86_64.so.1 failed: %s\n",
103-
dlerror());
104-
abort();
105-
}
87+
void *handle = dlopen("libc.musl-x86_64.so.1", RTLD_LAZY);
88+
if (!handle) {
89+
fprintf(stderr, "dlopen of libc.musl-x86_64.so.1 failed: %s\n",
90+
dlerror());
91+
abort();
92+
}
10693
#endif
107-
real_atfork = dlsym(handle, "pthread_atfork");
108-
if (!real_atfork) {
109-
fprintf(stderr, "dlsym of pthread_atfork failed: %s\n", dlerror());
110-
abort();
111-
}
94+
real_atfork = dlsym(handle, "pthread_atfork");
95+
if (!real_atfork) {
96+
fprintf(stderr, "dlsym of pthread_atfork failed: %s\n", dlerror());
97+
abort();
11298
}
99+
}
113100

114-
return real_atfork(prepare, parent, child);
101+
return real_atfork(prepare, parent, child);
115102
}
116103

117104
// the symbol strerror_r in glibc is not the POSIX version; it returns char *
118105
// __xpg_sterror_r is exported by both glibc and musl
119106
int strerror_r(int errnum, char *buf, size_t buflen) {
120-
int __xpg_strerror_r(int, char *, size_t);
121-
return __xpg_strerror_r(errnum, buf, buflen);
107+
int __xpg_strerror_r(int, char *, size_t);
108+
return __xpg_strerror_r(errnum, buf, buflen);
109+
}
110+
111+
// when compiling with --coverage, some references to atexit show up.
112+
// glibc doesn't provide atexit for similar reasons as pthread_atfork presumably
113+
int __cxa_atexit(void (*func)(void *), void *arg, void *dso_handle);
114+
int atexit(void (*function)(void)) {
115+
if (!__dso_handle) {
116+
fprintf(stderr, "Aborting because __dso_handle is NULL\n");
117+
abort();
118+
}
119+
120+
// the cast is harmless on amd64 and aarch64. Passing an extra argument to a
121+
// function that expects none causes no problems
122+
return __cxa_atexit((void (*)(void *))function, 0, __dso_handle);
122123
}
123124

124125
#endif

test/.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
__pycache__/
22
test.times
3+
/coverage_data.tar.gz

test/bin/run

+1-1
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,5 @@ printf " arch : %s\n" "${ARCH}"
1111
printf "base image : %s\n" "${BASE_IMAGE}"
1212
printf " test args : %s\n" "$@"
1313

14-
docker compose build --parallel
14+
docker compose build --parallel --progress=plain
1515
python3 -m unittest "$@"

0 commit comments

Comments
 (0)