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

NativeAOT the XUnitLogChecker #103795

Merged
merged 17 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from 9 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
11 changes: 4 additions & 7 deletions eng/testing/RunnerTemplate.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -109,15 +109,12 @@ if NOT "%__IsXUnitLogCheckerSupported%"=="1" (
echo ----- start =============== XUnitLogChecker Output =====================================================

set DOTNET_EXE=%RUNTIME_PATH%\dotnet.exe
set XUNITLOGCHECKER_DLL=%HELIX_CORRELATION_PAYLOAD%\XUnitLogChecker.dll
set XUNITLOGCHECKER_COMMAND=%DOTNET_EXE% --roll-forward Major %XUNITLOGCHECKER_DLL% --dumps-path %HELIX_DUMP_FOLDER%
set XUNITLOGCHECKER_EXE=%HELIX_CORRELATION_PAYLOAD%\XUnitLogChecker.exe
set XUNITLOGCHECKER_COMMAND=%XUNITLOGCHECKER_EXE% --dumps-path %HELIX_DUMP_FOLDER%
set XUNITLOGCHECKER_EXIT_CODE=1

if NOT EXIST %DOTNET_EXE% (
echo dotnet.exe does not exist in the expected location: %DOTNET_EXE%
GOTO XUNITLOGCHECKER_END
) else if NOT EXIST %XUNITLOGCHECKER_DLL% (
echo XUnitLogChecker.dll does not exist in the expected location: %XUNITLOGCHECKER_DLL%
if NOT EXIST %XUNITLOGCHECKER_EXE% (
echo XUnitLogChecker.dll does not exist in the expected location: %XUNITLOGCHECKER_EXE%
agocke marked this conversation as resolved.
Show resolved Hide resolved
GOTO XUNITLOGCHECKER_END
)

Expand Down
16 changes: 6 additions & 10 deletions eng/testing/RunnerTemplate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -79,23 +79,19 @@ function invoke_xunitlogchecker {
local dump_folder=$1

total_dumps=$(find $dump_folder -name "*.dmp" | wc -l)

if [[ $total_dumps > 0 ]]; then
echo "Total dumps found in $dump_folder: $total_dumps"
xunitlogchecker_file_name="$HELIX_CORRELATION_PAYLOAD/XUnitLogChecker.dll"
dotnet_file_name="$RUNTIME_PATH/dotnet"
xunitlogchecker_file_name="$HELIX_CORRELATION_PAYLOAD/XUnitLogChecker"

if [[ ! -f $dotnet_file_name ]]; then
echo "'$dotnet_file_name' was not found. Unable to run XUnitLogChecker."
xunitlogchecker_exit_code=1
elif [[ ! -f $xunitlogchecker_file_name ]]; then
if [[ ! -f $xunitlogchecker_file_name ]]; then
echo "'$xunitlogchecker_file_name' was not found. Unable to print dump file contents."
agocke marked this conversation as resolved.
Show resolved Hide resolved
xunitlogchecker_exit_code=2
elif [[ ! -d $dump_folder ]]; then
echo "The dump directory '$dump_folder' does not exist."
else
echo "Executing XUnitLogChecker in $dump_folder..."
cmd="$dotnet_file_name --roll-forward Major $xunitlogchecker_file_name --dumps-path $dump_folder"
cmd="$xunitlogchecker_file_name --dumps-path $dump_folder"
echo "$cmd"
$cmd
xunitlogchecker_exit_code=$?
Expand Down Expand Up @@ -207,7 +203,7 @@ if [[ $system_name == "Linux" && $test_exitcode -ne 0 ]]; then
if [[ -e /proc/sys/kernel/core_uses_pid && "1" == $(cat /proc/sys/kernel/core_uses_pid) ]]; then
core_name_uses_pid=1
fi

# The osx dumps are too large to egress the machine
echo Looking around for any Linux dumps...

Expand Down Expand Up @@ -241,7 +237,7 @@ elif [[ "$__IsXUnitLogCheckerSupported" != "1" ]]; then
echo "XUnitLogChecker not supported for this test case. Skipping."
else
echo ----- start =============== XUnitLogChecker Output =====================================================

invoke_xunitlogchecker "$HELIX_DUMP_FOLDER"

if [[ $xunitlogchecker_exit_code -ne 0 ]]; then
Expand Down
5 changes: 3 additions & 2 deletions src/libraries/tests.proj
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@
<ProjectExclusions Remove="$(MSBuildThisFileDirectory)System.Collections\tests\System.Collections.Tests.csproj" />
<ProjectExclusions Remove="$(MSBuildThisFileDirectory)System.IO.IsolatedStorage\tests\System.IO.IsolatedStorage.Tests.csproj" />
</ItemGroup>

<ItemGroup Condition="'$(TestNativeAot)' == 'true' or '$(TargetsMobile)' == 'true'">
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Resources.Extensions\tests\BinaryFormatTests\System.Resources.Extensions.BinaryFormat.Tests.csproj" />
<ProjectExclusions Include="$(MSBuildThisFileDirectory)System.Resources.Extensions\tests\CompatTests\System.Resources.Extensions.Compat.Tests.csproj" />
Expand Down Expand Up @@ -762,7 +762,8 @@
<ItemGroup Condition="'$(ArchiveTests)' == 'true' and '$(TargetFrameworkIdentifier)' == '.NETCoreApp' and '$(IsXUnitLogCheckerSupported)' == 'true'">
<ProjectReference
Include="$(RepoRoot)src\tests\Common\XUnitLogChecker\XUnitLogChecker.csproj"
AdditionalProperties="%(AdditionalProperties);Configuration=Release;OutDir=$(XUnitLogCheckerLibrariesOutDir)" />
Targets="Publish"
AdditionalProperties="%(AdditionalProperties);_IsPublishing=true;Configuration=Release;PublishDir=$(XUnitLogCheckerLibrariesOutDir)" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For my own education, why is _IsPublishing being passed? Why isn't int sufficient to specify Targets="Publish" here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is tricky. In short, _IsPublishing is required for publishing, but not set when you're using msbuild /t:publish, only when you're doing dotnet publish.

</ItemGroup>

<Target Name="GenerateMergedCoverageReport"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<NuGetTargetMoniker>$(NetCoreAppCurrentToolTargetFrameworkMoniker)</NuGetTargetMoniker>
<NuGetTargetMonikerShort>$(NetCoreAppToolCurrent)</NuGetTargetMonikerShort>
<RunAnalyzers>false</RunAnalyzers>
<Nullable>enable</Nullable>
<RunAnalyzers>true</RunAnalyzers>
<EnableAotAnalyzer>true</EnableAotAnalyzer>
<EnableTrimAnalyzer>true</EnableTrimAnalyzer>
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
</PropertyGroup>

<ItemGroup>
Expand Down
45 changes: 24 additions & 21 deletions src/tests/Common/Coreclr.TestWrapper/CoreclrTestWrapperLib.cs
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;
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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
{
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully we will not see null objects where the ! was added.

Copy link
Member Author

Choose a reason for hiding this comment

The 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
Expand All @@ -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;
}
Expand All @@ -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))
{
Expand All @@ -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))
{
Expand All @@ -507,7 +510,7 @@ public static bool TryPrintStackTraceFromCrashReport(string crashReportJsonFile,
}
}

string symbolizerOutput = null;
string? symbolizerOutput = null;

Process llvmSymbolizer = new Process()
{
Expand Down Expand Up @@ -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.");
Expand Down Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The 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);
Expand Down
8 changes: 4 additions & 4 deletions src/tests/Common/Coreclr.TestWrapper/MobileAppHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ private static int HandleMobileApp(string action, string platform, string catego
if (platformValueFlag && actionValueFlag)
{
int timeout = 240000; // Set timeout to 4 mins, because the installation on Android arm64/32 devices could take up to 10 mins on CI
string dotnetCmd_raw = System.Environment.GetEnvironmentVariable("__TestDotNetCmd");
string xharnessCmd_raw = System.Environment.GetEnvironmentVariable("XHARNESS_CLI_PATH");
string? dotnetCmd_raw = System.Environment.GetEnvironmentVariable("__TestDotNetCmd");
string? xharnessCmd_raw = System.Environment.GetEnvironmentVariable("XHARNESS_CLI_PATH");
string dotnetCmd = string.IsNullOrEmpty(dotnetCmd_raw) ? "dotnet" : dotnetCmd_raw;
string xharnessCmd = string.IsNullOrEmpty(xharnessCmd_raw) ? "xharness" : $"exec {xharnessCmd_raw}";
string appExtension = platform == "android" ? "apk" : "app";
Expand Down Expand Up @@ -194,9 +194,9 @@ private static string ConvertCmd2Arg(string cmd)

private static void CreateRetryFile(string fileName, int exitCode, string appName)
{
using (StreamWriter writer = new StreamWriter(fileName))
using (StreamWriter writer = new StreamWriter(fileName))
{
writer.WriteLine($"appName: {appName}; exitCode: {exitCode}");
writer.WriteLine($"appName: {appName}; exitCode: {exitCode}");
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/tests/Common/XUnitLogChecker/XUnitLogChecker.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ private enum TagCategory { OPENING, CLOSING }

private static LogCheckerConfigParameters s_configuration;

static int Main(string[] args)
public static int Main(string[] args)
{
s_configuration = new LogCheckerConfigParameters();

Expand Down
9 changes: 6 additions & 3 deletions src/tests/Common/XUnitLogChecker/XUnitLogChecker.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,18 @@
<TargetFramework>$(NetCoreAppToolCurrent)</TargetFramework>
<GenerateRuntimeConfigurationFiles>true</GenerateRuntimeConfigurationFiles>
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<PublishAot>true</PublishAot>
<SuppressTrimAnalysisWarnings>false</SuppressTrimAnalysisWarnings>
<LinkerFlavor Condition="'$(CrossBuild)' == 'true' and '$(TargetsLinux)' == 'true'">lld</LinkerFlavor>
carlossanlop marked this conversation as resolved.
Show resolved Hide resolved
</PropertyGroup>

<ItemGroup>
<Compile Include="XUnitLogChecker.cs" />
<Compile Include="$(MSBuildThisFileDirectory)XUnitLogChecker.cs" />
</ItemGroup>

<ItemGroup>
<Compile Include="../Coreclr.TestWrapper/CoreclrTestWrapperLib.cs" Link="CoreclrTestWrapperLib.cs" />
<Compile Include="../Coreclr.TestWrapper/MobileAppHandler.cs" Link="MobileAppHandler.cs" />
<Compile Include="$(MSBuildThisFileDirectory)../Coreclr.TestWrapper/CoreclrTestWrapperLib.cs" Link="CoreclrTestWrapperLib.cs" />
<Compile Include="$(MSBuildThisFileDirectory)../Coreclr.TestWrapper/MobileAppHandler.cs" Link="MobileAppHandler.cs" />
</ItemGroup>

</Project>
5 changes: 2 additions & 3 deletions src/tests/Common/helixpublishwitharcade.proj
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand All @@ -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>
Copy link
Member

Choose a reason for hiding this comment

The 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>
Expand Down
Loading
Loading