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 StackOverflow in non-recursive bindings checker #16908

Merged
merged 5 commits into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
2 changes: 1 addition & 1 deletion docs/release-notes/.FSharp.Compiler.Service/8.0.300.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
* Enforce AttributeTargets on enums ([PR #16887](https://github.com/dotnet/fsharp/pull/16887))
* Completion: fix for unfinished record field decl ([PR #16893](https://github.com/dotnet/fsharp/pull/16893))
* Enforce AttributeTargets on delegates ([PR #16891](https://github.com/dotnet/fsharp/pull/16891))

* Fix StackOverflow when checking non-recursive bindings in module or namespace in `fscAnyCpu`/`fsiAnyCpu`. ([PR #16908](https://github.com/dotnet/fsharp/pull/16908))

### Added

Expand Down
67 changes: 40 additions & 27 deletions src/Compiler/Checking/CheckDeclarations.fs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module internal FSharp.Compiler.CheckDeclarations

open System
open System.Collections.Generic
open System.Threading

open FSharp.Compiler.Diagnostics
open Internal.Utilities.Collections
Expand Down Expand Up @@ -5330,22 +5331,29 @@ let rec TcModuleOrNamespaceElementNonMutRec (cenv: cenv) parent typeNames scopem
}

/// The non-mutually recursive case for a sequence of declarations
and TcModuleOrNamespaceElementsNonMutRec cenv parent typeNames endm (defsSoFar, env, envAtEnd) (moreDefs: SynModuleDecl list) =
cancellable {
match moreDefs with
| firstDef :: otherDefs ->
// Lookahead one to find out the scope of the next declaration.
let scopem =
if isNil otherDefs then unionRanges firstDef.Range endm
else unionRanges (List.head otherDefs).Range endm
and [<TailCall>] TcModuleOrNamespaceElementsNonMutRec cenv parent typeNames endm (defsSoFar, env, envAtEnd) (moreDefs: SynModuleDecl list) (ct: CancellationToken) =

if ct.IsCancellationRequested then
ValueOrCancelled.Cancelled (OperationCanceledException())
else
match moreDefs with
| [] ->
ValueOrCancelled.Value (List.rev defsSoFar, envAtEnd)
| firstDef :: otherDefs ->
// Lookahead one to find out the scope of the next declaration.
let scopem =
if isNil otherDefs then
unionRanges firstDef.Range endm
else
unionRanges (List.head otherDefs).Range endm

let! firstDef, env, envAtEnd = TcModuleOrNamespaceElementNonMutRec cenv parent typeNames scopem env firstDef
let result = Cancellable.run ct (TcModuleOrNamespaceElementNonMutRec cenv parent typeNames scopem env firstDef)

// tail recursive
return! TcModuleOrNamespaceElementsNonMutRec cenv parent typeNames endm ( (firstDef :: defsSoFar), env, envAtEnd) otherDefs
| [] ->
return List.rev defsSoFar, envAtEnd
}
match result with
| ValueOrCancelled.Cancelled x ->
ValueOrCancelled.Cancelled x
| ValueOrCancelled.Value(firstDef, env, envAtEnd) ->
TcModuleOrNamespaceElementsNonMutRec cenv parent typeNames endm ((firstDef :: defsSoFar), env, envAtEnd) otherDefs ct

/// The mutually recursive case for a sequence of declarations (and nested modules)
and TcModuleOrNamespaceElementsMutRec (cenv: cenv) parent typeNames m envInitial mutRecNSInfo (defs: SynModuleDecl list) =
Expand Down Expand Up @@ -5470,20 +5478,25 @@ and TcModuleOrNamespaceElements cenv parent endm env xml mutRecNSInfo openDecls0
escapeCheck()
return (moduleContents, topAttrsNew, envAtEnd)

| None ->

let! compiledDefs, envAtEnd = TcModuleOrNamespaceElementsNonMutRec cenv parent typeNames endm ([], env, env) synModuleDecls

// Apply the functions for each declaration to build the overall expression-builder
let moduleDefs = List.collect p13 compiledDefs
let moduleDefs = match openDecls0 with [] -> moduleDefs | _ -> TMDefOpens openDecls0 :: moduleDefs
let moduleContents = TMDefs moduleDefs
| None ->
let! ct = Cancellable.token ()
let result = TcModuleOrNamespaceElementsNonMutRec cenv parent typeNames endm ([], env, env) synModuleDecls ct
//let! compiledDefs, envAtEnd = TcModuleOrNamespaceElementsNonMutRec cenv parent typeNames endm ([], env, env) synModuleDecls ct

match result with
| ValueOrCancelled.Value(compiledDefs, envAtEnd) ->
// Apply the functions for each declaration to build the overall expression-builder
let moduleDefs = List.collect p13 compiledDefs
let moduleDefs = match openDecls0 with [] -> moduleDefs | _ -> TMDefOpens openDecls0 :: moduleDefs
let moduleContents = TMDefs moduleDefs

// Collect up the attributes that are global to the file
let topAttrsNew = List.collect p33 compiledDefs
return (moduleContents, topAttrsNew, envAtEnd)
| ValueOrCancelled.Cancelled x ->
return! Cancellable(fun _ -> ValueOrCancelled.Cancelled x)
}

// Collect up the attributes that are global to the file
let topAttrsNew = compiledDefs |> List.collect p33
return (moduleContents, topAttrsNew, envAtEnd)
}


//--------------------------------------------------------------------------
// CheckOneImplFile - Typecheck all the namespace fragments in a file.
Expand Down
22 changes: 21 additions & 1 deletion src/Compiler/Facilities/DiagnosticsLogger.fs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ open System
open System.Diagnostics
open System.Reflection
open System.Threading
open System.Runtime.CompilerServices
open System.Runtime.InteropServices
open Internal.Utilities.Library
open Internal.Utilities.Library.Extras
open System.Collections.Concurrent
Expand Down Expand Up @@ -853,7 +855,25 @@ type StackGuard(maxDepth: int, name: string) =
let mutable depth = 1

[<DebuggerHidden; DebuggerStepThrough>]
member _.Guard(f) =
member _.Guard
(
f,
[<CallerMemberName; Optional; DefaultParameterValue("")>] memberName: string,
[<CallerFilePath; Optional; DefaultParameterValue("")>] path: string,
[<CallerLineNumber; Optional; DefaultParameterValue(0)>] line: int
) =
use _ =
Activity.start
"DiagnosticsLogger.StackGuard.Guard"
[|
Activity.Tags.stackGuardName, name
Activity.Tags.stackGuardCurrentDepth, string depth
Activity.Tags.stackGuardMaxDepth, string maxDepth
Activity.Tags.callerMemberName, memberName
Activity.Tags.callerFilePath, path
Activity.Tags.callerLineNumber, string line
|]

depth <- depth + 1

try
Expand Down
9 changes: 8 additions & 1 deletion src/Compiler/Facilities/DiagnosticsLogger.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ open System
open FSharp.Compiler.Diagnostics
open FSharp.Compiler.Features
open FSharp.Compiler.Text
open System.Runtime.CompilerServices
open System.Runtime.InteropServices

/// Represents the style being used to format errors
[<RequireQualifiedAccess>]
Expand Down Expand Up @@ -448,7 +450,12 @@ type StackGuard =
new: maxDepth: int * name: string -> StackGuard

/// Execute the new function, on a new thread if necessary
member Guard: f: (unit -> 'T) -> 'T
member Guard:
f: (unit -> 'T) *
[<CallerMemberName; Optional; DefaultParameterValue("")>] memberName: string *
[<CallerFilePath; Optional; DefaultParameterValue("")>] path: string *
[<CallerLineNumber; Optional; DefaultParameterValue(0)>] line: int ->
'T

static member GetDepthOption: string -> int

Expand Down
12 changes: 12 additions & 0 deletions src/Compiler/Utilities/Activity.fs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ module internal Activity =
let outputDllFile = "outputDllFile"
let buildPhase = "buildPhase"
let version = "version"
let stackGuardName = "stackGuardName"
let stackGuardCurrentDepth = "stackGuardCurrentDepth"
let stackGuardMaxDepth = "stackGuardMaxDepth"
let callerMemberName = "callerMemberName"
let callerFilePath = "callerFilePath"
let callerLineNumber = "callerLineNumber"

let AllKnownTags =
[|
Expand All @@ -50,6 +56,12 @@ module internal Activity =
gc2
outputDllFile
buildPhase
stackGuardName
stackGuardCurrentDepth
stackGuardMaxDepth
callerMemberName
callerFilePath
callerLineNumber
|]

module Events =
Expand Down
6 changes: 6 additions & 0 deletions src/Compiler/Utilities/Activity.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ module internal Activity =
val cache: string
val buildPhase: string
val version: string
val stackGuardName: string
val stackGuardCurrentDepth: string
val stackGuardMaxDepth: string
val callerMemberName: string
val callerFilePath: string
val callerLineNumber: string

module Events =
val cacheHit: string
Expand Down