From d5d8f8ec326787ad9ea0a70008e19eeaf2a0e29a Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Wed, 9 Mar 2022 15:33:15 +0100 Subject: [PATCH 1/8] Emit (+flatten) import scope tables --- src/fsharp/IlxGen.fs | 30 ++++++++++++++++++------------ src/fsharp/absil/ilwrite.fs | 18 +++++++++++++----- src/fsharp/absil/ilwritepdb.fs | 31 +++++++++++++++++-------------- 3 files changed, 48 insertions(+), 31 deletions(-) diff --git a/src/fsharp/IlxGen.fs b/src/fsharp/IlxGen.fs index 0e2cf020cfe..5fb00943a58 100644 --- a/src/fsharp/IlxGen.fs +++ b/src/fsharp/IlxGen.fs @@ -1413,28 +1413,34 @@ and AddDebugImportsToEnv _cenv eenv (openDecls: OpenDeclaration list) = for modul in openDecl.Modules do if modul.IsNamespace then ILDebugImport.ImportNamespace (fullDisplayTextOfModRef modul) - // Emit of 'open type' and 'open ' is causing problems - // See https://github.com/dotnet/fsharp/pull/12010#issuecomment-903339109 - // - // It may be nested types/modules in particular - //else - // ILDebugImport.ImportType (mkILNonGenericBoxedTy modul.CompiledRepresentationForNamedType) + else + ILDebugImport.ImportType (mkILNonGenericBoxedTy modul.CompiledRepresentationForNamedType) //for t in openDecl.Types do // let m = defaultArg openDecl.Range Range.range0 // ILDebugImport.ImportType (GenType cenv.amap m TypeReprEnv.Empty t) |] - |> Array.distinctBy (function - | ILDebugImport.ImportNamespace nsp -> nsp - | ILDebugImport.ImportType t -> t.QualifiedName) if ilImports.Length = 0 then eenv else + // We flatten _all_ the import scopes, creating repetition, because C# debug engine doesn't seem to handle + // nesting of import scopes at all. This means every new "open" in, say, a nested module in F# causes + // duplication of all the implicit/enclosing "open" in within the debug information. + // However overall there are not very many "open" declarations and debug information can be large + // so this is not considered a problem. let imports = - { Parent = eenv.imports - Imports = ilImports } + [| match eenv.imports with + | None -> () + | Some parent -> yield! parent.Imports + yield! ilImports |] + |> Array.filter (function + | ILDebugImport.ImportNamespace _ -> true + | ILDebugImport.ImportType t -> t.IsNominal) + |> Array.distinctBy (function + | ILDebugImport.ImportNamespace nsp -> nsp + | ILDebugImport.ImportType t -> t.QualifiedName) - { eenv with imports = Some imports } + { eenv with imports = Some { Parent = None; Imports = imports } } and AddBindingsForModuleDef allocVal cloc eenv x = match x with diff --git a/src/fsharp/absil/ilwrite.fs b/src/fsharp/absil/ilwrite.fs index a5c58fd7233..c0ed0e7926d 100644 --- a/src/fsharp/absil/ilwrite.fs +++ b/src/fsharp/absil/ilwrite.fs @@ -770,13 +770,17 @@ and GetResolutionScopeAsElem cenv (scoref, enc) = (rs_TypeRef, GetTypeDescAsTypeRefIdx cenv (scoref, enc2, n2)) -let emitTypeInfoAsTypeDefOrRefEncoded cenv (bb: ByteBuffer) (scoref, enc, nm) = +let getTypeInfoAsTypeDefOrRefEncoded cenv (scoref, enc, nm) = if isScopeRefLocal scoref then let idx = GetIdxForTypeDef cenv (TdKey(enc, nm)) - bb.EmitZ32 (idx <<< 2) // ECMA 22.2.8 TypeDefOrRefEncoded - ILTypeDef + idx <<< 2 // ECMA 22.2.8 TypeDefOrRefEncoded - ILTypeDef else let idx = GetTypeDescAsTypeRefIdx cenv (scoref, enc, nm) - bb.EmitZ32 ((idx <<< 2) ||| 0x01) // ECMA 22.2.8 TypeDefOrRefEncoded - ILTypeRef + ((idx <<< 2) ||| 0x01) // ECMA 22.2.8 TypeDefOrRefEncoded - ILTypeRef + +let emitTypeInfoAsTypeDefOrRefEncoded cenv (bb: ByteBuffer) (scoref, enc, nm) = + let tok = getTypeInfoAsTypeDefOrRefEncoded cenv (scoref, enc, nm) + bb.EmitZ32 tok let getTypeDefOrRefAsUncodedToken (tag, idx) = let tab = @@ -2219,9 +2223,13 @@ let GetFieldDefTypeAsBlobIdx cenv env ty = EmitType cenv env bb ty) GetBytesAsBlobIdx cenv bytes -let GenPdbImport (cenv: cenv) env (input: ILDebugImport) = +let GenPdbImport (cenv: cenv) _env (input: ILDebugImport) = match input with - | ILDebugImport.ImportType ty -> PdbImport.ImportType (getTypeDefOrRefAsUncodedToken (GetTypeAsTypeDefOrRef cenv env ty)) + | ILDebugImport.ImportType ty -> + let tspec = ty.TypeSpec + let tok = getTypeInfoAsTypeDefOrRefEncoded cenv (tspec.Scope, tspec.Enclosing, tspec.Name) + PdbImport.ImportType tok + | ILDebugImport.ImportNamespace nsp -> PdbImport.ImportNamespace nsp let rec GenPdbImports (cenv: cenv) env (input: ILDebugImports option) = diff --git a/src/fsharp/absil/ilwritepdb.fs b/src/fsharp/absil/ilwritepdb.fs index 3acc2ada217..c16ad3616a9 100644 --- a/src/fsharp/absil/ilwritepdb.fs +++ b/src/fsharp/absil/ilwritepdb.fs @@ -490,10 +490,10 @@ type PortablePdbGenerator (embedAllSource: bool, embedSourceList: string list, s // writer.WriteByte((byte)ImportDefinitionKind.ImportAssemblyReferenceAlias); // writer.WriteCompressedInteger(MetadataTokens.GetHeapOffset(_debugMetadataOpt.GetOrAddBlobUTF8(import.AliasOpt))); - let serializeImportsBlob (scope: PdbImports) = + let serializeImportsBlob (imports: PdbImport[]) = let writer = new BlobBuilder() - for import in scope.Imports do + for import in imports do serializeImport writer import metadata.GetOrAddBlob(writer) @@ -515,7 +515,7 @@ type PortablePdbGenerator (embedAllSource: bool, embedSourceList: string list, s | None -> moduleImportScopeHandle | Some parent -> getImportScopeIndex parent - let blob = serializeImportsBlob imports + let blob = serializeImportsBlob imports.Imports let result = metadata.AddImportScope(parentScopeHandle, blob) importScopesTable.Add(imports, result) @@ -542,19 +542,19 @@ type PortablePdbGenerator (embedAllSource: bool, embedSourceList: string list, s let writeMethodScopes methToken rootScope = let flattenedScopes = flattenScopes rootScope - - // Get or create the import scope for this method - let importScopeHandle = -#if EMIT_IMPORT_SCOPES - match s.Imports with - | None -> Unchecked.defaultof<_> - | Some imports -> getImportScopeIndex imports + + for scope in flattenedScopes do + // Get or create the import scope for this method + let importScopeHandle = +#if !NO_EMIT_IMPORT_SCOPES + match scope.Imports with + | None -> Unchecked.defaultof<_> + | Some imports -> getImportScopeIndex imports #else - getImportScopeIndex |> ignore // make sure this code counts as used - Unchecked.defaultof<_> + getImportScopeIndex |> ignore // make sure this code counts as used + Unchecked.defaultof<_> #endif - for scope in flattenedScopes do let lastRowNumber = MetadataTokens.GetRowNumber(LocalVariableHandle.op_Implicit lastLocalVariableHandle) let nextHandle = MetadataTokens.LocalVariableHandle(lastRowNumber + 1) @@ -685,7 +685,7 @@ type PortablePdbGenerator (embedAllSource: bool, embedSourceList: string list, s metadata.SetCapacity(TableIndex.MethodDebugInformation, info.Methods.Length) // Currently disabled, see -#if EMIT_IMPORT_SCOPES +#if !NO_EMIT_IMPORT_SCOPES defineModuleImportScope() #else defineModuleImportScope |> ignore // make sure this function counts as used @@ -994,6 +994,9 @@ let logDebugInfo (outfile: string) (info: PdbData) = fprintfn sw " %s- [%d-%d]" offs scope.StartOffset scope.EndOffset if scope.Locals.Length > 0 then fprintfn sw " %s Locals: %A" offs [ for p in scope.Locals -> sprintf "%d: %s" p.Index p.Name ] + + // TODO: Print scope.Imports + for child in scope.Children do writeScope (offs + " ") child match meth.RootScope with From 9d743075c64df81726d31b0de80a35e830225d2d Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Fri, 11 Mar 2022 16:54:53 +0100 Subject: [PATCH 2/8] Emit open types --- src/fsharp/IlxGen.fs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/fsharp/IlxGen.fs b/src/fsharp/IlxGen.fs index 5fb00943a58..b1fc275b1f4 100644 --- a/src/fsharp/IlxGen.fs +++ b/src/fsharp/IlxGen.fs @@ -1406,7 +1406,7 @@ let AddBindingsForTycon allocVal (cloc: CompileLocation) (tycon: Tycon) eenv = let rec AddBindingsForModuleDefs allocVal (cloc: CompileLocation) eenv mdefs = List.fold (AddBindingsForModuleDef allocVal cloc) eenv mdefs -and AddDebugImportsToEnv _cenv eenv (openDecls: OpenDeclaration list) = +and AddDebugImportsToEnv (cenv: cenv) eenv (openDecls: OpenDeclaration list) = let ilImports = [| for openDecl in openDecls do @@ -1415,9 +1415,9 @@ and AddDebugImportsToEnv _cenv eenv (openDecls: OpenDeclaration list) = ILDebugImport.ImportNamespace (fullDisplayTextOfModRef modul) else ILDebugImport.ImportType (mkILNonGenericBoxedTy modul.CompiledRepresentationForNamedType) - //for t in openDecl.Types do - // let m = defaultArg openDecl.Range Range.range0 - // ILDebugImport.ImportType (GenType cenv.amap m TypeReprEnv.Empty t) + for t in openDecl.Types do + let m = defaultArg openDecl.Range Range.range0 + ILDebugImport.ImportType (GenType cenv.amap m TypeReprEnv.Empty t) |] if ilImports.Length = 0 then From d0630a667c98ecf107988e128b3cfdad906ccc10 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Wed, 16 Mar 2022 14:14:09 +0100 Subject: [PATCH 3/8] Don't emit FSI_NNNN dynamic modules to import tables --- src/fsharp/IlxGen.fs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/fsharp/IlxGen.fs b/src/fsharp/IlxGen.fs index 90a909b90b8..bc397b4589e 100644 --- a/src/fsharp/IlxGen.fs +++ b/src/fsharp/IlxGen.fs @@ -1447,7 +1447,11 @@ and AddDebugImportsToEnv (cenv: cenv) eenv (openDecls: OpenDeclaration list) = yield! ilImports |] |> Array.filter (function | ILDebugImport.ImportNamespace _ -> true - | ILDebugImport.ImportType t -> t.IsNominal) + | ILDebugImport.ImportType t -> + t.IsNominal && + // We filter out FSI_NNNN types (dynamic modules), since we don't really need them in the import tables. + not (t.QualifiedName.StartsWithOrdinal FsiDynamicModulePrefix + && t.TypeRef.Scope = ILScopeRef.Local )) |> Array.distinctBy (function | ILDebugImport.ImportNamespace nsp -> nsp | ILDebugImport.ImportType t -> t.QualifiedName) From ea8a8eb420489ec3e4eae9d03e6b61aba1171025 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Thu, 17 Mar 2022 18:44:43 +0100 Subject: [PATCH 4/8] Added a simple portable PDB verifier for testing framework. More to come --- eng/Versions.props | 1 + .../Debugger/PortablePdbs.fs | 29 ++++++++++++++ .../FSharp.Compiler.ComponentTests.fsproj | 1 + tests/FSharp.Test.Utilities/Compiler.fs | 40 +++++++++++++++++++ .../FSharp.Test.Utilities.fsproj | 4 ++ 5 files changed, 75 insertions(+) create mode 100644 tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs diff --git a/eng/Versions.props b/eng/Versions.props index b6fdb3e2883..5c461218963 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -89,6 +89,7 @@ 4.0.0 4.3.0 4.3.0 + 1.6.0 4.3.0 4.3.0 4.3.0 diff --git a/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs b/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs new file mode 100644 index 00000000000..60931ee18a7 --- /dev/null +++ b/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs @@ -0,0 +1,29 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace FSharp.Compiler.ComponentTests.Debugger + +open Xunit +open FSharp.Test.Compiler + +module PortablePdbs = + + [] + let ``Valid Portable PDBs are produced by compiler`` () = + FSharp """ +open System + +module Foo = + let getcwd () = Environment.CurrentDirectory + +[] +let main _ = + printfn "%s" Environment.CurrentDirectory + 0 + """ + |> asExe + |> withPortablePdb + |> compile + |> shouldSucceed + |> verifyPdb + + diff --git a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj index 461cdbb4924..69206d64fa6 100644 --- a/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj +++ b/tests/FSharp.Compiler.ComponentTests/FSharp.Compiler.ComponentTests.fsproj @@ -100,6 +100,7 @@ + diff --git a/tests/FSharp.Test.Utilities/Compiler.fs b/tests/FSharp.Test.Utilities/Compiler.fs index 71c552cdd7d..e3d3a38456b 100644 --- a/tests/FSharp.Test.Utilities/Compiler.fs +++ b/tests/FSharp.Test.Utilities/Compiler.fs @@ -19,6 +19,7 @@ open System.Text open System.Text.RegularExpressions open FSharp.Test.CompilerAssertHelpers open TestFramework +open System.Reflection.Metadata module rec Compiler = type BaselineFile = { FilePath: string; Content: string option } @@ -279,6 +280,9 @@ module rec Compiler = let withXmlCommentStrictParamChecking (cUnit: CompilationUnit) : CompilationUnit = withOptionsHelper [ "--warnon:3391" ] "withXmlCommentChecking is only supported for F#" cUnit + let withPortablePdb (cUnit: CompilationUnit) : CompilationUnit = + withOptionsHelper ["--debug:portable"] "withPortablePdb is only supported for F#" cUnit + let asLibrary (cUnit: CompilationUnit) : CompilationUnit = match cUnit with | FS fs -> FS { fs with OutputType = CompileOutput.Library } @@ -687,6 +691,42 @@ module rec Compiler = let verifyBaselines = verifyBaseline >> verifyILBaseline + + let private verifyPortablePdb (result: Output) : unit = + match result.OutputPath with + | Some assemblyPath -> + let pdbPath = Path.ChangeExtension(assemblyPath, ".pdb") + if not (FileSystem.FileExistsShim pdbPath) then + failwith $"PDB file does not exists: {pdbPath}" + + use fileStream = File.OpenRead pdbPath; + use provider = MetadataReaderProvider.FromPortablePdbStream fileStream + let reader = provider.GetMetadataReader() + + if reader.MetadataVersion <> "PDB v1.0" then + failwith $"Invalid PDB file version. Expected: \"PDB v1.0\"; Got {reader.MetadataVersion}" + + if reader.MetadataKind <> MetadataKind.Ecma335 then + failwith $"Invalid metadata kind detected. Expected {MetadataKind.Ecma335}; Got {reader.MetadataKind}" + + // This should not happen, just a sanity check: + if reader.IsAssembly then + failwith $"Unexpected PDB type, `IsAssembly` should be `false`." + + // TODO (sanity check): + // assert (reader.DebugMetadataHeader.EntryPoint.IsNil = false) // Pass to this function, whether we are building Exe or Library, and check that there's no EntryPoint for Library + + | _ -> failwith "Output path is not set, please make sure compilation was successfull." + + () + + let verifyPdb (result: TestResult) : TestResult = + match result with + | Success r -> verifyPortablePdb r + | _ -> failwith "Result should be \"Success\" in order to verify PDB." + + result + [] module Assertions = let private getErrorNumber (error: ErrorType) : int = diff --git a/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj b/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj index 00cd1995e72..3ca59109937 100644 --- a/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj +++ b/tests/FSharp.Test.Utilities/FSharp.Test.Utilities.fsproj @@ -50,6 +50,10 @@ + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + From 0099df3d6b3e435d78f22a56fb2e3345019bc444 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Mon, 21 Mar 2022 18:31:30 +0100 Subject: [PATCH 5/8] Added imports verification tests --- src/fsharp/absil/ilwritepdb.fs | 2 - .../Debugger/PortablePdbs.fs | 12 +- tests/FSharp.Test.Utilities/Compiler.fs | 121 +++++++++++++----- 3 files changed, 100 insertions(+), 35 deletions(-) diff --git a/src/fsharp/absil/ilwritepdb.fs b/src/fsharp/absil/ilwritepdb.fs index 91085296867..130a95f0e49 100644 --- a/src/fsharp/absil/ilwritepdb.fs +++ b/src/fsharp/absil/ilwritepdb.fs @@ -995,8 +995,6 @@ let logDebugInfo (outfile: string) (info: PdbData) = if scope.Locals.Length > 0 then fprintfn sw " %s Locals: %A" offs [ for p in scope.Locals -> sprintf "%d: %s" p.Index p.Name ] - // TODO: Print scope.Imports - for child in scope.Children do writeScope (offs + " ") child match meth.RootScope with diff --git a/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs b/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs index 60931ee18a7..ed48cec9dd2 100644 --- a/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs +++ b/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs @@ -4,6 +4,7 @@ namespace FSharp.Compiler.ComponentTests.Debugger open Xunit open FSharp.Test.Compiler +open System.Reflection.Metadata module PortablePdbs = @@ -24,6 +25,15 @@ let main _ = |> withPortablePdb |> compile |> shouldSucceed - |> verifyPdb + |> verifyPdb [ + VerifyImportScopes [ + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Core" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Collections" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Control" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "System" } + ] + ] diff --git a/tests/FSharp.Test.Utilities/Compiler.fs b/tests/FSharp.Test.Utilities/Compiler.fs index e3d3a38456b..f9e10295935 100644 --- a/tests/FSharp.Test.Utilities/Compiler.fs +++ b/tests/FSharp.Test.Utilities/Compiler.fs @@ -76,7 +76,7 @@ module rec Compiler = | UnionCase of string | Field of string - member this.FullName () = + member this.ImportScope () = match this with | MemberOrFunctionOrValue fullname | Entity fullname @@ -129,7 +129,8 @@ module rec Compiler = Dependencies: string list Adjust: int Diagnostics: ErrorInfo list - Output: RunOutput option } + Output: RunOutput option + Compilation: CompilationUnit } type TestResult = | Success of Output @@ -298,10 +299,10 @@ module rec Compiler = | FS fs -> FS { fs with IgnoreWarnings = true } | _ -> failwith "TODO: Implement ignorewarnings for the rest." - let rec private asMetadataReference reference = + let rec private asMetadataReference (cUnit: CompilationUnit) reference = match reference with | CompilationReference (cmpl, _) -> - let result = compileFSharpCompilation cmpl false + let result = compileFSharpCompilation cmpl false cUnit match result with | Failure f -> let message = sprintf "Operation failed (expected to succeed).\n All errors:\n%A" (f.Diagnostics) @@ -334,7 +335,7 @@ module rec Compiler = let refs = loop [] cs.References let source = getSource cs.Source let name = defaultArg cs.Name null - let metadataReferences = List.map asMetadataReference refs + let metadataReferences = List.map (asMetadataReference x) refs let cmpl = CompilationUtil.CreateCSharpCompilation(source, cs.LangVersion, cs.TargetFramework, additionalReferences = metadataReferences.ToImmutableArray().As(), name = name) |> CompilationReference.Create @@ -343,7 +344,7 @@ module rec Compiler = | IL _ -> failwith "TODO: Process references for IL" loop [] references - let private compileFSharpCompilation compilation ignoreWarnings : TestResult = + let private compileFSharpCompilation compilation ignoreWarnings (cUnit: CompilationUnit) : TestResult = let ((err: FSharpDiagnostic[], outputFilePath: string), deps) = CompilerAssert.CompileRaw(compilation, ignoreWarnings) @@ -354,7 +355,8 @@ module rec Compiler = Dependencies = deps Adjust = 0 Diagnostics = diagnostics - Output = None } + Output = None + Compilation = cUnit } let (errors, warnings) = partitionErrors diagnostics @@ -379,9 +381,9 @@ module rec Compiler = let references = processReferences fs.References outputDirectory let compilation = Compilation.Create(source, sourceKind, output, options, references, name, outputDirectory) - compileFSharpCompilation compilation fs.IgnoreWarnings + compileFSharpCompilation compilation fs.IgnoreWarnings (FS fs) - let private compileCSharpCompilation (compilation: CSharpCompilation) : TestResult = + let private compileCSharpCompilation (compilation: CSharpCompilation) csSource : TestResult = let outputPath = tryCreateTemporaryDirectory() @@ -398,7 +400,8 @@ module rec Compiler = Dependencies = [] Adjust = 0 Diagnostics = [] - Output = None } + Output = None + Compilation = CS csSource } if cmplResult.Success then Success { result with OutputPath = Some output } @@ -418,7 +421,7 @@ module rec Compiler = let additionalReferences = match processReferences csSource.References outputDirectory with | [] -> ImmutableArray.Empty - | r -> (List.map asMetadataReference r).ToImmutableArray().As() + | r -> (List.map (asMetadataReference (CS csSource)) r).ToImmutableArray().As() let references = TargetFrameworkUtil.getReferences csSource.TargetFramework @@ -434,7 +437,7 @@ module rec Compiler = references.As().AddRange additionalReferences, CSharpCompilationOptions (OutputKind.DynamicallyLinkedLibrary)) - cmpl |> compileCSharpCompilation + compileCSharpCompilation cmpl csSource let compile (cUnit: CompilationUnit) : TestResult = match cUnit with @@ -455,7 +458,8 @@ module rec Compiler = Dependencies = [] Adjust = 0 Diagnostics = diagnostics - Output = None } + Output = None + Compilation = FS fsSource } if failed then Failure result @@ -488,7 +492,8 @@ module rec Compiler = Dependencies = [] Adjust = 0 Diagnostics = diagnostics - Output = None } + Output = None + Compilation = FS fsSource } let (errors, warnings) = partitionErrors diagnostics @@ -553,7 +558,8 @@ module rec Compiler = Dependencies = [] Adjust = 0 Diagnostics = diagnostics - Output = Some(EvalOutput evalresult) } + Output = Some(EvalOutput evalresult) + Compilation = FS fs } let (errors, warnings) = partitionErrors diagnostics @@ -607,7 +613,8 @@ module rec Compiler = Dependencies = [] Adjust = 0 Diagnostics = [] - Output = None } + Output = None + Compilation = cUnit } if errors.Count > 0 then let output = ExecutionOutput { @@ -691,8 +698,65 @@ module rec Compiler = let verifyBaselines = verifyBaseline >> verifyILBaseline + type ImportScope = { Kind: ImportDefinitionKind; Name: string } - let private verifyPortablePdb (result: Output) : unit = + type PdbVerificationOption = + | VerifyImportScopes of ImportScope list + | Dummy of unit + + let private verifyPdbFormat (reader: MetadataReader) compilationType = + if reader.MetadataVersion <> "PDB v1.0" then + failwith $"Invalid PDB file version. Expected: \"PDB v1.0\"; Got {reader.MetadataVersion}" + + if reader.MetadataKind <> MetadataKind.Ecma335 then + failwith $"Invalid metadata kind detected. Expected {MetadataKind.Ecma335}; Got {reader.MetadataKind}" + + // This should not happen, just a sanity check: + if reader.IsAssembly then + failwith $"Unexpected PDB type, expected `IsAssembly` to be `false`." + + let shouldHaveEntryPoint = (compilationType = CompileOutput.Exe) + + // Sanity check, we want to verify, that Entrypoint is non-nil, if we are building "Exe" target. + if reader.DebugMetadataHeader.EntryPoint.IsNil && shouldHaveEntryPoint then + failwith $"EntryPoint expected to be {shouldHaveEntryPoint}, but was {reader.DebugMetadataHeader.EntryPoint.IsNil}" + + let private verifyPdbImportTables (reader: MetadataReader) (scopes: ImportScope list) = + // There always should be 2 import scopes - 1 empty "root" one, and one flattened table of imports for current scope. + if reader.ImportScopes.Count <> 2 then + failwith $"Expected to have 2 import scopes, but found {reader.ImportScopes.Count}." + + // Sanity check: explicitly test that first import scope is indeed an apty one (i.e. there are no imports). + let rootScope = reader.ImportScopes.ToImmutableArray().Item(0) |> reader.GetImportScope + let rootScopeImportsLength = rootScope.GetImports().ToImmutableArray().Length + if rootScopeImportsLength <> 0 then + failwith $"Expected root scope to have 0 imports, but got {rootScopeImportsLength}." + + let mainScope = reader.ImportScopes.ToImmutableArray().Item(1) |> reader.GetImportScope + + let imports = [ for import in mainScope.GetImports() -> + match import.Kind with + | ImportDefinitionKind.ImportNamespace -> + let targetNamespaceBlob = import.TargetNamespace + let targetNamespaceBytes = reader.GetBlobBytes(targetNamespaceBlob) + let name = Encoding.UTF8.GetString(targetNamespaceBytes, 0, targetNamespaceBytes.Length) + Some { Kind = import.Kind; Name = name } + | _ -> None ] |> List.filter Option.isSome |> List.map Option.get + + if scopes.Length <> imports.Length then + failwith $"Expected import scopes amount is {scopes.Length}, but got {imports.Length}\nExpected:\n%A{scopes}\nActual:%A{imports}" + + if scopes <> imports then + failwith $"Expected import scopes are different from PDB.\nExpected:\n%A{scopes}\nActual:%A{imports}" + + + let private verifyPdbOptions reader options = + for option in options do + match option with + | VerifyImportScopes scopes -> verifyPdbImportTables reader scopes + | _ -> failwith $"Unknown verification option: {option.ToString()}" + + let private verifyPortablePdb (result: Output) options : unit = match result.OutputPath with | Some assemblyPath -> let pdbPath = Path.ChangeExtension(assemblyPath, ".pdb") @@ -702,27 +766,20 @@ module rec Compiler = use fileStream = File.OpenRead pdbPath; use provider = MetadataReaderProvider.FromPortablePdbStream fileStream let reader = provider.GetMetadataReader() + let compilationType = + match result.Compilation with + | FS r -> r.OutputType + | _ -> failwith "Only F# compilations are supported when verifying PDBs." - if reader.MetadataVersion <> "PDB v1.0" then - failwith $"Invalid PDB file version. Expected: \"PDB v1.0\"; Got {reader.MetadataVersion}" - - if reader.MetadataKind <> MetadataKind.Ecma335 then - failwith $"Invalid metadata kind detected. Expected {MetadataKind.Ecma335}; Got {reader.MetadataKind}" - - // This should not happen, just a sanity check: - if reader.IsAssembly then - failwith $"Unexpected PDB type, `IsAssembly` should be `false`." - - // TODO (sanity check): - // assert (reader.DebugMetadataHeader.EntryPoint.IsNil = false) // Pass to this function, whether we are building Exe or Library, and check that there's no EntryPoint for Library - + verifyPdbFormat reader compilationType + verifyPdbOptions reader options | _ -> failwith "Output path is not set, please make sure compilation was successfull." () - let verifyPdb (result: TestResult) : TestResult = + let verifyPdb (options: PdbVerificationOption list) (result: TestResult) : TestResult = match result with - | Success r -> verifyPortablePdb r + | Success r -> verifyPortablePdb r options | _ -> failwith "Result should be \"Success\" in order to verify PDB." result From 83251957d0fab7e4007c2671c2102d52171fa93b Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Tue, 22 Mar 2022 19:42:06 +0100 Subject: [PATCH 6/8] Removed conditional compilation, added tests for multiple scopes/namespaces --- src/fsharp/absil/ilwritepdb.fs | 15 ++----- .../Debugger/PortablePdbs.fs | 39 +++++++++++------ tests/FSharp.Test.Utilities/Compiler.fs | 42 +++++++++++-------- 3 files changed, 53 insertions(+), 43 deletions(-) diff --git a/src/fsharp/absil/ilwritepdb.fs b/src/fsharp/absil/ilwritepdb.fs index 130a95f0e49..2001ab128d9 100644 --- a/src/fsharp/absil/ilwritepdb.fs +++ b/src/fsharp/absil/ilwritepdb.fs @@ -542,18 +542,14 @@ type PortablePdbGenerator (embedAllSource: bool, embedSourceList: string list, s let writeMethodScopes methToken rootScope = let flattenedScopes = flattenScopes rootScope - + for scope in flattenedScopes do // Get or create the import scope for this method let importScopeHandle = -#if !NO_EMIT_IMPORT_SCOPES + match scope.Imports with | None -> Unchecked.defaultof<_> | Some imports -> getImportScopeIndex imports -#else - getImportScopeIndex |> ignore // make sure this code counts as used - Unchecked.defaultof<_> -#endif let lastRowNumber = MetadataTokens.GetRowNumber(LocalVariableHandle.op_Implicit lastLocalVariableHandle) let nextHandle = MetadataTokens.LocalVariableHandle(lastRowNumber + 1) @@ -684,14 +680,9 @@ type PortablePdbGenerator (embedAllSource: bool, embedSourceList: string list, s sortMethods showTimes info metadata.SetCapacity(TableIndex.MethodDebugInformation, info.Methods.Length) -// Currently disabled, see -#if !NO_EMIT_IMPORT_SCOPES defineModuleImportScope() -#else - defineModuleImportScope |> ignore // make sure this function counts as used -#endif - for minfo in info.Methods do + for minfo in info.Methods do emitMethod minfo let entryPoint = diff --git a/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs b/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs index ed48cec9dd2..67d1aca99cd 100644 --- a/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs +++ b/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs @@ -11,29 +11,42 @@ module PortablePdbs = [] let ``Valid Portable PDBs are produced by compiler`` () = FSharp """ +namespace UserNamespace + open System module Foo = let getcwd () = Environment.CurrentDirectory -[] -let main _ = - printfn "%s" Environment.CurrentDirectory - 0 + +namespace UserNamespace2 + +open System.IO + +module Bar = + let baz _ = () """ - |> asExe + |> asLibrary |> withPortablePdb |> compile |> shouldSucceed |> verifyPdb [ VerifyImportScopes [ - { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft" } - { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp" } - { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Core" } - { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Collections" } - { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Control" } - { Kind = ImportDefinitionKind.ImportNamespace; Name = "System" } + [ + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Core" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Collections" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Control" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "System" } + ] + [ + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Core" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Collections" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Control" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "System.IO" } + ] ] ] - - diff --git a/tests/FSharp.Test.Utilities/Compiler.fs b/tests/FSharp.Test.Utilities/Compiler.fs index f9e10295935..1826289ae78 100644 --- a/tests/FSharp.Test.Utilities/Compiler.fs +++ b/tests/FSharp.Test.Utilities/Compiler.fs @@ -76,7 +76,7 @@ module rec Compiler = | UnionCase of string | Field of string - member this.ImportScope () = + member this.FullName () = match this with | MemberOrFunctionOrValue fullname | Entity fullname @@ -701,7 +701,7 @@ module rec Compiler = type ImportScope = { Kind: ImportDefinitionKind; Name: string } type PdbVerificationOption = - | VerifyImportScopes of ImportScope list + | VerifyImportScopes of ImportScope list list | Dummy of unit let private verifyPdbFormat (reader: MetadataReader) compilationType = @@ -721,33 +721,39 @@ module rec Compiler = if reader.DebugMetadataHeader.EntryPoint.IsNil && shouldHaveEntryPoint then failwith $"EntryPoint expected to be {shouldHaveEntryPoint}, but was {reader.DebugMetadataHeader.EntryPoint.IsNil}" - let private verifyPdbImportTables (reader: MetadataReader) (scopes: ImportScope list) = + let private verifyPdbImportTables (reader: MetadataReader) (scopes: ImportScope list list) = // There always should be 2 import scopes - 1 empty "root" one, and one flattened table of imports for current scope. - if reader.ImportScopes.Count <> 2 then - failwith $"Expected to have 2 import scopes, but found {reader.ImportScopes.Count}." + if reader.ImportScopes.Count < 2 then + failwith $"Expected to have at least 2 import scopes, but found {reader.ImportScopes.Count}." // Sanity check: explicitly test that first import scope is indeed an apty one (i.e. there are no imports). let rootScope = reader.ImportScopes.ToImmutableArray().Item(0) |> reader.GetImportScope + let rootScopeImportsLength = rootScope.GetImports().ToImmutableArray().Length + if rootScopeImportsLength <> 0 then failwith $"Expected root scope to have 0 imports, but got {rootScopeImportsLength}." - let mainScope = reader.ImportScopes.ToImmutableArray().Item(1) |> reader.GetImportScope + let pdbScopes = [ for import in reader.ImportScopes -> reader.GetImportScope import ] |> List.skip 1 |> List.rev + + if pdbScopes.Length <> scopes.Length then + failwith $"Expected import scopes amount is {scopes.Length}, but got {pdbScopes.Length}." - let imports = [ for import in mainScope.GetImports() -> - match import.Kind with - | ImportDefinitionKind.ImportNamespace -> - let targetNamespaceBlob = import.TargetNamespace - let targetNamespaceBytes = reader.GetBlobBytes(targetNamespaceBlob) - let name = Encoding.UTF8.GetString(targetNamespaceBytes, 0, targetNamespaceBytes.Length) - Some { Kind = import.Kind; Name = name } - | _ -> None ] |> List.filter Option.isSome |> List.map Option.get + for (pdbScope, expectedScope) in List.zip pdbScopes scopes do + let imports = [ for import in pdbScope.GetImports() -> + match import.Kind with + | ImportDefinitionKind.ImportNamespace -> + let targetNamespaceBlob = import.TargetNamespace + let targetNamespaceBytes = reader.GetBlobBytes(targetNamespaceBlob) + let name = Encoding.UTF8.GetString(targetNamespaceBytes, 0, targetNamespaceBytes.Length) + Some { Kind = import.Kind; Name = name } + | _ -> None ] |> List.filter Option.isSome |> List.map Option.get - if scopes.Length <> imports.Length then - failwith $"Expected import scopes amount is {scopes.Length}, but got {imports.Length}\nExpected:\n%A{scopes}\nActual:%A{imports}" + if expectedScope.Length <> imports.Length then + failwith $"Expected imports amount is {expectedScope.Length}, but got {imports.Length}\nExpected:\n%A{expectedScope}\nActual:%A{imports}" - if scopes <> imports then - failwith $"Expected import scopes are different from PDB.\nExpected:\n%A{scopes}\nActual:%A{imports}" + if expectedScope <> imports then + failwith $"Expected imports are different from PDB.\nExpected:\n%A{expectedScope}\nActual:%A{imports}" let private verifyPdbOptions reader options = From dc5e88b959f7d4c1ea5d55fa4f831cf51be5fca1 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Tue, 22 Mar 2022 19:45:44 +0100 Subject: [PATCH 7/8] Removed conditional compilation, added tests for multiple scopes/namespaces --- .../Debugger/PortablePdbs.fs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs b/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs index 67d1aca99cd..2dc61c8ff95 100644 --- a/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs +++ b/tests/FSharp.Compiler.ComponentTests/Debugger/PortablePdbs.fs @@ -25,6 +25,11 @@ open System.IO module Bar = let baz _ = () + +open System.Collections.Generic + +module Baz = + let aiou _ = () """ |> asLibrary |> withPortablePdb @@ -48,5 +53,14 @@ module Bar = { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Control" } { Kind = ImportDefinitionKind.ImportNamespace; Name = "System.IO" } ] + [ + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Core" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Collections" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "Microsoft.FSharp.Control" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "System.IO" } + { Kind = ImportDefinitionKind.ImportNamespace; Name = "System.Collections.Generic" } + ] ] ] From 5bb431474c6545ec8f73502e193c9ea260e3ae76 Mon Sep 17 00:00:00 2001 From: Vlad Zarytovskii Date: Wed, 23 Mar 2022 12:36:09 +0100 Subject: [PATCH 8/8] Cleanup unused GetPdbImport and GetPdbImports parametes --- src/fsharp/absil/ilwrite.fs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/fsharp/absil/ilwrite.fs b/src/fsharp/absil/ilwrite.fs index c0ed0e7926d..59680d42021 100644 --- a/src/fsharp/absil/ilwrite.fs +++ b/src/fsharp/absil/ilwrite.fs @@ -2223,7 +2223,7 @@ let GetFieldDefTypeAsBlobIdx cenv env ty = EmitType cenv env bb ty) GetBytesAsBlobIdx cenv bytes -let GenPdbImport (cenv: cenv) _env (input: ILDebugImport) = +let GenPdbImport (cenv: cenv) (input: ILDebugImport) = match input with | ILDebugImport.ImportType ty -> let tspec = ty.TypeSpec @@ -2232,7 +2232,7 @@ let GenPdbImport (cenv: cenv) _env (input: ILDebugImport) = | ILDebugImport.ImportNamespace nsp -> PdbImport.ImportNamespace nsp -let rec GenPdbImports (cenv: cenv) env (input: ILDebugImports option) = +let rec GenPdbImports (cenv: cenv) (input: ILDebugImports option) = match input with | None -> None | Some ilImports -> @@ -2240,8 +2240,8 @@ let rec GenPdbImports (cenv: cenv) env (input: ILDebugImports option) = | true, v -> Some v | _ -> let v : PdbImports = - { Imports = ilImports.Imports |> Array.map (GenPdbImport cenv env) - Parent = GenPdbImports cenv env ilImports.Parent } + { Imports = ilImports.Imports |> Array.map (GenPdbImport cenv) + Parent = GenPdbImports cenv ilImports.Parent } cenv.pdbImports.[ilImports] <- v Some v @@ -2256,7 +2256,7 @@ let GenILMethodBody mname cenv env (il: ILMethodBody) = else [| |] - let imports = GenPdbImports cenv env il.DebugImports + let imports = GenPdbImports cenv il.DebugImports let requiredStringFixups, seh, code, seqpoints, scopes = Codebuf.EmitMethodCode cenv imports localSigs env mname il.Code let codeSize = code.Length use methbuf = ByteBuffer.Create (codeSize * 3)