Conversation
5fd9f6c to
f734cb0
Compare
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
libX11symbol collisions. Fixx11perfbuild errors, an ASan overflow inXCreateBitmapFromData, and make SDL event filter handoff safe across nested and multi-display closes.New Features
clang-format-20andshfmt; runcppcheckwith 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).-Wl,-Bsymbolicand mark lock globalsLIBX11_COMPAT_HIDDENto preventlibX11.so.6clobbering; define_GNU_SOURCEsostrdupis visible under-std=c99.Bug Fixes
PictOpInReverse,PictOpOutReverse,PictOpAtopReverse,PictOpSaturate) and renumberPictOpOutto fix thex11perfbuild.XCreateBitmapFromDatafor a 2x2 bitmap to remove an ASan stack-buffer-overflow.Array.Written for commit 3e371a6. Summary will update on new commits.