Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added audit/runtime/PropertyChangedStorm/Program.cs
Binary file not shown.
23 changes: 23 additions & 0 deletions audit/runtime/PropertyChangedStorm/PropertyChangedStorm.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<Project Sdk="Microsoft.NET.Sdk">
<!--
Own.NET Audit — PropertyChanged-storm profiler (Plan.md §2 cat. 6, §4.3).
Build-required, WINDOWS-ONLY: reads an ETW trace (.etl) captured while a FlaUI
scenario drives the target, counting per-property PropertyChanged frequency. NOT
part of the Linux CI gate — the Python ingest bridge (audit/runtime/ingest.py) is
what CI tests. net472 + x64 to match the legacy target and read its ETW manifest.
-->
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net472</TargetFramework>
<LangVersion>latest</LangVersion>
<Nullable>enable</Nullable>
<AssemblyName>PropertyChangedStorm</AssemblyName>
<RootNamespace>OwnNet.Audit.Runtime</RootNamespace>
<Platforms>x64</Platforms>
<PlatformTarget>x64</PlatformTarget>
</PropertyGroup>
<ItemGroup>
<PackageReference Include="Microsoft.Diagnostics.Tracing.TraceEvent" Version="3.1.16" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
</ItemGroup>
</Project>
58 changes: 47 additions & 11 deletions audit/runtime/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ a static finding clusters with it → **high confidence** (Plan.md §3.5).

This layer covers the categories static analysis honestly can't (Plan.md §2):
event/subscription & timer leaks confirmed under load (cat. 2/3), the
`DependencyPropertyDescriptor.AddValueChanged` leak (cat. 4), and the
**duplicated-immutable-data** detector — the project's "gold" (cat. 11). For these,
the runtime layer is the *only* tool, so they were `NO-TOOL` until now.
`DependencyPropertyDescriptor.AddValueChanged` leak (cat. 4), **PropertyChanged
storms** measured by raise-frequency (cat. 6), and the **duplicated-immutable-data**
detector — the project's "gold" (cat. 11). For these, the runtime layer is the *only*
tool, so they were `NO-TOOL` until now.

## Stack (Windows / build-required — Plan.md §4)

Expand Down Expand Up @@ -40,6 +41,9 @@ audit/runtime/
DuplicateDetector/ # C# duplicate-immutable detector — Windows/build-required, NOT CI-gated
DuplicateDetector.csproj # net472; Microsoft.Diagnostics.Runtime
Program.cs # ClrMD over a full dump: group identical strings, wasted-bytes findings
PropertyChangedStorm/ # C# PropertyChanged-storm profiler — Windows/build-required, NOT CI-gated
PropertyChangedStorm.csproj # net472; Microsoft.Diagnostics.Tracing.TraceEvent
Program.cs # TraceEvent over an .etl: per-property raise frequency, storm findings
```

## How the leak-harness works (Plan.md §4.1)
Expand Down Expand Up @@ -88,22 +92,54 @@ python audit/runtime/ingest.py --duplicate-detector artifacts/own-audit/duplicat
# -> run_static folds duplicate-detector.sarif in as a category-11 (P2) finding set.
```

## PropertyChanged-storm profiler (Plan.md §2 cat. 6)

Frequency — not correctness — is a runtime property. The static `INPC0xx` tier (cat. 5)
catches a missing `nameof` or a broken arg; it cannot see that `Total` fires
PropertyChanged 4 000x for one keystroke, half of them with **no value change**,
thrashing every binding. The profiler reads an ETW trace (`.etl`) captured while a
FlaUI scenario drove the target — a diagnostic build emits one event per raise via an
EventSource (`OwnNet-Sematix-INPC` / `Raised`, payload `{Type, Property, ValueChanged,
[SourceFile, SourceLine]}`) — aggregates per (type, property), and reports each
property over its per-operation threshold. When the build resolved a source file, a
storm clusters with a static `INPC0xx` hit in the same file → **high confidence**
(§3.5); otherwise (file-only with no line, or no location at all) it gets a unique
`inpc://<type>/<NNNN>-<property>` synthetic uri — the `<NNNN>` index keeps distinct
storming properties in distinct clusters even when their slugs collide.

```bash
# on Windows, against an .etl captured during the scenario (PerfView / xperf / logman):
PropertyChangedStorm.exe --trace artifacts/own-audit/scenario.etl --operations 1 \
--per-op-threshold 50 --out artifacts/own-audit/propertychanged-storm.json \
--scenario open-declaration --target acme/LegacyApp --commit "$COMMIT"

# then, anywhere (CI exercises this conversion):
python audit/runtime/ingest.py --propertychanged-storm \
artifacts/own-audit/propertychanged-storm.json \
--out artifacts/own-audit/propertychanged-storm.sarif
# -> run_static folds propertychanged-storm.sarif in as a category-6 (P2) finding set;
# a located storm clusters with a static INPC0xx in the same file.
```

## Selftest

`ingest.py` carries embedded-fixture selftests (no harness, no Windows needed) and
gates on Linux CI — including the end-to-end check that a static OWN014 plus a
runtime leak in the same file form one high-confidence cluster:
gates on Linux CI — including the end-to-end checks that a static OWN014 plus a
runtime leak (and a static `INPC0xx` plus a runtime storm) in the same file each form
one high-confidence cluster:

```bash
python audit/runtime/ingest.py --selftest
```

## Status

- **Done:** the runtime→pipeline bridge (`ingest.py`, CI-gated, for both the
leak-harness and the duplicate detector), the leak-harness scenario schema + one
scenario, runtime rule mappings in the taxonomy (categories 2/3/4/11), the C#
leak-harness skeleton, and the C# duplicate-immutable detector (strings).
- **Done:** the runtime→pipeline bridge (`ingest.py`, CI-gated, for the leak-harness,
the duplicate detector and the PropertyChanged-storm profiler), the leak-harness
scenario schema + one scenario, runtime rule mappings in the taxonomy (categories
2/3/4/6/11), the C# leak-harness skeleton, the C# duplicate-immutable detector
(strings), and the C# PropertyChanged-storm profiler (ETW).
- **Deferred:** duplicate detection for arbitrary immutable types (field-by-field
content equality), the PropertyChanged-storm profiler (C# over ETW),
PerfView/SematixTrace wiring, and a scenario corpus for the top-N screens.
content equality), the diagnostic-build INPC `EventSource` instrumentation in the
target + PerfView/SematixTrace capture wiring, and a scenario corpus for the top-N
screens.
156 changes: 141 additions & 15 deletions audit/runtime/ingest.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,20 +81,21 @@ def _location(f: dict[str, Any]) -> tuple[str, int]:
return (f.get("location") or f.get("type") or "runtime", int(f.get("line", 0) or 0))


_DUP_SLUG_RE = re.compile(r"[^0-9A-Za-z._-]+")
_SLUG_RE = re.compile(r"[^0-9A-Za-z._-]+")


def _dup_uri(type_name: str, value: str, index: int) -> str:
"""A UNIQUE synthetic uri for one heap-wide duplicate group. These findings have
no source line, so without a distinct uri every group would share the same
``(basename, line)`` and the scorer would collapse Country, Currency, ... into a
single cluster — corrupting totals/heatmap and hiding distinct remediation items
(Codex review on #103). The ``index`` guarantees uniqueness even when two display
values share a prefix; the slug keeps the path readable. All groups roll up under
one ``heap://<type>`` module, so the heatmap still buckets them together."""
typ = (type_name or "immutable").replace("\\", ".").replace("/", ".")
slug = _DUP_SLUG_RE.sub("_", value or "").strip("_")[:40] or "value"
return f"heap://{typ}/{index:04d}-{slug}"
def _synthetic_uri(scheme: str, type_name: str, value: str, index: int) -> str:
"""A UNIQUE synthetic uri for a runtime finding with no source line — a heap-wide
duplicate group (``heap://``), a storming property (``inpc://``). Without a
distinct uri every such finding would share the same ``(basename, line)`` and the
scorer would collapse Country/Currency or Total/Subtotal into a single cluster,
corrupting totals/heatmap and hiding distinct remediation items (Codex review on
#103). The ``index`` guarantees uniqueness even when two display values share a
prefix; the slug keeps the path readable. All findings of one ``scheme://<type>``
roll up under one heatmap module, so it still buckets them together."""
typ = (type_name or "runtime").replace("\\", ".").replace("/", ".")
slug = _SLUG_RE.sub("_", value or "").strip("_")[:40] or "value"
return f"{scheme}://{typ}/{index:04d}-{slug}"


def leak_harness_to_sarif(result: dict[str, Any]) -> dict[str, Any]:
Expand Down Expand Up @@ -133,9 +134,10 @@ def duplicate_detector_to_sarif(result: dict[str, Any]) -> dict[str, Any]:
if not f.get("report", True):
continue
# heap-wide duplicate groups have no source line; synthesize a UNIQUE uri per
# value so distinct groups stay distinct clusters (see _dup_uri). Always
# value so distinct groups stay distinct clusters (see _synthetic_uri). Always
# file-level (line 0) -> no fabricated region.
uri = _dup_uri(str(f.get("type", "")), str(f.get("value", "")), len(findings))
uri = _synthetic_uri("heap", str(f.get("type", "")), str(f.get("value", "")),
len(findings))
findings.append({
"rule": f.get("rule", "RUNTIME-DUP-IMMUTABLE"),
"message": f.get("message", ""),
Expand All @@ -151,13 +153,57 @@ def duplicate_detector_to_sarif(result: dict[str, Any]) -> dict[str, Any]:
level="warning")


def propertychanged_storm_to_sarif(result: dict[str, Any]) -> dict[str, Any]:
"""PropertyChanged-storm profiler JSON → SARIF (Plan.md §2 cat. 6, §4.3). The
profiler counts, over one user operation, how often each property raises
PropertyChanged and how many of those raises carry no value change (a missing
equality guard). A property over its per-operation threshold is a storm. When the
instrumentation resolved a source file the finding keeps it — so a storm clusters
with a static ``INPC0xx`` hit (cat. 5) in the same file → high confidence (§3.5);
otherwise we synthesize a unique per-property uri so distinct storming properties
stay distinct clusters. ``report: false`` (below threshold) is dropped; level
``warning`` (a P2 perf finding, not a correctness error)."""
findings = []
for f in result.get("findings", []):
if not f.get("report", True):
continue
# A source location is usable for clustering ONLY when the line is resolved
# (>= 1). The C# falls back to line 0 when only the file is known; writing the
# bare file at line 0 would collapse every file-only storm into one cluster and
# could false-match a static finding on lines 1-3 (Codex review on #104). In
# that case keep a unique per-property synthetic uri instead.
loc = f.get("location")
line = int(f.get("line", 0) or 0)
if loc and line >= 1:
uri = str(loc)
else:
uri = _synthetic_uri("inpc", str(f.get("type", "")),
str(f.get("property", "")), len(findings))
line = 0
findings.append({
"rule": f.get("rule", "RUNTIME-PROPCHANGED-STORM"),
"message": f.get("message", ""),
"uri": uri, "line": line,
"properties": {k: f[k] for k in
("type", "property", "raises", "redundantRaises",
"perOperation", "threshold")
if k in f},
})
return _runtime_sarif(
result.get("tool", "propertychanged-storm"), findings,
{"target": result.get("target", ""), "commit": result.get("commit", ""),
"scenario": result.get("scenario", ""), "operations": result.get("operations", 0)},
level="warning")


def main(argv: list[str] | None = None) -> int:
ap = argparse.ArgumentParser(description="Convert a runtime tool's JSON result to SARIF.")
# exactly one source tool — reject both so a stray flag fails fast instead of
# silently converting the wrong file (CodeRabbit review on #103).
src = ap.add_mutually_exclusive_group()
src.add_argument("--leak-harness", help="leak-harness result JSON")
src.add_argument("--duplicate-detector", help="duplicate-immutable-detector result JSON")
src.add_argument("--propertychanged-storm", help="PropertyChanged-storm profiler result JSON")
ap.add_argument("--out", default="", help="output SARIF (defaults per tool under artifacts/)")
ap.add_argument("--selftest", action="store_true")
args = ap.parse_args(argv)
Expand All @@ -172,8 +218,13 @@ def main(argv: list[str] | None = None) -> int:
result = json.loads(Path(args.duplicate_detector).read_text(encoding="utf-8"))
sarif = duplicate_detector_to_sarif(result)
default_out = "artifacts/own-audit/duplicate-detector.sarif"
elif args.propertychanged_storm:
result = json.loads(Path(args.propertychanged_storm).read_text(encoding="utf-8"))
sarif = propertychanged_storm_to_sarif(result)
default_out = "artifacts/own-audit/propertychanged-storm.sarif"
else:
ap.error("one of --leak-harness / --duplicate-detector is required (or --selftest)")
ap.error("one of --leak-harness / --duplicate-detector / "
"--propertychanged-storm is required (or --selftest)")

out = Path(args.out or default_out)
out.parent.mkdir(parents=True, exist_ok=True)
Expand Down Expand Up @@ -302,6 +353,81 @@ def check(ok: bool, msg: str) -> None:
check(dscored["by_category"].get("duplicate-immutable") == 2,
f"both duplicate values must count under category 11, got {dscored['by_category']}")

# PropertyChanged-storm profiler (cat. 6): per-operation raise frequency. A storm
# with a resolved source file keeps its line (so it clusters with a static INPC0xx
# hit in the same file); storms with no source line get unique per-property uris so
# distinct properties stay distinct clusters; below-threshold is dropped.
storm = propertychanged_storm_to_sarif({
"tool": "propertychanged-storm", "target": "acme/LegacyApp",
"scenario": "open-declaration", "operations": 1,
"findings": [
{"rule": "RUNTIME-PROPCHANGED-STORM", "type": "Acme.Vm.DeclarationViewModel",
"property": "Total", "location": "src/Vm/DeclarationViewModel.cs", "line": 88,
"raises": 4200, "redundantRaises": 3990, "perOperation": 4200.0,
"threshold": 50, "report": True,
"message": "Total raised PropertyChanged 4200x/op (3990 with no value change)"},
{"rule": "RUNTIME-PROPCHANGED-STORM", "type": "Acme.Vm.DeclarationViewModel",
"property": "Subtotal", "raises": 900, "redundantRaises": 880,
"perOperation": 900.0, "threshold": 50, "report": True,
"message": "Subtotal raised PropertyChanged 900x/op (880 with no value change)"},
# file resolved but LINE unresolved (C# falls back to 0): must NOT use the
# bare file uri — keep a synthetic uri so it can't collapse/false-match.
{"rule": "RUNTIME-PROPCHANGED-STORM", "type": "Acme.Vm.DeclarationViewModel",
"property": "Discount", "location": "src/Vm/DeclarationViewModel.cs", "line": 0,
"raises": 700, "redundantRaises": 690, "perOperation": 700.0,
"threshold": 50, "report": True,
"message": "Discount raised PropertyChanged 700x/op (file known, line unresolved)"},
{"rule": "RUNTIME-PROPCHANGED-STORM", "type": "Acme.Vm.DeclarationViewModel",
"property": "Title", "raises": 2, "redundantRaises": 0, "perOperation": 2.0,
"threshold": 50, "report": False, "message": "below threshold"},
]})
sres = storm["runs"][0]["results"]
check(len(sres) == 3, f"below-threshold storm must be dropped: got {len(sres)}")
check(all(r["level"] == "warning" for r in sres),
"propertychanged-storm must be SARIF level warning (P2)")
located = [r for r in sres
if r["locations"][0]["physicalLocation"]["artifactLocation"]["uri"]
== "src/Vm/DeclarationViewModel.cs"]
check(len(located) == 1
and located[0]["locations"][0]["physicalLocation"].get("region", {})
.get("startLine") == 88,
"only the line-resolved storm keeps the bare source file (line>=1)")
# the file-only (line 0) storm falls back to a synthetic inpc:// uri, no region.
file_only = [r for r in sres if "Discount" in r["message"]["text"]]
check(len(file_only) == 1
and file_only[0]["locations"][0]["physicalLocation"]["artifactLocation"]["uri"]
.startswith("inpc://")
and "region" not in file_only[0]["locations"][0]["physicalLocation"],
"a file-only storm (line 0) must fall back to a synthetic inpc:// uri")
suris = {r["locations"][0]["physicalLocation"]["artifactLocation"]["uri"] for r in sres}
check(len(suris) == 3, f"distinct storming properties need distinct uris, got {suris}")
# the TWO unresolved storms (Subtotal: no location, Discount: file-only line 0)
# must get DISTINCT synthetic uris — not collapse onto one (CodeRabbit review #104).
synthetic = [r["locations"][0]["physicalLocation"]["artifactLocation"]["uri"] for r in sres
if r["locations"][0]["physicalLocation"]["artifactLocation"]["uri"]
.startswith("inpc://")]
check(len(synthetic) == 2 and len(set(synthetic)) == 2,
f"unresolved storming properties need distinct synthetic uris, got {synthetic}")
snorm = normalize_results(parse_sarif(json.dumps(storm), "propertychanged-storm", []), tax)
scat = {f.rule: (f.category, f.category_name) for f in snorm}
check(scat.get("RUNTIME-PROPCHANGED-STORM") == (6, "propertychanged-storm"),
f"propertychanged-storm -> category 6, got {scat.get('RUNTIME-PROPCHANGED-STORM')}")
# Plan §3.5: the located storm + a static INPC0xx in the same file -> high confidence.
inpc_sarif = json.dumps({"version": "2.1.0", "runs": [{
"tool": {"driver": {"name": "Own.NET"}}, "results": [
{"ruleId": "INPC003", "level": "warning",
"message": {"text": "raise PropertyChanged without an equality check"},
"locations": [{"physicalLocation": {
"artifactLocation": {"uri": "src/Vm/DeclarationViewModel.cs"},
"region": {"startLine": 87}}}]}]}]})
both_s = normalize_results(parse_sarif(inpc_sarif, "own-check", []), tax) + snorm
sscored = score(both_s, tax, line_tol=3)
confirmed_s = [c for c in sscored["clusters"] if c.confidence == "high"
and set(c.tools) == {"own-check", "propertychanged-storm"}]
check(len(confirmed_s) == 1,
f"static INPC0xx + runtime storm in the same file must form 1 high-confidence "
f"cluster, got {[(c.module, c.tools, c.confidence) for c in sscored['clusters']]}")

fails = [c for c in checks if c]
for f in fails:
print(f"INGEST SELFTEST FAIL: {f}")
Expand Down
Loading
Loading