summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAndy Ayers <andya@microsoft.com>2018-08-06 12:44:35 -0700
committerGitHub <noreply@github.com>2018-08-06 12:44:35 -0700
commit77e831456a682e40093df16d72dc826360f8712f (patch)
tree0174b36c127d8a332d6e9f418b7b23978fda86ca
parent594400d009b4f85aee267dfdef75cd33f5bbad35 (diff)
parent760418ee6e569697a1c08b6f0296a74a3526c863 (diff)
downloadcoreclr-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.cpp12
-rw-r--r--tests/src/JIT/Regression/JitBlue/GitHub_19149/GitHub_19149.cs249
-rw-r--r--tests/src/JIT/Regression/JitBlue/GitHub_19149/GitHub_19149.csproj35
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>