Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mono] IL Strip make sure the MemoryStream is disposed #112142

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 25 additions & 28 deletions src/tasks/MonoTargetsTasks/ILStrip/ILStrip.cs
Original file line number Diff line number Diff line change
Expand Up @@ -305,33 +305,32 @@ private Dictionary<int, int> ComputeMethodBodyUsage(MetadataReader mr, StreamRea
private void CreateTrimmedAssembly(PEReader peReader, string trimmedAssemblyFilePath, FileStream fs, Dictionary<int, int> methodBodyUses)
{
using FileStream os = File.Open(trimmedAssemblyFilePath, FileMode.Create);
{
fs.Position = 0;
MemoryStream memStream = new MemoryStream((int)fs.Length);
fs.CopyTo(memStream);
using MemoryStream memStream = new MemoryStream((int)fs.Length);

fs.Position = 0;
fs.CopyTo(memStream);

foreach (var kvp in methodBodyUses)
foreach (var kvp in methodBodyUses)
{
int rva = kvp.Key;
int count = kvp.Value;
if (count == 0)
{
int rva = kvp.Key;
int count = kvp.Value;
if (count == 0)
{
int methodSize = ComputeMethodSize(peReader, rva);
int actualLoc = ComputeMethodHash(peReader, rva);
int headerSize = ComputeMethodHeaderSize(memStream, actualLoc);
if (headerSize == 1) //Set code size to zero for TinyFormat
SetCodeSizeToZeroForTiny(ref memStream, actualLoc);
ZeroOutMethodBody(ref memStream, methodSize, actualLoc, headerSize);
}
else if (count < 0)
{
Log.LogError($"Method usage count is less than zero for rva: {rva}.");
}
int methodSize = ComputeMethodSize(peReader, rva);
int actualLoc = ComputeMethodHash(peReader, rva);
int headerSize = ComputeMethodHeaderSize(memStream, actualLoc);
if (headerSize == 1) //Set code size to zero for TinyFormat
SetCodeSizeToZeroForTiny(memStream, actualLoc);
ZeroOutMethodBody(memStream, methodSize, actualLoc, headerSize);
}
else if (count < 0)
{
Log.LogError($"Method usage count is less than zero for rva: {rva}.");
}

memStream.Position = 0;
memStream.CopyTo(os);
}

memStream.Position = 0;
memStream.CopyTo(os);
}

private static int ComputeMethodSize(PEReader peReader, int rva) => peReader.GetMethodBody(rva).Size;
Expand All @@ -351,17 +350,15 @@ private static int ComputeMethodHeaderSize(MemoryStream memStream, int actualLoc
return (headerFlag == 2 ? 1 : 4);
}

private static void SetCodeSizeToZeroForTiny(ref MemoryStream memStream, int actualLoc)
private static void SetCodeSizeToZeroForTiny(MemoryStream memStream, int actualLoc)
{
memStream.Position = actualLoc;
byte[] header = {0b10};
memStream.Write(header, 0, 1);
memStream.WriteByte(0b10);
}

private static void ZeroOutMethodBody(ref MemoryStream memStream, int methodSize, int actualLoc, int headerSize)
private static void ZeroOutMethodBody(MemoryStream memStream, int methodSize, int actualLoc, int headerSize)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mapping the region as Span and then Span.Clear will be cheaper than renting new buffer + copy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be missing something but why are we using a copied MemoryStream at all rather than just directly writing to the FileStream?

{
memStream.Position = actualLoc + headerSize;

byte[] zeroBuffer;
zeroBuffer = ArrayPool<byte>.Shared.Rent(methodSize);
Array.Clear(zeroBuffer, 0, zeroBuffer.Length);
Expand Down
Loading