From 39b48783095e11d714d9cf2bffcabef21b7f1e05 Mon Sep 17 00:00:00 2001 From: Radek Zikmund <32671551+rzikm@users.noreply.github.com> Date: Wed, 10 Apr 2024 14:47:40 +0200 Subject: [PATCH 01/11] Support SSLKEYLOGFILE in Release builds (#100665) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Allow SSLKEYLOGFILE functionality in Release builds * Unify SSLKEYLOGFILE logging, gate behind AppContextSwitch * Add more points where QUIC secrets are logged * Enable in Debug builds without appctx switch * Update src/libraries/System.Net.Quic/src/System.Net.Quic.csproj Co-authored-by: Marie Píchová <11718369+ManickaP@users.noreply.github.com> * Code review feedback * More code review changes * iAdjust appctx switch name --------- Co-authored-by: Marie Píchová <11718369+ManickaP@users.noreply.github.com> --- .../Interop.OpenSsl.cs | 21 +-- .../src/System/Net/Security/SslKeyLogger.cs | 134 ++++++++++++++++++ .../src/System.Net.Quic.csproj | 1 + .../Net/Quic/Internal/MsQuicTlsSecret.cs | 96 ++++++++----- .../src/System/Net/Quic/QuicConnection.cs | 15 +- .../MsQuicRemoteExecutorTests.cs | 32 +++-- .../src/System.Net.Security.csproj | 2 + .../SslStreamRemoteExecutorTests.cs | 35 +++-- .../System.Net.Security.Unit.Tests.csproj | 2 + 9 files changed, 257 insertions(+), 81 deletions(-) create mode 100644 src/libraries/Common/src/System/Net/Security/SslKeyLogger.cs diff --git a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs index 310981b194d0e1..8636bbe4884866 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.OpenSsl.cs @@ -21,10 +21,6 @@ internal static partial class Interop { internal static partial class OpenSsl { -#if DEBUG - private static readonly string? s_keyLogFile = Environment.GetEnvironmentVariable("SSLKEYLOGFILE"); - private static readonly FileStream? s_fileStream = s_keyLogFile != null ? File.Open(s_keyLogFile, FileMode.Append, FileAccess.Write, FileShare.ReadWrite) : null; -#endif private const string TlsCacheSizeCtxName = "System.Net.Security.TlsCacheSize"; private const string TlsCacheSizeEnvironmentVariable = "DOTNET_SYSTEM_NET_SECURITY_TLSCACHESIZE"; private const SslProtocols FakeAlpnSslProtocol = (SslProtocols)1; // used to distinguish server sessions with ALPN @@ -209,12 +205,10 @@ internal static unsafe SafeSslContextHandle AllocateSslContext(SslAuthentication Ssl.SslCtxSetDefaultOcspCallback(sslCtx); } } -#if DEBUG - if (s_fileStream != null) + if (SslKeyLogger.IsEnabled) { Ssl.SslCtxSetKeylogCallback(sslCtx, &KeyLogCallback); } -#endif } catch { @@ -757,23 +751,12 @@ private static unsafe void RemoveSessionCallback(IntPtr ctx, IntPtr session) ctxHandle.RemoveSession(name); } -#if DEBUG [UnmanagedCallersOnly] private static unsafe void KeyLogCallback(IntPtr ssl, char* line) { - Debug.Assert(s_fileStream != null); ReadOnlySpan data = MemoryMarshal.CreateReadOnlySpanFromNullTerminated((byte*)line); - if (data.Length > 0) - { - lock (s_fileStream) - { - s_fileStream.Write(data); - s_fileStream.WriteByte((byte)'\n'); - s_fileStream.Flush(); - } - } + SslKeyLogger.WriteLineRaw(data); } -#endif private static int BioRead(SafeBioHandle bio, Span buffer, int count) { diff --git a/src/libraries/Common/src/System/Net/Security/SslKeyLogger.cs b/src/libraries/Common/src/System/Net/Security/SslKeyLogger.cs new file mode 100644 index 00000000000000..e6e9d9cd039297 --- /dev/null +++ b/src/libraries/Common/src/System/Net/Security/SslKeyLogger.cs @@ -0,0 +1,134 @@ +// 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 System.Diagnostics; +using System.IO; +using System.Net; + +internal static class SslKeyLogger +{ + private static readonly string? s_keyLogFile = Environment.GetEnvironmentVariable("SSLKEYLOGFILE"); + private static readonly FileStream? s_fileStream; + +#pragma warning disable CA1810 // Initialize all static fields when declared and remove cctor + static SslKeyLogger() + { + s_fileStream = null; + + try + { +#if DEBUG + bool isEnabled = true; +#else + bool isEnabled = AppContext.TryGetSwitch("System.Net.EnableSslKeyLogging", out bool enabled) && enabled; +#endif + + if (isEnabled && s_keyLogFile != null) + { + s_fileStream = File.Open(s_keyLogFile, FileMode.Append, FileAccess.Write, FileShare.ReadWrite); + } + } + catch (Exception ex) + { + if (NetEventSource.Log.IsEnabled()) + { + NetEventSource.Error(null, $"Failed to open SSL key log file '{s_keyLogFile}': {ex}"); + } + } + } +#pragma warning restore CA1810 + + public static bool IsEnabled => s_fileStream != null; + + public static void WriteLineRaw(ReadOnlySpan data) + { + Debug.Assert(s_fileStream != null); + if (s_fileStream == null) + { + return; + } + + if (data.Length > 0) + { + lock (s_fileStream) + { + s_fileStream.Write(data); + s_fileStream.WriteByte((byte)'\n'); + s_fileStream.Flush(); + } + } + } + + public static void WriteSecrets( + ReadOnlySpan clientRandom, + ReadOnlySpan clientHandshakeTrafficSecret, + ReadOnlySpan serverHandshakeTrafficSecret, + ReadOnlySpan clientTrafficSecret0, + ReadOnlySpan serverTrafficSecret0, + ReadOnlySpan clientEarlyTrafficSecret) + { + Debug.Assert(s_fileStream != null); + Debug.Assert(!clientRandom.IsEmpty); + + if (s_fileStream == null || + clientRandom.IsEmpty || + + // return early if there is nothing to log + (clientHandshakeTrafficSecret.IsEmpty && + serverHandshakeTrafficSecret.IsEmpty && + clientTrafficSecret0.IsEmpty && + serverTrafficSecret0.IsEmpty && + clientEarlyTrafficSecret.IsEmpty)) + { + return; + } + + Span clientRandomUtf8 = clientRandom.Length <= 1024 ? stackalloc byte[clientRandom.Length * 2] : new byte[clientRandom.Length * 2]; + HexEncode(clientRandom, clientRandomUtf8); + + lock (s_fileStream) + { + WriteSecretCore("CLIENT_HANDSHAKE_TRAFFIC_SECRET"u8, clientRandomUtf8, clientHandshakeTrafficSecret); + WriteSecretCore("SERVER_HANDSHAKE_TRAFFIC_SECRET"u8, clientRandomUtf8, serverHandshakeTrafficSecret); + WriteSecretCore("CLIENT_TRAFFIC_SECRET_0"u8, clientRandomUtf8, clientTrafficSecret0); + WriteSecretCore("SERVER_TRAFFIC_SECRET_0"u8, clientRandomUtf8, serverTrafficSecret0); + WriteSecretCore("CLIENT_EARLY_TRAFFIC_SECRET"u8, clientRandomUtf8, clientEarlyTrafficSecret); + + s_fileStream.Flush(); + } + } + + private static void WriteSecretCore(ReadOnlySpan labelUtf8, ReadOnlySpan clientRandomUtf8, ReadOnlySpan secret) + { + if (secret.Length == 0) + { + return; + } + + // write the secret line in the format {label} {client_random (hex)} {secret (hex)} e.g. + // SERVER_HANDSHAKE_TRAFFIC_SECRET bae582227f0f46ca663cb8c3d62e68cec38c2b947e7c4a9ec6f4e262b5ed5354 48f6bd5b0c8447d97129c6dad080f34c7f9f11ade8eeabb011f33811543411d7ab1013b1374bcd81bfface6a2deef539 + int totalLength = labelUtf8.Length + 1 + clientRandomUtf8.Length + 1 + 2 * secret.Length + 1; + Span line = totalLength <= 1024 ? stackalloc byte[totalLength] : new byte[totalLength]; + + labelUtf8.CopyTo(line); + line[labelUtf8.Length] = (byte)' '; + + clientRandomUtf8.CopyTo(line.Slice(labelUtf8.Length + 1)); + line[labelUtf8.Length + 1 + clientRandomUtf8.Length] = (byte)' '; + + HexEncode(secret, line.Slice(labelUtf8.Length + 1 + clientRandomUtf8.Length + 1)); + line[^1] = (byte)'\n'; + + s_fileStream!.Write(line); + } + + private static void HexEncode(ReadOnlySpan source, Span destination) + { + for (int i = 0; i < source.Length; i++) + { + HexConverter.ToBytesBuffer(source[i], destination.Slice(i * 2)); + } + } + +} diff --git a/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj b/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj index aafca7f3e93b2a..8245e3304ad69c 100644 --- a/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj +++ b/src/libraries/System.Net.Quic/src/System.Net.Quic.csproj @@ -38,6 +38,7 @@ + diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicTlsSecret.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicTlsSecret.cs index 26f69c1653aa83..44f57b8b86bcfe 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicTlsSecret.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/Internal/MsQuicTlsSecret.cs @@ -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. -#if DEBUG using System.Diagnostics; using System.IO; using System.Runtime.InteropServices; @@ -13,14 +12,11 @@ namespace System.Net.Quic; internal sealed class MsQuicTlsSecret : IDisposable { - private static readonly string? s_keyLogFile = Environment.GetEnvironmentVariable("SSLKEYLOGFILE"); - private static readonly FileStream? s_fileStream = s_keyLogFile != null ? File.Open(s_keyLogFile, FileMode.Append, FileAccess.Write, FileShare.ReadWrite) : null; - private unsafe QUIC_TLS_SECRETS* _tlsSecrets; public static unsafe MsQuicTlsSecret? Create(MsQuicContextSafeHandle handle) { - if (s_fileStream is null) + if (!SslKeyLogger.IsEnabled) { return null; } @@ -55,40 +51,69 @@ private unsafe MsQuicTlsSecret(QUIC_TLS_SECRETS* tlsSecrets) public unsafe void WriteSecret() { - Debug.Assert(_tlsSecrets is not null); - Debug.Assert(s_fileStream is not null); + ReadOnlySpan clientRandom = _tlsSecrets->IsSet.ClientRandom != 0 + ? new ReadOnlySpan(_tlsSecrets->ClientRandom, 32) + : ReadOnlySpan.Empty; + + Span clientHandshakeTrafficSecret = _tlsSecrets->IsSet.ClientHandshakeTrafficSecret != 0 + ? new Span(_tlsSecrets->ClientHandshakeTrafficSecret, _tlsSecrets->SecretLength) + : Span.Empty; + + Span serverHandshakeTrafficSecret = _tlsSecrets->IsSet.ServerHandshakeTrafficSecret != 0 + ? new Span(_tlsSecrets->ServerHandshakeTrafficSecret, _tlsSecrets->SecretLength) + : Span.Empty; + + Span clientTrafficSecret0 = _tlsSecrets->IsSet.ClientTrafficSecret0 != 0 + ? new Span(_tlsSecrets->ClientTrafficSecret0, _tlsSecrets->SecretLength) + : Span.Empty; - lock (s_fileStream) + Span serverTrafficSecret0 = _tlsSecrets->IsSet.ServerTrafficSecret0 != 0 + ? new Span(_tlsSecrets->ServerTrafficSecret0, _tlsSecrets->SecretLength) + : Span.Empty; + + Span clientEarlyTrafficSecret = _tlsSecrets->IsSet.ClientEarlyTrafficSecret != 0 + ? new Span(_tlsSecrets->ClientEarlyTrafficSecret, _tlsSecrets->SecretLength) + : Span.Empty; + + SslKeyLogger.WriteSecrets( + clientRandom, + clientHandshakeTrafficSecret, + serverHandshakeTrafficSecret, + clientTrafficSecret0, + serverTrafficSecret0, + clientEarlyTrafficSecret); + + // clear secrets already logged, so they are not logged again on next call, + // keep ClientRandom as it is used for all secrets (and is not a secret itself) + if (!clientHandshakeTrafficSecret.IsEmpty) { - string clientRandom = string.Empty; - if (_tlsSecrets->IsSet.ClientRandom != 0) - { - clientRandom = Convert.ToHexString(new ReadOnlySpan(_tlsSecrets->ClientRandom, 32)); - } - if (_tlsSecrets->IsSet.ClientHandshakeTrafficSecret != 0) - { - s_fileStream.Write(Encoding.ASCII.GetBytes($"CLIENT_HANDSHAKE_TRAFFIC_SECRET {clientRandom} {Convert.ToHexString(new ReadOnlySpan(_tlsSecrets->ClientHandshakeTrafficSecret, _tlsSecrets->SecretLength))}\n")); - } - if (_tlsSecrets->IsSet.ServerHandshakeTrafficSecret != 0) - { - s_fileStream.Write(Encoding.ASCII.GetBytes($"SERVER_HANDSHAKE_TRAFFIC_SECRET {clientRandom} {Convert.ToHexString(new ReadOnlySpan(_tlsSecrets->ServerHandshakeTrafficSecret, _tlsSecrets->SecretLength))}\n")); - } - if (_tlsSecrets->IsSet.ClientTrafficSecret0 != 0) - { - s_fileStream.Write(Encoding.ASCII.GetBytes($"CLIENT_TRAFFIC_SECRET_0 {clientRandom} {Convert.ToHexString(new ReadOnlySpan(_tlsSecrets->ClientTrafficSecret0, _tlsSecrets->SecretLength))}\n")); - } - if (_tlsSecrets->IsSet.ServerTrafficSecret0 != 0) - { - s_fileStream.Write(Encoding.ASCII.GetBytes($"SERVER_TRAFFIC_SECRET_0 {clientRandom} {Convert.ToHexString(new ReadOnlySpan(_tlsSecrets->ServerTrafficSecret0, _tlsSecrets->SecretLength))}\n")); - } - if (_tlsSecrets->IsSet.ClientEarlyTrafficSecret != 0) - { - s_fileStream.Write(Encoding.ASCII.GetBytes($"CLIENT_EARLY_TRAFFIC_SECRET {clientRandom} {Convert.ToHexString(new ReadOnlySpan(_tlsSecrets->ClientEarlyTrafficSecret, _tlsSecrets->SecretLength))}\n")); - } - s_fileStream.Flush(); + clientHandshakeTrafficSecret.Clear(); + _tlsSecrets->IsSet.ClientHandshakeTrafficSecret = 0; } - NativeMemory.Clear(_tlsSecrets, (nuint)sizeof(QUIC_TLS_SECRETS)); + if (!serverHandshakeTrafficSecret.IsEmpty) + { + serverHandshakeTrafficSecret.Clear(); + _tlsSecrets->IsSet.ServerHandshakeTrafficSecret = 0; + } + + if (!clientTrafficSecret0.IsEmpty) + { + clientTrafficSecret0.Clear(); + _tlsSecrets->IsSet.ClientTrafficSecret0 = 0; + } + + if (!serverTrafficSecret0.IsEmpty) + { + serverTrafficSecret0.Clear(); + _tlsSecrets->IsSet.ServerTrafficSecret0 = 0; + } + + if (!clientEarlyTrafficSecret.IsEmpty) + { + clientEarlyTrafficSecret.Clear(); + _tlsSecrets->IsSet.ClientEarlyTrafficSecret = 0; + } } public unsafe void Dispose() @@ -110,4 +135,3 @@ public unsafe void Dispose() } } } -#endif diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs index 0846543a6aee66..315352d2797bb4 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs @@ -187,13 +187,11 @@ static async ValueTask StartConnectAsync(QuicClientConnectionOpt /// private SslApplicationProtocol _negotiatedApplicationProtocol; -#if DEBUG /// /// Will contain TLS secret after CONNECTED event is received and store it into SSLKEYLOGFILE. /// MsQuic holds the underlying pointer so this object can be disposed only after connection native handle gets closed. /// private readonly MsQuicTlsSecret? _tlsSecret; -#endif /// /// The remote endpoint used for this connection. @@ -254,9 +252,7 @@ private unsafe QuicConnection() throw; } -#if DEBUG _tlsSecret = MsQuicTlsSecret.Create(_handle); -#endif } /// @@ -284,9 +280,7 @@ internal unsafe QuicConnection(QUIC_HANDLE* handle, QUIC_NEW_CONNECTION_INFO* in _remoteEndPoint = MsQuicHelpers.QuicAddrToIPEndPoint(info->RemoteAddress); _localEndPoint = MsQuicHelpers.QuicAddrToIPEndPoint(info->LocalAddress); -#if DEBUG _tlsSecret = MsQuicTlsSecret.Create(_handle); -#endif } private async ValueTask FinishConnectAsync(QuicClientConnectionOptions options, CancellationToken cancellationToken = default) @@ -510,9 +504,8 @@ private unsafe int HandleEventConnected(ref CONNECTED_DATA data) QuicAddr localAddress = MsQuicHelpers.GetMsQuicParameter(_handle, QUIC_PARAM_CONN_LOCAL_ADDRESS); _localEndPoint = MsQuicHelpers.QuicAddrToIPEndPoint(&localAddress); -#if DEBUG + // Final (1-RTT) secrets have been derived, log them if desired to allow decrypting application traffic. _tlsSecret?.WriteSecret(); -#endif if (NetEventSource.Log.IsEnabled()) { @@ -535,6 +528,9 @@ private unsafe int HandleEventShutdownInitiatedByPeer(ref SHUTDOWN_INITIATED_BY_ } private unsafe int HandleEventShutdownComplete() { + // make sure we log at least some secrets in case of shutdown before handshake completes. + _tlsSecret?.WriteSecret(); + Exception exception = ExceptionDispatchInfo.SetCurrentStackTrace(_disposed == 1 ? new ObjectDisposedException(GetType().FullName) : ThrowHelper.GetOperationAbortedException()); _acceptQueue.Writer.TryComplete(exception); _connectedTcs.TrySetException(exception); @@ -578,6 +574,9 @@ private unsafe int HandleEventPeerCertificateReceived(ref PEER_CERTIFICATE_RECEI // worker threads. // + // Handshake keys should be available by now, log them now if desired. + _tlsSecret?.WriteSecret(); + var task = _sslConnectionOptions.StartAsyncCertificateValidation((IntPtr)data.Certificate, (IntPtr)data.Chain); if (task.IsCompletedSuccessfully) { diff --git a/src/libraries/System.Net.Quic/tests/FunctionalTests/MsQuicRemoteExecutorTests.cs b/src/libraries/System.Net.Quic/tests/FunctionalTests/MsQuicRemoteExecutorTests.cs index f57f4aef8ec670..d2a40a1f35af53 100644 --- a/src/libraries/System.Net.Quic/tests/FunctionalTests/MsQuicRemoteExecutorTests.cs +++ b/src/libraries/System.Net.Quic/tests/FunctionalTests/MsQuicRemoteExecutorTests.cs @@ -20,27 +20,43 @@ public class MsQuicRemoteExecutorTests : QuicTestBase public MsQuicRemoteExecutorTests() : base(null!) { } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - public void SslKeyLogFile_IsCreatedAndFilled() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [InlineData(true)] + [InlineData(false)] + public void SslKeyLogFile_IsCreatedAndFilled(bool enabledBySwitch) { - if (PlatformDetection.IsReleaseLibrary(typeof(QuicConnection).Assembly)) + if (PlatformDetection.IsDebugLibrary(typeof(QuicConnection).Assembly) && !enabledBySwitch) { - throw new SkipTestException("Retrieving SSL secrets is not supported in Release mode."); + // AppCtxSwitch is not checked for SSLKEYLOGFILE in Debug builds, the same code path + // will be tested by the enabledBySwitch = true case. Skip it here. + return; } var psi = new ProcessStartInfo(); var tempFile = Path.GetTempFileName(); psi.Environment.Add("SSLKEYLOGFILE", tempFile); - RemoteExecutor.Invoke(async () => + RemoteExecutor.Invoke(async (enabledBySwitch) => { + if (bool.Parse(enabledBySwitch)) + { + AppContext.SetSwitch("System.Net.EnableSslKeyLogging", true); + } + (QuicConnection clientConnection, QuicConnection serverConnection) = await CreateConnectedQuicConnection(); await clientConnection.DisposeAsync(); await serverConnection.DisposeAsync(); - }, new RemoteInvokeOptions { StartInfo = psi }).Dispose(); + } + , enabledBySwitch.ToString(), new RemoteInvokeOptions { StartInfo = psi }).Dispose(); - Assert.True(File.Exists(tempFile)); - Assert.True(File.ReadAllText(tempFile).Length > 0); + if (enabledBySwitch) + { + Assert.True(File.ReadAllText(tempFile).Length > 0); + } + else + { + Assert.True(File.ReadAllText(tempFile).Length == 0); + } } } } diff --git a/src/libraries/System.Net.Security/src/System.Net.Security.csproj b/src/libraries/System.Net.Security/src/System.Net.Security.csproj index 9a7fd09fd88b13..e6650f80670e17 100644 --- a/src/libraries/System.Net.Security/src/System.Net.Security.csproj +++ b/src/libraries/System.Net.Security/src/System.Net.Security.csproj @@ -387,6 +387,8 @@ Link="Common\Microsoft\Win32\SafeHandles\SafeHandleCache.cs" /> + diff --git a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamRemoteExecutorTests.cs b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamRemoteExecutorTests.cs index d162a54bf92172..82cd3114eb4772 100644 --- a/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamRemoteExecutorTests.cs +++ b/src/libraries/System.Net.Security/tests/FunctionalTests/SslStreamRemoteExecutorTests.cs @@ -21,21 +21,30 @@ public class SslStreamRemoteExecutorTests public SslStreamRemoteExecutorTests() { } - [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [ActiveIssue("https://github.com/dotnet/runtime/issues/94843", ~TestPlatforms.Linux)] - public void SslKeyLogFile_IsCreatedAndFilled() + [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [PlatformSpecific(TestPlatforms.Linux)] // SSLKEYLOGFILE is only supported on Linux for SslStream + [InlineData(true)] + [InlineData(false)] + public void SslKeyLogFile_IsCreatedAndFilled(bool enabledBySwitch) { - if (PlatformDetection.IsReleaseLibrary(typeof(SslStream).Assembly)) + if (PlatformDetection.IsDebugLibrary(typeof(SslStream).Assembly) && !enabledBySwitch) { - throw new SkipTestException("Retrieving SSL secrets is not supported in Release mode."); + // AppCtxSwitch is not checked for SSLKEYLOGFILE in Debug builds, the same code path + // will be tested by the enabledBySwitch = true case. Skip it here. + return; } var psi = new ProcessStartInfo(); var tempFile = Path.GetTempFileName(); psi.Environment.Add("SSLKEYLOGFILE", tempFile); - RemoteExecutor.Invoke(async () => + RemoteExecutor.Invoke(async (enabledBySwitch) => { + if (bool.Parse(enabledBySwitch)) + { + AppContext.SetSwitch("System.Net.EnableSslKeyLogging", true); + } + (Stream clientStream, Stream serverStream) = TestHelper.GetConnectedStreams(); using (clientStream) using (serverStream) @@ -55,10 +64,16 @@ await TestConfiguration.WhenAllOrAnyFailedWithTimeout( await TestHelper.PingPong(client, server); } - }, new RemoteInvokeOptions { StartInfo = psi }).Dispose(); + }, enabledBySwitch.ToString(), new RemoteInvokeOptions { StartInfo = psi }).Dispose(); - Assert.True(File.Exists(tempFile)); - Assert.True(File.ReadAllText(tempFile).Length > 0); + if (enabledBySwitch) + { + Assert.True(File.ReadAllText(tempFile).Length > 0); + } + else + { + Assert.True(File.ReadAllText(tempFile).Length == 0); + } } } -} \ No newline at end of file +} diff --git a/src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj b/src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj index e3e81ecb61a3d4..5602fc0cab3562 100644 --- a/src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj +++ b/src/libraries/System.Net.Security/tests/UnitTests/System.Net.Security.Unit.Tests.csproj @@ -121,6 +121,8 @@ Link="Common\Microsoft\Win32\SafeHandles\SafeHandleCache.cs" /> + From 36f0c6687799af52f7c079260214cb56d0d8a5e1 Mon Sep 17 00:00:00 2001 From: Ahmet Ibrahim Aksoy Date: Wed, 10 Apr 2024 15:02:12 +0200 Subject: [PATCH 02/11] Add Log to SendStreamLimit Http3 Test (#100857) * Add Log to SendStreamLimit Http3 Test * Add one more log and fix timestamps --- .../tests/FunctionalTests/HttpClientHandlerTest.Http3.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs index 0af7c66ac8ffcc..16abc260386b42 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs @@ -137,6 +137,7 @@ public async Task SendStreamLimitRequestsConcurrently_Succeeds(int streamLimit) { await using Http3LoopbackStream stream = await connection.AcceptRequestStreamAsync(); await stream.HandleRequestAsync(); + _output.WriteLine($"[{DateTime.Now:HH:mm:ss.fffffff}] Server: Finished request {i}"); } }); @@ -156,9 +157,11 @@ public async Task SendStreamLimitRequestsConcurrently_Succeeds(int streamLimit) }; tasks[i] = client.SendAsync(request); + _output.WriteLine($"[{DateTime.Now:HH:mm:ss.fffffff}] Client: Started request {i}"); }); var responses = await Task.WhenAll(tasks); + _output.WriteLine($"[{DateTime.Now:HH:mm:ss.fffffff}] Client: Got all responses"); foreach (var response in responses) { response.Dispose(); From e33b83226ba025b6da5f3b9a0de60dc4986e486d Mon Sep 17 00:00:00 2001 From: Jo Shields Date: Wed, 10 Apr 2024 09:22:27 -0400 Subject: [PATCH 03/11] Enable passing through special LLVM build modes from VMR outer build (#100822) --- eng/DotNetBuild.props | 3 +++ 1 file changed, 3 insertions(+) diff --git a/eng/DotNetBuild.props b/eng/DotNetBuild.props index 06ea2ee0466575..52abdf92f094a2 100644 --- a/eng/DotNetBuild.props +++ b/eng/DotNetBuild.props @@ -65,6 +65,9 @@ $(InnerBuildArgs) /p:WasmEnableThreads=true $(InnerBuildArgs) $(FlagParameterPrefix)s clr.nativeaotlibs+clr.nativeaotruntime+libs+packs /p:BuildNativeAOTRuntimePack=true /p:SkipLibrariesNativeRuntimePackages=true + $(InnerBuildArgs) /p:MonoEnableLLVM=$(DotNetBuildMonoEnableLLVM) + $(InnerBuildArgs) /p:MonoAOTEnableLLVM=$(DotNetBuildMonoAOTEnableLLVM) + $(InnerBuildArgs) /p:MonoBundleLLVMOptimizer=$(DotNetBuildMonoBundleLLVMOptimizer) $(InnerBuildArgs) $(FlagParameterPrefix)pgoinstrument From b229723b2b110f3e948c65c3065fb48da27fab6d Mon Sep 17 00:00:00 2001 From: Pavel Savara Date: Wed, 10 Apr 2024 15:50:03 +0200 Subject: [PATCH 04/11] [browser] unhandled JS exceptions (#100855) --- src/mono/browser/runtime/invoke-js.ts | 4 +-- src/mono/browser/runtime/pthreads/shared.ts | 6 +++- src/mono/browser/runtime/scheduling.ts | 31 +++++++++++++++------ 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/mono/browser/runtime/invoke-js.ts b/src/mono/browser/runtime/invoke-js.ts index fa6949f2d636aa..c5d385a17e4e44 100644 --- a/src/mono/browser/runtime/invoke-js.ts +++ b/src/mono/browser/runtime/invoke-js.ts @@ -59,8 +59,8 @@ export function mono_wasm_invoke_jsimport_MT (signature: JSFunctionSignature, ar } } return; - } catch (ex2: any) { - runtimeHelpers.nativeAbort(ex2); + } catch (ex: any) { + loaderHelpers.mono_exit(1, ex); return; } } diff --git a/src/mono/browser/runtime/pthreads/shared.ts b/src/mono/browser/runtime/pthreads/shared.ts index c266c2dfec66e4..69137957eeeebe 100644 --- a/src/mono/browser/runtime/pthreads/shared.ts +++ b/src/mono/browser/runtime/pthreads/shared.ts @@ -77,7 +77,11 @@ export function exec_synchronization_context_pump (): void { return; } forceThreadMemoryViewRefresh(); - tcwraps.mono_wasm_synchronization_context_pump(); + try { + tcwraps.mono_wasm_synchronization_context_pump(); + } catch (ex) { + loaderHelpers.mono_exit(1, ex); + } } export function mono_wasm_schedule_synchronization_context (): void { diff --git a/src/mono/browser/runtime/scheduling.ts b/src/mono/browser/runtime/scheduling.ts index 35552123845ada..0be8cb19a72336 100644 --- a/src/mono/browser/runtime/scheduling.ts +++ b/src/mono/browser/runtime/scheduling.ts @@ -35,20 +35,28 @@ function prevent_timer_throttling_tick () { if (!loaderHelpers.is_runtime_running()) { return; } - cwraps.mono_wasm_execute_timer(); - pump_count++; + try { + cwraps.mono_wasm_execute_timer(); + pump_count++; + } catch (ex) { + loaderHelpers.mono_exit(1, ex); + } mono_background_exec_until_done(); } function mono_background_exec_until_done () { if (WasmEnableThreads) return; Module.maybeExit(); - if (!loaderHelpers.is_runtime_running()) { - return; - } - while (pump_count > 0) { - --pump_count; - cwraps.mono_background_exec(); + try { + while (pump_count > 0) { + --pump_count; + if (!loaderHelpers.is_runtime_running()) { + return; + } + cwraps.mono_background_exec(); + } + } catch (ex) { + loaderHelpers.mono_exit(1, ex); } } @@ -78,5 +86,10 @@ function mono_wasm_schedule_timer_tick () { return; } lastScheduledTimeoutId = undefined; - cwraps.mono_wasm_execute_timer(); + try { + cwraps.mono_wasm_execute_timer(); + pump_count++; + } catch (ex) { + loaderHelpers.mono_exit(1, ex); + } } From dc2fa66903488778a9549847cf07bcd8d115a1fb Mon Sep 17 00:00:00 2001 From: t-mustafin <66252296+t-mustafin@users.noreply.github.com> Date: Wed, 10 Apr 2024 19:25:18 +0300 Subject: [PATCH 05/11] Restore StubSecretArg from stack (#100428) Fixes #100301 --- src/coreclr/jit/lower.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 0427cf666a3bcb..ea061b9ef75b76 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -5616,8 +5616,16 @@ void Lowering::InsertPInvokeMethodProlog() call->gtArgs.PushBack(comp, frameAddrArg); // for x86/arm32 don't pass the secretArg. #if !defined(TARGET_X86) && !defined(TARGET_ARM) - NewCallArg stubParamArg = - NewCallArg::Primitive(PhysReg(REG_SECRET_STUB_PARAM)).WellKnown(WellKnownArg::SecretStubParam); + GenTree* argNode; + if (comp->info.compPublishStubParam) + { + argNode = comp->gtNewLclvNode(comp->lvaStubArgumentVar, TYP_I_IMPL); + } + else + { + argNode = comp->gtNewIconNode(0, TYP_I_IMPL); + } + NewCallArg stubParamArg = NewCallArg::Primitive(argNode).WellKnown(WellKnownArg::SecretStubParam); call->gtArgs.PushBack(comp, stubParamArg); #endif From fd96b8044f80d327fd09fcfffe5c8a509f69bab1 Mon Sep 17 00:00:00 2001 From: Tomas Weinfurt Date: Wed, 10 Apr 2024 09:41:45 -0700 Subject: [PATCH 06/11] enable AutomaticOrManual_DoesntFailRegardlessOfWhetherClientCertsAreAvailable test (#100496) --- .../System/Net/Http/HttpClientHandlerTest.ClientCertificates.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.ClientCertificates.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.ClientCertificates.cs index 04a7414d046951..373928b90792b2 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.ClientCertificates.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.ClientCertificates.cs @@ -185,7 +185,6 @@ await LoopbackServer.CreateServerAsync(async (server, url) => }, options); } - [ActiveIssue("https://github.com/dotnet/runtime/issues/29419")] [Theory] [InlineData(ClientCertificateOption.Manual)] [InlineData(ClientCertificateOption.Automatic)] From 242a6aaa60d4e311daa30728e146172040a749f5 Mon Sep 17 00:00:00 2001 From: Thays Grazia Date: Wed, 10 Apr 2024 14:15:13 -0300 Subject: [PATCH 07/11] Disabling this test on Firefox. (#100868) Removing the other test it's flaky and this will not be responsability of BrowserDebugProxy anymore. --- .../EvaluateOnCallFrameTests.cs | 2 +- .../DebuggerTestSuite/SteppingTests.cs | 54 ------------------- 2 files changed, 1 insertion(+), 55 deletions(-) diff --git a/src/mono/browser/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs b/src/mono/browser/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs index 82ea36992d341b..5e099a787bc466 100644 --- a/src/mono/browser/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs +++ b/src/mono/browser/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs @@ -216,7 +216,7 @@ await EvaluateOnCallFrameAndCheck(id, ($"local_dt.Date.Year * 10", TNumber(10))); }); - [Theory] + [ConditionalTheory(nameof(RunningOnChrome))] [InlineData("")] [InlineData("this.")] public async Task InheritedAndPrivateMembersInAClass(string prefix) diff --git a/src/mono/browser/debugger/DebuggerTestSuite/SteppingTests.cs b/src/mono/browser/debugger/DebuggerTestSuite/SteppingTests.cs index 0074c536db81a7..2d7f86b950ee97 100644 --- a/src/mono/browser/debugger/DebuggerTestSuite/SteppingTests.cs +++ b/src/mono/browser/debugger/DebuggerTestSuite/SteppingTests.cs @@ -1166,60 +1166,6 @@ await StepAndCheck(StepKind.Into, "dotnet://Newtonsoft.Json.dll/JArray.cs", 350, ); } - [ConditionalTheory(nameof(RunningOnChrome))] - [InlineData("https://symbols.nuget.org/download/symbols", "")] - // Symbols are already loaded, so setting urls = [] won't affect it - [InlineData] - [InlineData("", "https://microsoft.com/non-existant/symbols")] - public async Task SteppingIntoLibrarySymbolsLoadedFromSymbolServerRemoveSymbolServerAndStepAgain(params string[] secondServers) - { - string cachePath = _env.CreateTempDirectory("symbols-cache"); - _testOutput.WriteLine($"Using cachePath: {cachePath}"); - var searchPaths = new JArray - { - "https://symbols.nuget.org/download/symbols", - "https://msdl.microsoft.com/download/bad-non-existant", - "https://msdl.microsoft.com/download/symbols" - }; - var waitForScript = WaitForScriptParsedEventsAsync(new string [] { "JArray.cs" }); - var symbolOptions = JObject.FromObject(new { symbolOptions = JObject.FromObject(new { cachePath, searchPaths })}); - await SetJustMyCode(false); - await SetSymbolOptions(symbolOptions); - - await EvaluateAndCheck( - "window.setTimeout(function() { invoke_static_method ('[debugger-test] TestLoadSymbols:Run'); invoke_static_method ('[debugger-test] TestLoadSymbols:Run'); }, 1);", - "dotnet://debugger-test.dll/debugger-test.cs", 1572, 8, - "TestLoadSymbols.Run" - ); - - await waitForScript; - - await StepAndCheck(StepKind.Into, "dotnet://Newtonsoft.Json.dll/JArray.cs", 350, 12, "Newtonsoft.Json.Linq.JArray.Add", - locals_fn: async (locals) => - { - await CheckObject(locals, "this", "Newtonsoft.Json.Linq.JArray", description: "[]"); - }, times: 2 - ); - searchPaths.Clear(); - foreach (string secondServer in secondServers) - searchPaths.Add(secondServer); - - symbolOptions = JObject.FromObject(new { symbolOptions = JObject.FromObject(new { cachePath, searchPaths })}); - await SetSymbolOptions(symbolOptions); - - await SendCommandAndCheck(null, "Debugger.resume", - "dotnet://debugger-test.dll/debugger-test.cs", 1572, 8, - "TestLoadSymbols.Run" - ); - - await StepAndCheck(StepKind.Into, "dotnet://Newtonsoft.Json.dll/JArray.cs", 350, 12, "Newtonsoft.Json.Linq.JArray.Add", - locals_fn: async (locals) => - { - await CheckObject(locals, "this", "Newtonsoft.Json.Linq.JArray", description: "[]"); - }, times: 2 - ); - } - [ConditionalTheory(nameof(RunningOnChrome))] [InlineData(true)] [InlineData(false)] From f8624eed1c9e71f1ffd0d3b93a579b5641b214d6 Mon Sep 17 00:00:00 2001 From: Andy Gocke Date: Wed, 10 Apr 2024 10:19:02 -0700 Subject: [PATCH 08/11] Import pool providers (#100851) Import pool-providers in common variables. This should allow all stages access to the pool provider variables. --- eng/pipelines/common/variables.yml | 7 +++++++ eng/pipelines/runtime-official.yml | 2 ++ 2 files changed, 9 insertions(+) diff --git a/eng/pipelines/common/variables.yml b/eng/pipelines/common/variables.yml index f868c3d953fb1e..be6ee86df631e4 100644 --- a/eng/pipelines/common/variables.yml +++ b/eng/pipelines/common/variables.yml @@ -1,3 +1,8 @@ +parameters: + - name: templatePath + type: string + default: 'templates' + variables: # These values enable longer delays, configurable number of retries, and special understanding of TCP hang-up @@ -54,3 +59,5 @@ variables: eq(variables['isRollingBuild'], true))) ] - template: /eng/pipelines/common/perf-variables.yml + +- template: /eng/common/${{ parameters.templatePath }}/variables/pool-providers.yml \ No newline at end of file diff --git a/eng/pipelines/runtime-official.yml b/eng/pipelines/runtime-official.yml index ba09a957605eb9..0e17c0aefeb4e8 100644 --- a/eng/pipelines/runtime-official.yml +++ b/eng/pipelines/runtime-official.yml @@ -23,6 +23,8 @@ pr: none variables: - template: /eng/pipelines/common/variables.yml + parameters: + templatePath: 'templates-official' - template: /eng/pipelines/common/internal-variables.yml parameters: teamName: dotnet-core-acquisition From b8803cb2790ca4beb7f47bd87d78560db6b80d01 Mon Sep 17 00:00:00 2001 From: Katelyn Gadd Date: Wed, 10 Apr 2024 10:27:43 -0700 Subject: [PATCH 09/11] Optimize mono_metadata_type_dup_with_cmods (#100849) mono_metadata_type_dup_with_cmods is a minor hotspot during WASM startup, so apply some minor optimizations to it --- src/mono/mono/metadata/metadata.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/mono/mono/metadata/metadata.c b/src/mono/mono/metadata/metadata.c index f837c66d9b471e..0453de9828395e 100644 --- a/src/mono/mono/metadata/metadata.c +++ b/src/mono/mono/metadata/metadata.c @@ -6201,17 +6201,26 @@ mono_metadata_type_dup_with_cmods (MonoImage *image, const MonoType *o, const Mo uint8_t num_mods = MAX (mono_type_custom_modifier_count (o), mono_type_custom_modifier_count (cmods_source)); gboolean aggregate = mono_type_is_aggregate_mods (o) || mono_type_is_aggregate_mods (cmods_source); - size_t sizeof_r = mono_sizeof_type_with_mods (num_mods, aggregate); + size_t sizeof_r = mono_sizeof_type_with_mods (num_mods, aggregate), + sizeof_o = mono_sizeof_type (o), + sizeof_cmods_source = cmods_source->has_cmods ? mono_sizeof_type (cmods_source) : 0, + sizeof_header = MAX(sizeof_o, sizeof_cmods_source); + uint8_t *r_bytes = NULL; - r = image ? (MonoType *)mono_image_alloc0 (image, (guint)sizeof_r) : (MonoType *)g_malloc0 (sizeof_r); + r = image ? (MonoType *)mono_image_alloc (image, (guint)sizeof_r) : (MonoType *)g_malloc (sizeof_r); + r_bytes = (uint8_t *)r; if (cmods_source->has_cmods) { /* FIXME: if it's aggregate what do we assert here? */ g_assert (!image || (!aggregate && image == mono_type_get_cmods (cmods_source)->image)); - memcpy (r, cmods_source, mono_sizeof_type (cmods_source)); + // Copy the portion of the source type that won't be overwritten by o below + memcpy (r_bytes + sizeof_o, ((uint8_t *)cmods_source) + sizeof_o, sizeof_cmods_source - sizeof_o); } - memcpy (r, o, mono_sizeof_type (o)); + memcpy (r, o, sizeof_o); + // Zero the remaining uninitialized bytes + if (sizeof_r > sizeof_header) + memset (r_bytes + sizeof_header, 0, sizeof_r - sizeof_header); /* reset custom mod count and aggregateness to be correct. */ mono_type_with_mods_init (r, num_mods, aggregate); From 674ba3fade49e5d617bc3ad3a555558572532593 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 10 Apr 2024 19:32:06 +0200 Subject: [PATCH 10/11] JIT: Track sideness of arrOp in GetCheckedBoundArithInfo (#100848) --- src/coreclr/jit/optcse.cpp | 8 +++++-- src/coreclr/jit/valuenum.cpp | 14 ++++++----- src/coreclr/jit/valuenum.h | 2 ++ .../JitBlue/Runtime_100809/Runtime_100809.cs | 23 +++++++++++++++++++ .../Runtime_100809/Runtime_100809.csproj | 5 ++++ 5 files changed, 44 insertions(+), 8 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_100809/Runtime_100809.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_100809/Runtime_100809.csproj diff --git a/src/coreclr/jit/optcse.cpp b/src/coreclr/jit/optcse.cpp index 41b15792e24ae1..7fcaac35c43b45 100644 --- a/src/coreclr/jit/optcse.cpp +++ b/src/coreclr/jit/optcse.cpp @@ -4813,8 +4813,12 @@ void CSE_HeuristicCommon::PerformCSE(CSE_Candidate* successfulCandidate) assert(vnStore->IsVNCompareCheckedBoundArith(oldCmpVN)); vnStore->GetCompareCheckedBoundArithInfo(oldCmpVN, &info); - newCmpArgVN = vnStore->VNForFunc(vnStore->TypeOfVN(info.arrOp), (VNFunc)info.arrOper, - info.arrOp, theConservativeVN); + + ValueNum arrOp1 = info.arrOpLHS ? info.arrOp : theConservativeVN; + ValueNum arrOp2 = info.arrOpLHS ? theConservativeVN : info.arrOp; + + newCmpArgVN = + vnStore->VNForFunc(vnStore->TypeOfVN(info.arrOp), (VNFunc)info.arrOper, arrOp1, arrOp2); } ValueNum newCmpVN = vnStore->VNForFunc(vnStore->TypeOfVN(oldCmpVN), (VNFunc)info.cmpOper, info.cmpOp, newCmpArgVN); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 7a6136577ddba3..65239b356e461f 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -6669,15 +6669,17 @@ void ValueNumStore::GetCheckedBoundArithInfo(ValueNum vn, CompareCheckedBoundAri bool isOp1CheckedBound = IsVNCheckedBound(funcArith.m_args[1]); if (isOp1CheckedBound) { - info->arrOper = funcArith.m_func; - info->arrOp = funcArith.m_args[0]; - info->vnBound = funcArith.m_args[1]; + info->arrOper = funcArith.m_func; + info->arrOp = funcArith.m_args[0]; + info->vnBound = funcArith.m_args[1]; + info->arrOpLHS = true; } else { - info->arrOper = funcArith.m_func; - info->arrOp = funcArith.m_args[1]; - info->vnBound = funcArith.m_args[0]; + info->arrOper = funcArith.m_func; + info->arrOp = funcArith.m_args[1]; + info->vnBound = funcArith.m_args[0]; + info->arrOpLHS = false; } } diff --git a/src/coreclr/jit/valuenum.h b/src/coreclr/jit/valuenum.h index 1f9171e13cef21..799d36472b00c9 100644 --- a/src/coreclr/jit/valuenum.h +++ b/src/coreclr/jit/valuenum.h @@ -932,12 +932,14 @@ class ValueNumStore ValueNum vnBound; unsigned arrOper; ValueNum arrOp; + bool arrOpLHS; // arrOp is on the left side of cmpOp expression unsigned cmpOper; ValueNum cmpOp; CompareCheckedBoundArithInfo() : vnBound(NoVN) , arrOper(GT_NONE) , arrOp(NoVN) + , arrOpLHS(false) , cmpOper(GT_NONE) , cmpOp(NoVN) { diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_100809/Runtime_100809.cs b/src/tests/JIT/Regression/JitBlue/Runtime_100809/Runtime_100809.cs new file mode 100644 index 00000000000000..eba4e112918c30 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_100809/Runtime_100809.cs @@ -0,0 +1,23 @@ +// 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 System.Runtime.CompilerServices; +using Xunit; + +public static class Runtime_100809 +{ + [Fact] + public static int TestEntryPoint() + { + return AlwaysFalse(96) ? -1 : 100; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static bool AlwaysFalse(int x) + { + var result = new byte[x]; + int count = result.Length - 2; + return (x < 0 || result.Length - count < 0); + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_100809/Runtime_100809.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_100809/Runtime_100809.csproj new file mode 100644 index 00000000000000..6c8c63b83414ad --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_100809/Runtime_100809.csproj @@ -0,0 +1,5 @@ + + + + + \ No newline at end of file From 4d5e5ffcf3ccec21749c856b8c734410eae6d478 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Wed, 10 Apr 2024 22:17:39 +0200 Subject: [PATCH 11/11] Align up structs to IntPtr.Size if they have gc pointers (#100289) --- .../Common/MetadataFieldLayoutAlgorithm.cs | 32 +-- .../InstanceFieldLayoutTests.cs | 2 +- .../Regressions/100220/Runtime_100220.cs | 190 ++++++++++++++++++ .../Regressions/100220/Runtime_100220.csproj | 8 + 4 files changed, 217 insertions(+), 15 deletions(-) create mode 100644 src/tests/Loader/classloader/explicitlayout/Regressions/100220/Runtime_100220.cs create mode 100644 src/tests/Loader/classloader/explicitlayout/Regressions/100220/Runtime_100220.csproj diff --git a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs index 92bb1126bba841..1ff2ad218cbdd4 100644 --- a/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs +++ b/src/coreclr/tools/Common/TypeSystem/Common/MetadataFieldLayoutAlgorithm.cs @@ -965,7 +965,7 @@ private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt { SizeAndAlignment result; - // Pad the length of structs to be 1 if they are empty so we have no zero-length structures + // Pad the length of structs to be 1 if they are empty, so we have no zero-length structures if (type.IsValueType && instanceSize == LayoutInt.Zero) { instanceSize = LayoutInt.One; @@ -973,25 +973,29 @@ private static SizeAndAlignment ComputeInstanceSize(MetadataType type, LayoutInt TargetDetails target = type.Context.Target; - if (classLayoutSize != 0) + if (type.IsValueType) { - LayoutInt parentSize; - if (type.IsValueType) - parentSize = new LayoutInt(0); - else - parentSize = type.BaseType.InstanceByteCountUnaligned; - - LayoutInt specifiedInstanceSize = parentSize + new LayoutInt(classLayoutSize); + if (classLayoutSize != 0) + { + instanceSize = LayoutInt.Max(new LayoutInt(classLayoutSize), instanceSize); - instanceSize = LayoutInt.Max(specifiedInstanceSize, instanceSize); - } - else - { - if (type.IsValueType) + // Size of a struct with gc references is always expected to be aligned to pointer size + if (type.ContainsGCPointers) + { + instanceSize = LayoutInt.AlignUp(instanceSize, new LayoutInt(target.PointerSize), target); + } + } + else { instanceSize = LayoutInt.AlignUp(instanceSize, alignment, target); } } + else if (classLayoutSize != 0 && type.IsSequentialLayout && !type.ContainsGCPointers) + { + // For classes, we respect classLayoutSize only for SequentialLayout + no gc fields + LayoutInt specifiedInstanceSize = type.BaseType.InstanceByteCountUnaligned + new LayoutInt(classLayoutSize); + instanceSize = LayoutInt.Max(specifiedInstanceSize, instanceSize); + } if (type.IsValueType) { diff --git a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/InstanceFieldLayoutTests.cs b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/InstanceFieldLayoutTests.cs index 8c49d5ad91b818..e2facd9e9923dc 100644 --- a/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/InstanceFieldLayoutTests.cs +++ b/src/coreclr/tools/aot/ILCompiler.TypeSystem.Tests/InstanceFieldLayoutTests.cs @@ -76,7 +76,7 @@ public void TestExplicitLayoutThatIsEmpty() public void TestExplicitTypeLayoutWithSize() { var explicitSizeType = _testModule.GetType("Explicit", "ExplicitSize"); - Assert.Equal(48, explicitSizeType.InstanceByteCount.AsInt); + Assert.Equal(32, explicitSizeType.InstanceByteCount.AsInt); } [Fact] diff --git a/src/tests/Loader/classloader/explicitlayout/Regressions/100220/Runtime_100220.cs b/src/tests/Loader/classloader/explicitlayout/Regressions/100220/Runtime_100220.cs new file mode 100644 index 00000000000000..0aa5d67f4839a0 --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/Regressions/100220/Runtime_100220.cs @@ -0,0 +1,190 @@ +// 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 System.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using Xunit; + +public static class Runtime_100220 +{ + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/100220", TestRuntimes.Mono)] + // Also, Mono needs RuntimeHelpers.GetRawObjectDataSize or equivalent to get an object size + public static int TestEntryPoint() + { + if (IntPtr.Size == 8) + { + // Classes + Assert.Equal(16, ObjectSize()); + Assert.Equal(16, ObjectSize()); + Assert.Equal(16, ObjectSize()); + Assert.Equal(16, ObjectSize()); + Assert.Equal(1000, ObjectSize()); + Assert.Equal(24, ObjectSize()); + Assert.Equal(40, ObjectSize()); + Assert.Equal(48, ObjectSize()); + Assert.Equal(32, ObjectSize()); + Assert.Equal(16, ObjectSize()); + + // Structs + Assert.Equal(16, Unsafe.SizeOf()); + Assert.Equal(16, Unsafe.SizeOf()); + Assert.Equal(16, Unsafe.SizeOf()); + Assert.Equal(16, Unsafe.SizeOf()); + Assert.Equal(1000, Unsafe.SizeOf()); + } + else + { + // Classes + Assert.Equal(8, ObjectSize()); + Assert.Equal(12, ObjectSize()); + Assert.Equal(8, ObjectSize()); + Assert.Equal(12, ObjectSize()); + Assert.Equal(1000, ObjectSize()); + Assert.Equal(20, ObjectSize()); + Assert.Equal(36, ObjectSize()); + Assert.Equal(44, ObjectSize()); + Assert.Equal(28, ObjectSize()); + Assert.Equal(8, ObjectSize()); + + // Structs + Assert.Equal(8, Unsafe.SizeOf()); + Assert.Equal(8, Unsafe.SizeOf()); + Assert.Equal(12, Unsafe.SizeOf()); + Assert.Equal(8, Unsafe.SizeOf()); + Assert.Equal(1000, Unsafe.SizeOf()); + } + + // Field offsets: + Assert.Equal("(5, 3, 4, 1, 2, 6)", FieldOffsets(new SubclassSubclassSeq { A = 1, B = 2, C = 3, D = 4, E = 5, F = 6 }).ToString()); + return 100; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static object FieldOffsets(SubclassSubclassSeq f) => (f.E, f.C, f.D, f.A, f.B, f.F); + + [StructLayout(LayoutKind.Sequential, Size = 20)] + public class BaseClassSeq + { + public byte A; + public nint B; + } + + [StructLayout(LayoutKind.Sequential, Size = 20)] + public class BaseClassWithGcSeq + { + public byte A; + public string B; + } + + public class SubclassOfBaseWithGcSeq : BaseClassWithGcSeq + { + public byte C; + } + + [StructLayout(LayoutKind.Sequential, Size = 16)] + public class SubclassSeq : BaseClassSeq + { + public byte C; + public nint D; + } + + [StructLayout(LayoutKind.Sequential, Size = 32)] + public class SubclassWithGcSeq : BaseClassSeq + { + public byte C; + public object D; + } + + public class SubclassSubclassSeq : SubclassSeq + { + public byte E; + public nint F; + } + + [MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.AggressiveOptimization)] + private static int ObjectSize() where T : new() + { + return (int)(nuint)typeof(RuntimeHelpers) + .GetMethod("GetRawObjectDataSize", BindingFlags.Static | BindingFlags.NonPublic) + .Invoke(null, [new T()]); + } + + private class MyClassAuto + { + private byte[] a; + private byte b; + } + + [StructLayout(LayoutKind.Explicit, Size = 1000)] + private class MyClass1000Exp + { + [FieldOffset(0)] + private byte[] a; + [FieldOffset(8)] + private byte b; + } + + [StructLayout(LayoutKind.Sequential, Size = 1000)] + private class MyClass1000Seq + { + private byte[] a; + private byte b; + } + + [StructLayout(LayoutKind.Explicit, Size = 1000)] + private class MyClass1000NoGcExp + { + [FieldOffset(0)] + private nint a; + [FieldOffset(8)] + private byte b; + } + + [StructLayout(LayoutKind.Sequential, Size = 1000)] + private class MyClass1000NoGcSeq + { + private nint a; + private byte b; + } + + private struct MyStructAuto + { + private byte[] a; + private byte b; + } + + [StructLayout(LayoutKind.Explicit, Size = 1000)] + private struct MyStruct1000Exp + { + [FieldOffset(0)] + private byte[] a; + [FieldOffset(8)] + private byte b; + } + + [StructLayout(LayoutKind.Sequential, Size = 1000)] + private struct MyStruct1000Seq + { + private byte[] a; + private byte b; + } + + [StructLayout(LayoutKind.Explicit, Size = 9)] + private struct MyStruct9Exp + { + [FieldOffset(0)] + private byte[] a; + [FieldOffset(8)] + private byte b; + } + + [StructLayout(LayoutKind.Sequential, Size = 9)] + private struct MyStruct9Seq + { + private byte[] a; + private byte b; + } +} diff --git a/src/tests/Loader/classloader/explicitlayout/Regressions/100220/Runtime_100220.csproj b/src/tests/Loader/classloader/explicitlayout/Regressions/100220/Runtime_100220.csproj new file mode 100644 index 00000000000000..efc38a9429de55 --- /dev/null +++ b/src/tests/Loader/classloader/explicitlayout/Regressions/100220/Runtime_100220.csproj @@ -0,0 +1,8 @@ + + + true + + + + +