Skip to content

Commit d272e78

Browse files
etrclaude
andcommitted
Merge TASK-060: scope or remove file-scoped -Warray-bounds suppressions
Eliminate the two unscoped `#pragma GCC diagnostic ignored "-Warray-bounds"` directives flagged in specs/tasks/v2-branch-gap-audit.md §1 and add a `lint-warning-suppressions` gate (wired into both `make check` and the CI lint lane) that fails if any TU under src/ regains a file-scoped suppression. Followed by a review-driven simplification pass: scanner broadened to all of src/*.cpp, two-pass awk collapsed to one, and a self-contained unit test added for the gate. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2 parents 9c2d2f1 + 4fd12c6 commit d272e78

9 files changed

Lines changed: 422 additions & 26 deletions

File tree

.github/workflows/verify-build.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,14 @@ jobs:
728728
run: ./scripts/check-file-size.sh
729729
if: ${{ matrix.build-type == 'lint' && matrix.os-type == 'ubuntu' }}
730730

731+
- name: Run warning-suppression gate
732+
# TASK-060: forbid file-scoped `#pragma GCC diagnostic ignored
733+
# "-Warray-bounds"` (and any future broadened list) on the watched
734+
# library TUs. A regression here would silently turn the warning
735+
# off TU-wide.
736+
run: ./scripts/check-warning-suppressions.sh
737+
if: ${{ matrix.build-type == 'lint' && matrix.os-type == 'ubuntu' }}
738+
731739
- name: Run libhttpserver configure
732740
# TASK-038: fixed CXXLAGS -> CXXFLAGS typo so C++ TUs are instrumented by the sanitizer matrix.
733741
run: |

Makefile.am

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ EXTRA_DIST = libhttpserver.pc.in $(DX_CONFIG) scripts/extract-release-notes.sh s
4545
scripts/lib/resolve-prefix.sh \
4646
scripts/check-complexity.sh scripts/check-duplication.sh \
4747
scripts/check-file-size.sh \
48+
scripts/check-warning-suppressions.sh \
4849
scripts/verify-installed-examples.sh \
4950
test/headers/consumer_direct.cpp test/headers/consumer_detail.cpp test/headers/consumer_detail_modded.cpp \
5051
test/headers/consumer_umbrella.cpp \
@@ -357,7 +358,7 @@ check-hygiene:
357358

358359
# check-local runs the following static (no-install) checks first:
359360
# check-headers, check-examples, check-readme, check-release-notes,
360-
# check-doxygen, check-hooks-doc-spotcheck
361+
# check-doxygen, check-hooks-doc-spotcheck, lint-warning-suppressions
361362
# Then runs check-install-layout and check-hygiene against a single shared
362363
# staged install to avoid paying two full `make install` costs on every
363364
# `make check`. Both install-backed sub-checks can still be invoked standalone
@@ -367,7 +368,7 @@ check-hygiene:
367368
# staged the install; sub-check skips the install step to avoid double cost.
368369
# This knob is set only by check-local; do not set it when invoking sub-checks
369370
# standalone.
370-
check-local: check-headers check-examples check-readme check-release-notes check-doxygen check-hooks-doc-spotcheck
371+
check-local: check-headers check-examples check-readme check-release-notes check-doxygen check-hooks-doc-spotcheck lint-warning-suppressions
371372
@echo "=== Shared staged install for check-install-layout and check-hygiene ==="
372373
@rm -rf "$(abs_top_builddir)/.shared-check-stage"
373374
@$(MAKE) $(AM_MAKEFLAGS) install DESTDIR="$(abs_top_builddir)/.shared-check-stage" >check-shared-install.log 2>&1 || { \
@@ -463,7 +464,17 @@ lint-file-size:
463464
@echo "=== lint-file-size: enforce per-file line-count ceiling ==="
464465
@$(top_srcdir)/scripts/check-file-size.sh
465466

466-
.PHONY: check-headers check-install-layout check-hygiene check-local check-examples check-readme check-release-notes check-doxygen check-hooks-doc-spotcheck check-soversion check-parallel-install lint-complexity lint-duplication lint-file-size
467+
# TASK-060: file-scoped warning-suppression gate. Fails if any .cpp
468+
# file under src/ carries a top-level `#pragma GCC diagnostic ignored
469+
# "-Warray-bounds"` that is not bracketed by a push/pop pair. Wired
470+
# into `make check` via check-local (requires only bash/awk/grep, so
471+
# runs in every CI lane and local developer builds without needing
472+
# external tools). Also invoked directly by CI's `lint` lane.
473+
lint-warning-suppressions:
474+
@echo "=== lint-warning-suppressions: forbid file-scoped GCC warning suppressions ==="
475+
@$(top_srcdir)/scripts/check-warning-suppressions.sh
476+
477+
.PHONY: check-headers check-install-layout check-hygiene check-local check-examples check-readme check-release-notes check-doxygen check-hooks-doc-spotcheck check-soversion check-parallel-install lint-complexity lint-duplication lint-file-size lint-warning-suppressions
467478

468479
# TASK-039: top-level convenience rule that descends into test/ to
469480
# build and run the bench binaries (see test/Makefile.am and
Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
#!/usr/bin/env bash
2+
#
3+
# check-warning-suppressions.sh — guard against file-scoped GCC warning
4+
# suppressions in the library sources flagged by TASK-060.
5+
#
6+
# The audit in specs/tasks/v2-branch-gap-audit.md §1 ("HIGH — Unscoped
7+
# warning suppressions") singled out two `#pragma GCC diagnostic
8+
# ignored "-Warray-bounds"` directives that sat at file scope and
9+
# silenced the warning for entire translation units. TASK-060 either
10+
# removes them or scopes them to a `push`/`pop` block; this gate makes
11+
# the regression visible by failing if any TU in src/ grows a new
12+
# file-scoped suppression.
13+
#
14+
# Watched files: all .cpp files under src/, discovered at runtime via
15+
# `find`. This is safe because at the time TASK-060 landed no TU in
16+
# src/ carried an unscoped -Warray-bounds pragma; the broad scan ensures
17+
# that future work which introduces new TUs is also guarded without
18+
# needing to remember to update a static list.
19+
#
20+
# Detection logic:
21+
# 1. Any `#pragma GCC diagnostic ignored "-Warray-bounds"` at the
22+
# beginning of a line is a candidate violation.
23+
# 2. A candidate is allowed iff it sits between a matching
24+
# `#pragma GCC diagnostic push` and `#pragma GCC diagnostic pop`
25+
# pair (push must appear earlier in the file, pop must appear
26+
# later). Conditional compilation around the push/pop is fine —
27+
# we only care about the textual ordering.
28+
# 3. Any candidate that fails the push/pop bracketing check is
29+
# reported and the script exits 1.
30+
#
31+
# Known limitation: the push/pop check uses "nearest push before" /
32+
# "nearest pop after" heuristics. An interleaved pattern such as
33+
# push@5, pop@8, pragma@10, pop@15 would be incorrectly allowed because
34+
# push_before=5 (non-zero) and pop_after=15 (non-zero). This shape is
35+
# not present in the codebase and is extremely unlikely in practice; if
36+
# the project ever adopts nested or interleaved push/pop patterns,
37+
# upgrade the detection to track bracket depth with a counter.
38+
#
39+
# Exit codes:
40+
# 0 no violations
41+
# 1 one or more watched files carries a file-scoped suppression
42+
set -euo pipefail
43+
44+
REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)"
45+
cd "$REPO_ROOT"
46+
47+
# Discover all .cpp files under src/ at runtime so new TUs are
48+
# automatically included without requiring a manual list update.
49+
# Use a while-read loop for bash 3.x / macOS compatibility (no mapfile).
50+
WATCHED_FILES=()
51+
while IFS= read -r f; do
52+
WATCHED_FILES+=("$f")
53+
done < <(find src -name '*.cpp' | sort)
54+
55+
echo "check-warning-suppressions: scanning ${#WATCHED_FILES[@]} file(s)"
56+
57+
violations=0
58+
for file in "${WATCHED_FILES[@]}"; do
59+
if [ ! -f "$file" ]; then
60+
echo " $file: missing — watched file no longer present" >&2
61+
violations=$((violations + 1))
62+
continue
63+
fi
64+
65+
# Each line that begins with the warning-suppression pragma is a
66+
# candidate. We then verify it is bracketed by push/pop.
67+
while IFS=: read -r lineno _; do
68+
# Single-pass awk: find the nearest push before and pop after
69+
# the candidate line in one read of the file.
70+
read -r push_before pop_after < <(awk -v target="$lineno" '
71+
/^#pragma[[:space:]]+GCC[[:space:]]+diagnostic[[:space:]]+push/ {
72+
if (NR < target) last_push = NR
73+
}
74+
/^#pragma[[:space:]]+GCC[[:space:]]+diagnostic[[:space:]]+pop/ {
75+
if (NR > target && first_pop == 0) first_pop = NR
76+
}
77+
END { print (last_push ? last_push : 0), (first_pop ? first_pop : 0) }
78+
' "$file")
79+
80+
if [ "$push_before" = "0" ] || [ "$pop_after" = "0" ]; then
81+
echo " $file:$lineno: file-scoped #pragma GCC diagnostic ignored \"-Warray-bounds\" (not bracketed by push/pop)" >&2
82+
violations=$((violations + 1))
83+
fi
84+
done < <(grep -nE '^#pragma GCC diagnostic ignored "-Warray-bounds"' "$file" || true)
85+
done
86+
87+
if [ "$violations" -gt 0 ]; then
88+
echo "check-warning-suppressions: FAIL — $violations file-scoped suppression(s) found" >&2
89+
echo " scope each pragma with #pragma GCC diagnostic push / pop and a comment naming the GCC version range" >&2
90+
exit 1
91+
fi
92+
93+
echo "check-warning-suppressions: PASS — no file-scoped -Warray-bounds suppressions in watched files"
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
#!/usr/bin/env bash
2+
# Unit test for check-warning-suppressions.sh
3+
# Tests that the push/pop bracketing detection works correctly.
4+
# Run from any directory; uses a temporary directory for fixtures.
5+
set -euo pipefail
6+
7+
PASS=0
8+
FAIL=0
9+
10+
ok() { echo " OK: $1"; PASS=$((PASS + 1)); }
11+
fail() { echo " FAIL: $1"; FAIL=$((FAIL + 1)); }
12+
13+
SCRIPT="$(cd "$(dirname "$0")" && pwd)/check-warning-suppressions.sh"
14+
15+
# Create a temp work area and override the src/ search inside the script.
16+
# We do this by creating a temp directory tree that mimics src/ and then
17+
# running the script with REPO_ROOT overridden via a symlink approach.
18+
TMPDIR_BASE="$(mktemp -d)"
19+
trap 'rm -rf "$TMPDIR_BASE"' EXIT
20+
21+
FAKE_REPO="$TMPDIR_BASE/repo"
22+
mkdir -p "$FAKE_REPO/scripts" "$FAKE_REPO/src"
23+
24+
# Copy the script so it can be run from the fake repo root.
25+
cp "$SCRIPT" "$FAKE_REPO/scripts/check-warning-suppressions.sh"
26+
chmod +x "$FAKE_REPO/scripts/check-warning-suppressions.sh"
27+
28+
# Test 1: No pragmas — should exit 0
29+
cat > "$FAKE_REPO/src/clean.cpp" <<'EOF'
30+
// no pragmas here
31+
int main() { return 0; }
32+
EOF
33+
if (cd "$FAKE_REPO" && bash scripts/check-warning-suppressions.sh 2>/dev/null); then
34+
ok "no pragmas exits 0"
35+
else
36+
fail "no pragmas should exit 0"
37+
fi
38+
39+
# Test 2: Properly bracketed pragma — should exit 0
40+
cat > "$FAKE_REPO/src/bracketed.cpp" <<'EOF'
41+
// Some code
42+
#pragma GCC diagnostic push
43+
#pragma GCC diagnostic ignored "-Warray-bounds"
44+
int x = arr[5];
45+
#pragma GCC diagnostic pop
46+
int main() { return 0; }
47+
EOF
48+
if (cd "$FAKE_REPO" && bash scripts/check-warning-suppressions.sh 2>/dev/null); then
49+
ok "bracketed pragma exits 0"
50+
else
51+
fail "bracketed pragma should exit 0"
52+
fi
53+
54+
# Test 3: File-scoped pragma (no push/pop) — should exit 1
55+
cat > "$FAKE_REPO/src/unbracketed.cpp" <<'EOF'
56+
#pragma GCC diagnostic ignored "-Warray-bounds"
57+
int arr[5];
58+
int x = arr[10];
59+
int main() { return 0; }
60+
EOF
61+
if ! (cd "$FAKE_REPO" && bash scripts/check-warning-suppressions.sh 2>/dev/null); then
62+
ok "unbracketed pragma exits 1"
63+
else
64+
fail "unbracketed pragma should exit 1"
65+
fi
66+
67+
# Test 4: push before but no pop after — should exit 1
68+
cat > "$FAKE_REPO/src/push_no_pop.cpp" <<'EOF'
69+
#pragma GCC diagnostic push
70+
#pragma GCC diagnostic ignored "-Warray-bounds"
71+
int arr[5];
72+
int main() { return 0; }
73+
EOF
74+
if ! (cd "$FAKE_REPO" && bash scripts/check-warning-suppressions.sh 2>/dev/null); then
75+
ok "push-no-pop exits 1"
76+
else
77+
fail "push-no-pop should exit 1"
78+
fi
79+
80+
# Test 5: pop after but no push before — should exit 1
81+
cat > "$FAKE_REPO/src/no_push_pop.cpp" <<'EOF'
82+
#pragma GCC diagnostic ignored "-Warray-bounds"
83+
int arr[5];
84+
#pragma GCC diagnostic pop
85+
int main() { return 0; }
86+
EOF
87+
if ! (cd "$FAKE_REPO" && bash scripts/check-warning-suppressions.sh 2>/dev/null); then
88+
ok "no-push-pop exits 1"
89+
else
90+
fail "no-push-pop should exit 1"
91+
fi
92+
93+
echo ""
94+
echo "Results: $PASS passed, $FAIL failed"
95+
[ "$FAIL" -eq 0 ]

specs/tasks/M7-v2-cleanup/TASK-060.md

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@
88
Eliminate the two unscoped `#pragma GCC diagnostic ignored "-Warray-bounds"` directives flagged as the worst-shaped suppressions in `specs/tasks/v2-branch-gap-audit.md` §1 "HIGH — Unscoped warning suppressions". Either localize each to the minimum offending line range with a `push`/`pop` pair plus a comment explaining the underlying false-positive, or investigate and remove the suppression entirely once the warning is silenced at the source.
99

1010
**Action Items:**
11-
- [ ] Reproduce the `-Warray-bounds` warning at `src/http_utils.cpp:62` under the supported GCC versions used in CI (matrix lanes in `.github/workflows/verify-build.yml`). Capture the exact diagnostic text and offending construct in the commit message.
12-
- [ ] Reproduce the same at `src/detail/ip_representation.cpp:55`. Capture diagnostic text.
13-
- [ ] For each site: either (a) rewrite the source so the warning no longer fires and delete the pragma, or (b) replace the file-scoped pragma with a tightly scoped `#pragma GCC diagnostic push` / `#pragma GCC diagnostic ignored "-Warray-bounds"` / `#pragma GCC diagnostic pop` block around the minimum offending lines, with a comment naming the GCC version range and the upstream report (if any).
14-
- [ ] If the suppression is kept, add a `TODO(TASK-NNN-followup)` style comment only if a concrete follow-up exists; otherwise the scoping comment is sufficient.
15-
- [ ] Add a guard test or static-analysis spot-check that fails if either file regains a file-scoped suppression.
11+
- [x] Reproduce the `-Warray-bounds` warning at `src/http_utils.cpp:62` under the supported GCC versions used in CI (matrix lanes in `.github/workflows/verify-build.yml`). Capture the exact diagnostic text and offending construct in the commit message.
12+
- [x] Reproduce the same at `src/detail/ip_representation.cpp:55`. Capture diagnostic text.
13+
- [x] For each site: either (a) rewrite the source so the warning no longer fires and delete the pragma, or (b) replace the file-scoped pragma with a tightly scoped `#pragma GCC diagnostic push` / `#pragma GCC diagnostic ignored "-Warray-bounds"` / `#pragma GCC diagnostic pop` block around the minimum offending lines, with a comment naming the GCC version range and the upstream report (if any).
14+
- [x] If the suppression is kept, add a `TODO(TASK-NNN-followup)` style comment only if a concrete follow-up exists; otherwise the scoping comment is sufficient.
15+
- [x] Add a guard test or static-analysis spot-check that fails if either file regains a file-scoped suppression.
1616

1717
**Dependencies:**
1818
- Blocked by: None
@@ -28,4 +28,4 @@ Eliminate the two unscoped `#pragma GCC diagnostic ignored "-Warray-bounds"` dir
2828
**Related Requirements:** PRD §2 code quality NFR
2929
**Related Decisions:** None new
3030

31-
**Status:** Backlog
31+
**Status:** Done

specs/tasks/M7-v2-cleanup/_index.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ TASK-093).
2424

2525
| ID | Name | Audit grade | Estimate | Status |
2626
|---|---|---|---|---|
27-
| TASK-060 | Scope or remove file-scoped `-Warray-bounds` suppressions | HIGH | S | Backlog |
27+
| TASK-060 | Scope or remove file-scoped `-Warray-bounds` suppressions | HIGH | S | Done |
2828
| TASK-061 | Mechanical cleanup sweep — unfinished prose, orphan comments, stale doc refs | HIGH | S | Backlog |
2929
| TASK-062 | RFC-7616-compliant Digest auth response factory | HIGH | L | Backlog |
3030
| TASK-063 | Honor or remove `http_response::pipe` `size_hint` parameter | HIGH | S | Backlog |

0 commit comments

Comments
 (0)