Skip to content

Retain independent API memory-safety facts - #5915

Merged
richlander merged 6 commits into
mainfrom
feat/5253-api-memory-safety-facts
Sep 5, 2026
Merged

richlander merged 6 commits into
mainfrom
feat/5253-api-memory-safety-facts

Conversation

@richlander

@richlander richlander commented Sep 5, 2026 •

Copy link
Copy Markdown
Owner

Adds shared, typed API facts so consumers can distinguish caller contracts
from pointer shape instead of making one Boolean answer both questions.
This is the Metadata-owned foundation for correct memory-safety presentation;
existing IsUnsafe consumers retain their behavior during migration.

Closes #5253.

Demo

The .NET 11 updated-rules fixture contains these neighboring declarations:

public static void PointerOnly(int* value) { unsafe { _ = *value; } }
public static unsafe int MethodContract() => 42;
public static unsafe int PropertyContract { get; set; }
public static unsafe event Action? EventContract;

Before, an API member exposed only the existing IsUnsafe projection. After,
its independent facts are available without re-reading metadata or parsing
that Boolean or the signature text:

Declaration Caller contract Signature pointer Existing IsUnsafe
PointerOnly under updated rules None Present true, unchanged
MethodContract under updated rules Explicit Absent true, unchanged
PropertyContract under updated rules Explicit Absent false, unchanged
EventContract under updated rules Explicit Absent false, unchanged
Pointer-bearing method under legacy rules Implicit Present true, unchanged

Property and event accessor MethodDefs retain their own contract results.
Ordinary auto-property and field-like-event backing matches carry the
convention, module MVID, field token, matched name, and static/instance fact.
Two matches are ambiguous; one positive match beside an undecodable candidate
is unknown with the positive evidence retained. A custom property with no
convention match is unknown, not declared storage-free.

Type agreement is deliberately conservative: exact same-module signature
encoding, not display spelling. Equal names from different scopes and unequal
array bounds do not establish a backing match; ordinary generic, array,
pointer, and function-pointer properties and generic events do. Equivalent
types encoded differently remain outside the positive-match convention.

Module rules, layout, member/accessor facts, and backing evidence survive
reader disposal and API JSON round-trip. Older JSON does not invent the facts.
The normal and compact production source-generated JSON contexts retain them.
The member command also preserves type facts through member filters and section
selection. For Utf8JsonReader in System.Text.Json.dll, --kind method
retains its Sequential layout and module rules alongside 41 methods;
-S Properties retains the same facts alongside 11 properties.

Design basis

The sole normative owner is ILInspector.Metadata API representation through
docs/design/type-member-api-representation.md#api-memory-safety-facts.
Its claim is an additive, reader-independent projection that preserves
contract, pointer, declaration-shape, and unavailable evidence independently.

Supporting owners and constraints:

  • MemorySafetyMetadataIndex supplies caller-contract and module-rule
    outcomes under assembly-inspection-query.md; no second contract resolver
    or new memory-safety semantics are introduced.
  • PointerDetector supplies shared structural signature evidence to the
    existing resolver and API extraction.
  • The landed Metadata semantic-substrate pattern supplies the evidence-grade
    distinction. Existing convention markers are reused with a narrower,
    encoding-based type match for retained facts; existing field folding is
    unchanged. Event association is not independently admitted as a new shared
    substrate family.
  • The API-extraction budget counts retained evidence text. Existing
    reader-lifetime, platform, and host-neutral boundaries remain in force.
  • CSharp snapshots and CLI accessor projections only preserve the new model
    facts; neither changes declaration, filtering, diff, or admission policy.

The conventional design is separate typed facts with explicit unavailable
states, rather than a replacement Boolean or a broad new result framework.
Existing compiler-produced legacy/updated fixtures and resolver cases supply
the oracle. The only added representation complexity serves the concrete
correctness need to avoid losing contract evidence or presenting unknown
storage/signatures as negative facts.

Production-host adoption and scope

Overall tracker: #5226. The primary declaration path has 3 stages:

  1. Separate API caller contracts from pointer shape #5253 publishes the Metadata facts in this PR.
  2. Use typed memory-safety facts in C# declaration spelling #5257 adopts them in the shared CSharp declaration producer, consuming
    Make Decompiler memory-safety rendering model-aware #5255's separately owned primary-constructor fallback.
  3. Use typed memory-safety facts in C# declaration spelling #5257 exercises that producer through CLI and browser/Wasm declaration
    surfaces.

This PR completes stage 1, not the later host behavior. JS-export policy
(#5258) and Research summaries (#5259) are separate adoption branches.
IsUnsafe is neither retired nor narrowed here; consumer migration precedes
that decision.

No new rendering domain or host-specific algorithm is added. Existing typed
API JSON uses its source-generated serializers; existing Markout/declaration
rendering is unchanged. The facts are available to both production hosts
through shared models. No platform exception is requested.

Validation

Focused Release evidence uses the existing Metadata native xUnit runner
(-class) and the CLI test project's MTP runner (--filter-class).
The admitted cases are ordinary compiler output, legacy versus updated
contracts, invalid/unsupported module markers, malformed signatures,
duplicate and incomplete backing candidates, JSON compatibility, and exact
retained-text accounting. No separate product-behavior claim is made for
unimplemented consumer adoption.

dotnet build tests/ILInspector.Metadata.Tests/ILInspector.Metadata.Tests.csproj -c Release
dotnet run --project tests/ILInspector.Metadata.Tests -c Release --no-build -- \
  -class ILInspector.Metadata.Tests.ApiMemorySafetyFactsTests \
  -class ILInspector.Metadata.Tests.ApiSurfaceUnsafeTests \
  -class ILInspector.Metadata.Tests.ArrayKindIdentityTests \
  -class ILInspector.Metadata.Tests.ApiSurfaceExtractorBoundsTests \
  -class ILInspector.Metadata.Tests.MemorySafetyMetadataIndexTests

dotnet build src/dotnet-inspect.Tests/dotnet-inspect.Tests.csproj -c Release
dotnet run --project src/dotnet-inspect.Tests -c Release --no-build -- \
  --filter-class DotnetInspector.Tests.ApiMemorySafetyJsonTests

The focused selections passed 220 Metadata cases and 5 production JSON /
accessor cases
. Local builds used -m:2 -p:UseSharedCompilation=false and
DOTNET_PROCESSOR_COUNT=2 to avoid contention with the shared compiler server.
Both selections also passed at the frozen integrated candidate
162a3beeefb121676d41544c4078057e3649df41: the Metadata gate before
publication and the production JSON gate alongside CI.

npx --no-install markdownlint-cli docs/design/type-member-api-representation.md
and git diff --check pass.

Review-driven projection fix

Round 1's three reviewers identified one omission: two CLI JSON projections
were dropping the newly retained type facts. Commit
9ba250ee18f31e858412feefa331cedb1a8c628b preserves Layout and MemorySafety
in both copies. Four actual MemberCommand cases cover member-kind filtering
and section selection in normal and compact JSON; all 9 cases in
ApiMemorySafetyJsonTests pass. The reported Utf8JsonReader scenarios now
retain both facts with their original member counts. No filtering or
IsUnsafe policy changed.

richlander and others added 4 commits September 4, 2026 16:37
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@richlander

Copy link
Copy Markdown
Owner Author

Round 1 candidate is locked at 162a3be; effective base main at 807a4db. This is the Metadata-owned foundation for independently meaningful API contract, pointer, and declaration-shape facts. The final integrated head passed the 220-case focused Metadata selection; the 5-case production JSON/accessor selection passed before integration and is repeating alongside CI. Review dispatch waits for successful current-head ci-required. Planned seats: GPT-6 Astra plus Claude Opus 5, following the current AGENTS roster and using a second model family. No consumer-policy migration, exact-codomain redesign, or merge authorization is included.

@richlander

Copy link
Copy Markdown
Owner Author

Final-head local gates are complete at 162a3be: 220 focused Metadata cases and 5 production source-generated JSON/accessor projection cases passed in Release. The JSON cases include the library, API surface, normal type, and compact type contexts, plus selection of each accessor’s own contract token. The candidate remains unchanged and locked; review dispatch still awaits ci-required under the existing bounded observation schedule.

@richlander

richlander commented Sep 5, 2026 •

Copy link
Copy Markdown
Owner Author

Round 1 is running at frozen head 162a3be, effective base 807a4db. Round start: 2026-09-05T01:06:21Z. Current-head ci-required succeeded, both local gates passed, and the latest main tip 191e8da merges locally without conflict. GPT-6 Astra, Claude Opus 5, and the additional GPT-5.6 Sol read are dispatched with the same complete canonical prompt and candidate frame, each in a separate read-only exact-head worktree. No author changes or base integrations occur while this round is locked; no merge is authorized.

@richlander

Copy link
Copy Markdown
Owner Author

Round 1 reconciliation at 162a3be: all three reviewers found the same API projection omission (Astra/Sol medium; Opus low), accepted as one medium implementation defect. Parent reproduction from the clean Opus exact-head worktree confirms Utf8JsonReader JSON retains Sequential layout and module rules unfiltered, but --kind method drops both with 41 members remaining; -S Properties also drops both with 11 members remaining. The Metadata owner already requires persistence; no design expansion is needed. The next candidate will copy Layout and MemorySafety in both ApiCommand type projections and gate the actual command path, preserving all filtering policy. Opus’s payload growth, mixed enum encoding, reserved-layout naming, and event-token notes are non-blocking observations; no wire redesign or consumer adoption is being folded into this fix. Current-head CI and the 220+5 local gates are green. Round 1 closes with 0/3 clean, converging feedback. No review-clean label or merge authorization exists.

richlander and others added 2 commits September 4, 2026 18:49
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@richlander

Copy link
Copy Markdown
Owner Author

Round 2 candidate locked at 9ba250e, effective base main at 191e8da (integrated before fixing; final fetch required no further integration). Fix commit 9ba250e resolves the single finding shared by Astra, Sol and Opus: both filtered and section-selected ApiType JSON copies now retain Layout and MemorySafety. The 9-case ApiMemorySafetyJsonTests gate passes, including 4 actual MemberCommand projection cases across normal/compact JSON. Parent replay of both reported Utf8JsonReader scenarios retains Sequential layout/module rules with the same 41 methods and 11 properties. No selection or IsUnsafe policy changes. Review dispatch waits for new-head ci-required; the next review will confirm this fix, carrying the previous findings forward. No merge authorization.

@richlander

Copy link
Copy Markdown
Owner Author

Round 2 is starting at fixed head 9ba250ee18f31e858412feefa331cedb1a8c628b, effective base 191e8dacc2fc1fec7faa887253cf5c7005d1a633 (2026-09-05T02:48:43Z). Theme: metadata-semantic-substrates.

This provides shared, typed API facts without conflating caller contracts and pointer shape. The sole normative owner remains docs/design/type-member-api-representation.md#api-memory-safety-facts; the existing resolver supplies contract semantics, and later consumers retain their separately owned policy decisions.

The previous round found one defect shared by all three reviewers: two CLI JSON type copies dropped Layout and MemorySafety. The replacement preserves both fields through kind filtering and section selection. Nine focused JSON cases pass, including four command-level normal/compact projection cases; the reported Utf8JsonReader scenarios preserve both facts and their original member counts. The prior 220-case Metadata result belongs to the previous head, with the Metadata implementation unchanged.

Current-head ci-required succeeded at 2026-09-05T02:39:57Z. The 02:48:21Z snapshot has no non-green or running checks; the fetched main tip is unchanged and mergeability is positive. Reusing Astra, Opus 5, and GPT-5.6 Sol in new isolated worktrees for a proportional confirmation round. Keeping the same three seats preserves continuity with the unanimous prior finding. The candidate stays locked. No merge authorization has been given.

@richlander

Copy link
Copy Markdown
Owner Author

Shared Metadata facts now survive the supported JSON projections without changing consumer policy. Round 2 is CLEAN: GPT-6 Astra, Claude Opus 5, and GPT-5.6 Sol each accepted exact head 9ba250ee18f31e858412feefa331cedb1a8c628b, effective base 191e8dacc2fc1fec7faa887253cf5c7005d1a633. No blocking findings or author changes remain.

The round-1 finding is resolved: both filtered and section-projected ApiType copies retain Layout and MemorySafety. Opus independently confirmed that the new command cases reach both distinct branches, replayed the 56/41/11-member Utf8JsonReader scenarios, and reran all 9 focused JSON cases successfully. Astra and Sol also returned CLEAN. The Metadata implementation is unchanged from the prior full review.

Design basis remains docs/design/type-member-api-representation.md#api-memory-safety-facts: additive, reader-independent retention of independent evidence. MemorySafetyMetadataIndex supplies existing semantics, the semantic-substrate pattern supplies evidence-grade distinctions, and CSharp/CLI projections preserve facts. No ownership drift or new policy adoption occurred. The three-stage #5253 -> #5257 shared producer -> CLI/browser host plan remains unchanged.

Non-actions: BuildFilteredTypeForSections still omits these facts, but its only callers compute discovery/emptiness and no current predicate consumes them; there is no current observable failure. Carry this note to #5257 before it introduces a relevant consumer. The prior JSON-size, enum-form, layout-name, and event-token observations remain nonblocking and unchanged. No speculative fix is added to the clean head.

Final snapshot at 2026-09-05T02:59:12Z: current-head ci-required is successful, no non-green or running checks, GraphQL MERGEABLE/CLEAN, non-draft, expected head/base ref, and fetched main remains the recorded effective base. No base integration is needed. Recording review-clean for this exact SHA; no merge authorization has been given and auto-merge remains unarmed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-clean PR has met the adversarial review standard per AGENTS.md.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Separate API caller contracts from pointer shape

1 participant