Fix FieldAccessException when optimizer relocates protected base-field access (#19963) (#19964)

* Fix FieldAccessException when optimizer relocates protected base-field access (#19963)

Author: T-Gro

docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@@ -5,6 +5,7 @@
 * Fix missing FS1182 ("unused binding") warning for unused `let` function bindings inside class types. ([Issue #13849](https://github.com/dotnet/fsharp/issues/13849), [PR #19805](https://github.com/dotnet/fsharp/pull/19805))
 * Fix inner mutually-recursive `let rec ... and ...` functions under `--realsig+` not being lifted to top-level static methods (TLR), causing `FSharpFunc` closure allocations and loss of `tail.` opcodes — the large struct-mutual-recursion perf regression reported in [Issue #17607](https://github.com/dotnet/fsharp/issues/17607). ([PR #19882](https://github.com/dotnet/fsharp/pull/19882))
 * Fix `TypeLoadException` ("Specialize tried to implicitly override a method with weaker type parameter constraints") and the related CLR crash with constrained inline calls by stripping constraints from closure-class typars in `EraseClosures.convIlxClosureDef`. ([Issue #14492](https://github.com/dotnet/fsharp/issues/14492), [Issue #19075](https://github.com/dotnet/fsharp/issues/19075), [PR #19882](https://github.com/dotnet/fsharp/pull/19882))
+* Fix `FieldAccessException` at runtime when the optimizer relocates a read of a `protected` (family) base-class field into a method outside the field's family (e.g. a trivial member inlined into module/startup code under `--optimize+`). Protected (family) IL field access is no longer hoisted out of its declaring family by inlining or method-splitting. ([Issue #19963](https://github.com/dotnet/fsharp/issues/19963), [PR #19964](https://github.com/dotnet/fsharp/pull/19964))
 
 * Suppress hover/symbol resolution for wildcard `_` patterns inside `member _.…` bodies that incorrectly showed `val _: T` tooltip. ([PR #19760](https://github.com/dotnet/fsharp/pull/19760))
 * Deduplicate format specifier locations in computation expressions so editor tooling no longer reports duplicate entries for the same `%` specifier. ([Issue #16419](https://github.com/dotnet/fsharp/issues/16419), [PR #19791](https://github.com/dotnet/fsharp/pull/19791))

src/Compiler/AbstractIL/il.fs

@@ -3980,6 +3980,18 @@ let mkNormalLdfld fspec = I_ldfld(Aligned, Nonvolatile, fspec)
 
 let mkNormalLdflda fspec = I_ldflda fspec
 
+/// Matches an IL instruction that loads or stores a field, returning the referenced field spec.
+[<return: Struct>]
+let (|ILFieldInstr|_|) instr =
+    match instr with
+    | I_ldsfld(_, fspec)
+    | I_ldfld(_, _, fspec)
+    | I_ldsflda fspec
+    | I_ldflda fspec
+    | I_stsfld(_, fspec)
+    | I_stfld(_, _, fspec) -> ValueSome fspec
+    | _ -> ValueNone
+
 let mkNormalLdobj dt = I_ldobj(Aligned, Nonvolatile, dt)
 
 let mkNormalStobj dt = I_stobj(Aligned, Nonvolatile, dt)

src/Compiler/AbstractIL/il.fsi

@@ -2144,6 +2144,11 @@ val internal mkNormalStsfld: ILFieldSpec -> ILInstr
 val internal mkNormalLdsfld: ILFieldSpec -> ILInstr
 val internal mkNormalLdfld: ILFieldSpec -> ILInstr
 val internal mkNormalLdflda: ILFieldSpec -> ILInstr
+
+/// Matches an IL instruction that loads or stores a field, returning the referenced field spec.
+[<return: Struct>]
+val internal (|ILFieldInstr|_|): instr: ILInstr -> ILFieldSpec voption
+
 val internal mkNormalLdobj: ILType -> ILInstr
 val internal mkNormalStobj: ILType -> ILInstr
 val internal mkLdcInt32: int32 -> ILInstr

src/Compiler/Checking/import.fs

@@ -159,6 +159,14 @@ let ImportILTypeRef (env: ImportMap) m (tref: ILTypeRef) =
 let CanImportILTypeRef (env: ImportMap) m (tref: ILTypeRef) =
     env.ILTypeRefToTyconRefCache.ContainsKey(tref) || CanImportILScopeRef env m tref.Scope
 
+/// Imports a reference to a type definition (ILTypeRef) as a TyconRef when it can be imported.
+[<return: Struct>]
+let (|TryImportILTypeRef|_|) (env: ImportMap) m (tref: ILTypeRef) =
+    if CanImportILTypeRef env m tref then
+        ValueSome(ImportILTypeRef env m tref)
+    else
+        ValueNone
+
 /// Import a type, given an AbstractIL ILTypeRef and an F# type instantiation.
 ///
 /// Prefer the F# abbreviation for some built-in types, e.g. 'string' rather than

src/Compiler/Checking/import.fsi

@@ -77,6 +77,10 @@ val internal ImportILTypeRef: ImportMap -> range -> ILTypeRef -> TyconRef
 /// Pre-check for ability to import a reference to a type definition, given an AbstractIL ILTypeRef, with caching
 val internal CanImportILTypeRef: ImportMap -> range -> ILTypeRef -> bool
 
+/// Imports a reference to a type definition (ILTypeRef) as a TyconRef when it can be imported.
+[<return: Struct>]
+val internal (|TryImportILTypeRef|_|): ImportMap -> range -> ILTypeRef -> TyconRef voption
+
 /// Import an IL type as an F# type.
 val internal ImportILType: ImportMap -> range -> TType list -> ILType -> TType
 

src/Compiler/Optimize/Optimizer.fs

@@ -1415,8 +1415,43 @@ let AbstractOptimizationInfoToEssentials =
 
     abstractLazyModulInfo
 
+/// True if the IL field has protected (family) accessibility.
+let private isProtectedILFieldSpec cenv m (fspec: ILFieldSpec) =
+    match fspec.DeclaringTypeRef with
+    | Import.TryImportILTypeRef cenv.amap m (ILTyconRawMetadata tdef) ->
+        tdef.Fields.LookupByName fspec.Name
+        |> List.exists (fun fdef -> fdef.Access = ILMemberAccess.Family || fdef.Access = ILMemberAccess.FamilyOrAssembly)
+    | _ -> false
+
+/// True if the expression loads or stores a protected (family) IL field anywhere in its body.
+let private exprReferencesProtectedILField cenv expr =
+    let mutable found = false
+
+    let folder =
+        { ExprFolder0 with
+            exprIntercept =
+                fun _recurseF noInterceptF z e ->
+                    if not found then
+                        match e with
+                        | Expr.Op(TOp.ILAsm(instrs, _), _, _, m) ->
+                            if instrs |> List.exists (function ILFieldInstr fspec -> isProtectedILFieldSpec cenv m fspec | _ -> false) then
+                                found <- true
+                        | _ -> ()
+
+                    noInterceptF z e }
+
+    FoldExpr folder () expr |> ignore
+    found
+
+/// True if the expression references constructs that are only valid within their defining method or
+/// family, and so must not be relocated by inlining or method-splitting: a protected/base call
+/// (UsesMethodLocalConstructs) or a protected (family) IL field access (issue #19963).
+let usesMethodLocalConstructsOrProtectedField cenv (fvs: FreeVars) expr =
+    fvs.UsesMethodLocalConstructs
+    || (fvs.ContainsILFieldAccess && exprReferencesProtectedILField cenv expr)
+
 /// Hide information because of a "let ... in ..." or "let rec ... in ... "
-let AbstractExprInfoByVars (boundVars: Val list, boundTyVars) ivalue =
+let AbstractExprInfoByVars cenv (boundVars: Val list, boundTyVars) ivalue =
     // Module and member bindings can be skipped when checking abstraction, since abstraction of these values has already been done when 
     // we hit the end of the module and called AbstractLazyModulInfoByHiding. If we don't skip these then we end up quadratically retraversing  
     // the inferred optimization data, i.e. at each binding all the way up a sequences of 'lets' in a module. 
@@ -1447,7 +1482,7 @@ let AbstractExprInfoByVars (boundVars: Val list, boundTyVars) ivalue =
             (let fvs = freeInExpr (if isNil boundTyVars then CollectLocalsWithStackGuard() else CollectTyparsAndLocals) expr
              (not (isNil boundVars) && List.exists (Zset.memberOf fvs.FreeLocals) boundVars) ||
              (not (isNil boundTyVars) && List.exists (Zset.memberOf fvs.FreeTyvars.FreeTypars) boundTyVars) ||
-             fvs.UsesMethodLocalConstructs) ->
+             usesMethodLocalConstructsOrProtectedField cenv fvs expr) ->
 
               // Trimming lambda
               UnknownValue
@@ -2851,7 +2886,7 @@ and OptimizeLetRec cenv env (binds, bodyExpr, m) =
         let fvs = List.fold (fun acc x -> unionFreeVars acc (fst x |> freeInBindingRhs CollectLocals)) fvs0 bindsR
         SplitValuesByIsUsedOrHasEffect cenv (fun () -> fvs.FreeLocals) bindsR
     // Trim out any optimization info that involves escaping values 
-    let evalueR = AbstractExprInfoByVars (vs, []) einfo.Info 
+    let evalueR = AbstractExprInfoByVars cenv (vs, []) einfo.Info 
     // REVIEW: size of constructing new closures - should probably add #freevars + #recfixups here 
     let bodyExprR = Expr.LetRec (bindsRR, bodyExprR, m, Construct.NewFreeVarsCache()) 
     let info = CombineValueInfos (einfo :: bindinfos) evalueR 
@@ -2926,7 +2961,7 @@ and OptimizeLinearExpr cenv env expr contf =
               Info = UnknownValue }
         else 
             // On the way back up: Trim out any optimization info that involves escaping values on the way back up
-            let evalueR = AbstractExprInfoByVars ([bindR.Var], []) bodyInfo.Info 
+            let evalueR = AbstractExprInfoByVars cenv ([bindR.Var], []) bodyInfo.Info 
 
             // Preserve the debug points for eliminated bindings that have debug points. 
             let bodyR =
@@ -3094,8 +3129,7 @@ and TryOptimizeVal cenv env (vOpt: ValRef option, shouldInline, inlineIfLambda,
 
     | CurriedLambdaValue (_, _, _, expr, _) when shouldInline || inlineIfLambda ->
         let fvs = freeInExpr CollectLocals expr
-        if fvs.UsesMethodLocalConstructs then
-            // Discarding lambda for binding because uses protected members --- TBD: Should we warn or error here
+        if usesMethodLocalConstructsOrProtectedField cenv fvs expr then
             None
         else
             let exprCopy = CopyExprForInlining cenv inlineIfLambda expr m
@@ -3893,7 +3927,7 @@ and OptimizeLambdas (vspec: Val option) cenv env valReprInfo expr exprTy =
           | None -> CurriedLambdaValue (lambdaId, arities, bsize, exprR, exprTy) 
           | Some baseVal -> 
               let fvs = freeInExpr CollectLocals bodyR
-              if fvs.UsesMethodLocalConstructs || fvs.FreeLocals.Contains baseVal then 
+              if usesMethodLocalConstructsOrProtectedField cenv fvs bodyR || fvs.FreeLocals.Contains baseVal then 
                   UnknownValue
               else 
                   let expr2 = mkMemberLambdas g m tps ctorThisValOpt None vsl (bodyR, bodyTy)
@@ -3975,7 +4009,7 @@ and ComputeSplitToMethodCondition flag threshold cenv env (e: Expr, einfo) =
     let m = e.Range
     (let fvs = freeInExpr (CollectLocalsWithStackGuard()) e
      not fvs.UsesUnboundRethrow &&
-     not fvs.UsesMethodLocalConstructs &&
+     not (usesMethodLocalConstructsOrProtectedField cenv fvs e) &&
      fvs.FreeLocals |> Zset.forall (fun v -> 
           // no direct-self-recursive references
           not (env.dontSplitVars.ContainsVal v) &&
@@ -4037,7 +4071,7 @@ and OptimizeDecisionTreeTarget cenv env _m (TTarget(vs, expr, flags)) =
     let env = BindInternalValsToUnknown cenv vs env 
     let exprR, einfo = OptimizeExpr cenv env expr 
     let exprR, einfo = ConsiderSplitToMethod cenv.settings.abstractBigTargets cenv.settings.bigTargetSize cenv env (exprR, einfo) 
-    let evalueR = AbstractExprInfoByVars (vs, []) einfo.Info 
+    let evalueR = AbstractExprInfoByVars cenv (vs, []) einfo.Info 
     TTarget(vs, exprR, flags), 
     { TotalSize=einfo.TotalSize 
       FunctionSize=einfo.FunctionSize
@@ -4168,8 +4202,7 @@ and OptimizeBinding cenv isRec env (TBind(vref, expr, spBind)) =
                     UnknownValue 
                 else
                     let fvs = freeInExpr CollectLocals body
-                    if fvs.UsesMethodLocalConstructs then
-                        // Discarding lambda for binding because uses protected members
+                    if usesMethodLocalConstructsOrProtectedField cenv fvs body then
                         UnknownValue
                     elif fvs.FreeLocals.ToArray() |> Seq.fold(fun acc v -> if not acc then v.Accessibility.IsPrivate else acc) false then
                         // Discarding lambda for binding because uses private members

src/Compiler/TypedTree/TypedTree.fs

@@ -6086,6 +6086,10 @@ type FreeVars =
       /// Rethrow may only occur in such locations. 
       UsesUnboundRethrow: bool 
 
+      /// Indicates if the expression contains a direct IL field load/store — a cheap over-approximate
+      /// gate the optimizer refines to protected (family) fields (issue #19963). Never read by escape checks.
+      ContainsILFieldAccess: bool 
+
       /// The summary of locally defined tycon representations used in the expression. These may be made private by a signature 
       /// or marked 'internal' or 'private' and we have to check various conditions associated with that. 
       FreeLocalTyconReprs: FreeTycons 

src/Compiler/TypedTree/TypedTree.fsi

@@ -4411,6 +4411,10 @@ type FreeVars =
         /// Rethrow may only occur in such locations.
         UsesUnboundRethrow: bool
 
+        /// Indicates if the expression contains a direct IL field load/store — a cheap over-approximate
+        /// gate the optimizer refines to protected (family) fields (issue #19963). Never read by escape checks.
+        ContainsILFieldAccess: bool
+
         /// The summary of locally defined tycon representations used in the expression. These may be made private by a signature
         /// or marked 'internal' or 'private' type we have to check various conditions associated with that.
         FreeLocalTyconReprs: FreeTycons

src/Compiler/TypedTree/TypedTreeBasics.fs

@@ -342,6 +342,11 @@ let (|AbbrevOrAppTy|_|) (ty: TType) =
     | TType_app (tcref, tinst, _) -> ValueSome(tcref, tinst)
     | _ -> ValueNone
 
+/// Matches a type definition reference backed by Abstract IL metadata, returning that metadata.
+[<return: Struct>]
+let (|ILTyconRawMetadata|_|) (tcref: TyconRef) =
+    if tcref.IsILTycon then ValueSome tcref.ILTyconRawMetadata else ValueNone
+
 //---------------------------------------------------------------------------
 // These make local/non-local references to values according to whether
 // the item is globally stable ("published") or not.

src/Compiler/TypedTree/TypedTreeBasics.fsi

@@ -7,6 +7,7 @@
 module internal FSharp.Compiler.TypedTreeBasics
 
 open Internal.Utilities.Library.Extras
+open FSharp.Compiler.AbstractIL.IL
 open FSharp.Compiler.Syntax
 open FSharp.Compiler.Text
 open FSharp.Compiler.TypedTree
@@ -155,6 +156,10 @@ val stripUnitEqns: unt: Measure -> Measure
 [<return: Struct>]
 val (|AbbrevOrAppTy|_|): ty: TType -> (TyconRef * TypeInst) voption
 
+/// Matches a type definition reference backed by Abstract IL metadata, returning that metadata.
+[<return: Struct>]
+val (|ILTyconRawMetadata|_|): tcref: TyconRef -> ILTypeDef voption
+
 val mkLocalValRef: v: Val -> ValRef
 
 val mkLocalModuleRef: v: ModuleOrNamespace -> EntityRef