vtjnash wrote:
> > Yes, concretely it was driven by encountering this pattern in the linux
> > kernel source code here, so it would require this PR to be able to annotate
> > that function definition correctly:
> > https://github.com/torvalds/linux/blob/fc02acf6ac0ccde0c805c2daa9148683cdd01ba8/include/linux/kref.h#L103-L105
>
> Does it work if 'lock' is reordered before the function pointer?
Yes, that exact function could likely be rewritten, but I'm reluctant to assume
it is going to be desirable to assert that projects may simply find it
mandatory to break their API to be able to adopt this analyzer.
> 1. Are you able to compare compile-times with/without? Say on the latest
> Linux kernel rc + perhaps another project that enables -Wthread-safety. The
> Linux kernel may not be entirely representative because it only enables
> -Wthread-safety for a subset of TUs, so analyzing some large TU only might be
> sufficient.
Benchmarks (hyperfine) couldn't measure any difference, either on the kernel or
on synthetic attempts to make it arbitrarily expensive (actually late parsing
was sometimes slightly faster, but far below the noise). See the whole harness
created, executed, analyzed, and summarized by Claude Opus below for evidence.
> 2. What's the stability risk? The feature is still marked "experimental", and
> I'm unfamiliar with how far it is from becoming stable. Anyone else have a
> tracking issue?
Not really something I can answer, but I think if it becomes load-bearing
(added tests here) that won't be easily broken.
---
<details>
<summary>Harness: attribute-heavy source generator
(<code>gen.py</code>)</summary>
Two clangs from the same tree — `0db95253f939` (parent) and this PR —
configured identically (`Release`, no assertions, same host compiler), compared
with `hyperfine` pinned to one core.
### Linux, with the kernel's own compile commands
Linux 7.2 (the tree carrying `CONFIG_WARN_CONTEXT_ANALYSIS`), `x86_64`
defconfig, compile commands taken verbatim from the kernel's `.cmd` files, so
the flags are the kernel's own: `-fexperimental-late-parse-attributes
-Wthread-safety -Wthread-safety-pointer -Wthread-safety-beta`. Both scopes,
since you noted the shipped config only analyzes a subset: the kernel's
`context-analysis-suppression.txt` as shipped, and
`CONFIG_WARN_CONTEXT_ANALYSIS_ALL=y`, which annotates all of `include/linux/`
in every TU. `-fsyntax-only` isolates the front end; two TUs are also measured
as full `-c -O2` compiles for scale.
| TU | analysis scope | mode | parent (ms) | PR (ms) | delta |
|---|---|---|---|---|---|
| `block/blk-core.c` | shipped suppressions | `-fsyntax-only` | 1150 | 1153 |
+0.3% |
| `drivers/ata/libata-core.c` | shipped suppressions | `-fsyntax-only` | 1261 |
1258 | -0.2% |
| `fs/namei.c` | shipped suppressions | `-fsyntax-only` | 925 | 923 | -0.2% |
| `kernel/fork.c` | shipped suppressions | `-fsyntax-only` | 1158 | 1156 |
-0.1% |
| `kernel/futex/core.c` | shipped suppressions | `-fsyntax-only` | 780 | 784 |
+0.6% |
| `mm/memory.c` | shipped suppressions | `-fsyntax-only` | 940 | 944 | +0.5% |
| `net/core/dev.c` | shipped suppressions | `-fsyntax-only` | 1645 | 1638 |
-0.4% |
| `block/blk-core.c` | tree-wide (`_ALL`) | `-fsyntax-only` | 1157 | 1178 |
+1.8% |
| `drivers/ata/libata-core.c` | tree-wide (`_ALL`) | `-fsyntax-only` | 1284 |
1267 | -1.3% |
| `fs/namei.c` | tree-wide (`_ALL`) | `-fsyntax-only` | 1063 | 1063 | +0.0% |
| `kernel/fork.c` | tree-wide (`_ALL`) | `-fsyntax-only` | 1339 | 1342 | +0.2% |
| `kernel/futex/core.c` | tree-wide (`_ALL`) | `-fsyntax-only` | 792 | 794 |
+0.2% |
| `mm/memory.c` | tree-wide (`_ALL`) | `-fsyntax-only` | 1096 | 1083 | -1.2% |
| `net/core/dev.c` | tree-wide (`_ALL`) | `-fsyntax-only` | 1888 | 1900 | +0.7%
|
| `kernel/fork.c` | tree-wide (`_ALL`) | full `-c -O2` | 1759 | 1757 | -0.1% |
| `mm/memory.c` | tree-wide (`_ALL`) | full `-c -O2` | 2544 | 2562 | +0.7% |
Mean delta +0.08%, spread -1.3% .. +1.8% in both directions. Two independently
linked clangs differ by about that much from code layout alone, so this is the
noise floor, not a signal.
### A source file that is nothing but these attributes
Since the kernel's density is low, I also generated a worst case: **18,011
capability attributes in 26,018 lines**, declarations only (no function bodies,
so the analysis itself doesn't dilute the parse), covering
`requires_capability`, `acquire_capability`, `release_capability`,
`assert_capability`, `try_acquire_capability`, `locks_excluded`,
`lock_returned`, `guarded_by`, `pt_guarded_by`, `acquired_after` — on fields,
on function-pointer fields, and on parameters.
Two spellings of the same 18,011 attributes, identical counts, differing only
in declaration order: `early` names everything before use (parses under both
compilers), `late` names everything after use (needs this PR). 100 runs each,
pinned, slowest 10% trimmed:
| what it measures | parent (ms) | PR (ms) | delta |
|---|---|---|---|
| `early` file, `-fexperimental-late-parse-attributes` — every capability
attribute newly deferred by this PR | 167.7 | 166.5 | -0.7% |
| `early` file, no flag — the default path, which this PR must not touch |
167.7 | 166.7 | -0.6% |
| 80k lines of parameter-heavy prototypes, no capability attributes at all —
the unconditional `ParseParameterDeclarationClause` change | 735 | 727 | -1.0% |
| full version with bodies + analysis (36k attributes, 74k lines) | 715 | 716 |
+0.2% |
And under the PR with the flag, the `late` spelling costs -0.3% against the
`early` spelling — i.e. deferring all 18,011 of them to the end of the record
or prototype is free within measurement error. Whatever the token-caching
costs, it is small next to the Sema work each declaration already does. That
bounds the per-attribute cost at well under 0.1 µs.
That matches the shape of the change: with the flag off the deferral list is
created with `LateAttrParseExperimentalExtOnly`, nothing lands in it, and the
eager path runs exactly as before; the only unconditional addition is one stack
`SmallVector` per parameter-declaration-clause, which is the third row above.
```python
#!/usr/bin/env python3
"""Generate thread-safety-attribute-heavy C sources for compile-time
benchmarking.
The variants below all share one code shape, so they are comparable:
pair-early / pair-late
- the matched pair: the same attributes in the same number, differing
only in whether every attribute names something declared before it
(parses under either compiler) or after it (needs late parsing).
compat - capability attributes used the way they must be written today: every
name an attribute mentions is already declared. Accepted by both
compilers, with and without -fexperimental-late-parse-attributes,
with no diagnostics either way.
fwd - the same density, but written with forward references (a parameter's
attribute naming a later parameter, a field's attribute naming a
later member). Only accepted by the PR, with the flag.
noattr - the same declarations with every attribute macro empty: the control
for "what does code that uses none of this pay?".
protos - nothing but function prototypes with many parameters and no
attributes at all, to isolate the per-parameter-clause overhead the
PR adds to ParseParameterDeclarationClause.
"""
import sys
PROLOGUE = r"""
#define REQUIRES(...) __attribute__((requires_capability(__VA_ARGS__)))
#define SHARED_REQUIRES(...) __attribute__((shared_locks_required(__VA_ARGS__)))
#define ACQUIRE(...) __attribute__((acquire_capability(__VA_ARGS__)))
#define RELEASE(...) __attribute__((release_capability(__VA_ARGS__)))
#define ASSERT_CAP(...) __attribute__((assert_capability(__VA_ARGS__)))
#define TRY_ACQUIRE(...)
__attribute__((try_acquire_capability(__VA_ARGS__)))
#define EXCLUDES(...) __attribute__((locks_excluded(__VA_ARGS__)))
#define RETURN_CAP(x) __attribute__((lock_returned(x)))
#define GUARDED_BY(x) __attribute__((guarded_by(x)))
#define PT_GUARDED_BY(x) __attribute__((pt_guarded_by(x)))
#define ACQUIRED_AFTER(...) __attribute__((acquired_after(__VA_ARGS__)))
struct __attribute__((capability("mutex"))) Mutex { int dummy; };
void mu_lock(struct Mutex *m) ACQUIRE(m);
void mu_unlock(struct Mutex *m) RELEASE(m);
int mu_trylock(struct Mutex *m) TRY_ACQUIRE(1, m);
"""
NOATTR_PROLOGUE = r"""
#define REQUIRES(...)
#define SHARED_REQUIRES(...)
#define ACQUIRE(...)
#define RELEASE(...)
#define ASSERT_CAP(...)
#define TRY_ACQUIRE(...)
#define EXCLUDES(...)
#define RETURN_CAP(x)
#define GUARDED_BY(x)
#define PT_GUARDED_BY(x)
#define ACQUIRED_AFTER(...)
struct Mutex { int dummy; };
void mu_lock(struct Mutex *m);
void mu_unlock(struct Mutex *m);
int mu_trylock(struct Mutex *m);
"""
# Everything an attribute names is declared before the attribute is written, so
# this parses with or without late parsing, under either compiler.
COMPAT_BLOCK = r"""
struct Obj_{i} {{
struct Mutex mu1;
struct Mutex mu2 ACQUIRED_AFTER(mu1);
int data GUARDED_BY(mu1);
int *ptr PT_GUARDED_BY(mu2);
void (*cb)(struct Mutex *m) REQUIRES(m);
int (*try_cb)(struct Mutex *m) TRY_ACQUIRE(1, m);
void (*rel_cb)(struct Mutex *m) RELEASE(m);
}};
void acq_{i}(struct Obj_{i} *o) ACQUIRE(o->mu1) EXCLUDES(o->mu2);
void rel_{i}(struct Obj_{i} *o) RELEASE(o->mu1);
int try_{i}(struct Obj_{i} *o) TRY_ACQUIRE(1, o->mu2);
void assert_{i}(struct Obj_{i} *o) ASSERT_CAP(o->mu1);
struct Mutex *getmu_{i}(struct Obj_{i} *o) RETURN_CAP(o->mu1);
void req_{i}(struct Obj_{i} *o) REQUIRES(o->mu1);
void shreq_{i}(struct Obj_{i} *o) SHARED_REQUIRES(o->mu1);
void with_cb_{i}(struct Obj_{i} *o, void (*cb)(struct Mutex *inner)
REQUIRES(inner))
REQUIRES(o->mu1);
void body_{i}(struct Obj_{i} *o) EXCLUDES(o->mu1, o->mu2) {{
acq_{i}(o);
o->data = {i};
req_{i}(o);
rel_{i}(o);
if (try_{i}(o)) {{
*o->ptr = {i};
mu_unlock(&o->mu2);
}}
}}
void body_req_{i}(struct Obj_{i} *o) REQUIRES(o->mu1) {{
assert_{i}(o);
o->data++;
shreq_{i}(o);
}}
"""
# The same attributes, every one of them naming something declared later.
FWD_BLOCK = r"""
struct Obj_{i} {{
int data GUARDED_BY(mu1);
int *ptr PT_GUARDED_BY(mu2);
void (*cb)(void) REQUIRES(mu1);
int (*try_cb)(void) TRY_ACQUIRE(1, mu2);
void (*rel_cb)(void) RELEASE(mu1);
void (*acq_cb)(void) ACQUIRE(mu2);
void (*assert_cb)(void) ASSERT_CAP(mu1);
struct Mutex mu1;
struct Mutex mu2 ACQUIRED_AFTER(mu1);
}};
void acq_{i}(struct Obj_{i} *o) ACQUIRE(o->mu1) EXCLUDES(o->mu2);
void rel_{i}(struct Obj_{i} *o) RELEASE(o->mu1);
int try_{i}(struct Obj_{i} *o) TRY_ACQUIRE(1, o->mu2);
void assert_{i}(struct Obj_{i} *o) ASSERT_CAP(o->mu1);
struct Mutex *getmu_{i}(struct Obj_{i} *o) RETURN_CAP(o->mu1);
void req_{i}(struct Obj_{i} *o) REQUIRES(o->mu1);
void shreq_{i}(struct Obj_{i} *o) SHARED_REQUIRES(o->mu1);
/* kref_put_lock() shape: a parameter's attribute names a later parameter. */
void put_lock_{i}(void (*release)(struct Obj_{i} *) RELEASE(mu), struct Mutex
*mu);
void body_{i}(struct Obj_{i} *o) EXCLUDES(o->mu1, o->mu2) {{
acq_{i}(o);
o->data = {i};
req_{i}(o);
rel_{i}(o);
if (try_{i}(o)) {{
*o->ptr = {i};
mu_unlock(&o->mu2);
}}
}}
void body_req_{i}(struct Obj_{i} *o) REQUIRES(o->mu1) {{
assert_{i}(o);
o->data++;
shreq_{i}(o);
}}
"""
# No attributes at all: just prototypes with parameters, which is the path the
# PR touches unconditionally (ParseParameterDeclarationClause).
PROTO_BLOCK = r"""
int proto_{i}_a(int a, long b, char *c, double d, void *e, unsigned f);
int proto_{i}_b(struct Mutex *m, int (*cb)(struct Mutex *inner, int x), char
**argv);
int proto_{i}_c(int a, int b, int c, int d, int e, int f, int g, int h, int i,
int j);
void proto_{i}_d(void (*cb)(int, long, char *), int n, const char *s, float
*out);
"""
# Declarations only: no function bodies, so the analysis does not run and what
# is left is dominated by parsing and Sema of the attributes themselves.
DECLS_BLOCK = COMPAT_BLOCK.split("void body_")[0]
FWD_DECLS_BLOCK = FWD_BLOCK.split("void body_")[0]
# A matched pair: identical attributes, identical count, differing only in the
# order the names are declared in. "late" needs this PR (every attribute names
# something declared after it); "early" is the same text reordered so that it
# parses under either compiler. The difference between the two is exactly what
# deferring the parse costs.
PAIR_LATE = """
struct Obj_{i} {{
int data GUARDED_BY(mu1);
int *ptr PT_GUARDED_BY(mu2);
void (*cb)(void) REQUIRES(mu1);
int (*try_cb)(void) TRY_ACQUIRE(1, mu2);
void (*rel_cb)(void) RELEASE(mu1);
void (*acq_cb)(void) ACQUIRE(mu2);
void (*assert_cb)(void) ASSERT_CAP(mu1);
struct Mutex mu1;
struct Mutex mu2 ACQUIRED_AFTER(mu1);
}};
void put_lock_{i}(void (*release)(struct Obj_{i} *) RELEASE(mu), struct Mutex
*mu);
"""
PAIR_EARLY = """
struct Obj_{i} {{
struct Mutex mu1;
struct Mutex mu2 ACQUIRED_AFTER(mu1);
int data GUARDED_BY(mu1);
int *ptr PT_GUARDED_BY(mu2);
void (*cb)(void) REQUIRES(mu1);
int (*try_cb)(void) TRY_ACQUIRE(1, mu2);
void (*rel_cb)(void) RELEASE(mu1);
void (*acq_cb)(void) ACQUIRE(mu2);
void (*assert_cb)(void) ASSERT_CAP(mu1);
}};
void put_lock_{i}(struct Mutex *mu, void (*release)(struct Obj_{i} *)
RELEASE(mu));
"""
def main():
variant, n, out = sys.argv[1], int(sys.argv[2]), sys.argv[3]
block = {
"compat": COMPAT_BLOCK,
"fwd": FWD_BLOCK,
"noattr": COMPAT_BLOCK,
"protos": PROTO_BLOCK,
"decls": DECLS_BLOCK,
"fwddecls": FWD_DECLS_BLOCK,
"pair-late": PAIR_LATE,
"pair-early": PAIR_EARLY,
}[variant]
prologue = NOATTR_PROLOGUE if variant in ("noattr", "protos") else PROLOGUE
with open(out, "w") as f:
f.write(prologue)
for i in range(n):
f.write(block.format(i=i))
main()
```
Usage: `python3 gen.py
{pair-early,pair-late,compat,fwd,decls,fwddecls,noattr,protos} <blocks> <out.c>`
</details>
<details>
<summary>Harness: synthetic benchmark</summary>
```bash
BASE=build-base/bin/clang # parent, 0db95253f939
PR=build-pr/bin/clang # this PR
TSA="-Wthread-safety -Wthread-safety-beta -Wthread-safety-pointer"
LATE="-fexperimental-late-parse-attributes"
python3 gen.py pair-early 2000 pair_early.c # 18011 attributes, 26018 lines
python3 gen.py pair-late 2000 pair_late.c # same attributes,
forward-referencing
python3 gen.py protos 16000 protos.c # no capability attributes at all
# both compilers must agree, or the comparison is meaningless
for c in $BASE $PR; do $c -fsyntax-only $TSA $LATE pair_early.c; done # silent
$PR -fsyntax-only $TSA $LATE pair_late.c # silent
$BASE -fsyntax-only $TSA $LATE pair_late.c #
errors, as expected
HF="hyperfine -N --warmup 5 --min-runs 100 --style basic"
$HF -n base "taskset -c 11 $BASE -fsyntax-only $TSA $LATE pair_early.c" \
-n pr "taskset -c 11 $PR -fsyntax-only $TSA $LATE pair_early.c"
$HF -n base "taskset -c 11 $BASE -fsyntax-only $TSA pair_early.c" \
-n pr "taskset -c 11 $PR -fsyntax-only $TSA pair_early.c"
$HF -n early "taskset -c 11 $PR -fsyntax-only $TSA $LATE pair_early.c" \
-n late "taskset -c 11 $PR -fsyntax-only $TSA $LATE pair_late.c"
$HF -n base "taskset -c 11 $BASE -fsyntax-only $TSA $LATE protos.c" \
-n pr "taskset -c 11 $PR -fsyntax-only $TSA $LATE protos.c"
```
</details>
<details>
<summary>Harness: Linux TUs, reusing the kernel's own compile commands</summary>
```bash
#!/bin/bash
# Compile a kernel config with a given clang and capture all diagnostics.
# usage: kbuild.sh <label> <clang-bin-dir> <config: defconfig|allmodconfig>
<all: y|n>
set -u
LABEL=$1; CLANGDIR=$2; BASECFG=$3; ALLSRC=$4
TMP=${TMP:-/tmp/tsa-bench} # holds the linux checkout and the build dirs
SRC=$TMP/linux
OUT=$TMP/out-$LABEL
LOG=$TMP/log-$LABEL.txt
export PATH=/opt/bb2-x86_64-linux-gnu/wrappers:$PATH
mkdir -p "$OUT"
# The sandbox's host elf.h predates these ARM relocation names, which modpost
# references; the values are the ABI-fixed ones. Host tools use the sandbox gcc
# (our clang has no host sysroot); only the kernel objects (freestanding,
# -nostdinc) go through the clang under test.
HOSTDEFS="-DR_ARM_MOVW_ABS_NC=43 -DR_ARM_MOVT_ABS=44 -DR_ARM_CALL=28 \
-DR_ARM_JUMP24=29 -DR_ARM_THM_MOVW_ABS_NC=47 -DR_ARM_THM_MOVT_ABS=48 \
-DR_ARM_THM_JUMP19=51 -DR_ARM_THM_JUMP24=30"
kmake() {
make -C "$SRC" O="$OUT" ARCH=x86_64 \
CC="$CLANGDIR/clang" HOSTCC=cc HOSTCXX=c++ \
LD=ld AR=ar NM=nm OBJCOPY=objcopy OBJDUMP=objdump STRIP=strip
READELF=readelf \
HOSTLD=ld HOSTAR=ar HOSTCFLAGS="$HOSTDEFS" "$@"
}
kmake "$BASECFG" >/dev/null 2>&1 || { echo "$BASECFG failed"; exit 1; }
# Force context analysis on, tree-wide if requested.
cfg() { "$SRC/scripts/config" --file "$OUT/.config" "$@"; }
cfg -d TRACE_BRANCH_PROFILING
cfg -e EXPERT
cfg -d COMPILE_TEST
# Keep these as warnings: -Werror plus clang's default error limit would
# truncate a TU after 19 diagnostics and hide the rest.
cfg -d WERROR
cfg -d OBJTOOL_WERROR
cfg -e WARN_CONTEXT_ANALYSIS
if [ "$ALLSRC" = y ]; then cfg -e WARN_CONTEXT_ANALYSIS_ALL; else cfg -d
WARN_CONTEXT_ANALYSIS_ALL; fi
cfg -e CONTEXT_ANALYSIS_TEST
kmake olddefconfig >/dev/null 2>&1
echo "=== $LABEL config ==="
grep -E
'CONFIG_(WARN_CONTEXT_ANALYSIS|WARN_CONTEXT_ANALYSIS_ALL|CONTEXT_ANALYSIS_TEST)='
"$OUT/.config"
# Compile only: build the top-level subdirectories, no vmlinux link.
# -k so one failure does not hide the rest of the diagnostics.
# --output-sync=target is essential: without it 100 parallel compilers
# interleave their writes and diagnostics get shredded mid-line.
kmake -k -j100 --output-sync=target \
arch/x86/ block/ crypto/ drivers/ fs/ init/ io_uring/ ipc/ kernel/ \
lib/ mm/ net/ security/ sound/ virt/ \
>"$LOG" 2>&1
echo "$LABEL: make exit=$? log=$LOG lines=$(wc -l <"$LOG")"
```
That build is only needed once, to make the kernel record its `.cmd` files.
Then, per TU, take the command it recorded and swap only the compiler:
```bash
#!/bin/bash
# Real-world compile-time comparison for PR 212615: Linux 7.2 TUs built with
# CONFIG_WARN_CONTEXT_ANALYSIS (which passes
-fexperimental-late-parse-attributes
# -Wthread-safety ...), using the kernel's own compile commands.
set -u
SP=$SP
BASE=$SP/build-base/bin/clang
PR=$SP/build-pr/bin/clang
KT=$TMP
HF="$SP/hyperfine --style basic --export-json"
# $1 = out dir (config), $2 = object, $3 = mode (full|syntax)
bench_tu() {
local out=$KT/$1 obj=$2 mode=$3
local cmdfile="$out/$(dirname $obj)/.$(basename $obj).cmd"
[ -f "$cmdfile" ] || { echo "skip $1/$obj (no .cmd)"; return; }
local cmd args
cmd=$(sed -n "s|^savedcmd_$obj := ||p" "$cmdfile")
args=${cmd#* } # drop the compiler path
args=$(echo "$args" | sed -E "s| -Wp,-MMD,[^ ]+||; s| -o $obj| -o /dev/null|")
if [ "$mode" = syntax ]; then
args=$(echo "$args" | sed -E "s| -c | -fsyntax-only |; s| -o /dev/null||")
fi
local runs=5; [ "$mode" = syntax ] && runs=10
local tag="$1-$(echo $obj | tr / _)-$mode"
echo "=== $1 $obj ($mode)"
( cd "$out" && $HF "$SP/hf-k-$tag.json" --warmup 1 --min-runs $runs \
-n "base" "$BASE $args" -n "pr" "$PR $args" ) 2>&1 | tail -20
}
TUS="mm/memory.o kernel/fork.o fs/namei.o block/blk-core.o kernel/futex/core.o
drivers/ata/libata-core.o net/core/dev.o"
for cfg in out-new-all out-new-supp; do
for tu in $TUS; do
bench_tu $cfg $tu syntax
done
done
for cfg in out-new-all; do
for tu in mm/memory.o kernel/fork.o; do
bench_tu $cfg $tu full
done
done
```
</details>
https://github.com/llvm/llvm-project/pull/212615
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
