python / python/cpython

CI: UBSan runs GIL-only and never against the JIT; two configurations already shipped by distros are unsanitized

Open
#154,927 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

infra type-feature
Dominant language
Python
Stars
77.2k
Forks
35.9k
PR merge metrics
PR metrics pending

Description

Feature or enhancement

Proposal

The UBSan job in .github/workflows/build.yml runs against exactly one configuration: GIL-enabled, no JIT. Two configurations that CI already builds — and that distributions already ship — are never sanitized. Both are cheap to add, and I have measured what each would have caught.

1. UBSan × free-threading: true

The sanitizer matrix pairs TSan with free-threading: {false, true} but UBSan only with false:

        free-threading:
        - false
        - true
        sanitizer:
        - TSan
        include:
        - check-name: Undefined behavior
          sanitizer: UBSan
          free-threading: false

So files that only compile under Py_GIL_DISABLED are never built under UBSan. That is not a hypothetical gap: Python/uniqueid.c:185 shifted a negative per-thread refcount delta left, which is UB, and it fired from interpreter startup and from every GC cycle. Over the full suite with all suppressions disabled:

UB reports test failures failing files
before 1762 529 61
after #154915 0 0 0

Once #154915 lands, this configuration is green, so adding it is a regression guard rather than a source of new work.

2. UBSan × --enable-experimental-jit

jit.yml builds the JIT in several configurations and tail-call.yml builds --with-tail-call-interp, but neither is ever combined with a sanitizer. The JIT's own C code — the stencil patcher in Python/jit.c, and the tier-2 optimiser in Python/optimizer*.c which only compiles under -D_Py_TIER2=1 — is therefore never sanitized in CI.

This one has already proven productive when done by hand: gh-139269 (an unaligned uint64_t store in Python/jit.c's patch_* functions, which segfaulted release builds) was found by building --enable-experimental-jit with -fsanitize=address,undefined, and gh-142476 was an ASan-found leak in allocate_executor.

I ran the full test suite against a machine-code JIT build under UBSan, with every entry in Tools/ubsan/suppressions.txt disabled: clean, zero reports, 51,459 tests across 493 files, 4m50s. Verified non-vacuous — _testinternalcapi.get_jit_backend() returns jit and a hot loop produces real machine-code ranges, so this was the copy-and-patch JIT rather than a silent fallback to the uop interpreter.

Since it is already green, this is also a pure regression guard.

Two smaller findings from the same sweep

  • local-bounds is enabled nowhere. --with-undefined-behavior-sanitizer injects plain -fsanitize=undefined, and local-bounds is not in that group even though it detects genuine UB. I built with it on and ran the full suite: zero findings, including no false positives from the trailing variable-length array idiom, which was the obvious risk. It looks free to add.
  • float-divide-by-zero (also not in the group) yields exactly one hit, Modules/expat/xmlparse.c:869, in vendored libexpat, which deliberately relies on IEEE-754 +inf and documents that in a comment. Not worth enabling without excluding expat.

On cost

The usual objection to widening the matrix is build time. For reference, a full PGO + ThinLTO + UBSan build — the heaviest combination I tried, including the instrumented build, the profile run, the rebuild and the ThinLTO link — took 9 min 43 s at --jobs=8. A plain UBSan build is far cheaper, and the JIT suite run above took under five minutes.

(That PGO/LTO configuration found nothing new, incidentally. It is worth noting only because --enable-optimizations and --with-lto appear in no workflow at all, while Arch, Debian and python-build-standalone all ship them. UBSan's checks are source-level, so PGO/LTO mostly changes which UB is reached; miscompilation from UB would show up as test failures rather than sanitizer reports, which is a different exercise.)

Linked PRs

  • #154915 — fixes the free-threaded UB, prerequisite for adding that matrix entry

Has this already been discussed elsewhere?

This is a follow-up to gh-148286.

Links to previous discussion of this feature:

  • gh-148286
  • gh-139269
  • gh-142476

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with .github/workflows/build.yml and compare its UBSan matrix with the free-threading, JIT, and tail-call workflow configurations. Check the prerequisite in #154915, then update the relevant CI configuration so UBSan covers the proposed builds and local-bounds consideration. Done means the new jobs run successfully without sanitizer reports and preserve the existing CI coverage.

Written by the indexing model from the issue text.

Assessment

Tech stack
github-actions, python
Domain
build-system, ci-cd, testing-qa
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
62/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.