From 176296c6bdd8a067c2280aae99e9dfc87971657d Mon Sep 17 00:00:00 2001 From: Tomas Rylek Date: Sun, 4 Sep 2022 20:22:38 +0200 Subject: [PATCH 1/7] Initial provisions for semantic Crossgen2 PDB validation This Quality Week work item is motivated by my findings from a few months back that Crossgen2 PDB generator had been producing bogus symbol files for almost half a year due to a trivial bug and no testing in place was able to catch that. This change adds initial provisions for semantic PDB validation. In this initial commit I'm adding a new managed app PdbChecker that uses the DIA library to read a given PDB and optionally check it for the presence of given symbols. In parallel I'm making test infra changes that enable selective PDB validation support in individual tests including checks for the presence of expected symbols. This change by itself introduces rudimentary PR / CI validation of PDB files produced by Crossgen2. As next step I plan to introduce additional provisions for running this logic for all tests to be added to one of the Crossgen2-specific pipelines, and validation of PDBs produced during compilation of the framework assemblies. Thanks Tomas --- src/tests/Common/CLRTest.CrossGen.targets | 31 ++++++++-- src/tests/Common/Directory.Build.targets | 3 +- .../Common/PdbChecker/MSDiaSymbolReader.cs | 58 ++++++++++++++++++ src/tests/Common/PdbChecker/PdbChecker.csproj | 17 ++++++ src/tests/Common/PdbChecker/PdbChecker.sln | 25 ++++++++ src/tests/Common/PdbChecker/Program.cs | 60 +++++++++++++++++++ .../test_dependencies.csproj | 1 + .../crossgen2/crossgen2smoke.csproj | 7 +++ 8 files changed, 197 insertions(+), 5 deletions(-) create mode 100644 src/tests/Common/PdbChecker/MSDiaSymbolReader.cs create mode 100644 src/tests/Common/PdbChecker/PdbChecker.csproj create mode 100644 src/tests/Common/PdbChecker/PdbChecker.sln create mode 100644 src/tests/Common/PdbChecker/Program.cs diff --git a/src/tests/Common/CLRTest.CrossGen.targets b/src/tests/Common/CLRTest.CrossGen.targets index ca3bde4776b65..7aff011c62b25 100644 --- a/src/tests/Common/CLRTest.CrossGen.targets +++ b/src/tests/Common/CLRTest.CrossGen.targets @@ -80,6 +80,7 @@ if [ ! -z ${RunCrossGen2+x} ]%3B then OneFileCrossgen2() { date +%H:%M:%S __OutputFile=$1 + __PdbFile=$ __ResponseFile="$__OutputFile.rsp" rm $__ResponseFile 2>/dev/null @@ -168,8 +169,12 @@ if /i "$(AlwaysUseCrossGen2)" == "true" ( REM CrossGen2 Script if defined RunCrossGen2 ( set ExtraCrossGen2Args=!ExtraCrossGen2Args! $(CrossGen2TestExtraArguments) + set CrossGen2TestCheckPdb=$(CrossGen2TestCheckPdb) + set __CreatePdb=$(__CreatePdb) if defined LargeVersionBubble ( set ExtraCrossGen2Args=!ExtraCrossGen2Args! --inputbubble) + if defined CrossGen2TestCheckPdb ( set __CreatePdb=1) + set CrossGen2Status=0 set compilationDoneFlagFile=!ScriptPath!IL-CG2\done if exist "IL-CG2" ( @@ -192,6 +197,7 @@ if defined RunCrossGen2 ( if defined CompositeBuildMode ( set ExtraCrossGen2Args=!ExtraCrossGen2Args! --composite set __OutputFile=!scriptPath!\composite-r2r.dll + set __PdbFile=!scriptPath!\composite-r2r.ni.pdb rem In composite mode, treat all dll's in the test folder as rooting inputs set __InputFile=!scriptPath!IL-CG2\*.dll call :CompileOneFileCrossgen2 @@ -200,6 +206,7 @@ if defined RunCrossGen2 ( set ExtraCrossGen2Args=!ExtraCrossGen2Args! -r:!scriptPath!IL-CG2\*.dll for %%I in (!scriptPath!IL-CG2\*.dll) do ( set __OutputFile=!scriptPath!%%~nI.dll + set __PdbFile=!scriptPath!%%~nI.ni.pdb set __InputFile=%%I call :CompileOneFileCrossgen2 IF NOT !CrossGen2Status!==0 ( @@ -216,13 +223,14 @@ if defined RunCrossGen2 ( set __ResponseFile=!__OutputFile!.rsp del /Q !__ResponseFile! 2>nul - set __Command=!_DebuggerFullPath! REM Tests run locally need __TestDotNetCmd (set by runtest.py) or a compatible 5.0 dotnet runtime in the path if defined __TestDotNetCmd ( - set __Command=!__Command! "!__TestDotNetCmd!" + set __DotNet="!__TestDotNetCmd!" ) else ( - set __Command=!__Command! "dotnet" + set __DotNet="dotnet" ) + set __Command=!_DebuggerFullPath! + set __Command=!__Command! !__DotNet! set __Command=!__Command! "!CORE_ROOT!\crossgen2\crossgen2.dll" set __Command=!__Command! @"!__ResponseFile!" set __Command=!__Command! !ExtraCrossGen2Args! @@ -237,7 +245,7 @@ if defined RunCrossGen2 ( echo -r:!CORE_ROOT!\netstandard.dll>>!__ResponseFile! echo -O>>!__ResponseFile! - if not "$(__CreatePdb)" == "" ( + if defined __CreatePdb ( echo --pdb>>!__ResponseFile! ) @@ -259,6 +267,21 @@ if defined RunCrossGen2 ( endlocal set CrossGen2Status=!ERRORLEVEL! echo %time% + + if !CrossGen2Status!==0 ( + if defined CrossGen2TestCheckPdb ( + set __CheckPdbCommand=!__DotNet! + set __CheckPdbCommand=!__CheckPdbCommand! "!CORE_ROOT!\PdbChecker\PdbChecker.dll" + set __CheckPdbCommand=!__CheckPdbCommand! !__PdbFile! @(CheckPdbSymbol->'%22%(Identity)%22', ' ') + echo "!__CheckPdbCommand!" + call !__CheckPdbCommand! + if not !ERRORLEVEL!==0 ( + echo PDB check failed for file !__PdbFile! >2 + set CrossGen2Status=42 + ) + ) + ) + Exit /b 0 :DoneCrossgen2Operations diff --git a/src/tests/Common/Directory.Build.targets b/src/tests/Common/Directory.Build.targets index dde2731e82237..7046ef0ca995d 100644 --- a/src/tests/Common/Directory.Build.targets +++ b/src/tests/Common/Directory.Build.targets @@ -44,6 +44,7 @@ + @@ -174,7 +175,7 @@ - + _pdbSymbols; + + public MSDiaSymbolReader(string pdbFile) + { + try + { + _diaDataSource = new DiaSourceClass(); + _diaDataSource.loadDataFromPdb(pdbFile); + _diaDataSource.openSession(out _diaSession); + + _pdbSymbols = new List(); + + _diaSession.getSymbolsByAddr(out IDiaEnumSymbolsByAddr symbolEnum); + int symbolsTotal = 0; + for (IDiaSymbol symbol = symbolEnum.symbolByRVA(0); symbol != null; symbolEnum.Next(1, out symbol, out uint fetched)) + { + symbolsTotal++; + if (symbol.symTag == (uint)SymTagEnum.SymTagFunction || symbol.symTag == (uint)SymTagEnum.SymTagPublicSymbol) + { + _pdbSymbols.Add(symbol.name); + } + } + + Console.WriteLine("PDB file: {0}", pdbFile); + Console.WriteLine("Total symbols: {0}", symbolsTotal); + Console.WriteLine("Public symbols: {0}", _pdbSymbols.Count); + } + catch (Exception ex) + { + throw new Exception($"Error opening PDB file {pdbFile}", ex); + } + } + + public void DumpSymbols() + { + Console.WriteLine("PDB public symbol list:"); + foreach (string symbol in _pdbSymbols.OrderBy(s => s)) + { + Console.WriteLine(symbol); + } + Console.WriteLine("End of PDB public symbol list"); + } + + public bool ContainsSymbol(string symbolName) => _pdbSymbols.Any(s => s.Contains(symbolName)); +} diff --git a/src/tests/Common/PdbChecker/PdbChecker.csproj b/src/tests/Common/PdbChecker/PdbChecker.csproj new file mode 100644 index 0000000000000..a2114f8be3229 --- /dev/null +++ b/src/tests/Common/PdbChecker/PdbChecker.csproj @@ -0,0 +1,17 @@ + + + + Exe + net7.0 + enable + enable + true + + + + + + + + + diff --git a/src/tests/Common/PdbChecker/PdbChecker.sln b/src/tests/Common/PdbChecker/PdbChecker.sln new file mode 100644 index 0000000000000..3ba7e9ae4c337 --- /dev/null +++ b/src/tests/Common/PdbChecker/PdbChecker.sln @@ -0,0 +1,25 @@ + +Microsoft Visual Studio Solution File, Format Version 12.00 +# Visual Studio Version 17 +VisualStudioVersion = 17.3.32708.82 +MinimumVisualStudioVersion = 10.0.40219.1 +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "PdbChecker", "PdbChecker.csproj", "{6247A503-5387-4BE1-ACA3-027CADA30CA9}" +EndProject +Global + GlobalSection(SolutionConfigurationPlatforms) = preSolution + Debug|Any CPU = Debug|Any CPU + Release|Any CPU = Release|Any CPU + EndGlobalSection + GlobalSection(ProjectConfigurationPlatforms) = postSolution + {6247A503-5387-4BE1-ACA3-027CADA30CA9}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {6247A503-5387-4BE1-ACA3-027CADA30CA9}.Debug|Any CPU.Build.0 = Debug|Any CPU + {6247A503-5387-4BE1-ACA3-027CADA30CA9}.Release|Any CPU.ActiveCfg = Release|Any CPU + {6247A503-5387-4BE1-ACA3-027CADA30CA9}.Release|Any CPU.Build.0 = Release|Any CPU + EndGlobalSection + GlobalSection(SolutionProperties) = preSolution + HideSolutionNode = FALSE + EndGlobalSection + GlobalSection(ExtensibilityGlobals) = postSolution + SolutionGuid = {4033231C-763B-4C57-BA35-7C1AC007AD0E} + EndGlobalSection +EndGlobal diff --git a/src/tests/Common/PdbChecker/Program.cs b/src/tests/Common/PdbChecker/Program.cs new file mode 100644 index 0000000000000..41f4209ce648d --- /dev/null +++ b/src/tests/Common/PdbChecker/Program.cs @@ -0,0 +1,60 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using Dia2Lib; +class Program +{ + public static int Main(string[] args) + { + try + { + TryMain(args); + return 0; + } + catch (Exception ex) + { + Console.Error.WriteLine("Fatal error: {0}", ex); + return 1; + } + } + + private static void TryMain(string[] args) + { + if (args.Length == 0) + { + DisplayUsage(); + return; + } + MSDiaSymbolReader reader = new MSDiaSymbolReader(args[0]); + int matchedSymbols = 0; + int missingSymbols = 0; + for (int symbolArgIndex = 1; symbolArgIndex < args.Length; symbolArgIndex++) + { + string symbolName = args[symbolArgIndex]; + if (reader.ContainsSymbol(symbolName)) + { + matchedSymbols++; + } + else + { + missingSymbols++; + Console.Error.WriteLine("Missing symbol: {0}", symbolName); + } + } + if (missingSymbols > 0) + { + reader.DumpSymbols(); + throw new Exception($"{missingSymbols} missing symbols ({matchedSymbols} symbols matched)"); + } + if (matchedSymbols > 0) + { + Console.WriteLine("Matched all {0} symbols", matchedSymbols); + } + } + + private static void DisplayUsage() + { + Console.WriteLine("Usage: PdbChecker { }"); + } +} diff --git a/src/tests/Common/test_dependencies/test_dependencies.csproj b/src/tests/Common/test_dependencies/test_dependencies.csproj index 8f10be173327d..bffd70cc1561a 100644 --- a/src/tests/Common/test_dependencies/test_dependencies.csproj +++ b/src/tests/Common/test_dependencies/test_dependencies.csproj @@ -10,6 +10,7 @@ + diff --git a/src/tests/readytorun/crossgen2/crossgen2smoke.csproj b/src/tests/readytorun/crossgen2/crossgen2smoke.csproj index c233cd8dcbd04..dfefd2c243fb1 100644 --- a/src/tests/readytorun/crossgen2/crossgen2smoke.csproj +++ b/src/tests/readytorun/crossgen2/crossgen2smoke.csproj @@ -5,6 +5,13 @@ true + true + + + + + + From ec1cf32b6e2e0d1f1845d9ef32193b986683835e Mon Sep 17 00:00:00 2001 From: Tomas Date: Wed, 7 Sep 2022 03:25:04 +0200 Subject: [PATCH 2/7] Revert unnecessary change in the Unix section of CLRTest.Crossgen.targets --- src/tests/Common/CLRTest.CrossGen.targets | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tests/Common/CLRTest.CrossGen.targets b/src/tests/Common/CLRTest.CrossGen.targets index 7aff011c62b25..52b8cbcc04e7a 100644 --- a/src/tests/Common/CLRTest.CrossGen.targets +++ b/src/tests/Common/CLRTest.CrossGen.targets @@ -80,7 +80,6 @@ if [ ! -z ${RunCrossGen2+x} ]%3B then OneFileCrossgen2() { date +%H:%M:%S __OutputFile=$1 - __PdbFile=$ __ResponseFile="$__OutputFile.rsp" rm $__ResponseFile 2>/dev/null From 901c3f2878a26c1217287925abfa07ec42e4e37d Mon Sep 17 00:00:00 2001 From: Tomas Rylek Date: Thu, 8 Sep 2022 22:41:21 +0200 Subject: [PATCH 3/7] Instrument the test to fail to check its proper behavior in the lab --- src/tests/readytorun/crossgen2/crossgen2smoke.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/readytorun/crossgen2/crossgen2smoke.csproj b/src/tests/readytorun/crossgen2/crossgen2smoke.csproj index dfefd2c243fb1..4808e35ee4899 100644 --- a/src/tests/readytorun/crossgen2/crossgen2smoke.csproj +++ b/src/tests/readytorun/crossgen2/crossgen2smoke.csproj @@ -11,6 +11,7 @@ + From 0e4f09adcea7c8c0fa4235aba81212fa214fd2a5 Mon Sep 17 00:00:00 2001 From: Tomas Rylek Date: Fri, 16 Sep 2022 19:46:37 +0200 Subject: [PATCH 4/7] Modify PdbChecker project to include DiaSymReader --- src/tests/Common/PdbChecker/PdbChecker.csproj | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/tests/Common/PdbChecker/PdbChecker.csproj b/src/tests/Common/PdbChecker/PdbChecker.csproj index a2114f8be3229..cb4415fd8aa94 100644 --- a/src/tests/Common/PdbChecker/PdbChecker.csproj +++ b/src/tests/Common/PdbChecker/PdbChecker.csproj @@ -10,8 +10,30 @@ + + + + $(TargetArchitectureForSharedLibraries) + amd64 + Microsoft.DiaSymReader.Native.$(DiaSymReaderTargetArch).dll + $(PkgMicrosoft_DiaSymReader_Native)\runtimes\win\native\$(DiaSymReaderTargetArchFileName) + + $(CoreCLRArtifactsPath)crossgen2/$(DiaSymReaderTargetArchFileName) + + + + - + From 4c974164396793b9565dd26f3e4d2661d721340c Mon Sep 17 00:00:00 2001 From: Tomas Date: Tue, 20 Sep 2022 01:59:27 +0200 Subject: [PATCH 5/7] Retry building against Dia2Lib --- src/tests/Common/PdbChecker/PdbChecker.csproj | 25 +++---------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/src/tests/Common/PdbChecker/PdbChecker.csproj b/src/tests/Common/PdbChecker/PdbChecker.csproj index cb4415fd8aa94..214485340c189 100644 --- a/src/tests/Common/PdbChecker/PdbChecker.csproj +++ b/src/tests/Common/PdbChecker/PdbChecker.csproj @@ -12,28 +12,11 @@ - - $(TargetArchitectureForSharedLibraries) - amd64 - Microsoft.DiaSymReader.Native.$(DiaSymReaderTargetArch).dll - $(PkgMicrosoft_DiaSymReader_Native)\runtimes\win\native\$(DiaSymReaderTargetArchFileName) - - $(CoreCLRArtifactsPath)crossgen2/$(DiaSymReaderTargetArchFileName) - - - - - + From 1f1a5702f70f13f3479c4def3859425ed0dfb78b Mon Sep 17 00:00:00 2001 From: Tomas Date: Tue, 20 Sep 2022 15:04:25 +0200 Subject: [PATCH 6/7] Make the package reference to TraceEvent unconditional --- src/tests/Common/PdbChecker/PdbChecker.csproj | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/src/tests/Common/PdbChecker/PdbChecker.csproj b/src/tests/Common/PdbChecker/PdbChecker.csproj index 214485340c189..cc9d3881353b6 100644 --- a/src/tests/Common/PdbChecker/PdbChecker.csproj +++ b/src/tests/Common/PdbChecker/PdbChecker.csproj @@ -10,13 +10,7 @@ - - - - + From d22eaa503094644f2f648a42d80a5d91e88ff382 Mon Sep 17 00:00:00 2001 From: Tomas Date: Tue, 20 Sep 2022 19:14:24 +0200 Subject: [PATCH 7/7] Fix build of PdbChecker in test_dependencies.csproj --- src/tests/Common/test_dependencies/test_dependencies.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/Common/test_dependencies/test_dependencies.csproj b/src/tests/Common/test_dependencies/test_dependencies.csproj index bffd70cc1561a..b20052c390b0d 100644 --- a/src/tests/Common/test_dependencies/test_dependencies.csproj +++ b/src/tests/Common/test_dependencies/test_dependencies.csproj @@ -10,7 +10,7 @@ - +