From 24f474878e272ae943076b345da3df7cbf5d6540 Mon Sep 17 00:00:00 2001 From: Pavel Punsky Date: Mon, 4 May 2026 18:49:18 -0700 Subject: [PATCH] Filc harness and pointer typedefs (#1896) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - Add a self-contained Fil-C build/test harness under `filc/` that mirrors the existing `fuzzing/` pattern: one host script (`filc/run-local.sh`) builds an Ubuntu 24.04 image with the [Fil-C](https://github.com/pizlonator/fil-c) optfil 0.678 toolchain, builds turnserver with `CC=filcc`/`CXX=fil++`, runs unit tests + system tests, and drops a per-run timestamped log directory with `SUMMARY.txt` + `ISSUES.txt`. - Fix the two real Fil-C compatibility bugs the harness surfaces by changing `ur_map_value_type` and `ur_addr_map_value_type` from `uintptr_t` to `void *` in `src/server/ns_turn_maps.h`. ## Why [Fil-C](https://fil-c.org) is a memory-safe C/C++ compiler (Clang 20 fork) that pairs every pointer with an "InvisiCap" capability and turns UB into deterministic panics with no `unsafe` escape hatch. Putting coturn through it answers two questions: (a) does it compile unmodified, and (b) does it run correctly under capability-enforced memory safety. After this PR, the answer is **yes** for both — turnserver, turnutils_peer, and turnutils_uclient relay TCP/TLS/UDP/DTLS traffic with full Fil-C enforcement, all unit tests pass, and `examples/run_tests_conf.sh` runs end-to-end. ## What's in the PR ### `filc/` harness (commit 1) | File | Purpose | |---|---| | `filc/Dockerfile` | Ubuntu 24.04 + Fil-C optfil 0.678 (extracts the nested `fil.tar.xz` to `/opt/fil`); `--platform linux/amd64` so it works on Apple Silicon under emulation. | | `filc/run-local.sh` | Host-side: build image, create `filc/logs//`, run container with source mounted read-only and log dir mounted r/w. | | `filc/docker-entrypoint.sh` | In-container orchestrator. Phases: env / source-copy / build / unit-tests / system-cli / system-conf. Runs every phase even when a prior one fails (no aborting mid-pass). Captures per-phase logs + a combined `all.log` + JUnit XML for ctest. Greps panics/errors into `ISSUES.txt`. Downgrades `system-*` phases to FAIL when `examples/run_tests*.sh` prints `FAIL` despite exiting 0 (existing fragility in those scripts). | | `filc/build.sh` | `cmake … -DBUILD_TESTING=ON -DCMAKE_C_COMPILER=filcc -DCMAKE_CXX_COMPILER=fil++ -DCMAKE_BUILD_TYPE=RelWithDebInfo`, then build. | | `filc/.gitignore` | Ignore the on-host `logs/` dir. | The harness also bumps the post-launch sleep in `examples/run_tests.sh` from 2s to 6s **only inside the container** (sed-in-place on the copied source; upstream is untouched). Under linux/amd64 emulation the Fil-C-built turnserver isn't accepting TCP at 2s, so the first sub-test races and prints `FAIL`. Matches the 5s sleep already used by `run_tests_conf.sh`. ### Pointer-typedef fixes (commit 2) `src/server/ns_turn_maps.h`: ```diff -typedef uintptr_t ur_map_value_type; +typedef void *ur_map_value_type; ... -typedef uintptr_t ur_addr_map_value_type; +typedef void *ur_addr_map_value_type; ``` **Why this is necessary.** Both maps store pointers, but their value slot is integer-typed. Every existing `_put` site casts a pointer through `(ur_*_value_type)` to store, and every `_get` site casts back. Under standard C this is a well-defined no-op. Under Fil-C, casting a pointer to `uintptr_t` discards its InvisiCap; casting back yields a pointer with a non-null address but a NULL Fil-C object — the next dereference panics with `cannot read pointer with null object`. The harness caught two such panics, both in the auth-resume / relay-allocate flow: 1. `src/server/ns_turn_server.c:3248` — `ss->client_socket`, where `ss` came from `sessions_map` (a `ur_map`). 2. `src/apps/relay/turn_ports.c:225` — `tp->mutex`, where `tp` came from `ip_to_turnports_*` (a `ur_addr_map`) via `turnipports_add`. **Why this is also a correctness improvement on a normal build.** The new typedef makes the API strictly more type-safe — the compiler now enforces "you put a pointer in." It eliminates a class of accidental misuse (storing a non-pointer integer where a pointer was expected) that the integer typedef silently allowed. Same generated code on a normal build; different (correct) Fil-C semantics. **Audit.** Verified before changing: - All `ur_map_put` / `lm_map_put` / `ur_addr_map_put` callers store pointer-typed values exclusively (no callers store raw integers). - No internal arithmetic on the value type anywhere in `ns_turn_maps.c`. - `ur_map_del_func` / `ur_addr_map_func` implementations either don't exist (all `_del` callers pass `NULL`) or immediately cast their parameter to a real pointer type — no source change needed. - `KHASH_MAP_INIT_INT64(3, ur_map_value_type)` works identically with `void *`. - `ur_addr_map`'s `addr_elem.value` is assigned, read, compared for truthiness, and cleared with `= 0` — all valid for `void *`. ## Test plan - [ ] `filc/run-local.sh` reports all six phases PASS (env / source-copy / build / unit-tests / system-cli / system-conf), `ISSUES.txt` carries no Fil-C panic / safety / sanitizer entries. - [ ] Local `cmake -S . -B build -DBUILD_TESTING=ON && cmake --build build && ctest --test-dir build --output-on-failure` is green (no regression on the regular build). - [ ] `examples/run_tests.sh` and `examples/run_tests_conf.sh` are green on Linux per `CLAUDE.md`. - [ ] Existing `fuzzing/run-local.sh ASan 0 -runs=1` still passes (the new `filc/` directory is independent and shouldn't perturb anything). --- filc/.gitignore | 1 + filc/Dockerfile | 59 +++++++++++ filc/build.sh | 33 ++++++ filc/docker-entrypoint.sh | 212 ++++++++++++++++++++++++++++++++++++++ filc/run-local.sh | 90 ++++++++++++++++ src/server/ns_turn_maps.h | 4 +- 6 files changed, 397 insertions(+), 2 deletions(-) create mode 100644 filc/.gitignore create mode 100644 filc/Dockerfile create mode 100755 filc/build.sh create mode 100755 filc/docker-entrypoint.sh create mode 100755 filc/run-local.sh diff --git a/filc/.gitignore b/filc/.gitignore new file mode 100644 index 00000000..333c1e91 --- /dev/null +++ b/filc/.gitignore @@ -0,0 +1 @@ +logs/ diff --git a/filc/Dockerfile b/filc/Dockerfile new file mode 100644 index 00000000..a74f73f1 --- /dev/null +++ b/filc/Dockerfile @@ -0,0 +1,59 @@ +FROM --platform=linux/amd64 ubuntu:24.04 + +ENV DEBIAN_FRONTEND=noninteractive + +ARG FILC_VERSION=0.678 +ARG FILC_URL=https://github.com/pizlonator/fil-c/releases/download/v${FILC_VERSION}/optfil-${FILC_VERSION}-linux-x86_64.tar.xz + +RUN apt-get update \ + && apt-get install -y --no-install-recommends \ + bash \ + ca-certificates \ + cmake \ + curl \ + file \ + gawk \ + git \ + iproute2 \ + make \ + net-tools \ + pkg-config \ + procps \ + python3 \ + tar \ + unzip \ + xz-utils \ + && rm -rf /var/lib/apt/lists/* + +# The optfil release tarball wraps a nested fil.tar.xz that extracts to +# /opt/fil. We avoid the bundled setup.sh (interactive, attempts SSH/user +# setup) and just unpack the inner archive directly. set -eux makes any +# failure self-describing in the docker-build log. +RUN set -eux; \ + curl -fL --retry 5 --retry-delay 2 --retry-all-errors --max-time 600 \ + -o /tmp/optfil-outer.tar.xz "${FILC_URL}"; \ + ls -la /tmp/optfil-outer.tar.xz; \ + mkdir -p /tmp/optfil-extract /opt; \ + tar -C /tmp/optfil-extract -xf /tmp/optfil-outer.tar.xz; \ + inner=$(ls /tmp/optfil-extract/*/fil.tar.xz | head -n 1); \ + test -n "${inner}"; \ + tar -C /opt -xf "${inner}"; \ + rm -rf /tmp/optfil-outer.tar.xz /tmp/optfil-extract; \ + test -x /opt/fil/bin/filcc; \ + /opt/fil/bin/filcc --version + +ENV PATH=/opt/fil/bin:${PATH} +ENV CC=filcc +ENV CXX=fil++ +# Make CMake's find_package look at /opt/fil first so coturn links the +# Fil-C-built OpenSSL/libevent/sqlite, not the Ubuntu ones. +ENV CMAKE_PREFIX_PATH=/opt/fil +ENV PKG_CONFIG_PATH=/opt/fil/lib/pkgconfig:/opt/fil/lib64/pkgconfig + +WORKDIR /src + +COPY docker-entrypoint.sh /usr/local/bin/coturn-filc-local +COPY build.sh /usr/local/bin/coturn-filc-build +RUN chmod +x /usr/local/bin/coturn-filc-local /usr/local/bin/coturn-filc-build + +ENTRYPOINT ["/usr/local/bin/coturn-filc-local"] diff --git a/filc/build.sh b/filc/build.sh new file mode 100755 index 00000000..5e40d09f --- /dev/null +++ b/filc/build.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +# Configure and build coturn with the Fil-C toolchain. Called from +# docker-entrypoint.sh inside the container. + +set -euo pipefail + +src=/work/coturn +build="${src}/build" +log_dir="${LOG_DIR:-/out}" + +cd "${src}" + +echo "## cmake configure" +cmake -S . -B "${build}" \ + -DCMAKE_BUILD_TYPE=RelWithDebInfo \ + -DBUILD_TESTING=ON \ + -DCMAKE_C_COMPILER=filcc \ + -DCMAKE_CXX_COMPILER=fil++ \ + 2>&1 | tee "${log_dir}/configure.log" + +echo +echo "## cmake build" +cmake --build "${build}" --parallel "$(nproc)" \ + 2>&1 | tee "${log_dir}/build.log" + +echo +echo "## built binaries" +ls -la "${build}/bin" 2>/dev/null || true + +echo +echo "## sanity-check the relay binary" +file "${build}/bin/turnserver" 2>/dev/null || true +"${build}/bin/turnserver" -h 2>&1 | head -5 || true diff --git a/filc/docker-entrypoint.sh b/filc/docker-entrypoint.sh new file mode 100755 index 00000000..724cab47 --- /dev/null +++ b/filc/docker-entrypoint.sh @@ -0,0 +1,212 @@ +#!/usr/bin/env bash +# In-container orchestrator. Runs each phase, captures stdout+stderr to its +# own log file under /out, records exit codes in /out/SUMMARY.txt, and never +# aborts on a single phase failure — so one run surfaces every issue. + +# Deliberately not setting `set -e` at the top level: each phase manages its +# own failure handling so subsequent phases still run. +set -uo pipefail + +readonly mounted_src=/src +readonly work_src=/work/coturn +readonly log_dir=/out + +mkdir -p "${log_dir}" +: > "${log_dir}/all.log" +: > "${log_dir}/SUMMARY.txt" +printf '%-18s %-6s %-5s %s\n' phase status exit log >> "${log_dir}/SUMMARY.txt" + +overall_status=0 + +ts() { date -u +'%Y-%m-%dT%H:%M:%SZ'; } + +# run_phase +# Streams output to both the per-phase log and /out/all.log, with timestamps. +# Records the result in SUMMARY.txt. Updates overall_status on failure. +run_phase() { + local name=$1 + local log_file=$2 + shift 2 + + echo + echo "===== [$(ts)] PHASE: ${name} =====" + { + echo "===== [$(ts)] PHASE: ${name} =====" + echo "Command: $*" + } | tee -a "${log_dir}/${log_file}" "${log_dir}/all.log" >/dev/null + + local start_epoch end_epoch dur status + start_epoch=$(date -u +%s) + + set +e + ( "$@" ) 2>&1 \ + | gawk '{ print strftime("[%Y-%m-%dT%H:%M:%SZ] ", systime(), 1) $0; fflush(); }' \ + | tee -a "${log_dir}/${log_file}" "${log_dir}/all.log" + status=${PIPESTATUS[0]} + set -e + + end_epoch=$(date -u +%s) + dur=$((end_epoch - start_epoch)) + + local label=PASS + if [ "${status}" -ne 0 ]; then + label=FAIL + overall_status=1 + fi + + printf '%-18s %-6s %-5s %s\n' "${name}" "${label}" "${status}" "${log_file}" \ + >> "${log_dir}/SUMMARY.txt" + echo "===== [$(ts)] PHASE ${name}: ${label} (exit=${status}, ${dur}s) =====" + + return 0 +} + +# ----- phase: env ------------------------------------------------------------- +phase_env() { + echo "## uname" + uname -a + echo + echo "## /etc/os-release" + cat /etc/os-release + echo + echo "## CPU model" + grep -m1 'model name' /proc/cpuinfo || true + echo + echo "## Apt-installed build deps" + dpkg -l | grep -E 'libssl|libevent|sqlite|cmake|pkg-config|^ii curl' || true + echo + echo "## filcc --version" + filcc --version || true + echo + echo "## /opt/fil contents" + ls -la /opt/fil/bin 2>/dev/null | head -50 || true + echo + echo "## Fil-C-built libs available in /opt/fil" + ls /opt/fil/lib*/libssl* 2>/dev/null || true + ls /opt/fil/lib*/libcrypto* 2>/dev/null || true + ls /opt/fil/lib*/libevent* 2>/dev/null || true + ls /opt/fil/lib*/libsqlite* 2>/dev/null || true + echo + echo "## pkg-config sees" + for pkg in openssl libevent sqlite3; do + printf '%-12s ' "${pkg}" + pkg-config --modversion "${pkg}" 2>&1 || echo "(missing)" + done + echo + echo "## env" + env | sort | grep -E '^(CC|CXX|PATH|CMAKE_|PKG_CONFIG|LD_)' || true +} + +# ----- phase: source-copy ----------------------------------------------------- +phase_source_copy() { + rm -rf "${work_src}" + mkdir -p "${work_src}" + echo "Copying ${mounted_src} -> ${work_src} (excluding .git, build*, .DS_Store)" + tar \ + --exclude='.git' \ + --exclude='.DS_Store' \ + --exclude='build' \ + --exclude='build-*' \ + --exclude='filc/logs' \ + -C "${mounted_src}" \ + -cf - . | tar -C "${work_src}" -xf - + echo "Copy complete." + du -sh "${work_src}" + ls "${work_src}" +} + +# ----- phase: build ----------------------------------------------------------- +phase_build() { + LOG_DIR="${log_dir}" coturn-filc-build +} + +# ----- phase: unit-tests ------------------------------------------------------ +phase_unit_tests() { + ctest \ + --test-dir "${work_src}/build" \ + --output-on-failure \ + --output-junit "${log_dir}/unit-tests.junit.xml" +} + +# ----- phase: system-tests-cli ------------------------------------------------ +phase_system_cli() { + cd "${work_src}/examples" + # Bump the post-launch sleep from 2s to 6s. Under linux/amd64 emulation on + # Apple Silicon, the Fil-C-built turnserver is not yet accepting TCP at 2s, + # so the very first sub-test races and prints FAIL. Matches run_tests_conf.sh. + sed -i 's/^sleep 2$/sleep 6/' run_tests.sh + ./run_tests.sh +} + +# ----- phase: system-tests-conf ----------------------------------------------- +phase_system_conf() { + cd "${work_src}/examples" + ./run_tests_conf.sh +} + +# Run every phase, but skip later ones if a prerequisite already failed. +run_phase env env.log phase_env +run_phase source-copy source-copy.log phase_source_copy + +if grep -E '^source-copy +PASS' "${log_dir}/SUMMARY.txt" >/dev/null; then + run_phase build build.log phase_build +fi + +if grep -E '^build +PASS' "${log_dir}/SUMMARY.txt" >/dev/null; then + run_phase unit-tests unit-tests.log phase_unit_tests + run_phase system-cli run_tests.log phase_system_cli + run_phase system-conf run_tests_conf.log phase_system_conf +else + echo "Build failed; skipping test phases." + for skipped in unit-tests system-cli system-conf; do + printf '%-18s %-6s %-5s %s\n' "${skipped}" SKIP -- skipped \ + >> "${log_dir}/SUMMARY.txt" + done +fi + +# examples/run_tests.sh prints "FAIL" on failed test cases but exits 0. +# Catch that explicitly. +for phase_log in run_tests.log run_tests_conf.log; do + if [ -f "${log_dir}/${phase_log}" ] && \ + grep -Eq '(^|\] )FAIL$' "${log_dir}/${phase_log}"; then + echo "Detected FAIL marker in ${phase_log}; downgrading phase status." + case "${phase_log}" in + run_tests.log) target=system-cli ;; + run_tests_conf.log) target=system-conf ;; + esac + # Mark failure and update overall status. + awk -v t="${target}" ' + $1==t { printf "%-18s %-6s %-5s %s\n", $1, "FAIL", "1", $4; next } + { print } + ' "${log_dir}/SUMMARY.txt" > "${log_dir}/SUMMARY.txt.new" + mv "${log_dir}/SUMMARY.txt.new" "${log_dir}/SUMMARY.txt" + overall_status=1 + fi +done + +# ----- issues extraction ------------------------------------------------------ +{ + echo "# Issues extracted from /out/*.log" + echo "# Generated: $(ts)" + echo + for f in "${log_dir}"/*.log; do + [ -f "${f}" ] || continue + base=$(basename "${f}") + [ "${base}" = "all.log" ] && continue + matches=$(grep -nE \ + 'error:|undefined reference|warning:|FAIL$|SEGV|Segmentation fault|panic|Filc panic|filc panic|AddressSanitizer|LeakSanitizer|UndefinedBehaviorSanitizer|MemorySanitizer|assertion|Assertion .* failed|cannot find' \ + "${f}" || true) + if [ -n "${matches}" ]; then + echo "## ${base}" + echo "${matches}" + echo + fi + done +} > "${log_dir}/ISSUES.txt" + +echo +echo "===== [$(ts)] FINAL SUMMARY =====" +cat "${log_dir}/SUMMARY.txt" +echo "===== overall exit: ${overall_status} =====" + +exit "${overall_status}" diff --git a/filc/run-local.sh b/filc/run-local.sh new file mode 100755 index 00000000..7b2c96c2 --- /dev/null +++ b/filc/run-local.sh @@ -0,0 +1,90 @@ +#!/usr/bin/env bash +# Build coturn with the Fil-C compiler in Docker and run the test suite. +# All output is captured under filc/logs// on the host. +# +# Fil-C is Linux/x86_64 only; on Apple Silicon hosts the image runs under +# Docker Desktop's amd64 emulation (Rosetta or QEMU) and is therefore slow. + +set -euo pipefail + +usage() { + cat <<'EOF' +Usage: + filc/run-local.sh [args passed through to the in-container entrypoint] + +Environment: + COTURN_FILC_IMAGE image tag to build/run (default: coturn-filc-local) + FILC_VERSION Fil-C release to bake into the image (default: 0.678) + +Examples: + filc/run-local.sh + COTURN_FILC_IMAGE=coturn-filc-local:dev filc/run-local.sh + FILC_VERSION=0.678 filc/run-local.sh +EOF +} + +case "${1:-}" in + -h|--help|help) + usage + exit 0 + ;; +esac + +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +image_name="${COTURN_FILC_IMAGE:-coturn-filc-local}" +filc_version="${FILC_VERSION:-0.678}" + +ts="$(date -u +%Y%m%dT%H%M%SZ)" +log_dir="${repo_root}/filc/logs/${ts}" +mkdir -p "${log_dir}" + +echo "Logs: ${log_dir}" +echo "Image: ${image_name}" +echo "Fil-C version: ${filc_version}" +echo + +docker_run_flags=(--rm --platform linux/amd64) +if [ -t 0 ] && [ -t 1 ]; then + docker_run_flags+=(-it) +fi + +# Build the image. Use the host log dir so a failing build is still triagable. +set +e +docker build \ + --platform linux/amd64 \ + --build-arg "FILC_VERSION=${filc_version}" \ + -f "${repo_root}/filc/Dockerfile" \ + -t "${image_name}" \ + "${repo_root}/filc" 2>&1 | tee "${log_dir}/docker-build.log" +build_status=${PIPESTATUS[0]} +set -e + +if [ "${build_status}" -ne 0 ]; then + echo "docker build failed (exit ${build_status}). See ${log_dir}/docker-build.log" >&2 + exit "${build_status}" +fi + +# Run the build + tests inside the container. The entrypoint writes per-phase +# logs to /out (mounted to ${log_dir}) and never aborts mid-run, so we get a +# full picture of every issue in a single pass. +set +e +docker run "${docker_run_flags[@]}" \ + -v "${repo_root}:/src:ro" \ + -v "${log_dir}:/out" \ + "${image_name}" \ + "$@" +run_status=$? +set -e + +echo +echo "Logs: ${log_dir}" +if [ -f "${log_dir}/SUMMARY.txt" ]; then + echo "----- SUMMARY.txt -----" + cat "${log_dir}/SUMMARY.txt" + echo "-----------------------" +fi +if [ -f "${log_dir}/ISSUES.txt" ] && [ -s "${log_dir}/ISSUES.txt" ]; then + echo "ISSUES.txt is non-empty — see ${log_dir}/ISSUES.txt" +fi + +exit "${run_status}" diff --git a/src/server/ns_turn_maps.h b/src/server/ns_turn_maps.h index 7e461465..7889094b 100644 --- a/src/server/ns_turn_maps.h +++ b/src/server/ns_turn_maps.h @@ -52,7 +52,7 @@ typedef struct _ur_map ur_map; //////////////// Common Definitions ////// typedef uint64_t ur_map_key_type; -typedef uintptr_t ur_map_value_type; +typedef void *ur_map_value_type; typedef void (*ur_map_del_func)(ur_map_value_type); @@ -176,7 +176,7 @@ bool lm_map_foreach_arg(lm_map *map, foreachcb_arg_type func, void *arg); //////////////// UR ADDR MAP ////////////////// -typedef uintptr_t ur_addr_map_value_type; +typedef void *ur_addr_map_value_type; #define ADDR_MAP_SIZE (1024) #define ADDR_ARRAY_SIZE (4)