-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
NativeAOT the XUnitLogChecker #103795
NativeAOT the XUnitLogChecker #103795
Changes from all commits
e3b702f
44a6ae1
1c6e909
222f40e
8da3453
eac9c40
f98ee2f
6f6d23a
fdd5f11
27ecb7b
c8fb8a8
1640b4e
7555fa2
75e73dd
08791d0
8de2c1d
f10a4e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
// | ||
#nullable disable | ||
|
||
using System; | ||
using System.Collections.Generic; | ||
|
@@ -100,7 +99,7 @@ public unsafe struct ProcessEntry32W | |
static class @libproc | ||
{ | ||
[DllImport(nameof(libproc))] | ||
private static extern int proc_listchildpids(int ppid, int[] buffer, int byteSize); | ||
private static extern int proc_listchildpids(int ppid, int[]? buffer, int byteSize); | ||
|
||
public static unsafe bool ListChildPids(int ppid, out int[] buffer) | ||
{ | ||
|
@@ -171,7 +170,7 @@ private unsafe static IEnumerable<Process> Windows_GetChildren(Process process) | |
private static IEnumerable<Process> Linux_GetChildren(Process process) | ||
{ | ||
var children = new List<Process>(); | ||
List<int> childPids = null; | ||
List<int>? childPids = null; | ||
|
||
try | ||
{ | ||
|
@@ -188,7 +187,7 @@ private static IEnumerable<Process> Linux_GetChildren(Process process) | |
pgrepInfo.RedirectStandardOutput = true; | ||
pgrepInfo.Arguments = $"-P {process.Id}"; | ||
|
||
using Process pgrep = Process.Start(pgrepInfo); | ||
using Process pgrep = Process.Start(pgrepInfo)!; | ||
|
||
string[] pidStrings = pgrep.StandardOutput.ReadToEnd().Split('\n', StringSplitOptions.RemoveEmptyEntries); | ||
pgrep.WaitForExit(); | ||
|
@@ -271,7 +270,11 @@ static bool CollectCrashDumpWithMiniDumpWriteDump(Process process, string crashD | |
|
||
static bool CollectCrashDumpWithCreateDump(Process process, string crashDumpPath, StreamWriter outputWriter) | ||
{ | ||
string coreRoot = Environment.GetEnvironmentVariable("CORE_ROOT"); | ||
string? coreRoot = Environment.GetEnvironmentVariable("CORE_ROOT"); | ||
if (coreRoot is null) | ||
{ | ||
throw new InvalidOperationException("CORE_ROOT environment variable is not set."); | ||
} | ||
string createdumpPath = Path.Combine(coreRoot, "createdump"); | ||
string arguments = $"--crashreport --name \"{crashDumpPath}\" {process.Id} --withheap"; | ||
Process createdump = new Process(); | ||
|
@@ -388,7 +391,7 @@ public static bool TryPrintStackTraceFromCrashReport(string crashReportJsonFile, | |
} | ||
|
||
Console.WriteLine("========================================="); | ||
string userName = Environment.GetEnvironmentVariable("USER"); | ||
string? userName = Environment.GetEnvironmentVariable("USER"); | ||
if (!string.IsNullOrEmpty(userName)) | ||
{ | ||
if (!RunProcess("sudo", $"chown {userName} {crashReportJsonFile}", Console.Out)) | ||
|
@@ -426,8 +429,8 @@ public static bool TryPrintStackTraceFromCrashReport(string crashReportJsonFile, | |
Console.WriteLine($"Error reading {crashReportJsonFile}: {ex.ToString()}"); | ||
return false; | ||
} | ||
dynamic crashReport = JsonSerializer.Deserialize<JsonObject>(contents); | ||
var threads = crashReport["payload"]["threads"]; | ||
var crashReport = JsonNode.Parse(contents)!; | ||
var threads = (JsonArray)crashReport["payload"]!["threads"]!; | ||
Comment on lines
+432
to
+433
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hopefully we will not see null objects where the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct. And if we do, we would throw an exception anyway, so I figure any further checking is a waste. |
||
|
||
// The logic happens in 3 steps: | ||
// 1. Read the crashReport.json file, locate all the addresses of interest and then build | ||
|
@@ -443,7 +446,7 @@ public static bool TryPrintStackTraceFromCrashReport(string crashReportJsonFile, | |
foreach (var thread in threads) | ||
{ | ||
|
||
if (thread["native_thread_id"] == null) | ||
if (thread!["native_thread_id"] == null) | ||
{ | ||
continue; | ||
} | ||
|
@@ -452,21 +455,21 @@ public static bool TryPrintStackTraceFromCrashReport(string crashReportJsonFile, | |
addrBuilder.AppendLine("----------------------------------"); | ||
addrBuilder.AppendLine($"Thread Id: {thread["native_thread_id"]}"); | ||
addrBuilder.AppendLine(" Child SP IP Call Site"); | ||
var stack_frames = thread["stack_frames"]; | ||
var stack_frames = (JsonArray)thread["stack_frames"]!; | ||
foreach (var frame in stack_frames) | ||
{ | ||
addrBuilder.Append($"{SKIP_LINE_TAG} {frame["stack_pointer"]} {frame["native_address"]} "); | ||
bool isNative = (string)frame["is_managed"] == "false"; | ||
addrBuilder.Append($"{SKIP_LINE_TAG} {frame!["stack_pointer"]} {frame["native_address"]} "); | ||
bool isNative = (string)frame["is_managed"]! == "false"; | ||
|
||
if (isNative) | ||
{ | ||
string nativeModuleName = (string)frame["native_module"]; | ||
string unmanagedName = (string)frame["unmanaged_name"]; | ||
var nativeModuleName = (string)frame["native_module"]!; | ||
var unmanagedName = (string)frame["unmanaged_name"]!; | ||
|
||
if ((nativeModuleName != null) && (knownNativeModules.Contains(nativeModuleName))) | ||
{ | ||
// Need to use llvm-symbolizer (only if module_address != 0) | ||
AppendAddress(addrBuilder, coreRoot, nativeModuleName, (string)frame["native_address"], (string)frame["module_address"]); | ||
AppendAddress(addrBuilder, coreRoot, nativeModuleName, (string)frame["native_address"]!, (string)frame["module_address"]!); | ||
} | ||
else if ((nativeModuleName != null) || (unmanagedName != null)) | ||
{ | ||
|
@@ -482,8 +485,8 @@ public static bool TryPrintStackTraceFromCrashReport(string crashReportJsonFile, | |
} | ||
else | ||
{ | ||
string fileName = (string)frame["filename"]; | ||
string methodName = (string)frame["method_name"]; | ||
var fileName = (string)frame["filename"]!; | ||
var methodName = (string)frame["method_name"]!; | ||
|
||
if ((fileName != null) || (methodName != null)) | ||
{ | ||
|
@@ -507,7 +510,7 @@ public static bool TryPrintStackTraceFromCrashReport(string crashReportJsonFile, | |
} | ||
} | ||
|
||
string symbolizerOutput = null; | ||
string? symbolizerOutput = null; | ||
|
||
Process llvmSymbolizer = new Process() | ||
{ | ||
|
@@ -602,7 +605,7 @@ private static void AppendAddress(StringBuilder sb, string coreRoot, string nati | |
|
||
public static bool TryPrintStackTraceFromDmp(string dmpFile, TextWriter outputWriter) | ||
{ | ||
string targetArchitecture = Environment.GetEnvironmentVariable(TEST_TARGET_ARCHITECTURE_ENVIRONMENT_VAR); | ||
string? targetArchitecture = Environment.GetEnvironmentVariable(TEST_TARGET_ARCHITECTURE_ENVIRONMENT_VAR); | ||
if (string.IsNullOrEmpty(targetArchitecture)) | ||
{ | ||
outputWriter.WriteLine($"Environment variable {TEST_TARGET_ARCHITECTURE_ENVIRONMENT_VAR} is not set."); | ||
|
@@ -675,10 +678,10 @@ public int RunTest(string executable, string outputFile, string errorFile, strin | |
|
||
// If a timeout was given to us by an environment variable, use it instead of the default | ||
// timeout. | ||
string environmentVar = Environment.GetEnvironmentVariable(TIMEOUT_ENVIRONMENT_VAR); | ||
string? environmentVar = Environment.GetEnvironmentVariable(TIMEOUT_ENVIRONMENT_VAR); | ||
int timeout = environmentVar != null ? int.Parse(environmentVar) : DEFAULT_TIMEOUT_MS; | ||
bool collectCrashDumps = Environment.GetEnvironmentVariable(COLLECT_DUMPS_ENVIRONMENT_VAR) != null; | ||
string crashDumpFolder = Environment.GetEnvironmentVariable(CRASH_DUMP_FOLDER_ENVIRONMENT_VAR); | ||
string? crashDumpFolder = Environment.GetEnvironmentVariable(CRASH_DUMP_FOLDER_ENVIRONMENT_VAR); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For these two env vars and the ones in the other files, should we throw if they're not found? |
||
|
||
var outputStream = new FileStream(outputFile, FileMode.Create); | ||
var errorStream = new FileStream(errorFile, FileMode.Create); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -67,7 +67,7 @@ | |
<PropertyGroup> | ||
<CoreRootDirectory>$(TestBinDir)Tests\Core_Root\</CoreRootDirectory> | ||
<CoreRootDirectory>$([MSBuild]::NormalizeDirectory($(CoreRootDirectory)))</CoreRootDirectory> | ||
<XUnitLogCheckerDirectory>$(TestBinDir)Common\XUnitLogChecker\</XUnitLogCheckerDirectory> | ||
<XUnitLogCheckerDirectory>$(TestBinDir)XUnitLogChecker\</XUnitLogCheckerDirectory> | ||
<XUnitLogCheckerDirectory>$([MSBuild]::NormalizeDirectory($(XUnitLogCheckerDirectory)))</XUnitLogCheckerDirectory> | ||
<LegacyPayloadsRootDirectory>$(TestBinDir)LegacyPayloads\</LegacyPayloadsRootDirectory> | ||
<LegacyPayloadsRootDirectory>$([MSBuild]::NormalizeDirectory($(LegacyPayloadsRootDirectory)))</LegacyPayloadsRootDirectory> | ||
|
@@ -392,7 +392,6 @@ | |
<PropertyGroup> | ||
<XUnitLogCheckerHelixPath Condition="'$(TestWrapperTargetsWindows)' != 'true'">%24HELIX_CORRELATION_PAYLOAD/</XUnitLogCheckerHelixPath> | ||
<XUnitLogCheckerHelixPath Condition="'$(TestWrapperTargetsWindows)' == 'true'">%25HELIX_CORRELATION_PAYLOAD%25/</XUnitLogCheckerHelixPath> | ||
<XUnitLogCheckerHelixPath>$(XUnitLogCheckerHelixPath)XUnitLogChecker/</XUnitLogCheckerHelixPath> | ||
|
||
<XUnitLogCheckerArgs>--results-path $(_MergedWrapperRunScriptDirectoryRelative)</XUnitLogCheckerArgs> | ||
<XUnitLogCheckerArgs>$(XUnitLogCheckerArgs) --test-wrapper $(_MergedWrapperName)</XUnitLogCheckerArgs> | ||
|
@@ -401,7 +400,7 @@ | |
<XUnitLogCheckerArgs Condition="'$(TestWrapperTargetsWindows)' != 'true'">$(XUnitLogCheckerArgs) %24HELIX_DUMP_FOLDER</XUnitLogCheckerArgs> | ||
<XUnitLogCheckerArgs Condition="'$(TestWrapperTargetsWindows)' == 'true'">$(XUnitLogCheckerArgs) %25HELIX_DUMP_FOLDER%25</XUnitLogCheckerArgs> | ||
|
||
<XUnitLogCheckerCommand>dotnet $(XUnitLogCheckerHelixPath)XUnitLogChecker.dll $(XUnitLogCheckerArgs)</XUnitLogCheckerCommand> | ||
<XUnitLogCheckerCommand>$(XUnitLogCheckerHelixPath)XUnitLogChecker$(ExeSuffix) $(XUnitLogCheckerArgs)</XUnitLogCheckerCommand> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So you're now putting XUnitLogChecker.exe in the root helix path. Nice. |
||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
|
@@ -426,7 +425,7 @@ | |
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' != 'true'" Include="test_exit_code=%24%3F" /> | ||
|
||
<!-- Add the XUnitLogChecker's running command and store its exit code. --> | ||
<HelixCommandLines Include="$(XUnitLogCheckerCommand)" /> | ||
<HelixCommandLines Condition="'$(IsXunitLogCheckerSupported)' == 'true'" Include="$(XUnitLogCheckerCommand)" /> | ||
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' == 'true'" Include="set xunitlogchecker_exit_code=%25ERRORLEVEL%25" /> | ||
<HelixCommandLines Condition="'$(TestWrapperTargetsWindows)' != 'true'" Include="xunitlogchecker_exit_code=%24%3F" /> | ||
|
||
|
@@ -761,7 +760,7 @@ | |
<HelixCorrelationPayload Include="$(CoreRootDirectory)" /> | ||
|
||
<!-- Browser-Wasm and iOS platforms follow a very different workflow, which is currently out of scope of the Log Checker. It's not useful on any platform that uses xharness. --> | ||
<HelixCorrelationPayload Include="$(XUnitLogCheckerDirectory)" Condition="'$(TargetsBrowser)' != 'true' and '$(TargetsAppleMobile)' != 'true'" /> | ||
<HelixCorrelationPayload Include="$(XUnitLogCheckerDirectory)" Condition="'$(IsXunitLogCheckerSupported)' == 'true' and '$(TargetsBrowser)' != 'true' and '$(TargetsAppleMobile)' != 'true'" /> | ||
<HelixCorrelationPayload Condition="'$(TestWrapperTargetsWindows)' == 'true'" Include="dotnet-sos"> | ||
<Destination>sos</Destination> | ||
<Uri>https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-tools/nuget/v3/flat2/dotnet-sos/$(DotnetSosVersion)/dotnet-sos.$(DotnetSosVersion).nupkg</Uri> | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct me if I'm wrong: the 'Runtime Identifier' additional property here is the equivalent of using the CLI command argument
-r <rid>
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, that's correct