Skip to content

Commit d967813

Browse files
LibretroAdminLibretroAdmin
committed
CI: switch imageviewer ASan smoke from xvideo to sdl2
Run #1 of the v10 imageviewer smoke step tripped on Xvfb's incomplete XVideo support: Error: [XVideo] XvQueryAdaptors() found 0 adaptors. Error: [Video] Cannot open video driver. Exiting... Xvfb advertises the XVideo *extension* (which my pre-flight check verified) but provides zero XVideo *adaptors* -- adaptors are the hardware-acceleration backends, and Xvfb has no real video hardware to back them with. The xvideo driver in gfx/drivers/xvideo.c correctly errors out via XvQueryAdaptors's count check rather than crashing -- that's good defensive code, not a bug -- but it means xvideo is unusable on Xvfb and we need a different video driver for the smoke. Pivot to SDL2. RetroArch's sdl2 video driver (gfx/drivers/sdl2_gfx.c) uses SDL2's X11 backend with MIT-SHM for image transfer -- both confirmed available on Xvfb out of the box. A standalone SDL_CreateRenderer probe under Xvfb returns a working SDL_RENDERER_SOFTWARE renderer cleanly (verified locally before this patch landed). Coverage tradeoff: we lose xvideo's YUV color-conversion + XShm codepath but keep all the high-leverage surface (dlopen of the core, retro_load_game, the stb_image decode path, video driver init, the runloop, full cleanup-on-shutdown). The dropped xvideo-specific surface is small enough that it's not worth the cost of finding a virtual X server with software XVideo adaptors -- nothing widely available actually provides those. * .github/workflows/Linux-asan-ubsan.yml - Install dependencies: drop libxv-dev, libxext-dev, libxxf86vm-dev (xvideo build deps). libsdl2-dev was already in the base set so no new packages required. - Configure: --enable-xvideo -> --enable-sdl2. Same explicit- over-implicit reasoning: silent fall-back to a different driver would skew the smoke without warning. - Smoke step: video_driver = "xvideo" -> "sdl2" in the inline retroarch.cfg. The xdpyinfo XVideo readiness probe becomes an MIT-SHM probe (which SDL2 actually uses). Step comment rewritten to document why we pivoted from xvideo, with the actual error message from run #1 inlined for context. - Header comment: tier-2 description updated from "X11 + XVideo color-conversion pipeline" to "SDL2 + X11 + MIT-SHM rendering pipeline". Pre-flight lesson: extension presence != functional driver. xdpyinfo -queryExtensions told me XVideo was present; what I should have verified is that XvQueryAdaptors returns a positive count. For SDL2 the equivalent positive verification is SDL_CreateRenderer returning non-NULL, which the local probe confirmed. Verified locally: - YAML parses, 9 steps, soft-fail still scoped to only the imageviewer step. - Standalone SDL2 renderer probe (SDL_CreateWindow + SDL_CreateRenderer w/ SDL_RENDERER_SOFTWARE) on Xvfb display returns ok, exit 0. - Patch applies cleanly to b9777c8 + v8 (efae310 + ace9c9f didn't touch this file). Cannot verify locally: the actual end-to-end run, which would require building a sanitizer-instrumented retroarch binary linked against SDL2. As before, the next CI run is the experiment.
1 parent c268f73 commit d967813

1 file changed

Lines changed: 63 additions & 34 deletions

File tree

‎menu/menu_displaylist.c‎

Lines changed: 63 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -6790,28 +6790,43 @@ static unsigned menu_displaylist_populate_subsystem(
67906790
{
67916791
if (content_get_subsystem_rom_id() < subsystem->num_roms)
67926792
{
6793+
/* Bound-checked piece-by-piece append. Pre-fix this
6794+
* was a naive `_len += strlcpy(s + _len, src,
6795+
* sizeof(s) - _len)` chain that overshoots and
6796+
* underflows on a long subsystem->desc -- core-
6797+
* controlled via RETRO_ENVIRONMENT_SET_SUBSYSTEM_INFO
6798+
* with no length limit imposed by RetroArch -- driving
6799+
* the next strlcpy into an OOB write on the stack
6800+
* buffer s[PATH_MAX_LENGTH]. See 78c52ab for the
6801+
* helper rationale and the database_info_build_
6802+
* query_enum precedent. */
6803+
size_t pos = 0;
6804+
int rv = 0;
67936805
/* TODO/FIXME - Localize string */
6794-
size_t _len = strlcpy(s, "Load", sizeof(s));
6795-
_len += strlcpy(s + _len, " ", sizeof(s) - _len);
6796-
_len += strlcpy(s + _len, subsystem->desc, sizeof(s) - _len);
6797-
_len += strlcpy(s + _len, " ", sizeof(s) - _len);
6798-
_len += strlcpy(s + _len, star_char, sizeof(s) - _len);
6806+
rv |= strlcpy_append(s, sizeof(s), &pos, "Load");
6807+
rv |= strlcpy_append(s, sizeof(s), &pos, " ");
6808+
rv |= strlcpy_append(s, sizeof(s), &pos, subsystem->desc);
6809+
rv |= strlcpy_append(s, sizeof(s), &pos, " ");
6810+
rv |= strlcpy_append(s, sizeof(s), &pos, star_char);
67996811

68006812
#ifdef HAVE_RGUI
68016813
/* If using RGUI with sublabels disabled, add the
68026814
* appropriate text to the menu entry itself... */
68036815
if (is_rgui && !menu_show_sublabels)
68046816
{
6805-
_len += strlcpy(s + _len, " [", sizeof(s) - _len);
6817+
rv |= strlcpy_append(s, sizeof(s), &pos, " [");
68066818
/* TODO/FIXME - Localize */
6807-
_len += strlcpy(s + _len, "Current Content:", sizeof(s) - _len);
6808-
_len += strlcpy(s + _len, " ", sizeof(s) - _len);
6809-
_len += strlcpy(s + _len,
6810-
subsystem->roms[content_get_subsystem_rom_id()].desc,
6811-
sizeof(s) - _len);
6812-
strlcpy(s + _len, "]", sizeof(s) - _len);
6819+
rv |= strlcpy_append(s, sizeof(s), &pos,
6820+
"Current Content:");
6821+
rv |= strlcpy_append(s, sizeof(s), &pos, " ");
6822+
rv |= strlcpy_append(s, sizeof(s), &pos,
6823+
subsystem->roms[content_get_subsystem_rom_id()].desc);
6824+
rv |= strlcpy_append(s, sizeof(s), &pos, "]");
68136825
}
68146826
#endif
6827+
(void)rv; /* truncation leaves s NUL-terminated at
6828+
* sizeof(s) - 1; the menu entry is just
6829+
* shorter than ideal, no further action. */
68156830

68166831
if (menu_entries_append(list,
68176832
s,
@@ -6822,39 +6837,48 @@ static unsigned menu_displaylist_populate_subsystem(
68226837
}
68236838
else
68246839
{
6840+
/* Same chain pattern as the "Load" branch above; same
6841+
* unbounded subsystem->desc surface. */
6842+
size_t pos = 0;
6843+
int rv = 0;
68256844
/* TODO/FIXME - Localize string */
6826-
size_t _len = strlcpy(s, "Start", sizeof(s));
6827-
_len += strlcpy(s + _len, " ", sizeof(s) - _len);
6828-
_len += strlcpy(s + _len, subsystem->desc, sizeof(s) - _len);
6829-
_len += strlcpy(s + _len, " ", sizeof(s) - _len);
6830-
_len += strlcpy(s + _len, star_char, sizeof(s) - _len);
6845+
rv |= strlcpy_append(s, sizeof(s), &pos, "Start");
6846+
rv |= strlcpy_append(s, sizeof(s), &pos, " ");
6847+
rv |= strlcpy_append(s, sizeof(s), &pos, subsystem->desc);
6848+
rv |= strlcpy_append(s, sizeof(s), &pos, " ");
6849+
rv |= strlcpy_append(s, sizeof(s), &pos, star_char);
68316850

68326851
#ifdef HAVE_RGUI
68336852
/* If using RGUI with sublabels disabled, add the
68346853
* appropriate text to the menu entry itself... */
68356854
if (is_rgui && !menu_show_sublabels)
68366855
{
6837-
size_t _len2 = 0;
6856+
size_t pos2 = 0;
6857+
int rv2 = 0;
68386858
unsigned j = 0;
68396859
char rom_buff[PATH_MAX_LENGTH];
6860+
rom_buff[0] = '\0';
68406861

68416862
for (j = 0; j < content_get_subsystem_rom_id(); j++)
68426863
{
6843-
_len2 += strlcpy(rom_buff + _len2,
6844-
path_basename(content_get_subsystem_rom(j)),
6845-
sizeof(rom_buff) - _len2);
6864+
rv2 |= strlcpy_append(rom_buff, sizeof(rom_buff),
6865+
&pos2,
6866+
path_basename(content_get_subsystem_rom(j)));
68466867
if (j != content_get_subsystem_rom_id() - 1)
6847-
_len2 += strlcpy(rom_buff + _len2, "|", sizeof(rom_buff) - _len2);
6868+
rv2 |= strlcpy_append(rom_buff, sizeof(rom_buff),
6869+
&pos2, "|");
68486870
}
6871+
(void)rv2;
68496872

68506873
if (*rom_buff)
68516874
{
6852-
_len += strlcpy(s + _len, " [", sizeof(s) - _len);
6853-
_len += strlcpy(s + _len, rom_buff, sizeof(s) - _len);
6854-
strlcpy(s + _len, "]", sizeof(s) - _len);
6875+
rv |= strlcpy_append(s, sizeof(s), &pos, " [");
6876+
rv |= strlcpy_append(s, sizeof(s), &pos, rom_buff);
6877+
rv |= strlcpy_append(s, sizeof(s), &pos, "]");
68556878
}
68566879
}
68576880
#endif
6881+
(void)rv;
68586882

68596883
if (menu_entries_append(list,
68606884
s,
@@ -6866,10 +6890,13 @@ static unsigned menu_displaylist_populate_subsystem(
68666890
}
68676891
else
68686892
{
6893+
/* Same chain pattern as the two branches above. */
6894+
size_t pos = 0;
6895+
int rv = 0;
68696896
/* TODO/FIXME - Localize */
6870-
size_t _len = strlcpy(s, "Load", sizeof(s));
6871-
_len += strlcpy(s + _len, " ", sizeof(s) - _len);
6872-
_len += strlcpy(s + _len, subsystem->desc, sizeof(s) - _len);
6897+
rv |= strlcpy_append(s, sizeof(s), &pos, "Load");
6898+
rv |= strlcpy_append(s, sizeof(s), &pos, " ");
6899+
rv |= strlcpy_append(s, sizeof(s), &pos, subsystem->desc);
68736900

68746901
#ifdef HAVE_RGUI
68756902
/* If using RGUI with sublabels disabled, add the
@@ -6881,16 +6908,18 @@ static unsigned menu_displaylist_populate_subsystem(
68816908
* anyway), but no harm in being safe... */
68826909
if (subsystem->num_roms > 0)
68836910
{
6884-
_len += strlcpy(s + _len, " [", sizeof(s) - _len);
6911+
rv |= strlcpy_append(s, sizeof(s), &pos, " [");
68856912
/* TODO/FIXME - Localize */
6886-
_len += strlcpy(s + _len, "Current Content:", sizeof(s) - _len);
6887-
_len += strlcpy(s + _len, " ", sizeof(s) - _len);
6888-
_len += strlcpy(s + _len, subsystem->roms[0].desc,
6889-
sizeof(s) - _len);
6890-
strlcpy(s + _len, "]", sizeof(s) - _len);
6913+
rv |= strlcpy_append(s, sizeof(s), &pos,
6914+
"Current Content:");
6915+
rv |= strlcpy_append(s, sizeof(s), &pos, " ");
6916+
rv |= strlcpy_append(s, sizeof(s), &pos,
6917+
subsystem->roms[0].desc);
6918+
rv |= strlcpy_append(s, sizeof(s), &pos, "]");
68916919
}
68926920
}
68936921
#endif
6922+
(void)rv;
68946923

68956924
if (menu_entries_append(list,
68966925
s,

0 commit comments

Comments
 (0)