drm: close the round-14 review findings

- the .so contract and deb assertions no longer pipe into grep. under
  `set -o pipefail`, `producer | grep -q` reports a FALSE FAILURE once
  the producer outruns the 64 KB pipe buffer: grep -q exits at the
  first match, the producer dies on SIGPIPE, and pipefail makes that
  the pipeline's status - so a library that HAS the symbol is reported
  as missing it and the step fails on a good build. measured on a real
  EGL-enabled .so (101 KB of strings, both markers present): the piped
  form reported both missing. this was introduced by the strictness
  fix two rounds ago and only passes today because a release-sized .so
  fits in the buffer. NOTE the obvious repair does not work either -
  materializing the output and piping the variable keeps the pipe and
  fails identically (measured), so these now match with bash's own
  pattern operator and no subprocess at all. verified with positive
  and negative controls.
- warm_availability decides X11 for itself, inside its retry loop,
  with the UNMEMOISED `scrap::is_x11()`. this is the same one-shot-at
  -startup bug the pre-warm had, in its sibling call site, left behind
  when that one was fixed: the check ran during startup, where
  loginctl cannot yet name the seat0 session and the answer defaults
  to "x11", so a Wayland host that came up slowly skipped the warm for
  the life of the process and got back the cold-probe "No displays"
  symptom the warm exists to remove. the memoised form would have
  moved the bug rather than fixed it, since it latches its first
  answer.
- the grab_desc SAFETY comment says what the frame protocol actually
  is instead of promising a release on every return path: traced in
  the C, a failing grab_desc leaves nothing to release (-EINVAL
  returns before allocating, a failed inner grab has already cleaned
  up, and -ENOTSUP releases the frame itself), so releasing on those
  paths would be a double free.
This commit is contained in:
Mariano Abad
2026-07-30 16:57:09 -03:00
parent ad246d2714
commit 78bfe61554
4 changed files with 66 additions and 14 deletions

View File

@@ -215,8 +215,22 @@ jobs:
echo "::error::extracted only $nsyms loader symbols from drmtap_dl.rs (expected >= 13); the extraction pattern no longer matches the loader"
missing=1
fi
# Inspect the object ONCE into a variable, then match with bash's own pattern operator --
# NO PIPE ANYWHERE IN THESE CHECKS. `anything | grep -q` under `set -o pipefail` reports a
# FALSE FAILURE as soon as the producer outruns the 64 KB pipe buffer: grep -q exits at the
# first match, the producer dies on SIGPIPE (141), and pipefail makes that the pipeline's
# status, so a library that HAS the symbol is reported as missing it. Measured on a real
# EGL-enabled .so (101 KB of `strings`, both markers present): the piped form reported both
# missing and failed the step. Note the obvious repair does NOT work -- materializing the
# output and then doing `printf '%s\n' "$var" | grep -q` keeps the pipe and just swaps the
# producer, and it fails identically (measured). Today's release-sized .so happens to fit in
# the buffer, which is the only reason this has not fired yet.
exported="$(nm -D --defined-only "$SO")"
strs="$(strings "$SO")"
for sym in $syms; do
if ! nm -D --defined-only "$SO" | grep -q " T $sym\$"; then
# Line-anchored: wrap in newlines so the pattern can require a whole line, the same
# thing `grep " T $sym$"` was expressing.
if [[ $'\n'"$exported"$'\n' != *$'\n'*" T $sym"$'\n'* ]]; then
echo "::error::libdrmtap does not export $sym, which the runtime loader resolves"
missing=1
fi
@@ -226,7 +240,7 @@ jobs:
# for: the naive ELF check reports "no EGL" on a perfectly good library. What a CPU-only stub
# build really lacks is the dlopen target name and the import call itself.
for s in "libEGL.so.1" "eglCreateImageKHR"; do
if ! strings "$SO" | grep -qF "$s"; then
if [[ "$strs" != *"$s"* ]]; then
echo "::error::libdrmtap looks like a CPU-only stub (no $s): the EGL detile hot path is missing"
missing=1
fi
@@ -346,8 +360,19 @@ jobs:
fi
deb="${debs[0]}"
echo "::notice::built $deb ($(stat -c %s "$deb") bytes)"
dpkg -c "$deb" | grep -E 'usr/lib/rustdesk/libdrmtap\.so\.0\.[0-9]+\.[0-9]+$'
dpkg -c "$deb" | grep -E 'usr/lib/rustdesk/libdrmtap\.so\.0 ->'
# Pipe-free for the same reason as the .so contract step above (see the comment there:
# a producer feeding a grep that can exit early is a SIGPIPE reported as a failure under
# pipefail). `grep -E` without -q reads to EOF so these two happen to be safe, but the
# shape is the hazard and the next `-q` added here would inherit it silently.
contents="$(dpkg -c "$deb")"
if [[ ! "$contents" =~ usr/lib/rustdesk/libdrmtap\.so\.0\.[0-9]+\.[0-9]+ ]]; then
echo "::error::the deb does not contain a versioned libdrmtap.so.0.x.y"
exit 1
fi
if [[ "$contents" != *"usr/lib/rustdesk/libdrmtap.so.0 ->"* ]]; then
echo "::error::the deb does not contain the libdrmtap.so.0 soname symlink"
exit 1
fi
# The library alone does not make this a drm build: build.py stages it whenever --drm is
# passed, independently of what was compiled, and the deb name is what tells a user this
# is the consent-bypass variant. Assert the BINARY too, by the absolute dlopen path that