Skip to content
Open
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
7 changes: 7 additions & 0 deletions .github/instructions/TypedTreePickle.instructions.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
applyTo:
- "src/Compiler/TypedTree/TypedTreePickle.{fs,fsi}"
- "src/Compiler/TypedTree/TypedTree.{fs,fsi}"
- "src/Compiler/Driver/CompilerImports.{fs,fsi}"
---

Expand All @@ -21,6 +22,12 @@ This means:
2. **Additions must be invisible to old readers.** New data goes in stream B, where readers that don't know about it get `0` (the default sentinel) past end-of-stream. New readers detect presence via a tag byte they write unconditionally.
3. **Tag values are forever.** Once a byte value means something in a reader's `match`, that meaning cannot change. Old DLLs encode that value with the old semantics.

## Flag Enums: Reinterpreting a Tag Breaks Old Binaries

The `ValFlags`, `EntityFlags`, and `TyparFlags` types in `TypedTree.fs` pack enum cases into bit patterns exposed as `PickledBits` and serialized verbatim. Adding a case to such an enum must **not** reuse a bit pattern that already exists in shipped metadata with different semantics. Normalizing the new case on the *write* side protects only future binaries — an older compiler already emitted the old pattern into DLLs that exist permanently. If you reuse a pattern, add matching *read*-side normalization (see `ValFlags.OfPickledBits`) that maps the legacy pattern back to its original meaning; otherwise prefer an unused pattern.

For a detailed example of what goes wrong when a serialized flag pattern is reinterpreted, see `docs/postmortems/regression-legacy-inline-metadata-dynamic-invocation.md`.

## Reading and Writing Must Be Perfectly Aligned

The format uses two parallel byte streams. Every `p_*` (write) function has a corresponding `u_*` (read) function. They must produce and consume the **exact same byte sequence** under **every possible code path** — including paths gated by feature flags, language versions, or target frameworks that your current build may not exercise.
Expand Down
5 changes: 5 additions & 0 deletions docs/postmortems/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,8 @@
Detailed write-ups of bugs that were hard to diagnose, had non-obvious root causes, or taught us something worth preserving. Each document captures the symptoms, root cause, fix, and timeline so that future contributors can recognize similar patterns early.

These are referenced from [agentic instructions](../../.github/instructions/) and serve as deeper reading — the instructions tell you *what* to do, the postmortems explain *why* the rules exist.

## Index

- [`regression-fs0229-bstream-misalignment.md`](regression-fs0229-bstream-misalignment.md) — a conditional write with an unconditional read shifted the pickle B-stream, producing `FS0229` when reading older metadata.
- [`regression-legacy-inline-metadata-dynamic-invocation.md`](regression-legacy-inline-metadata-dynamic-invocation.md) — a new inline-flag case reused a serialized bit pattern that already meant "required inline" in F# 5 binaries, breaking cross-assembly SRTP at runtime.
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
# Regression: Legacy inline metadata decoded as non-inline, breaking cross-assembly SRTP

## Summary

Adding a new `ValInline.InlinedDefinition` case reused the serialized inline-flag bit pattern `0x00`, which already meant "required inline" in assemblies compiled by F# 5.0 and earlier. A newer compiler reading those older assemblies decoded the value as *not* inlined, dropped the inline body at the call site, and emitted a direct call to a dynamic-invocation stub. Cross-assembly SRTP APIs such as Aether 8.3.1 then threw `System.NotSupportedException` at runtime. Shipped in .NET SDK 10.0.400.

## Error Manifestation

A program that consumes an inline SRTP API from a pre-F#6 library compiles cleanly but throws at runtime, including in optimized Release builds:

```text
Unhandled exception. System.NotSupportedException:
Dynamic invocation of op_HatEquals is not supported
at Aether.Optic.set[a,b,c](a optic, b value)
```

The same source built with SDK 10.0.303 prints the expected result. `--always-inline+` does not help; rebuilding the referenced library with a current compiler does.

## Root Cause

`ValFlags` packs a value's inline declaration into two bits of an `int64` that is serialized verbatim into assembly metadata. The bit patterns are a permanent on-disk contract.

In F# 5.0 and earlier the field had a `PseudoVal` case — "must always be inlined, no IL body needed" — encoded as `0x00` with `ShouldInline = true`:

```fsharp
match (flags &&& 0b110000L) with
| 0b000000L -> ValInline.PseudoVal // ShouldInline = true
| 0b010000L -> ValInline.Always
| ...
```

PR #6811 (July 2021, F# 6) removed `PseudoVal` and folded it into `Always`. Crucially, `0x00` kept decoding to a `ShouldInline = true` value, so libraries built before the removal continued to import correctly.

PR #19548 introduced `ValInline.InlinedDefinition` and reused the now-"free-looking" `0x00` bit pattern for it — but with the *opposite* semantics, `ShouldInline = false`. The reader was changed so `0x00` decoded to `InlinedDefinition`. That silently reinterpreted every `0x00` inline value already sitting in shipped DLLs: a required-inline definition from an old library now imported as non-inline, so the consuming compiler emitted a direct call to the SRTP dynamic-invocation stub instead of inlining the resolved witness.

The violated assumption is the "tag values are forever" rule: a bit pattern that already has a meaning in shipped metadata cannot be given a new, incompatible meaning.

## Why It Escaped

PR #19548 *did* add write-side normalization so a current compiler serializes `InlinedDefinition` as `Always` (`0x10`), keeping fresh round-trips correct. That protection is exactly what hid the bug:

- Any in-repo test compiles the producer library **with the new compiler**, which never writes `0x00` for an inline value. So no test that builds its own fixtures could reproduce it — the poisoned byte only exists in binaries produced by an F# 5.0-or-earlier compiler.
- The `CompilerCompat` cross-version suite exercises recent SDKs (9 ↔ current), not pre-2021 F# 5 binaries, so the format generation that still emits `0x00` inline bits was outside its matrix.

The gap was read-side: the new meaning was applied to old bytes, and nothing in CI reads bytes written by a 2021-era compiler.

## Fix

PR #20260 adds `ValFlags.OfPickledBits`, used by `u_ValData` when importing metadata. Because the write side always normalizes `InlinedDefinition` to `Always`, a serialized `0x00` inline field can only originate from a legacy compiler, where it meant required inline. `OfPickledBits` therefore maps legacy `0x00` back to `Always` on import. The serialized byte layout is unchanged; only interpretation of the legacy pattern is restored.

## Timeline

| Date | Event |
|---|---|
| ≤ 2021 | F# 5.0 and earlier encode required-inline values (`PseudoVal`) as inline bits `0x00`, `ShouldInline = true`. |
| 2021-07-19 | PR #6811 removes `PseudoVal`; `0x00` still decodes to a `ShouldInline = true` value. Old libraries keep working. |
| 2026-04-16 | Commit `761c8635b8` adds write-side normalization for the upcoming `InlinedDefinition` (`0x00` → `0x10` on pickle). |
| 2026-07-02 | PR #19548 merges: `InlinedDefinition` reuses `0x00` with `ShouldInline = false`; reader decodes `0x00` → `InlinedDefinition`. Latent regression for legacy binaries. |
| ~2026-08 | Ships in .NET SDK 10.0.400. |
| 2026-08-13 | Issue #20253 filed: Aether 8.3.1 SRTP call throws `NotSupportedException` under 10.0.400. |
| — | PR #20260 adds read-side normalization (`OfPickledBits`), restoring the invariant. |

## Prevention

The generalized rule — flag bit patterns baked into pickled metadata are permanent, and a pattern that already has a meaning in shipped DLLs must never be reinterpreted — is encoded in [`.github/instructions/TypedTreePickle.instructions.md`](../../.github/instructions/TypedTreePickle.instructions.md), whose `applyTo` covers the flag-encoding types in `src/Compiler/TypedTree/TypedTree.{fs,fsi}` and the pickle path. When adding a case to a serialized flag enum, either allocate an unused bit pattern or add read-side normalization that maps legacy patterns to their original semantics — write-side normalization alone only protects future binaries, never the ones already in the wild.
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/11.0.100.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
* Extend the `=` adjacent to an interpolated string fix to the verbatim (`=$@"…"`, `=@$"…"`) and extended multi-dollar (`=$$"""…"""`) interpolated-string forms. ([Issue #16696](https://github.com/dotnet/fsharp/issues/16696), [PR #19984](https://github.com/dotnet/fsharp/pull/19984))
* Preserve type abbreviations (`string`, user-defined aliases) in the refined type of bindings introduced after a `| null` pattern in a `match` expression. ([Issue #19646](https://github.com/dotnet/fsharp/issues/19646), [PR #19745](https://github.com/dotnet/fsharp/pull/19745))
* Fix attributes on return type of unparenthesized tuple methods being silently dropped from IL. ([Issue #462](https://github.com/dotnet/fsharp/issues/462), [PR #19714](https://github.com/dotnet/fsharp/pull/19714))
* Fix cross-assembly calls to inline SRTP functions from libraries compiled with F# 4.7 and earlier. ([Issue #20253](https://github.com/dotnet/fsharp/issues/20253), [PR #20260](https://github.com/dotnet/fsharp/pull/20260))
* Fix enum values losing their type when used in a custom attribute argument of type `obj` (they were stored as the underlying integer instead of the enum). ([Issue #995](https://github.com/dotnet/fsharp/issues/995), [PR #19975](https://github.com/dotnet/fsharp/pull/19975))
* Fix false-positive nullness warning (FS3261) when pattern matching narrows nullness inside seq/list/array comprehensions. ([Issue #19644](https://github.com/dotnet/fsharp/issues/19644), [PR #19743](https://github.com/dotnet/fsharp/pull/19743))
* Fix internal error FS0073 "Undefined or unsolved type variable" in IlxGen when nested inline SRTP functions with multiple overloads leave unsolved typars in the non-witness codegen path. ([Issue #19709](https://github.com/dotnet/fsharp/issues/19709), [PR #19710](https://github.com/dotnet/fsharp/pull/19710))
Expand Down
11 changes: 11 additions & 0 deletions src/Compiler/TypedTree/TypedTree.fs
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,17 @@ type ValFlags(flags: int64) =
else
bits

/// Reconstruct flags from the F# binary metadata. PickledBits always writes
/// ValInline.InlinedDefinition (0x00) out as ValInline.Always (0x01), so zero inline bits
/// are never produced by a compiler that has this normalization. Any zero bits seen here
/// are therefore legacy metadata from compilers older than PR #19548, which used the same
/// 0x00 bits to mean ValInline.Always (ShouldInline=true), and must be imported as such.
static member OfPickledBits(bits: int64) =
if bits &&& 0b00000000000000110000L = 0L then
ValFlags(bits ||| 0b00000000000000010000L)
else
ValFlags bits

/// Represents the kind of a type parameter
[<RequireQualifiedAccess (* ; StructuredFormatDisplay("{DebugText}") *) >]
type TyparKind =
Expand Down
7 changes: 6 additions & 1 deletion src/Compiler/TypedTree/TypedTree.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,12 @@ type ValFlags =
isGeneratedEventVal: bool ->
ValFlags

new: flags: int64 -> ValFlags
/// Reconstruct flags from the F# binary metadata. PickledBits always writes
/// ValInline.InlinedDefinition (0x00) out as ValInline.Always (0x01), so zero inline bits
/// are never produced by a compiler that has this normalization. Any zero bits seen here
/// are therefore legacy metadata from compilers older than PR #19548, which used the same
/// 0x00 bits to mean ValInline.Always (ShouldInline=true), and must be imported as such.
static member OfPickledBits: bits: int64 -> ValFlags

member WithIsCompilerGenerated: isCompGen: bool -> ValFlags

Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/TypedTree/TypedTreePickle.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3301,7 +3301,7 @@ and u_ValData st =
| Some(a, _) -> a)
val_type = x2
val_stamp = newStamp ()
val_flags = ValFlags x4
val_flags = ValFlags.OfPickledBits x4
val_opt_data =
match x1z, x1a, x10, x14, x13, x15, x8, x13b, x12, x9 with
| None, None, None, None, TAccess [], None, None, ParentNone, "", [] -> None
Expand Down
Loading