Repository navigation
lint-c passes on main again: the JIT request table, DM_PROGRAMS, the web page's scheme handler and the Vulkan occlusion query no longer reach NULL (baseline 241 to 240) - #1207
Closed
oliverames wants to merge 1 commit into
Conversation
…web page's scheme handler and the Vulkan occlusion query no longer reach NULL (baseline 241 to 240) lint-c has failed on main since 0b8e0dc (Super Mario 3D All-Stars; run 41819072419, still failing at 41842097689) with four file/check pairs above the baseline and one below it. The five findings were real paths to NULL, not noise: - hle_jit.c (bugprone-suspicious-realloc-usage, 4): jit_serve_load grew the request table and each request's three relocation arrays with `p = realloc(p, ...)`, so a failed realloc lost the block and the next store wrote through NULL. They go through jit_realloc, which stops with "jit: no memory for the request table" instead. - hle_oe.c (clang-analyzer-unix.StdCLibraryFunctions): DM_PROGRAMS was sized with ftell, whose -1 became a SIZE_MAX count for fread and a calloc of 0 bytes. A failed ftell is logged and the file read as empty. - web_apple.m (clang-analyzer-nullability.NullablePassedToNonnull, 5): json() passed a failed JSON serialization to initWithData:, the font override path (an environment value that is not UTF-8 boxes to nil) reached dataWithContentsOfFile: and pathExtension as nil, the scheme handler passed a response that initWithURL: may return nil to didReceiveResponse:, the page address went to requestWithURL: unchecked and webshot passed TIFFRepresentation, which may be nil, to initWithData:. Each now takes its fallback: an empty literal, an empty path, didFailWithError:, a logged address that is not one, no bitmap. - hle_vk.m (clang-analyzer-core.NullDereference): occ_start read s->sq->nvis after replay's own `s->sq &&` checks said it might be NULL. replay always sets it, so this one is a false positive; occ_start now returns on NULL as the other readers of s->sq are written to. - hle_vk.m clang-analyzer-security.ArrayBound had 1 finding against 2 in the baseline, so the baseline follows (lint_c.py tidy --update; 241 to 240 findings). No count was raised and no NOLINT added. Checked with LLVM 23.1.4 (the CI release, Homebrew's llvm@23): lint_c.py tidy over the whole project: 240 findings in 73 files, 134 file/check pairs, 0 above the baseline and 0 below it; test_lint_c.py 19 tests OK; make -C dm/host/tests run exits 0 (504 checks); make -C dm/host dmhost builds. Not run: the JIT serve path with a real jit.tsv, the web page in a window, the Vulkan occlusion query on a game. Front-ends: none
2 of 3 tasks
Contributor
Author
|
Closing as superseded: f8a5618d takes every item of this PR onto main: hle_jit.c's realloc guards, hle_oe.c's DM_PROGRAMS size guard, hle_vk.m's submit-queue guard, the web_apple.m JSON / font-path / URL and snapshot guards, with the same one-drop of the baseline. I checked each of the four files on main against this branch's diff; nothing unique is left, and the branch no longer merges cleanly. Re-ping if a site there should be guarded differently. |
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.
lint-c has failed on main since 0b8e0dc (run 41819072419, and again at 41842097689): four file/check pairs above the baseline and one below it. This is the whole remainder after a27d684; it does not repeat the allocation fixes already on main. It keeps clang-tidy red on open PRs (#1178, #1168, #1140, #1104).
Findings and fixes (each one is a path to NULL; one is a false positive)
dm/host/hle_jit.cbugprone-suspicious-realloc-usage(4)jit_serve_loadgrew the request table and each request'srel_at,rel_off,rel_modwithp = realloc(p, ...). A failed realloc lost the block and the next store wrote through NULL. They now go throughjit_realloc, which stops withjit: no memory for the request table.dm/host/hle_oe.cclang-analyzer-unix.StdCLibraryFunctionsDM_PROGRAMSwas sized withftell; its -1 became a SIZE_MAX count forfreadover a 0-byte calloc. A failedftellis logged and the file read as empty.dm/host/web_apple.mclang-analyzer-nullability.NullablePassedToNonnull(5)json()passed a failed serialization toinitWithData:(now returns""); the font override (@(env)is nil for non-UTF-8) reacheddataWithContentsOfFile:andpathExtensionas nil (now?: @"");initWithURL:may return nil, so the scheme handler fails the task withdidFailWithError:instead of passing it on; the page address went torequestWithURL:unchecked (now skipped with a[RED]log); webshot passedTIFFRepresentationunchecked (now nil means no bitmap).dm/gfx/vk/hle_vk.mclang-analyzer-core.NullDereferencereplayalways setss->sq, but its owns->sq &&check at the visibility buffer tells the analyzer it may be NULL.occ_startnow returns on a NULLsq, as that check does.dm/tools/lint_c_baseline.tsvhle_vk.mclang-analyzer-security.ArrayBoundlint_c.py tidy --update, 241 to 240 findings.No count was raised and no
NOLINTwas added. No documentation entry: nothing here changes what a game or tool does.Checked locally (Apple silicon Mac, LLVM 23.1.4 from Homebrew
llvm@23, the release CI pins;make-built compile database forGAME=010015100B514000as CI uses)python3 dm/tools/lint_c.py tidyon the four files reproduced the same 11 findings as CI.python3 dm/tools/lint_c.py tidy -j 6over the whole project: 240 findings in 73 files, 134 file/check pairs,baseline 240 findings; 0 file/check pairs above it, 0 below it.python3 -m unittest dm.tools.tests.test_lint_c: 19 tests OK.make -C dm/host/tests GAME=010015100B514000 run: exit 0, 504 checks (includest_oe, which compileshle_oe.c).make -C dm/host GAME=010015100B514000 dmhost: builds.Not verified
jit.tsv, an out-of-memory or failedftell, the web page in a window (the Vulkan occlusion query was not run on a game either). The changes are guard clauses on paths that did not fail before.Front-ends: none