From 450698833530ac61384d788029ffceb33b3bcd00 Mon Sep 17 00:00:00 2001 From: chrg6 Date: Fri, 13 Nov 2026 19:36:09 -0500 Subject: [PATCH] Make upscaler_idle_check fail where a place that lets something go frees it at once. The review of 2b9b3061 found that the check missed the dangerous regression. It only checked that each place that lets something go calls vk_upscaler_let_go, so with destroy_size put back in vk_dlaa.c free_size, or hl_destroy in hudless_from's size change, it still passed, while either frees images or a DLSS feature a frame still in flight uses. The check now also fails when: - a function that lets go of what the GPU may still use (vk_dlaa.c free_size, free_snap, stale_sizes, release_kept, retire_idle, evaluate and the retire_size, retire_feat, retire_snap, retire_hl and retire_pb helpers; vk_ffx.cpp context_for, trim, retire_up, vk_ffx_trim_idle and vk_ffx_release_idle; vk_ffx4.cpp context_for, vk_ffx4_trim, retire_up and vk_ffx4_release_idle) calls a destroy that frees at once (destroy_size, destroy_snap, hl_destroy, pb_destroy, feat_release, N.release, vkDestroyImageView, vmaDestroyImage; A.destroy in the FSR files); - hudless_from, pb_images or snap_for call one of those before their first image2d call (after it, only their half-made new images are destroyed); - upd_take_all, which releases a whole list whatever its gates, is called outside teardown (vk_dlaa_release, vk_ffx_release, vk_ffx4_release) and a full list's fallback (dead_put, the FSR files' retire_up), or there before vk_upscaler_settle's wait for the device. docs/building.md "Checks" says so. Each of 19 mutations of scratch copies fails the check (destroy_size in free_size and retire_idle, hl_destroy in hudless_from, pb_destroy in pb_images, destroy_snap in free_snap and snap_for, feat_release in release_kept, N.release in stale_sizes, vmaDestroyImage in retire_size, upd_take_all in retire_hl, A.destroy in FSR 3.1's retire_up and in either file's trim and release_idle, dead_put and FSR 4's retire_up without the settle, FSR 3.1 and FSR 4 without the retry); the unchanged sources pass, and stability-pass's sources fail with 46 problems. tools/check_content.py --staged passes. Co-Authored-By: Claude Opus 5.5 --- docs/building.md | 10 ++++-- tests/upscaler_idle_check.py | 70 ++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) diff --git a/docs/building.md b/docs/building.md index 6ac588d3..f537894e 100644 --- a/docs/building.md +++ b/docs/building.md @@ -817,9 +817,13 @@ GPU may still use it", `runtime/vk_ffx.cpp` and `runtime/vk_ffx4.cpp` `context_f and the `--probe-ffx-cache` path, that function only behind the rollback switch, a full list and a refused creation, every place that lets something go calling `vk_upscaler_let_go`, every creation of a DLSS feature or an FSR context (`vk_dlaa.c` `evaluate`, `vk_ffx.cpp` and `vk_ffx4.cpp` `context_for`) trying a refusal once more after a wait for the -GPU to pass what was let go of (`vk_upscaler_gpu_wait`), `upscaler_idle_wait`'s `sections[]` entry and -`nx2navi.ini.example` line, a name in `perf.c` for each perf mark kind, and `pb_images` noting the images' last use -after it makes them. Any Python, no game files. +GPU to pass what was let go of (`vk_upscaler_gpu_wait`), no place that lets something go (those, the `retire_*` +helpers, the FSR trims and `release_idle` calls) calling a destroy that frees at once (`destroy_size`, `destroy_snap`, +`hl_destroy`, `pb_destroy`, `feat_release`, `N.release`, `vkDestroyImageView`, `vmaDestroyImage`, the FSR files' +`A.destroy`), except after the first `image2d` of `hudless_from`, `pb_images` and `snap_for` for their half-made +images, a whole list taken (`upd_take_all`) only at teardown or after `vk_upscaler_settle` for a full list, +`upscaler_idle_wait`'s `sections[]` entry and `nx2navi.ini.example` line, a name in `perf.c` for each perf mark kind, +and `pb_images` noting the images' last use after it makes them. Any Python, no game files. A change to which frames the flight recorder takes for a one-frame flash (`runtime/flight_flash.h`, `runtime/flight.c` `flight_cells`) passes `python tests\flight_flash_check.py`, which builds the header alone, draws pictures of the diff --git a/tests/upscaler_idle_check.py b/tests/upscaler_idle_check.py index 5fafa5f3..80c762bb 100644 --- a/tests/upscaler_idle_check.py +++ b/tests/upscaler_idle_check.py @@ -13,6 +13,16 @@ It fails when: - one of the places that create a DLSS feature or an FSR context (vk_dlaa.c evaluate, vk_ffx.cpp and vk_ffx4.cpp context_for) does not call vk_upscaler_gpu_wait: a creation refused while what was let go of is still in flight then fails for want of its video memory, where the wait for the device of 0.10.1 had freed it first; +- one of the places that let go of what the GPU may still use (those above, the helpers that hand it to the lists, + vk_dlaa.c retire_size, retire_feat, retire_snap, retire_hl and retire_pb, vk_ffx.cpp and vk_ffx4.cpp retire_up, and + the FSR files' trims and release_idle calls) frees something at once: a destroy that takes an idle device or a + half-made object (vk_dlaa.c destroy_size, destroy_snap, hl_destroy, pb_destroy, feat_release, N.release, + vkDestroyImageView, vmaDestroyImage; the FSR files' A.destroy), which on a GPU still running a frame is a use after + free. vk_dlaa.c hudless_from, pb_images and snap_for may destroy only after their first image2d call, what of the new + images was made and never used; +- a whole list is taken whatever its gates (upd_take_all) outside teardown (vk_dlaa.c vk_dlaa_release, vk_ffx.cpp + vk_ffx_release, vk_ffx4.cpp vk_ffx4_release) and a full list's fallback (vk_dlaa.c dead_put, the FSR files' + retire_up), or there before vk_upscaler_settle's wait for the device; - NX2NAVI_UPSCALER_IDLE_WAIT has no sections[] entry in cfg.c or no line in nx2navi.ini.example (its known[] name and its metas[] row, and a // that would hide them, are tests/cfg_rows_check.py's); - perf.c's mark_name has another count of names than host.h has PERF_M_* kinds: a kind without a name prints a null @@ -43,6 +53,29 @@ REFUSED = { # functions that create a feature or context and must retry a refu 'vk_ffx.cpp': ['context_for'], 'vk_ffx4.cpp': ['context_for'], } +# Callees that free at once, without a wait: each is for an idle device (teardown) or for what no submission named yet +# (a half-made object). +FREE_NOW = { + 'vk_dlaa.c': ['destroy_size', 'destroy_snap', 'hl_destroy', 'pb_destroy', 'feat_release', 'N.release', + 'vkDestroyImageView', 'vmaDestroyImage'], + 'vk_ffx.cpp': ['A.destroy'], + 'vk_ffx4.cpp': ['A.destroy'], +} +NO_FREE_NOW = { # functions that let go of what the GPU may still use: no FREE_NOW callee anywhere in them + 'vk_dlaa.c': ['free_size', 'free_snap', 'stale_sizes', 'release_kept', 'retire_idle', 'evaluate', 'retire_size', + 'retire_feat', 'retire_snap', 'retire_hl', 'retire_pb'], + 'vk_ffx.cpp': ['context_for', 'trim', 'retire_up', 'vk_ffx_trim_idle', 'vk_ffx_release_idle'], + 'vk_ffx4.cpp': ['context_for', 'vk_ffx4_trim', 'retire_up', 'vk_ffx4_release_idle'], +} +# Functions that let go of the old images and then make new ones: no FREE_NOW callee before their first image2d call; +# after it, the failure path destroys what of the new images was made, which no submission named. +NO_FREE_BEFORE_MAKE = {'vk_dlaa.c': ['hudless_from', 'pb_images', 'snap_for']} +# Where upd_take_all may be called, and whether vk_upscaler_settle's wait for the device must come before it there. +TAKE_ALL = { + 'vk_dlaa.c': {'vk_dlaa_release': False, 'dead_put': True}, + 'vk_ffx.cpp': {'vk_ffx_release': False, 'retire_up': True}, + 'vk_ffx4.cpp': {'vk_ffx4_release': False, 'retire_up': True}, +} # A function definition: at column 0, a return type, the name, the parameters and the body's brace (the runtime's # style); declarations end with ';' before any '{'. Read in code whose string literals are blanked: extern "C" is # extern " " there. @@ -114,6 +147,11 @@ def calls(code, funcs, callee): return out +def call_offsets(code, callee, a, b): + """The offsets of each call of callee between a and b.""" + return [m.start() for m in re.compile(r'\b%s\s*\(' % re.escape(callee)).finditer(code, a, b)] + + def body(code, funcs, name): for n, a, b in funcs: if n == name: @@ -157,6 +195,38 @@ def main(): fails.append(f'{f}: no function {name}') elif not re.search(r'\bvk_upscaler_gpu_wait\s*\(', b): fails.append(f'{f} {name}: no retry of a refused creation after vk_upscaler_gpu_wait') + for f, names in NO_FREE_NOW.items(): + code, funcs = sources[f] + defined = {n for n, _, _ in funcs} + fails += [f'{f}: no function {name}' for name in names if name not in defined] + for callee in FREE_NOW[f]: + for line, fn in calls(code, funcs, callee): + if fn in names: + fails.append(f'{f}:{line}: {fn} calls {callee}, which frees at once what the GPU may still use') + for f, names in NO_FREE_BEFORE_MAKE.items(): + code, funcs = sources[f] + for name in names: + spans = [(a, b) for n, a, b in funcs if n == name] + if not spans: + fails.append(f'{f}: no function {name}') + for a, b in spans: + made = call_offsets(code, 'image2d', a, b) + if not made: + fails.append(f'{f} {name}: no image2d call') + continue + for callee in FREE_NOW[f]: + for at in call_offsets(code, callee, a, made[0]): + fails.append(f'{f}:{code.count(chr(10), 0, at) + 1}: {name} calls {callee} before it makes ' + f'its images, on what the GPU may still use') + for f, allowed in TAKE_ALL.items(): + code, funcs = sources[f] + for name, a, b in funcs: + for at in call_offsets(code, 'upd_take_all', a, b): + line = code.count('\n', 0, at) + 1 + if name not in allowed: + fails.append(f'{f}:{line}: upd_take_all in {name}, allowed only in {sorted(allowed)}') + elif allowed[name] and not call_offsets(code, 'vk_upscaler_settle', a, at): + fails.append(f'{f}:{line}: upd_take_all in {name} before vk_upscaler_settle') key = 'UPSCALER_IDLE_WAIT' cfg = (rt / 'cfg.c').read_text(encoding='utf-8-sig')