Skip to content

Commit

Permalink
Three fixes for R2RDump bugs I discovered while investigating dotnet#…
Browse files Browse the repository at this point in the history 
…38290 (dotnet#41303)

1) As ReadyToRunMethod newly looks at method bodies, in the
composite case we need to pass around per-module IL info, not only
the metadata reader. I have replaced such usages of MetadataReader
with the new interface IAssemblyMetadata and I implemented the
standalone variant StandaloneAssemblyMetadata.

2) I hit an UnwindInfo decoding issue and I found out that the
UnwindCode decoder is weirdly split between the UnwindCode ctor
and a separate method ParseUnwindCode that weren't completely
consistent in whether the UnwindInfoArray refers to the raw 16-bit
entries in the UnwindInfo or to the parsed UnwindCode instances.
I have basically migrated ParseUnwindCode into the UnwindCode ctor
and I simplified the related logic.

3) We shouldn't block method dump on header dump, otherwise there's
no way to dump both headers and methods in one R2RDump execution.

Thanks

Tomas
  • Loading branch information
@trylek
trylek authored Aug 26, 2020
1 parent f1f6b3c commit cd91bf5
Show file tree
Hide file tree
Showing 13 changed files with 240 additions and 233 deletions.
233 changes: 96 additions & 137 deletions src/coreclr/src/tools/aot/ILCompiler.Reflection.ReadyToRun/Amd64/UnwindInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Text;

namespace ILCompiler.Reflection.ReadyToRun.Amd64
Expand Down Expand Up @@ -43,8 +42,6 @@ public enum UnwindFlags
/// </summary>
public class UnwindCode
{
public int Index { get; set; }

public byte CodeOffset { get; set; }
public UnwindOpCodes UnwindOp { get; set; } //4 bits

Expand All @@ -54,43 +51,101 @@ public class UnwindCode
public byte OffsetLow { get; set; }
public byte OffsetHigh { get; set; } //4 bits

public uint FrameOffset { get; set; }
public int FrameOffset { get; set; }
public int NextFrameOffset { get; set; }

public bool IsOpInfo { get; set; }

public UnwindCode() { }

public UnwindCode(byte[] image, int index, ref int offset)
/// <summary>
/// Unwinde code parsing is based on <a href="https://github.com/dotnet/coreclr/blob/master/src/jit/unwindamd64.cpp">src\jit\unwindamd64.cpp</a> DumpUnwindInfo
/// </summary>
public UnwindCode(byte[] image, ref int frameOffset, ref int offset)
{
Index = index;

int off = offset;
CodeOffset = NativeReader.ReadByte(image, ref off);
byte op = NativeReader.ReadByte(image, ref off);
CodeOffset = NativeReader.ReadByte(image, ref offset);
byte op = NativeReader.ReadByte(image, ref offset);
UnwindOp = (UnwindOpCodes)(op & 15);
OpInfo = (byte)(op >> 4);

OffsetLow = CodeOffset;
OffsetHigh = OpInfo;

FrameOffset = NativeReader.ReadUInt16(image, ref offset);
NextFrameOffset = -1;
FrameOffset = frameOffset;

if (UnwindOp == UnwindOpCodes.UWOP_ALLOC_LARGE)
switch (UnwindOp)
{
uint codedSize;
if (OpInfo == 0)
{
codedSize = NativeReader.ReadUInt16(image, ref offset);
}
else if (OpInfo == 1)
{
codedSize = NativeReader.ReadUInt32(image, ref offset);
}
case UnwindOpCodes.UWOP_PUSH_NONVOL:
OpInfoStr = $"{(Registers)OpInfo}({OpInfo})";
break;
case UnwindOpCodes.UWOP_ALLOC_LARGE:
OpInfoStr = $"{OpInfo} - ";
if (OpInfo == 0)
{
OpInfoStr += "Scaled small";
NextFrameOffset = 8 * NativeReader.ReadUInt16(image, ref offset);
}
else if (OpInfo == 1)
{
OpInfoStr += "Unscaled large";
uint nextOffset = NativeReader.ReadUInt16(image, ref offset);
NextFrameOffset = (int)((uint)(NativeReader.ReadUInt16(image, ref offset) << 16) | nextOffset);
}
else
{
OpInfoStr += "Unknown";
}
break;
case UnwindOpCodes.UWOP_ALLOC_SMALL:
int opInfo = OpInfo * 8 + 8;
OpInfoStr = $"{opInfo}";
break;
case UnwindOpCodes.UWOP_SET_FPREG:
OpInfoStr = $"Unused({OpInfo})";
break;
case UnwindOpCodes.UWOP_SET_FPREG_LARGE:
{
OpInfoStr = $"Unused({OpInfo})";
uint nextOffset = NativeReader.ReadUInt16(image, ref offset);
nextOffset = ((uint)(NativeReader.ReadUInt16(image, ref offset) << 16) | nextOffset);
NextFrameOffset = (int)nextOffset * 16;
if ((NextFrameOffset & 0xF0000000) != 0)
{
throw new BadImageFormatException("Warning: Illegal unwindInfo unscaled offset: too large");
}
}
break;
case UnwindOpCodes.UWOP_SAVE_NONVOL:
{
OpInfoStr = $"{(Registers)OpInfo}({OpInfo})";
NextFrameOffset = NativeReader.ReadUInt16(image, ref offset) * 8;
}
break;
case UnwindOpCodes.UWOP_SAVE_NONVOL_FAR:
{
OpInfoStr = $"{(Registers)OpInfo}({OpInfo})";
uint nextOffset = NativeReader.ReadUInt16(image, ref offset);
NextFrameOffset = (int)((uint)(NativeReader.ReadUInt16(image, ref offset) << 16) | nextOffset);
}
break;
case UnwindOpCodes.UWOP_SAVE_XMM128:
{
OpInfoStr = $"XMM{OpInfo}({OpInfo})";
NextFrameOffset = (int)NativeReader.ReadUInt16(image, ref offset) * 16;
}
break;
case UnwindOpCodes.UWOP_SAVE_XMM128_FAR:
{
OpInfoStr = $"XMM{OpInfo}({OpInfo})";
uint nextOffset = NativeReader.ReadUInt16(image, ref offset);
NextFrameOffset = (int)((uint)(NativeReader.ReadUInt16(image, ref offset) << 16) | nextOffset);
}
break;
default:
throw new NotImplementedException(UnwindOp.ToString());
}

IsOpInfo = false;
NextFrameOffset = frameOffset;
}
}

Expand All @@ -108,8 +163,8 @@ public class UnwindInfo : BaseUnwindInfo
public byte CountOfUnwindCodes { get; set; }
public Registers FrameRegister { get; set; } //4 bits
public byte FrameOffset { get; set; } //4 bits
public UnwindCode[] UnwindCodeArray { get; set; }
public Dictionary<int, UnwindCode> UnwindCodes { get; set; }
public Dictionary<int, int> CodeOffsetToUnwindCodeIndex { get; set; }
public List<UnwindCode> UnwindCodes { get; set; }
public uint PersonalityRoutineRVA { get; set; }

public UnwindInfo() { }
Expand All @@ -128,20 +183,19 @@ public UnwindInfo(byte[] image, int offset)
FrameRegister = (Registers)(frameRegisterAndOffset & 15);
FrameOffset = (byte)(frameRegisterAndOffset >> 4);

UnwindCodeArray = new UnwindCode[CountOfUnwindCodes];
UnwindCodes = new Dictionary<int, UnwindCode>();
for (int i = 0; i < CountOfUnwindCodes; i++)
UnwindCodes = new List<UnwindCode>(CountOfUnwindCodes);
CodeOffsetToUnwindCodeIndex = new Dictionary<int, int>();
int frameOffset = FrameOffset;
int sizeOfUnwindCodes = CountOfUnwindCodes * _sizeofUnwindCode;
int endOffset = offset + sizeOfUnwindCodes;
while (offset < endOffset)
{
UnwindCodeArray[i] = new UnwindCode(image, i, ref offset);
}
for (int i = 0; i < CountOfUnwindCodes; i++)
{
ParseUnwindCode(ref i);
Debug.Assert(!UnwindCodes.ContainsKey(UnwindCodeArray[i].CodeOffset));
UnwindCodes.Add(UnwindCodeArray[i].CodeOffset, UnwindCodeArray[i]);
UnwindCode unwindCode = new UnwindCode(image, ref frameOffset, ref offset);
CodeOffsetToUnwindCodeIndex.Add(unwindCode.CodeOffset, UnwindCodes.Count);
UnwindCodes.Add(unwindCode);
}

Size = _offsetofUnwindCode + CountOfUnwindCodes * _sizeofUnwindCode;
Size = _offsetofUnwindCode + sizeOfUnwindCodes;
int alignmentPad = -Size & 3;
Size += alignmentPad + sizeof(uint);

Expand Down Expand Up @@ -177,16 +231,14 @@ public override string ToString()
sb.AppendLine($" FrameOffset: {FrameOffset}");
sb.AppendLine($" Unwind Codes:");
sb.AppendLine($" ------------------");
for (int i = 0; i < CountOfUnwindCodes; i++)
foreach (UnwindCode unwindCode in UnwindCodes)
{
if (!UnwindCodeArray[i].IsOpInfo)
continue;
sb.AppendLine($" CodeOffset: 0x{UnwindCodeArray[i].CodeOffset:X2}");
sb.AppendLine($" UnwindOp: {UnwindCodeArray[i].UnwindOp}({(byte)UnwindCodeArray[i].UnwindOp})");
sb.AppendLine($" OpInfo: {UnwindCodeArray[i].OpInfoStr}");
if (UnwindCodeArray[i].NextFrameOffset != -1)
sb.AppendLine($" CodeOffset: 0x{unwindCode.CodeOffset:X2}");
sb.AppendLine($" UnwindOp: {unwindCode.UnwindOp}({(byte)unwindCode.UnwindOp})");
sb.AppendLine($" OpInfo: {unwindCode.OpInfoStr}");
if (unwindCode.NextFrameOffset != -1)
{
sb.AppendLine($" FrameOffset: {UnwindCodeArray[i].NextFrameOffset}");
sb.AppendLine($" FrameOffset: {unwindCode.NextFrameOffset}");
}
sb.AppendLine($" ------------------");
}
Expand All @@ -196,98 +248,5 @@ public override string ToString()
return sb.ToString();
}

/// <summary>
/// based on <a href="https://github.com/dotnet/coreclr/blob/master/src/jit/unwindamd64.cpp">src\jit\unwindamd64.cpp</a> DumpUnwindInfo
/// </summary>
private void ParseUnwindCode(ref int i)
{
UnwindCode code = UnwindCodeArray[i];
code.IsOpInfo = true;
switch (code.UnwindOp)
{
case UnwindOpCodes.UWOP_PUSH_NONVOL:
code.OpInfoStr = $"{(Registers)code.OpInfo}({code.OpInfo})";
break;
case UnwindOpCodes.UWOP_ALLOC_LARGE:
code.OpInfoStr = $"{code.OpInfo} - ";
if (code.OpInfo == 0)
{
i++;
UnwindCodeArray[i].OpInfoStr += "Scaled small";
code.NextFrameOffset = (int)UnwindCodeArray[i].FrameOffset * 8;
}
else if (code.OpInfo == 1)
{
i++;
UnwindCodeArray[i].OpInfoStr += "Unscaled large";
uint offset = UnwindCodeArray[i].FrameOffset;
i++;
offset = ((UnwindCodeArray[i].FrameOffset << 16) | offset);
code.NextFrameOffset = (int)offset;
}
else
{
code.OpInfoStr += "Unknown";
}
break;
case UnwindOpCodes.UWOP_ALLOC_SMALL:
int opInfo = code.OpInfo * 8 + 8;
code.OpInfoStr = $"{opInfo}";
break;
case UnwindOpCodes.UWOP_SET_FPREG:
code.OpInfoStr = $"Unused({code.OpInfo})";
break;
case UnwindOpCodes.UWOP_SET_FPREG_LARGE:
{
code.OpInfoStr = $"Unused({code.OpInfo})";
i++;
uint offset = UnwindCodeArray[i].FrameOffset;
i++;
offset = ((UnwindCodeArray[i].FrameOffset << 16) | offset);
code.NextFrameOffset = (int)offset * 16;
if ((UnwindCodeArray[i].FrameOffset & 0xF0000000) != 0)
{
throw new BadImageFormatException("Warning: Illegal unwindInfo unscaled offset: too large");
}
}
break;
case UnwindOpCodes.UWOP_SAVE_NONVOL:
{
code.OpInfoStr = $"{(Registers)code.OpInfo}({code.OpInfo})";
i++;
uint offset = UnwindCodeArray[i].FrameOffset * 8;
code.NextFrameOffset = (int)offset;
}
break;
case UnwindOpCodes.UWOP_SAVE_NONVOL_FAR:
{
code.OpInfoStr = $"{(Registers)code.OpInfo}({code.OpInfo})";
i++;
uint offset = UnwindCodeArray[i].FrameOffset;
i++;
offset = ((UnwindCodeArray[i].FrameOffset << 16) | offset);
code.NextFrameOffset = (int)offset;
}
break;
case UnwindOpCodes.UWOP_SAVE_XMM128:
{
code.OpInfoStr = $"XMM{code.OpInfo}({code.OpInfo})";
i++;
uint offset = UnwindCodeArray[i].FrameOffset * 16;
code.NextFrameOffset = (int)offset;
}
break;
case UnwindOpCodes.UWOP_SAVE_XMM128_FAR:
{
code.OpInfoStr = $"XMM{code.OpInfo}({code.OpInfo})";
i++;
uint offset = UnwindCodeArray[i].FrameOffset;
i++;
offset = ((UnwindCodeArray[i].FrameOffset << 16) | offset);
code.NextFrameOffset = (int)offset;
}
break;
}
}
}
}
2 changes: 1 addition & 1 deletion src/coreclr/src/tools/aot/ILCompiler.Reflection.ReadyToRun/EHInfo.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public EHClause(ReadyToRunReader reader, int offset)
}
else
{
ClassName = MetadataNameFormatter.FormatHandle(reader.GetGlobalMetadataReader(), MetadataTokens.Handle((int)ClassTokenOrFilterOffset));
ClassName = MetadataNameFormatter.FormatHandle(reader.GetGlobalMetadata()?.MetadataReader, MetadataTokens.Handle((int)ClassTokenOrFilterOffset));
}
}
}
Expand Down
18 changes: 18 additions & 0 deletions src/coreclr/src/tools/aot/ILCompiler.Reflection.ReadyToRun/IAssemblyMetadata.cs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Reflection.Metadata;
using System.Reflection.PortableExecutable;

namespace ILCompiler.Reflection.ReadyToRun
{
/// <summary>
/// This interface represents MSIL information for a single component assembly.
/// </summary>
public interface IAssemblyMetadata
{
PEReader ImageReader { get; }

MetadataReader MetadataReader { get; }
}
}
4 changes: 2 additions & 2 deletions src/coreclr/src/tools/aot/ILCompiler.Reflection.ReadyToRun/IAssemblyResolver.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ namespace ILCompiler.Reflection.ReadyToRun
{
public interface IAssemblyResolver
{
MetadataReader FindAssembly(MetadataReader metadataReader, AssemblyReferenceHandle assemblyReferenceHandle, string parentFile);
MetadataReader FindAssembly(string simpleName, string parentFile);
IAssemblyMetadata FindAssembly(MetadataReader metadataReader, AssemblyReferenceHandle assemblyReferenceHandle, string parentFile);
IAssemblyMetadata FindAssembly(string simpleName, string parentFile);
// TODO (refactoring) - signature formatting options should be independent of assembly resolver
bool Naked { get; }
bool SignatureBinary { get; }
Expand Down
Loading

0 comments on commit cd91bf5

Please sign in to comment.