Skip to content

Enable GitHub Actions CI#1

Merged
jserv merged 1 commit into
mainfrom
cicd
Jun 1, 2026
Merged

Enable GitHub Actions CI#1
jserv merged 1 commit into
mainfrom
cicd

Conversation

@jserv
Copy link
Copy Markdown
Contributor

@jserv jserv commented Jun 1, 2026

Wire up lint, build, and sanitize jobs on ubuntu-24.04. The lint job installs clang-format-20 from apt.llvm.org plus shfmt from noble and runs the shell checks under .ci/ (newline, clang-format, shfmt, security patterns, cppcheck). The build job exercises make and make check; the sanitize job rebuilds with ASan + UBSan on both CFLAGS_EXTRA and LDFLAGS so the .so and test binaries share a runtime.

cppcheck is non-blocking for now because the existing tree has real findings (doubleFree in font.c, va_end_missing in input-method.c) plus false positives on SDL_VERSION_ATLEAST and XID-registry ownership transfer that still need triage.


Summary by cubic

Enable GitHub Actions CI on ubuntu-24.04 for lint, build, and ASan/UBSan tests, and harden Linux builds to avoid libX11 symbol collisions. Fix x11perf build errors, an ASan overflow in XCreateBitmapFromData, and make SDL event filter handoff safe across nested and multi-display closes.

  • New Features

    • CI: lint (newline/format/security checks; enforce clang-format-20 and shfmt; run cppcheck with a 180s timeout, non-blocking), build (make, make check, examples, -DDEBUG_LIBX11_COMPAT), and sanitize (ASan+UBSan on compile and link with a shared runtime; tuned ASAN/UBSAN options).
    • Linux: link with -Wl,-Bsymbolic and mark lock globals LIBX11_COMPAT_HIDDEN to prevent libX11.so.6 clobbering; define _GNU_SOURCE so strdup is visible under -std=c99.
  • Bug Fixes

    • Add missing Xrender ops (PictOpInReverse, PictOpOutReverse, PictOpAtopReverse, PictOpSaturate) and renumber PictOpOut to fix the x11perf build.
    • Provide two bytes to XCreateBitmapFromData for a 2x2 bitmap to remove an ASan stack-buffer-overflow.
    • SDL event filter: track open displays, hand off the filter to the most-recently-opened survivor, and clear it only on the final close; replace the fixed-size registry with a dynamic Array.

Written for commit 3e371a6. Summary will update on new commits.

Review in cubic

cubic-dev-ai[bot]

This comment was marked as resolved.

cubic-dev-ai[bot]

This comment was marked as resolved.

cubic-dev-ai[bot]

This comment was marked as resolved.

@jserv jserv force-pushed the cicd branch 2 times, most recently from 5fd9f6c to f734cb0 Compare June 1, 2026 17:40
Wire up lint, build, and sanitize jobs on ubuntu-24.04 plus the
Makefile and source plumbing they need to compile and pass cleanly
under AddressSanitizer + UndefinedBehaviorSanitizer on Linux.

Workflow (.github/workflows/ci.yml)
 * lint installs clang-format-20 (LLVM apt repo, signed-by keyring,
   HTTPS) and shfmt from noble, then runs the four .ci/ shell scripts.
   cppcheck stays continue-on-error pending triage of pre-existing
   findings (doubleFree in font.c, va_end_missing in input-method.c)
   and false positives on SDL_VERSION_ATLEAST plus XID-registry
   ownership transfer.
 * build exercises make, make check, make examples, and a
   -DDEBUG_LIBX11_COMPAT rebuild.
 * sanitize rebuilds with -fsanitize=address,undefined and
   -fno-sanitize-recover=undefined on both CFLAGS_EXTRA and LDFLAGS so
   the .so and test binaries share a runtime, then runs make check.

.ci/ scripts
 * check-format.sh hard-requires clang-format major 20.
 * check-newline.sh enforces newline-at-EOF on every tracked *.c/*.h.
 * check-cppcheck.sh runs cppcheck on src/*.c with -DNARROWPROTO
   -DXTHREADS and a 180s timeout.
 * check-security.sh scans src/ and include/X11/ for banned libc
   calls, secrets, and dangerous preprocessor directives (covers
   #define _FORTIFY_SOURCE 0, bare #undef _FORTIFY_SOURCE, and either
   polarity of __SSP__). The comment-skip filter is tight enough to
   leave code starting with '*' (e.g. '*ptr = strdup(s);') in scope.

mk/ infrastructure
 * toolchain.mk auto-detects a clang-format binary whose --version
   reports the required major, exposes UNAME_S, and declares SHFMT.
 * format.mk gates the format targets behind _clang_format_guard and
   _shfmt_guard, and drives shfmt against .ci/*.sh using the new
   [*.sh] section of .editorconfig.
 * config.mk adds -D_GNU_SOURCE so glibc exposes strdup under
   -std=c99; macOS already exposes it.
 * library.mk passes -Wl,-Bsymbolic on Linux (gated by UNAME_S) so
   intra-library references bind locally at link time. Mach-O ld on
   macOS does not accept the flag.

Avoiding the libX11.so.6 ABI collision

SDL2 on Linux transitively pulls /lib/.../libX11.so.6 into the test
binary's process. The staged upstream locking.c exports global
function-pointer variables (_XInitDisplayLock_fn, _XFreeDisplayLock_fn,
_XLockMutex_fn, _XUnlockMutex_fn, _XCreateMutex_fn, _XFreeMutex_fn,
_Xthread_self_fn, _Xglobal_lock, _Xi18n_lock, _conv_lock) under the
same names as libX11.so.6. The dynamic linker picks one canonical slot
per shared name, so libX11.so.6's init could write into our storage
and our XOpenDisplay would then dispatch through libX11.so.6 routines
against the libX11-compat Display layout -- LockDisplay segfaulted at
offset 0x50 inside libX11.so.6. Mark each of those data symbols with
LIBX11_COMPAT_HIDDEN (new helper in util.h) so they leave the dynamic
symbol table entirely. Intra-library references then resolve directly
to local storage, libX11.so.6 cannot reach them, and display->lock_fns
stays NULL (XInitThreads is never called by the tests) so the
LockDisplay macro short-circuits as intended. Public Xlib entry points
keep default visibility and stay exported.

CI-driven source fixes
 * include/X11/extensions/Xrender.h shipped the original 1996 subset
   and mis-numbered PictOpOut as 6, so x11perf failed to compile when
   it referenced PictOpInReverse, PictOpOutReverse, PictOpAtopReverse,
   or PictOpSaturate. Insert the four ops at their canonical positions
   and renumber PictOpOut to 7.
 * tests/check.c passed a one-byte char bits[] = {0x01} to
   XCreateBitmapFromData(..., 2, 2). X11 packs bitmaps as
   ceil(width/8) * height bytes, so a 2x2 image needs two; ASan caught
   the stack-buffer-overflow in bitmapBitIsSet. Extend the literal to
   two bytes (the second row stays clear so the existing assertions
   still hold).
 * src/events.c maintains a small registry of displays that installed
   onSdlEvent. SDL_SetEventFilter stores one (callback, userdata) pair,
   so a nested XOpenDisplay silently overwrites the slot. Without a
   handoff the next XCloseDisplay either left a dangling userdata
   (heap-use-after-free in test_extensions) or stranded the surviving
   display. closeEventPipe(), called from XCloseDisplay before free(),
   removes the display from the registry and -- only when it was the
   active slot -- hands the filter to the most-recently-opened
   surviving display, or nulls it when none remain.
@jserv jserv merged commit 4ad9422 into main Jun 1, 2026
6 checks passed
@jserv jserv deleted the cicd branch June 1, 2026 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant