es-ude / es-ude/OnDeviceTraining

ci/build: unit_test_ubsan preset sanitizes only 2 UB classes — shift UB rides on the clang asan job alone

Open Beginner friendly
#441 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C
Stars
1
Forks
3
Avg merge
1d 1h
Merged PRs (30d)
8

Description

Follow-up from BFP epic PR6 (#439). Surfaced twice during the PR: a zero-mantissa-code shift-count UB (`0 << up` with `up >= 32`) and the Task-1 death-test RED were both only reproducible with a standalone `-fsanitize=shift` compile — neither the local preset nor CI's ubsan job would have flagged them.

## The gap

`unit_test_ubsan` (CMakePresets.json:66-72 @ d9b46097) sanitizes exactly two classes:

```
-fsanitize=signed-integer-overflow,float-cast-overflow
```

CI's `c-ubsan-build-and-test` job uses this preset with gcc, so the job named "ubsan" misses most of UBSan: `shift` (both exponent and base), `bounds`, `null`, `vla-bound`, `integer-divide-by-zero`, …

The only full-UB net is `c-asan-build-and-test`: `unit_test_asan` carries `-fsanitize=address,undefined` under clang. That coverage is real (it is why PR6 was safe to merge), but it is one compiler's UBSan only, and the preset's name/display ("UBSan overflow") undersells what the dedicated job does NOT check — a contributor running `ctest --preset unit_test_ubsan` locally reasonably believes shift UB is covered.

## Proposal

Extend `unit_test_ubsan` to the full `undefined` group (or at minimum add `shift`), keeping `-fno-sanitize-recover=all`:

```
-fsanitize=undefined -fno-sanitize-recover=all
```

That also buys gcc-vs-clang UBSan diversity (the two implementations flag different corners). If any check turns out noisy on the existing suite, carve it out explicitly with `-fno-sanitize=` and a comment — an explicit carve-out documents the gap instead of hiding it.

Acceptance: the PR6 zero-code shift mutant (revert `(up >= 31u) ? 0 : ...` in `src/layer/Softmax.c`) must turn the `unit_test_ubsan` preset RED.

Part of the #410 epic hygiene trail.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Contributor guide

Open the contributing guide

Research direction

Inspect CMakePresets.json:66-72 and the unit_test_ubsan preset, then run ctest --preset unit_test_ubsan. Use the PR6 zero-code shift mutant in src/layer/Softmax.c as the acceptance check: the preset should turn RED, while any sanitizer exclusions should be explicit and documented.

Written by the indexing model from the issue text.

Assessment

Tech stack
c, cmake
Domain
build-system, ci-cd, testing-qa
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.