Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 16105 - Release inline optimization leads to MethodAccessException if used with assembly:InternalsVisibleTo attribute #16737

Merged
merged 8 commits into from
Feb 27, 2024
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
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/8.0.300.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* `[<CliEvent>]` member should not produce property symbol. ([Issue #16640](https://github.com/dotnet/fsharp/issues/16640), [PR #16658](https://github.com/dotnet/fsharp/pull/16658))
* Fix discriminated union initialization. ([#PR 16661](https://github.com/dotnet/fsharp/pull/16661))
* Allow calling method with both Optional and ParamArray. ([#PR 16688](https://github.com/dotnet/fsharp/pull/16688), [suggestions #1120](https://github.com/fsharp/fslang-suggestions/issues/1120))
* Fix release inline optimization, which leads to MethodAccessException if used with `assembly:InternalsVisibleTo`` attribute. ([Issue #16105](https://github.com/dotnet/fsharp/issues/16105), ([PR #16737](https://github.com/dotnet/fsharp/pull/16737))
* Enforce AttributeTargets on let values and functions. ([PR #16692](https://github.com/dotnet/fsharp/pull/16692))

### Added
Expand Down
43 changes: 34 additions & 9 deletions src/Compiler/Optimize/Optimizer.fs
Original file line number Diff line number Diff line change
Expand Up @@ -496,7 +496,7 @@ let rec IsPartialExprVal x =
| TupleValue args | RecdValue (_, args) | UnionCaseValue (_, args) -> Array.exists IsPartialExprVal args
| ConstValue _ | CurriedLambdaValue _ | ConstExprValue _ -> false
| ValValue (_, a)
| SizeValue(_, a) -> IsPartialExprVal a
| SizeValue (_, a) -> IsPartialExprVal a

let CheckInlineValueIsComplete (v: Val) res =
if v.MustInline && IsPartialExprVal res then
Expand Down Expand Up @@ -690,10 +690,25 @@ let GetInfoForVal cenv env m (vref: ValRef) =
GetInfoForLocalValue cenv env vref.binding m
else
GetInfoForNonLocalVal cenv env vref
res

let GetInfoForValWithCheck cenv env m (vref: ValRef) =
let res = GetInfoForVal cenv env m vref
check vref res |> ignore
res

let IsPartialExpr cenv env m x =
let rec isPartialExpression x =
match x with
| Expr.App (func, _, _, args, _) -> func :: args |> Seq.exists isPartialExpression
| Expr.Lambda (_, _, _, _, expr, _, _) -> expr |> isPartialExpression
| Expr.Let (TBind (_,expr,_), body, _, _) -> expr :: [body] |> List.exists isPartialExpression
| Expr.LetRec (bindings, body, _, _) -> body :: (bindings |> List.map (fun (TBind (_,expr,_)) -> expr)) |> List.exists isPartialExpression
| Expr.Sequential (expr1, expr2, _, _) -> [expr1; expr2] |> Seq.exists isPartialExpression
| Expr.Val (vr, _, _) when not vr.IsLocalRef -> ((GetInfoForVal cenv env m vr).ValExprInfo) |> IsPartialExprVal
| _ -> false
isPartialExpression x

//-------------------------------------------------------------------------
// Try to get information about values of particular types
//-------------------------------------------------------------------------
Expand Down Expand Up @@ -3062,11 +3077,14 @@ and TryOptimizeVal cenv env (vOpt: ValRef option, mustInline, inlineIfLambda, va
failwith "tuple, union and record values cannot be marked 'inline'"

| UnknownValue when mustInline ->
warning(Error(FSComp.SR.optValueMarkedInlineHasUnexpectedValue(), m)); None
warning(Error(FSComp.SR.optValueMarkedInlineHasUnexpectedValue(), m))
None

| _ when mustInline ->
warning(Error(FSComp.SR.optValueMarkedInlineCouldNotBeInlined(), m)); None
| _ -> None
warning(Error(FSComp.SR.optValueMarkedInlineCouldNotBeInlined(), m))
None

| _ -> None

and TryOptimizeValInfo cenv env m vinfo =
if vinfo.HasEffect then None else TryOptimizeVal cenv env (None, false, false, vinfo.Info, m)
Expand All @@ -3089,7 +3107,7 @@ and OptimizeVal cenv env expr (v: ValRef, m) =

let g = cenv.g

let valInfoForVal = GetInfoForVal cenv env m v
let valInfoForVal = GetInfoForValWithCheck cenv env m v

match TryOptimizeVal cenv env (Some v, v.MustInline, v.InlineIfLambda, valInfoForVal.ValExprInfo, m) with
| Some e ->
Expand Down Expand Up @@ -3402,15 +3420,15 @@ and TryInlineApplication cenv env finfo (tyargs: TType list, args: Expr list, m)
| _ -> false
| _ -> false
| _ -> false
| _ -> false
| _ -> false

if isValFromLazyExtensions then None else

let isSecureMethod =
match finfo.Info with
| ValValue(vref, _) ->
vref.Attribs |> List.exists (fun a -> (IsSecurityAttribute g cenv.amap cenv.casApplied a m) || (IsSecurityCriticalAttribute g a))
| _ -> false
| _ -> false

if isSecureMethod then None else

Expand All @@ -3421,6 +3439,13 @@ and TryInlineApplication cenv env finfo (tyargs: TType list, args: Expr list, m)

if isGetHashCode then None else

let isApplicationPartialExpr =
match finfo.Info with
| ValValue (_, CurriedLambdaValue (_, _, _, expr, _) ) -> IsPartialExpr cenv env m expr
| _ -> false

if isApplicationPartialExpr then None else

// Inlining lambda
let f2R = CopyExprForInlining cenv false f2 m

Expand Down Expand Up @@ -3597,8 +3622,8 @@ and OptimizeApplication cenv env (f0, f0ty, tyargs, args, m) =
// This includes recursive calls to the function being defined (in which case we get a non-critical, closed-world tailcall).
// Note we also have to check the argument count to ensure this is a direct call (or a partial application).
let doesNotMakeCriticalTailcall =
vref.MakesNoCriticalTailcalls ||
(let valInfoForVal = GetInfoForVal cenv env m vref in valInfoForVal.ValMakesNoCriticalTailcalls) ||
vref.MakesNoCriticalTailcalls ||
(let valInfoForVal = GetInfoForValWithCheck cenv env m vref in valInfoForVal.ValMakesNoCriticalTailcalls) ||
(match env.functionVal with | None -> false | Some (v, _) -> valEq vref.Deref v)
if doesNotMakeCriticalTailcall then
let numArgs = otherArgs.Length + newArgs.Length
Expand Down
11 changes: 2 additions & 9 deletions tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,7 @@ module Baz =
Line 16, Col 20, Line 16, Col 22
Line 21, Col 20, Line 21, Col 22
]
VerifyDocuments [
Path.Combine(Environment.CurrentDirectory, "test.fs")
]
VerifyDocuments [ "test.fs" ]
]

[<Fact>]
Expand All @@ -100,9 +98,4 @@ module M =
|> withPortablePdb
|> compile
|> shouldSucceed
|> verifyPdb [
VerifyDocuments [
Path.Combine(Environment.CurrentDirectory, "test.fsi")
Path.Combine(Environment.CurrentDirectory, "test.fs")
]
]
|> verifyPdb [ VerifyDocuments [ "test.fsi"; "test.fs" ] ]
Loading
Loading