diff options
author | Andy Ayers <andya@microsoft.com> | 2018-08-06 12:44:35 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-08-06 12:44:35 -0700 |
commit | 77e831456a682e40093df16d72dc826360f8712f (patch) | |
tree | 0174b36c127d8a332d6e9f418b7b23978fda86ca | |
parent | 594400d009b4f85aee267dfdef75cd33f5bbad35 (diff) | |
parent | 760418ee6e569697a1c08b6f0296a74a3526c863 (diff) | |
download | coreclr-77e831456a682e40093df16d72dc826360f8712f.tar.gz coreclr-77e831456a682e40093df16d72dc826360f8712f.tar.bz2 coreclr-77e831456a682e40093df16d72dc826360f8712f.zip |
Merge pull request #19224 from AndyAyersMS/Port19156toRelease21
JIT: port extra check to struct of struct of x promotion to release/2.1
-rw-r--r-- | src/jit/lclvars.cpp | 12 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_19149/GitHub_19149.cs | 249 | ||||
-rw-r--r-- | tests/src/JIT/Regression/JitBlue/GitHub_19149/GitHub_19149.csproj | 35 |
3 files changed, 296 insertions, 0 deletions
diff --git a/src/jit/lclvars.cpp b/src/jit/lclvars.cpp index 765a2eee3f..866b7117ce 100644 --- a/src/jit/lclvars.cpp +++ b/src/jit/lclvars.cpp @@ -1626,6 +1626,18 @@ void Compiler::lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd, // natural boundary. if (fieldSize == 0 || fieldSize != TARGET_POINTER_SIZE || varTypeIsFloating(fieldVarType)) { + JITDUMP("Promotion blocked: struct contains struct field with one field," + " but that field has invalid size or type"); + return; + } + + // Insist this wrapped field occupy all of its parent storage. + unsigned innerStructSize = info.compCompHnd->getClassSize(pFieldInfo->fldTypeHnd); + + if (fieldSize != innerStructSize) + { + JITDUMP("Promotion blocked: struct contains struct field with one field," + " but that field is not the same size as its parent."); return; } diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_19149/GitHub_19149.cs b/tests/src/JIT/Regression/JitBlue/GitHub_19149/GitHub_19149.cs new file mode 100644 index 0000000000..e62ee0bb1e --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_19149/GitHub_19149.cs @@ -0,0 +1,249 @@ +// 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. + +// Bug report thanks to @mgravell +// +// JIT bug affecting how fixed buffers are handled +// +// Affects: netcoreapp2.1, debug and release +// Does not seem to affect: netcoreapp2.0, net47 +// +// the idea behind CommandBytes is that it is a fixed-sized string-like thing +// used for matching commands; it is *implemented* as a fixed buffer +// of **longs**, but: the first byte of the first element is coerced into +// a byte and used to store the length; the actual text payload (ASCII) +// starts at the second byte of the first element +// +// as far as I can tell, it is all validly implemented, and it works fine +// in isolation, however: when used in a dictionary, it goes bad; +// - items not being found despite having GetHashCode and Equals match +// - items over 1 chunk size becoming corrupted (see: ToInnerString) +// +// however, if I replace the fixed buffer with the same number of +// regular fields (_c0,_c1,_c2) and use *all the same code*, it +// all works correctly! +// +// The "Main" method populates a dictionary in the expected way, +// then attempts to find things - either via TryGetValue or manually; +// it then compares the contents +// +// Yes, this code is evil; it is for a very specific optimized scenario. + +using System; +using System.Collections.Generic; +using System.Text; + +unsafe struct CommandBytes : IEquatable<CommandBytes> +{ + private const int ChunkLength = 3; + public const int MaxLength = (ChunkLength * 8) - 1; + + fixed long _chunks[ChunkLength]; + + public override int GetHashCode() + { + fixed (long* lPtr = _chunks) + { + var hashCode = -1923861349; + long* x = lPtr; + for (int i = 0; i < ChunkLength; i++) + { + hashCode = hashCode * -1521134295 + (*x++).GetHashCode(); + } + return hashCode; + } + } + + public override string ToString() + { + fixed (long* lPtr = _chunks) + { + var bPtr = (byte*)lPtr; + return Encoding.ASCII.GetString(bPtr + 1, bPtr[0]); + } + } + public int Length + { + get + { + fixed (long* lPtr = _chunks) + { + var bPtr = (byte*)lPtr; + return bPtr[0]; + } + } + } + public byte this[int index] + { + get + { + fixed (long* lPtr = _chunks) + { + byte* bPtr = (byte*)lPtr; + int len = bPtr[0]; + if (index < 0 || index >= len) throw new IndexOutOfRangeException(); + return bPtr[index + 1]; + } + } + } + + public CommandBytes(string value) + { + value = value.ToLowerInvariant(); + var len = Encoding.ASCII.GetByteCount(value); + if (len > MaxLength) throw new ArgumentOutOfRangeException("Maximum command length exceeed"); + + fixed (long* lPtr = _chunks) + { + Clear(lPtr); + byte* bPtr = (byte*)lPtr; + bPtr[0] = (byte)len; + fixed (char* cPtr = value) + { + Encoding.ASCII.GetBytes(cPtr, value.Length, bPtr + 1, len); + } + } + } + public override bool Equals(object obj) => obj is CommandBytes cb && Equals(cb); + + public string ToInnerString() + { + fixed (long* lPtr = _chunks) + { + long* x = lPtr; + var sb = new StringBuilder(); + for (int i = 0; i < ChunkLength; i++) + { + if (sb.Length != 0) sb.Append(','); + sb.Append(*x++); + } + return sb.ToString(); + } + } + public bool Equals(CommandBytes value) + { + fixed (long* lPtr = _chunks) + { + long* x = lPtr; + long* y = value._chunks; + for (int i = 0; i < ChunkLength; i++) + { + if (*x++ != *y++) return false; + } + return true; + } + } + private static void Clear(long* ptr) + { + for (int i = 0; i < ChunkLength; i++) + { + *ptr++ = 0L; + } + } +} + +static class Program +{ + static int Main() + { + var lookup = new Dictionary<CommandBytes, string>(); + void Add(string val) + { + var cb = new CommandBytes(val); + // prove we didn't screw up + if (cb.ToString() != val) + throw new InvalidOperationException("oops!"); + lookup.Add(cb, val); + } + Add("client"); + Add("cluster"); + Add("command"); + Add("config"); + Add("dbsize"); + Add("decr"); + Add("del"); + Add("echo"); + Add("exists"); + Add("flushall"); + Add("flushdb"); + Add("get"); + Add("incr"); + Add("incrby"); + Add("info"); + Add("keys"); + Add("llen"); + Add("lpop"); + Add("lpush"); + Add("lrange"); + Add("memory"); + Add("mget"); + Add("mset"); + Add("ping"); + Add("quit"); + Add("role"); + Add("rpop"); + Add("rpush"); + Add("sadd"); + Add("scard"); + Add("select"); + Add("set"); + Add("shutdown"); + Add("sismember"); + Add("spop"); + Add("srem"); + Add("strlen"); + Add("subscribe"); + Add("time"); + Add("unlink"); + Add("unsubscribe"); + + bool HuntFor(string lookFor) + { + Console.WriteLine($"Looking for: '{lookFor}'"); + var hunt = new CommandBytes(lookFor); + bool result = lookup.TryGetValue(hunt, out var found); + + if (result) + { + Console.WriteLine($"Found via TryGetValue: '{found}'"); + } + else + { + Console.WriteLine("**NOT FOUND** via TryGetValue"); + } + + Console.WriteLine("looking manually"); + foreach (var pair in lookup) + { + if (pair.Value == lookFor) + { + Console.WriteLine($"Found manually: '{pair.Value}'"); + var key = pair.Key; + void Compare<T>(string caption, Func<CommandBytes, T> func) + { + T x = func(hunt), y = func(key); + Console.WriteLine($"{caption}: {EqualityComparer<T>.Default.Equals(x, y)}, '{x}' vs '{y}'"); + } + Compare("GetHashCode", _ => _.GetHashCode()); + Compare("ToString", _ => _.ToString()); + Compare("Length", _ => _.Length); + Compare("ToInnerString", _ => _.ToInnerString()); + Console.WriteLine($"Equals: {key.Equals(hunt)}, {hunt.Equals(key)}"); + var eq = EqualityComparer<CommandBytes>.Default; + + Console.WriteLine($"EqualityComparer: {eq.Equals(key, hunt)}, {eq.Equals(hunt, key)}"); + Compare("eq GetHashCode", _ => eq.GetHashCode(_)); + } + } + Console.WriteLine(); + + return result; + } + + bool result1 = HuntFor("ping"); + bool result2 = HuntFor("subscribe"); + + return (result1 && result2) ? 100 : -1; + } +} diff --git a/tests/src/JIT/Regression/JitBlue/GitHub_19149/GitHub_19149.csproj b/tests/src/JIT/Regression/JitBlue/GitHub_19149/GitHub_19149.csproj new file mode 100644 index 0000000000..f8ba4995e0 --- /dev/null +++ b/tests/src/JIT/Regression/JitBlue/GitHub_19149/GitHub_19149.csproj @@ -0,0 +1,35 @@ +<?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> + <AssemblyName>$(MSBuildProjectName)</AssemblyName> + <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> + </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> + <PropertyGroup> + <DebugType></DebugType> + <AllowUnsafeBlocks>true</AllowUnsafeBlocks> + <Optimize>True</Optimize> + </PropertyGroup> + <ItemGroup> + <Compile Include="$(MSBuildProjectName).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> |