Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Commit 77e8314

Browse files
authored
Merge pull request #19224 from AndyAyersMS/Port19156toRelease21
JIT: port extra check to struct of struct of x promotion to release/2.1
2 parents 594400d + 760418e commit 77e8314

File tree

3 files changed

+296
-0
lines changed

3 files changed

+296
-0
lines changed

src/jit/lclvars.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1626,6 +1626,18 @@ void Compiler::lvaCanPromoteStructType(CORINFO_CLASS_HANDLE typeHnd,
16261626
// natural boundary.
16271627
if (fieldSize == 0 || fieldSize != TARGET_POINTER_SIZE || varTypeIsFloating(fieldVarType))
16281628
{
1629+
JITDUMP("Promotion blocked: struct contains struct field with one field,"
1630+
" but that field has invalid size or type");
1631+
return;
1632+
}
1633+
1634+
// Insist this wrapped field occupy all of its parent storage.
1635+
unsigned innerStructSize = info.compCompHnd->getClassSize(pFieldInfo->fldTypeHnd);
1636+
1637+
if (fieldSize != innerStructSize)
1638+
{
1639+
JITDUMP("Promotion blocked: struct contains struct field with one field,"
1640+
" but that field is not the same size as its parent.");
16291641
return;
16301642
}
16311643

Lines changed: 249 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,249 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
// Bug report thanks to @mgravell
6+
//
7+
// JIT bug affecting how fixed buffers are handled
8+
//
9+
// Affects: netcoreapp2.1, debug and release
10+
// Does not seem to affect: netcoreapp2.0, net47
11+
//
12+
// the idea behind CommandBytes is that it is a fixed-sized string-like thing
13+
// used for matching commands; it is *implemented* as a fixed buffer
14+
// of **longs**, but: the first byte of the first element is coerced into
15+
// a byte and used to store the length; the actual text payload (ASCII)
16+
// starts at the second byte of the first element
17+
//
18+
// as far as I can tell, it is all validly implemented, and it works fine
19+
// in isolation, however: when used in a dictionary, it goes bad;
20+
// - items not being found despite having GetHashCode and Equals match
21+
// - items over 1 chunk size becoming corrupted (see: ToInnerString)
22+
//
23+
// however, if I replace the fixed buffer with the same number of
24+
// regular fields (_c0,_c1,_c2) and use *all the same code*, it
25+
// all works correctly!
26+
//
27+
// The "Main" method populates a dictionary in the expected way,
28+
// then attempts to find things - either via TryGetValue or manually;
29+
// it then compares the contents
30+
//
31+
// Yes, this code is evil; it is for a very specific optimized scenario.
32+
33+
using System;
34+
using System.Collections.Generic;
35+
using System.Text;
36+
37+
unsafe struct CommandBytes : IEquatable<CommandBytes>
38+
{
39+
private const int ChunkLength = 3;
40+
public const int MaxLength = (ChunkLength * 8) - 1;
41+
42+
fixed long _chunks[ChunkLength];
43+
44+
public override int GetHashCode()
45+
{
46+
fixed (long* lPtr = _chunks)
47+
{
48+
var hashCode = -1923861349;
49+
long* x = lPtr;
50+
for (int i = 0; i < ChunkLength; i++)
51+
{
52+
hashCode = hashCode * -1521134295 + (*x++).GetHashCode();
53+
}
54+
return hashCode;
55+
}
56+
}
57+
58+
public override string ToString()
59+
{
60+
fixed (long* lPtr = _chunks)
61+
{
62+
var bPtr = (byte*)lPtr;
63+
return Encoding.ASCII.GetString(bPtr + 1, bPtr[0]);
64+
}
65+
}
66+
public int Length
67+
{
68+
get
69+
{
70+
fixed (long* lPtr = _chunks)
71+
{
72+
var bPtr = (byte*)lPtr;
73+
return bPtr[0];
74+
}
75+
}
76+
}
77+
public byte this[int index]
78+
{
79+
get
80+
{
81+
fixed (long* lPtr = _chunks)
82+
{
83+
byte* bPtr = (byte*)lPtr;
84+
int len = bPtr[0];
85+
if (index < 0 || index >= len) throw new IndexOutOfRangeException();
86+
return bPtr[index + 1];
87+
}
88+
}
89+
}
90+
91+
public CommandBytes(string value)
92+
{
93+
value = value.ToLowerInvariant();
94+
var len = Encoding.ASCII.GetByteCount(value);
95+
if (len > MaxLength) throw new ArgumentOutOfRangeException("Maximum command length exceeed");
96+
97+
fixed (long* lPtr = _chunks)
98+
{
99+
Clear(lPtr);
100+
byte* bPtr = (byte*)lPtr;
101+
bPtr[0] = (byte)len;
102+
fixed (char* cPtr = value)
103+
{
104+
Encoding.ASCII.GetBytes(cPtr, value.Length, bPtr + 1, len);
105+
}
106+
}
107+
}
108+
public override bool Equals(object obj) => obj is CommandBytes cb && Equals(cb);
109+
110+
public string ToInnerString()
111+
{
112+
fixed (long* lPtr = _chunks)
113+
{
114+
long* x = lPtr;
115+
var sb = new StringBuilder();
116+
for (int i = 0; i < ChunkLength; i++)
117+
{
118+
if (sb.Length != 0) sb.Append(',');
119+
sb.Append(*x++);
120+
}
121+
return sb.ToString();
122+
}
123+
}
124+
public bool Equals(CommandBytes value)
125+
{
126+
fixed (long* lPtr = _chunks)
127+
{
128+
long* x = lPtr;
129+
long* y = value._chunks;
130+
for (int i = 0; i < ChunkLength; i++)
131+
{
132+
if (*x++ != *y++) return false;
133+
}
134+
return true;
135+
}
136+
}
137+
private static void Clear(long* ptr)
138+
{
139+
for (int i = 0; i < ChunkLength; i++)
140+
{
141+
*ptr++ = 0L;
142+
}
143+
}
144+
}
145+
146+
static class Program
147+
{
148+
static int Main()
149+
{
150+
var lookup = new Dictionary<CommandBytes, string>();
151+
void Add(string val)
152+
{
153+
var cb = new CommandBytes(val);
154+
// prove we didn't screw up
155+
if (cb.ToString() != val)
156+
throw new InvalidOperationException("oops!");
157+
lookup.Add(cb, val);
158+
}
159+
Add("client");
160+
Add("cluster");
161+
Add("command");
162+
Add("config");
163+
Add("dbsize");
164+
Add("decr");
165+
Add("del");
166+
Add("echo");
167+
Add("exists");
168+
Add("flushall");
169+
Add("flushdb");
170+
Add("get");
171+
Add("incr");
172+
Add("incrby");
173+
Add("info");
174+
Add("keys");
175+
Add("llen");
176+
Add("lpop");
177+
Add("lpush");
178+
Add("lrange");
179+
Add("memory");
180+
Add("mget");
181+
Add("mset");
182+
Add("ping");
183+
Add("quit");
184+
Add("role");
185+
Add("rpop");
186+
Add("rpush");
187+
Add("sadd");
188+
Add("scard");
189+
Add("select");
190+
Add("set");
191+
Add("shutdown");
192+
Add("sismember");
193+
Add("spop");
194+
Add("srem");
195+
Add("strlen");
196+
Add("subscribe");
197+
Add("time");
198+
Add("unlink");
199+
Add("unsubscribe");
200+
201+
bool HuntFor(string lookFor)
202+
{
203+
Console.WriteLine($"Looking for: '{lookFor}'");
204+
var hunt = new CommandBytes(lookFor);
205+
bool result = lookup.TryGetValue(hunt, out var found);
206+
207+
if (result)
208+
{
209+
Console.WriteLine($"Found via TryGetValue: '{found}'");
210+
}
211+
else
212+
{
213+
Console.WriteLine("**NOT FOUND** via TryGetValue");
214+
}
215+
216+
Console.WriteLine("looking manually");
217+
foreach (var pair in lookup)
218+
{
219+
if (pair.Value == lookFor)
220+
{
221+
Console.WriteLine($"Found manually: '{pair.Value}'");
222+
var key = pair.Key;
223+
void Compare<T>(string caption, Func<CommandBytes, T> func)
224+
{
225+
T x = func(hunt), y = func(key);
226+
Console.WriteLine($"{caption}: {EqualityComparer<T>.Default.Equals(x, y)}, '{x}' vs '{y}'");
227+
}
228+
Compare("GetHashCode", _ => _.GetHashCode());
229+
Compare("ToString", _ => _.ToString());
230+
Compare("Length", _ => _.Length);
231+
Compare("ToInnerString", _ => _.ToInnerString());
232+
Console.WriteLine($"Equals: {key.Equals(hunt)}, {hunt.Equals(key)}");
233+
var eq = EqualityComparer<CommandBytes>.Default;
234+
235+
Console.WriteLine($"EqualityComparer: {eq.Equals(key, hunt)}, {eq.Equals(hunt, key)}");
236+
Compare("eq GetHashCode", _ => eq.GetHashCode(_));
237+
}
238+
}
239+
Console.WriteLine();
240+
241+
return result;
242+
}
243+
244+
bool result1 = HuntFor("ping");
245+
bool result2 = HuntFor("subscribe");
246+
247+
return (result1 && result2) ? 100 : -1;
248+
}
249+
}
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
3+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
4+
<PropertyGroup>
5+
<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
6+
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>
7+
<AssemblyName>$(MSBuildProjectName)</AssemblyName>
8+
<SchemaVersion>2.0</SchemaVersion>
9+
<ProjectGuid>{95DFC527-4DC1-495E-97D7-E94EE1F7140D}</ProjectGuid>
10+
<OutputType>Exe</OutputType>
11+
<ProjectTypeGuids>{786C830F-07A1-408B-BD7F-6EE04809D6DB};{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}</ProjectTypeGuids>
12+
<SolutionDir Condition="$(SolutionDir) == '' Or $(SolutionDir) == '*Undefined*'">..\..\</SolutionDir>
13+
</PropertyGroup>
14+
<!-- Default configurations to help VS understand the configurations -->
15+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' "></PropertyGroup>
16+
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Release|AnyCPU' "></PropertyGroup>
17+
<ItemGroup>
18+
<CodeAnalysisDependentAssemblyPaths Condition=" '$(VS100COMNTOOLS)' != '' " Include="$(VS100COMNTOOLS)..\IDE\PrivateAssemblies">
19+
<Visible>False</Visible>
20+
</CodeAnalysisDependentAssemblyPaths>
21+
</ItemGroup>
22+
<PropertyGroup>
23+
<DebugType></DebugType>
24+
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
25+
<Optimize>True</Optimize>
26+
</PropertyGroup>
27+
<ItemGroup>
28+
<Compile Include="$(MSBuildProjectName).cs" />
29+
</ItemGroup>
30+
<ItemGroup>
31+
<Service Include="{82A7F48D-3B50-4B1E-B82E-3ADA8210C358}" />
32+
</ItemGroup>
33+
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
34+
<PropertyGroup Condition=" '$(MsBuildProjectDirOverride)' != '' "></PropertyGroup>
35+
</Project>

0 commit comments

Comments
 (0)