Skip to content

Upstream codeql fixes#12

Open
deeprnd wants to merge 30 commits into
mainfrom
upstream-codeql-fixes
Open

Upstream codeql fixes#12
deeprnd wants to merge 30 commits into
mainfrom
upstream-codeql-fixes

Conversation

@deeprnd

@deeprnd deeprnd commented Jun 5, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add new CodeQL nightly query ImplicitIntegerTruncation.ql that detects implicit narrowing conversions between integer types without explicit casts (e.g. ulonguchar), including integer-promotion artifacts from small literals
  • Fix filter.qll to support both production and CodeQL test databases: production DBs have paths prefixed with src/, while test DBs use bare filenames with no directory component — previously tests would find zero results because the path filter excluded all test files
  • Fix LargeMemset.expected: error messages for fd_memset calls incorrectly said "call to memset" instead of "call to fd_memset"
  • Update TrivialMemcpy.expected: line numbers shifted by 2 to reflect current test file content

Changed Files

  • contrib/codeql/lib/filter.qll — path filter fix for test databases
  • contrib/codeql/src/nightly/ImplicitIntegerTruncation.ql — new query (added)
  • contrib/codeql/test/query-tests/ImplicitIntegerTruncation/ImplicitIntegerTruncation.c — new test (added)
  • contrib/codeql/test/query-tests/ImplicitIntegerTruncation/ImplicitIntegerTruncation.expected — new expected output (added)
  • contrib/codeql/test/query-tests/ImplicitIntegerTruncation/ImplicitIntegerTruncation.qlref — new query ref (added)
  • contrib/codeql/test/query-tests/LargeMemset/LargeMemset.expected — corrected fd_memset message text
  • contrib/codeql/test/query-tests/TrivialMemcpy/TrivialMemcpy.expected — updated line numbers

Background — why this matters

config/extra/with-brutality.mk enables -Werror -Wconversion, which would reject all implicit narrowing at compile time. However, brutality builds are not wired into CI — they are local-discipline only. As a result, implicit integer truncations can silently enter the codebase and go undetected. This query fills that gap in the nightly CodeQL pipeline.

Tests

  • New ImplicitIntegerTruncation query is covered by a full inline-expectations test (.c + .expected + .qlref) covering return truncation, variable initialisation, integer literal narrowing, function-call argument truncation, and struct-member assignment
  • Existing LargeMemset and TrivialMemcpy expected files updated to match current query output
  • Relevant checks pass locally and/or in CI
  • codeql test run contrib/codeql/test/query-tests/ passed

Two real instances in the codebase were modified to introduce ulong → uchar narrowing:

The UL suffix makes the literal type unsigned long (8 bytes), creating an implicit truncation to uchar (1 byte).

codeql test run contrib/codeql/test/query-tests/ImplicitIntegerTruncation/

FAILED: contrib/codeql/test/query-tests/ImplicitIntegerTruncation/ImplicitIntegerTruncation.qlref

The test fails because the query finds the two new patterns but the .expected file does not list them — confirming the query catches ulong-literal → uchar narrowing correctly. Restoring the original = 0 / = 64 values and updating .expected restores all tests to passing.

Type of Change

  • Bug fix (filter.qll test-database support; LargeMemset/TrivialMemcpy expected file corrections)
  • New feature (ImplicitIntegerTruncation nightly query)
  • Refactor
  • Documentation update

Notes

The ImplicitIntegerTruncation query intentionally includes integer-promotion artifacts (e.g. uchar c = a + b where a + b is int due to C integer promotion). Suppressions are done via explicit casts. The query excludes bool types and macro expansions to reduce noise.

lidatong and others added 30 commits June 4, 2026 12:21
Test is broken and unused
Remove unused test-only coroutine library
- deps.sh: update Alpine package deps
- Fix PAGE_SIZE identifier usage (conflicts with libc)
- Fix sendmmsg flags integer type (different on musl vs glibc)
- Fix usage of off64_t (better to use long)
- Fix <sys/poll.h> include (better to use <poll.h>)
- Fix sockaddr type punning with connect(2)
- Fix usage of dirent64 (better to use dirent, which is identical)
- Don't assume struct rlimit->resource is uint
- Enable bpf syscall wrappers for musl libc
- Fix missing <linux/ip.h> include when using <linux/if_tunnel.h>
Fossil code, broken since several years
Remove all alloca usages except in monitor.c and watch.c.
FD_HAS_INT128 implies 'has fast hardware support for int128'.
But all supported compilers still provide software emulation for
floating point numbers, wide/128-bit multiplies, etc.

Therefore, provide basic uint128 type support.
This fixes build flakiness with noarch64 targets and FD_HAS_INT128
guard spam.

Full Firedancer *requires* uint128 compiler support, but tolerates
the absence of fast hardware wide multiplies on exotic targets.

Therefore, remove unnecessary FD_HAS_INT128 compile guards and just
assume that compilers can soft-emulate uint128 type support.
(True for all supported compilers)

Also clean up unnecessary uint128 usages along the way, mostly
used as a futile attempt to get 16-byte atomic load/store.
This doesn't actually work in practice.

src/util and other libraries written by Kevin are more portable to
exotic targets.  But full Solana runtime support without compiler
uint128 support has proven impossible to do in C, and keeps causing
random build failures with noarch64.

The only remaining Firedancer usages of FD_HAS_INT128 gates decide
whether to use uint128 arithmetic (due to HW support) or whether
to soft-emulate.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants