diff --git a/src/coreclr/vm/peimagelayout.cpp b/src/coreclr/vm/peimagelayout.cpp index 5442b4d17e9cfe..803f1d34786d06 100644 --- a/src/coreclr/vm/peimagelayout.cpp +++ b/src/coreclr/vm/peimagelayout.cpp @@ -877,17 +877,29 @@ void* FlatImageLayout::LoadImageByCopyingParts(SIZE_T* m_imageParts) const #endif // FEATURE_ENABLE_NO_ADDRESS_SPACE_RANDOMIZATION DWORD allocationType = MEM_RESERVE | MEM_COMMIT; + DWORD initialProtection = PAGE_READWRITE; #if defined(HOST_UNIX) && defined(FEATURE_DYNAMIC_CODE_COMPILED) // Tell PAL to use the executable memory allocator to satisfy this request for virtual memory. // This is required on MacOS and otherwise will allow us to place native R2R code close to the // coreclr library and thus improve performance by avoiding jump stubs in managed code. allocationType |= MEM_RESERVE_EXECUTABLE; #endif +#if defined(HOST_OSX) && defined(HOST_ARM64) + // On Apple Silicon, allocating the image region as plain PAGE_READWRITE and then promoting + // executable sections to PAGE_EXECUTE_READ via mprotect has been observed to + // intermittently leave pages non-executable at the kernel level. This manifests as sporadic + // AccessViolationException crashes. + // + // Avoid the unreliable transition - reserve the whole region as PAGE_NOACCESS first, then + // commit each section directly with its final protection. + allocationType &= ~MEM_COMMIT; + initialProtection = PAGE_NOACCESS; +#endif COUNT_T allocSize = ALIGN_UP(this->GetVirtualSize(), g_SystemInfo.dwAllocationGranularity); - LPVOID base = ClrVirtualAlloc(preferredBase, allocSize, allocationType, PAGE_READWRITE); + LPVOID base = ClrVirtualAlloc(preferredBase, allocSize, allocationType, initialProtection); if (base == NULL && preferredBase != NULL) - base = ClrVirtualAlloc(NULL, allocSize, allocationType, PAGE_READWRITE); + base = ClrVirtualAlloc(NULL, allocSize, allocationType, initialProtection); if (base == NULL) ThrowLastError(); @@ -895,6 +907,56 @@ void* FlatImageLayout::LoadImageByCopyingParts(SIZE_T* m_imageParts) const // when loading by copying we have only one part to free. m_imageParts[0] = AllocatedPart(base); + IMAGE_SECTION_HEADER* sectionStart = IMAGE_FIRST_SECTION(FindNTHeaders()); + IMAGE_SECTION_HEADER* sectionEnd = sectionStart + VAL16(FindNTHeaders()->FileHeader.NumberOfSections); + +#if defined(HOST_OSX) && defined(HOST_ARM64) + _ASSERTE((allocationType & MEM_COMMIT) == 0); + _ASSERTE(initialProtection != PAGE_READWRITE); + + DWORD sizeOfHeaders = VAL32(FindNTHeaders()->OptionalHeader.SizeOfHeaders); + + // Commit and copy headers, then apply read-only protection. + if (ClrVirtualAlloc(base, sizeOfHeaders, MEM_COMMIT, PAGE_READWRITE) == NULL) + ThrowLastError(); + + CopyMemory(base, (void*)GetBase(), sizeOfHeaders); + + DWORD oldProtection; // PAL layer doesn't properly set the previous protection, so we don't try to validate it here. + if (!ClrVirtualProtect((void*)base, sizeOfHeaders, PAGE_READONLY, &oldProtection)) + ThrowLastError(); + + // Commit and copy each section with its desired protection. + for (IMAGE_SECTION_HEADER* section = sectionStart; section < sectionEnd; section++) + { + DWORD virtualSize = VAL32(section->Misc.VirtualSize); + if (virtualSize == 0) + continue; + + bool isExec = (section->Characteristics & IMAGE_SCN_MEM_EXECUTE) != 0; + BYTE* sectionBase = (BYTE*)base + VAL32(section->VirtualAddress); + DWORD copySize = min(VAL32(section->SizeOfRawData), virtualSize); + + if (ClrVirtualAlloc(sectionBase, virtualSize, MEM_COMMIT, isExec ? PAGE_EXECUTE_READWRITE : PAGE_READWRITE) == NULL) + ThrowLastError(); + + if (isExec) + PAL_JitWriteProtect(true); + + CopyMemory(sectionBase, (BYTE*)GetBase() + VAL32(section->PointerToRawData), copySize); + + if (isExec) + { + PAL_JitWriteProtect(false); + } + else if ((section->Characteristics & IMAGE_SCN_MEM_WRITE) == 0) + { + DWORD oldProt; + if (!ClrVirtualProtect(sectionBase, virtualSize, PAGE_READONLY, &oldProt)) + ThrowLastError(); + } + } +#else // We're going to copy everything first, and write protect what we need to later. // First, copy headers @@ -902,9 +964,6 @@ void* FlatImageLayout::LoadImageByCopyingParts(SIZE_T* m_imageParts) const // Now, copy all sections to appropriate virtual address - IMAGE_SECTION_HEADER* sectionStart = IMAGE_FIRST_SECTION(FindNTHeaders()); - IMAGE_SECTION_HEADER* sectionEnd = sectionStart + VAL16(FindNTHeaders()->FileHeader.NumberOfSections); - IMAGE_SECTION_HEADER* section = sectionStart; while (section < sectionEnd) { @@ -928,13 +987,9 @@ void* FlatImageLayout::LoadImageByCopyingParts(SIZE_T* m_imageParts) const // Finally, apply proper protection to copied sections for (section = sectionStart; section < sectionEnd; section++) { - DWORD executableProtection = PAGE_EXECUTE_READ; -#if defined(HOST_OSX) && defined(HOST_ARM64) - executableProtection = PAGE_EXECUTE_READWRITE; -#endif // Add appropriate page protection. DWORD newProtection = section->Characteristics & IMAGE_SCN_MEM_EXECUTE ? - executableProtection : + PAGE_EXECUTE_READ : section->Characteristics & IMAGE_SCN_MEM_WRITE ? PAGE_READWRITE : PAGE_READONLY; @@ -946,6 +1001,7 @@ void* FlatImageLayout::LoadImageByCopyingParts(SIZE_T* m_imageParts) const ThrowLastError(); } } +#endif // defined(HOST_OSX) && defined(HOST_ARM64) return base; } diff --git a/src/installer/tests/AppHost.Bundle.Tests/AppLaunch.cs b/src/installer/tests/AppHost.Bundle.Tests/AppLaunch.cs index 17d79ede5fa217..7f3e26b9ac56f6 100644 --- a/src/installer/tests/AppHost.Bundle.Tests/AppLaunch.cs +++ b/src/installer/tests/AppHost.Bundle.Tests/AppLaunch.cs @@ -110,6 +110,24 @@ private void NonAsciiCharacterSelfContainedApp() } } + // Regression test: on macOS Apple Silicon, compressed self-contained single-file + // apps intermittently hit an AccessViolationException based on how we load images. + // This was observed via concurrent child process launches, so this test launches + // multiple child processes in parallel as a regression test. + [Fact] + [PlatformSpecific(TestPlatforms.OSX)] + public void SelfContained_Compressed_SpawnsChildren() + { + string singleFile = sharedTestState.SelfContainedApp.Bundle(BundleOptions.EnableCompression); + + Command.Create(singleFile, "launch_self") + .CaptureStdErr() + .CaptureStdOut() + .Execute() + .Should().Pass() + .And.HaveStdOutContaining("Hello World!"); + } + [Theory( SkipType = typeof(Binaries.CetCompat), SkipUnless = nameof(Binaries.CetCompat.IsSupported), diff --git a/src/installer/tests/Assets/Projects/HelloWorld/Program.cs b/src/installer/tests/Assets/Projects/HelloWorld/Program.cs index 60bb05c8b641cf..7fd61b604c4c04 100644 --- a/src/installer/tests/Assets/Projects/HelloWorld/Program.cs +++ b/src/installer/tests/Assets/Projects/HelloWorld/Program.cs @@ -2,9 +2,12 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Diagnostics; +using System.Linq; using System.Reflection; using System.Runtime.InteropServices; using System.Runtime.Loader; +using System.Threading.Tasks; namespace HelloWorld { @@ -53,6 +56,24 @@ public static void Main(string[] args) // Disable core dumps - test is intentionally crashing Utilities.CoreDump.Disable(); throw new Exception("Goodbye World!"); + case "launch_self": + // Launch copies of this app in parallel. + // When run via dotnet , GetCommandLineArgs[0] is the managed + // dll - forward it so the child knows what to launch. + string fileName = Environment.ProcessPath!; + string entry = Environment.GetCommandLineArgs()[0]; + bool forwardEntry = entry != Environment.ProcessPath; + + var tasks = Enumerable.Range(0, 5).Select(_ => Task.Run(() => + { + var startInfo = new ProcessStartInfo { FileName = fileName }; + if (forwardEntry) + startInfo.ArgumentList.Add(entry); + using var process = Process.Start(startInfo)!; + process.WaitForExit(); + })).ToArray(); + Task.WaitAll(tasks); + break; default: break; }