diff options
author | Vladimir Sadov <vsadov@microsoft.com> | 2019-06-27 10:48:47 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-06-27 10:48:47 -0700 |
commit | 7a74a0b4f94b993f61f1c02ce0b8a2c6611771c3 (patch) | |
tree | a7451044b444d5b3e7d67411ca19c36db38e1553 | |
parent | c671a7f1356b0ee25a661c0925c8ead88fb19a42 (diff) | |
download | coreclr-7a74a0b4f94b993f61f1c02ce0b8a2c6611771c3.tar.gz coreclr-7a74a0b4f94b993f61f1c02ce0b8a2c6611771c3.tar.bz2 coreclr-7a74a0b4f94b993f61f1c02ce0b8a2c6611771c3.zip |
Fix SIGSEGV in GC when dealing with large double[] on ARM32 (#25403)
* Fix for #25252
* revert to special casing 8-alignement for double[] in GC helpers for x86
-rw-r--r-- | src/vm/gchelpers.cpp | 53 | ||||
-rw-r--r-- | tests/src/GC/Regressions/v3.0/25252/25252.cs | 20 | ||||
-rw-r--r-- | tests/src/GC/Regressions/v3.0/25252/25252.csproj | 32 |
3 files changed, 83 insertions, 22 deletions
diff --git a/src/vm/gchelpers.cpp b/src/vm/gchelpers.cpp index 07429a070a..0dcebd5e17 100644 --- a/src/vm/gchelpers.cpp +++ b/src/vm/gchelpers.cpp @@ -473,6 +473,11 @@ OBJECTREF AllocateSzArray(MethodTable* pArrayMT, INT32 cElements, GC_ALLOC_FLAGS } #endif + if (totalSize >= g_pConfig->GetGCLOHThreshold()) + { + bAllocateInLargeHeap = TRUE; + } + flags |= (pArrayMT->ContainsPointers() ? GC_ALLOC_CONTAINS_REF : GC_ALLOC_NO_FLAGS); ArrayBase* orArray = NULL; @@ -483,7 +488,22 @@ OBJECTREF AllocateSzArray(MethodTable* pArrayMT, INT32 cElements, GC_ALLOC_FLAGS } else { - if ((DATA_ALIGNMENT < sizeof(double)) && (elemType == ELEMENT_TYPE_R8)) +#ifdef FEATURE_64BIT_ALIGNMENT + MethodTable* pElementMT = pArrayMT->GetApproxArrayElementTypeHandle().GetMethodTable(); + if (pElementMT->RequiresAlign8() && pElementMT->IsValueType()) + { + // This platform requires that certain fields are 8-byte aligned (and the runtime doesn't provide + // this guarantee implicitly, e.g. on 32-bit platforms). Since it's the array payload, not the + // header that requires alignment we need to be careful. However it just so happens that all the + // cases we care about (single and multi-dim arrays of value types) have an even number of DWORDs + // in their headers so the alignment requirements for the header and the payload are the same. + _ASSERTE(((pArrayMT->GetBaseSize() - SIZEOF_OBJHEADER) & 7) == 0); + orArray = (ArrayBase*)AllocAlign8(totalSize, flags); + } + else +#else + if ((DATA_ALIGNMENT < sizeof(double)) && (elemType == ELEMENT_TYPE_R8) && + (totalSize < g_pConfig->GetGCLOHThreshold() - MIN_OBJECT_SIZE)) { // Creation of an array of doubles, not in the large object heap. // We want to align the doubles to 8 byte boundaries, but the GC gives us pointers aligned @@ -517,24 +537,10 @@ OBJECTREF AllocateSzArray(MethodTable* pArrayMT, INT32 cElements, GC_ALLOC_FLAGS orDummyObject->SetMethodTable(g_pObjectClass); } else +#endif // FEATURE_64BIT_ALIGNMENT + { -#ifdef FEATURE_64BIT_ALIGNMENT - MethodTable* pElementMT = pArrayMT->GetApproxArrayElementTypeHandle().GetMethodTable(); - if (pElementMT->RequiresAlign8() && pElementMT->IsValueType()) - { - // This platform requires that certain fields are 8-byte aligned (and the runtime doesn't provide - // this guarantee implicitly, e.g. on 32-bit platforms). Since it's the array payload, not the - // header that requires alignment we need to be careful. However it just so happens that all the - // cases we care about (single and multi-dim arrays of value types) have an even number of DWORDs - // in their headers so the alignment requirements for the header and the payload are the same. - _ASSERTE(((pArrayMT->GetBaseSize() - SIZEOF_OBJHEADER) & 7) == 0); - orArray = (ArrayBase*)AllocAlign8(totalSize, flags); - } - else -#endif - { - orArray = (ArrayBase*)Alloc(totalSize, flags); - } + orArray = (ArrayBase*)Alloc(totalSize, flags); } orArray->SetArrayMethodTable(pArrayMT); } @@ -544,8 +550,7 @@ OBJECTREF AllocateSzArray(MethodTable* pArrayMT, INT32 cElements, GC_ALLOC_FLAGS bool bProfilerNotifyLargeAllocation = false; - if (bAllocateInLargeHeap || - (totalSize >= g_pConfig->GetGCLOHThreshold())) + if (bAllocateInLargeHeap) { GCHeapUtilities::GetGCHeap()->PublishObject((BYTE*)orArray); bProfilerNotifyLargeAllocation = TrackLargeAllocations(); @@ -739,6 +744,11 @@ OBJECTREF AllocateArrayEx(MethodTable *pArrayMT, INT32 *pArgs, DWORD dwNumArgs, } #endif + if (totalSize >= g_pConfig->GetGCLOHThreshold()) + { + bAllocateInLargeHeap = TRUE; + } + flags |= (pArrayMT->ContainsPointers() ? GC_ALLOC_CONTAINS_REF : GC_ALLOC_NO_FLAGS); if (bAllocateInLargeHeap) @@ -773,8 +783,7 @@ OBJECTREF AllocateArrayEx(MethodTable *pArrayMT, INT32 *pArgs, DWORD dwNumArgs, bool bProfilerNotifyLargeAllocation = false; - if (bAllocateInLargeHeap || - (totalSize >= g_pConfig->GetGCLOHThreshold())) + if (bAllocateInLargeHeap) { GCHeapUtilities::GetGCHeap()->PublishObject((BYTE*)orArray); bProfilerNotifyLargeAllocation = TrackLargeAllocations(); diff --git a/tests/src/GC/Regressions/v3.0/25252/25252.cs b/tests/src/GC/Regressions/v3.0/25252/25252.cs new file mode 100644 index 0000000000..9245423437 --- /dev/null +++ b/tests/src/GC/Regressions/v3.0/25252/25252.cs @@ -0,0 +1,20 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System; + +class Program +{ + static int Main() + { + var matrix = new double[128 * 128]; + + Console.WriteLine("SIGSEGV upcoming"); + GC.Collect(GC.MaxGeneration); + + GC.KeepAlive(matrix); + return 100; + } +} + diff --git a/tests/src/GC/Regressions/v3.0/25252/25252.csproj b/tests/src/GC/Regressions/v3.0/25252/25252.csproj new file mode 100644 index 0000000000..ed9874daa8 --- /dev/null +++ b/tests/src/GC/Regressions/v3.0/25252/25252.csproj @@ -0,0 +1,32 @@ +<?xml version="1.0" encoding="utf-8"?> +<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003"> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" /> + <PropertyGroup> + <Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration> + <Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform> + <SchemaVersion>2.0</SchemaVersion> + <ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid> + <OutputType>Exe</OutputType> + <ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids> + <SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir> + <AllowUnsafeBlocks>true</AllowUnsafeBlocks> + <CLRTestPriority>0</CLRTestPriority> + </PropertyGroup> + <!-- Default configurations to help VS understand the configurations --> + <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup> + <PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup> + <ItemGroup> + <CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies"> + <Visible>False</Visible> + </CodeAnalysisDependentAssemblyPaths> + </ItemGroup> + <ItemGroup> + <!-- Add Compile Object Here --> + <Compile Include="25252.cs" /> + </ItemGroup> + <ItemGroup> + <Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" /> + </ItemGroup> + <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" /> + <PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup> +</Project>
\ No newline at end of file |