From 8db482834d6b158f944b08f2f64bc20ba205edee Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 19 Feb 2022 02:38:01 +1100 Subject: [PATCH 01/23] Handle empty XMP profiles. Fix #2012 --- .../Common/Extensions/StreamExtensions.cs | 2 +- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 10 ++- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 4 +- src/ImageSharp/Formats/Gif/LzwDecoder.cs | 2 +- .../Sections/GifXmpApplicationExtension.cs | 72 +++++++++++-------- src/ImageSharp/Metadata/ImageMetadata.cs | 3 +- .../Formats/Gif/GifDecoderTests.cs | 12 ++++ .../Formats/WebP/WebpMetaDataTests.cs | 3 +- .../Metadata/Profiles/XMP/XmpProfileTests.cs | 13 ++-- tests/ImageSharp.Tests/TestImages.cs | 1 + ...2012_Stronghold-Crusader-Extreme-Cover.png | 3 + ...2012_Stronghold-Crusader-Extreme-Cover.gif | 3 + 12 files changed, 82 insertions(+), 46 deletions(-) create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012EmptyXmp_Rgba32_issue2012_Stronghold-Crusader-Extreme-Cover.png create mode 100644 tests/Images/Input/Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif diff --git a/src/ImageSharp/Common/Extensions/StreamExtensions.cs b/src/ImageSharp/Common/Extensions/StreamExtensions.cs index 1193eccee3..8746989b38 100644 --- a/src/ImageSharp/Common/Extensions/StreamExtensions.cs +++ b/src/ImageSharp/Common/Extensions/StreamExtensions.cs @@ -13,7 +13,7 @@ namespace SixLabors.ImageSharp internal static class StreamExtensions { /// - /// Writes data from a stream into the provided buffer. + /// Writes data from a stream from the provided buffer. /// /// The stream. /// The buffer. diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 72e3ed144c..0a5eba7895 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -265,10 +265,14 @@ private void ReadApplicationExtension() this.stream.Read(this.buffer, 0, GifConstants.ApplicationBlockSize); bool isXmp = this.buffer.AsSpan().StartsWith(GifConstants.XmpApplicationIdentificationBytes); - if (isXmp) + if (isXmp && !this.IgnoreMetadata) { - var extension = GifXmpApplicationExtension.Read(this.stream); - this.metadata.XmpProfile = new XmpProfile(extension.Data); + var extension = GifXmpApplicationExtension.Read(this.stream, this.MemoryAllocator); + if (extension.Data.Length > 0) + { + this.metadata.XmpProfile = new XmpProfile(extension.Data); + } + return; } else diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index a21b050a81..e8bb136122 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -123,7 +123,8 @@ public void Encode(Image image, Stream stream, CancellationToken this.WriteComments(gifMetadata, stream); // Write application extensions. - this.WriteApplicationExtensions(stream, image.Frames.Count, gifMetadata.RepeatCount, metadata.XmpProfile); + XmpProfile xmpProfile = image.Metadata.XmpProfile ?? image.Frames.RootFrame.Metadata.XmpProfile; + this.WriteApplicationExtensions(stream, image.Frames.Count, gifMetadata.RepeatCount, xmpProfile); if (useGlobalTable) { @@ -137,7 +138,6 @@ public void Encode(Image image, Stream stream, CancellationToken // Clean up. quantized.Dispose(); - // TODO: Write extension etc stream.WriteByte(GifConstants.EndIntroducer); } diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index 68227db53d..1db05bb835 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -68,7 +68,7 @@ public LzwDecoder(MemoryAllocator memoryAllocator, BufferedReadStream stream) /// The pixel array to decode to. public void DecodePixels(int dataSize, Buffer2D pixels) { - Guard.MustBeLessThan(dataSize, int.MaxValue, nameof(dataSize)); + Guard.MustBeLessThanOrEqualTo(1 << dataSize, MaxStackSize, nameof(dataSize)); // The resulting index table length. int width = pixels.Width; diff --git a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs index 236508fe99..00bf4b7a2b 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs @@ -2,9 +2,9 @@ // Licensed under the Apache License, Version 2.0. using System; -using System.Collections.Generic; using System.IO; -using SixLabors.ImageSharp.Metadata.Profiles.Xmp; +using SixLabors.ImageSharp.IO; +using SixLabors.ImageSharp.Memory; namespace SixLabors.ImageSharp.Formats.Gif { @@ -14,7 +14,10 @@ namespace SixLabors.ImageSharp.Formats.Gif public byte Label => GifConstants.ApplicationExtensionLabel; - public int ContentLength => this.Data.Length + 269; // 12 + Data Length + 1 + 256 + // size : 1 + // identifier : 11 + // magic trailer : 257 + public int ContentLength => this.Data.Length + 269; /// /// Gets the raw Data. @@ -25,40 +28,23 @@ namespace SixLabors.ImageSharp.Formats.Gif /// Reads the XMP metadata from the specified stream. /// /// The stream to read from. + /// The memory allocator. /// The XMP metadata /// Thrown if the XMP block is not properly terminated. - public static GifXmpApplicationExtension Read(Stream stream) + public static GifXmpApplicationExtension Read(Stream stream, MemoryAllocator allocator) { - // Read data in blocks, until an \0 character is encountered. - // We overshoot, indicated by the terminatorIndex variable. - const int bufferSize = 256; - var list = new List(); - int terminationIndex = -1; - while (terminationIndex < 0) - { - byte[] temp = new byte[bufferSize]; - int bytesRead = stream.Read(temp); - list.Add(temp); - terminationIndex = Array.IndexOf(temp, (byte)1); - } + byte[] xmpBytes = ReadXmpData(stream, allocator); - // Pack all the blocks (except magic trailer) into one single array again. - int dataSize = ((list.Count - 1) * bufferSize) + terminationIndex; - byte[] buffer = new byte[dataSize]; - Span bufferSpan = buffer; - int pos = 0; - for (int j = 0; j < list.Count - 1; j++) + // Exclude the "magic trailer", see XMP Specification Part 3, 1.1.2 GIF + int xmpLength = xmpBytes.Length - 256; // 257 - unread 0x0 + byte[] buffer = Array.Empty(); + if (xmpLength > 0) { - list[j].CopyTo(bufferSpan.Slice(pos)); - pos += bufferSize; + buffer = new byte[xmpLength]; + xmpBytes.AsSpan(0, xmpLength).CopyTo(buffer); + stream.Skip(1); // Skip the terminator. } - // Last one only needs the portion until terminationIndex copied over. - Span lastBytes = list[list.Count - 1]; - lastBytes.Slice(0, terminationIndex).CopyTo(bufferSpan.Slice(pos)); - - // Skip the remainder of the magic trailer. - stream.Skip(258 - (bufferSize - terminationIndex)); return new GifXmpApplicationExtension(buffer); } @@ -67,7 +53,7 @@ public int WriteTo(Span buffer) int totalSize = this.ContentLength; if (buffer.Length < totalSize) { - throw new InsufficientMemoryException("Unable to write XMP metadata to GIF image"); + ThrowInsufficientMemory(); } int bytesWritten = 0; @@ -93,5 +79,29 @@ public int WriteTo(Span buffer) return totalSize; } + + private static byte[] ReadXmpData(Stream stream, MemoryAllocator allocator) + { + using ChunkedMemoryStream bytes = new(allocator); + + // XMP data doesn't have a fixed length nor is there an indicator of the length. + // So we simply read one byte at a time until we hit the 0x0 value at the end + // of the magic trailer or the end of the stream. + // Using ChunkedMemoryStream reduces the array resize allocation normally associated + // with writing from a non fixed-size buffer. + while (true) + { + int b = stream.ReadByte(); + if (b <= 0) + { + return bytes.ToArray(); + } + + bytes.WriteByte((byte)b); + } + } + + private static void ThrowInsufficientMemory() + => throw new InsufficientMemoryException("Unable to write XMP metadata to GIF image"); } } diff --git a/src/ImageSharp/Metadata/ImageMetadata.cs b/src/ImageSharp/Metadata/ImageMetadata.cs index 89592f776c..4760fa141e 100644 --- a/src/ImageSharp/Metadata/ImageMetadata.cs +++ b/src/ImageSharp/Metadata/ImageMetadata.cs @@ -68,6 +68,7 @@ private ImageMetadata(ImageMetadata other) this.ExifProfile = other.ExifProfile?.DeepClone(); this.IccProfile = other.IccProfile?.DeepClone(); this.IptcProfile = other.IptcProfile?.DeepClone(); + this.XmpProfile = other.XmpProfile?.DeepClone(); } /// @@ -175,7 +176,7 @@ public TFormatMetadata GetFormatMetadata(IImageFormat - public ImageMetadata DeepClone() => new ImageMetadata(this); + public ImageMetadata DeepClone() => new(this); /// /// Synchronizes the profiles with the current metadata. diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index c8ecdb717b..7ca64bea76 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -259,5 +259,17 @@ public void Issue1962(TestImageProvider provider) image.CompareFirstFrameToReferenceOutput(ImageComparer.Exact, provider); } + + // https://github.com/SixLabors/ImageSharp/issues/2012 + [Theory] + [WithFile(TestImages.Gif.Issues.Issue2012EmptyXmp, PixelTypes.Rgba32)] + public void Issue2012EmptyXmp(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(); + + image.DebugSave(provider); + image.CompareFirstFrameToReferenceOutput(ImageComparer.Exact, provider); + } } } diff --git a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs index 7fba86b4fe..eaa7fb5646 100644 --- a/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs +++ b/tests/ImageSharp.Tests/Formats/WebP/WebpMetaDataTests.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. using System.IO; +using System.Threading.Tasks; using SixLabors.ImageSharp.Formats.Webp; using SixLabors.ImageSharp.Metadata.Profiles.Exif; using SixLabors.ImageSharp.PixelFormats; @@ -66,7 +67,7 @@ public void IgnoreMetadata_ControlsWhetherIccpIsParsed(TestImageProvider [Theory] [WithFile(TestImages.Webp.Lossy.WithXmp, PixelTypes.Rgba32, false)] [WithFile(TestImages.Webp.Lossy.WithXmp, PixelTypes.Rgba32, true)] - public async void IgnoreMetadata_ControlsWhetherXmpIsParsed(TestImageProvider provider, bool ignoreMetadata) + public async Task IgnoreMetadata_ControlsWhetherXmpIsParsed(TestImageProvider provider, bool ignoreMetadata) where TPixel : unmanaged, IPixel { var decoder = new WebpDecoder { IgnoreMetadata = ignoreMetadata }; diff --git a/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs b/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs index 81dad699a1..a3c15d0b3f 100644 --- a/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs +++ b/tests/ImageSharp.Tests/Metadata/Profiles/XMP/XmpProfileTests.cs @@ -4,6 +4,7 @@ using System; using System.IO; using System.Text; +using System.Threading.Tasks; using System.Xml.Linq; using SixLabors.ImageSharp.Formats; using SixLabors.ImageSharp.Formats.Gif; @@ -31,7 +32,7 @@ public class XmpProfileTests [Theory] [WithFile(TestImages.Gif.Receipt, PixelTypes.Rgba32)] - public async void ReadXmpMetadata_FromGif_Works(TestImageProvider provider) + public async Task ReadXmpMetadata_FromGif_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(GifDecoder)) @@ -45,7 +46,7 @@ public async void ReadXmpMetadata_FromGif_Works(TestImageProvider(TestImageProvider provider) + public async Task ReadXmpMetadata_FromJpg_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(JpegDecoder)) @@ -57,7 +58,7 @@ public async void ReadXmpMetadata_FromJpg_Works(TestImageProvider(TestImageProvider provider) + public async Task ReadXmpMetadata_FromPng_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(PngDecoder)) @@ -69,7 +70,7 @@ public async void ReadXmpMetadata_FromPng_Works(TestImageProvider(TestImageProvider provider) + public async Task ReadXmpMetadata_FromTiff_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(TiffDecoder)) @@ -81,7 +82,7 @@ public async void ReadXmpMetadata_FromTiff_Works(TestImageProvider(TestImageProvider provider) + public async Task ReadXmpMetadata_FromWebp_Works(TestImageProvider provider) where TPixel : unmanaged, IPixel { using (Image image = await provider.GetImageAsync(WebpDecoder)) @@ -157,7 +158,7 @@ public void WritingJpeg_PreservesXmpProfile() } [Fact] - public async void WritingJpeg_PreservesExtendedXmpProfile() + public async Task WritingJpeg_PreservesExtendedXmpProfile() { // arrange var provider = TestImageProvider.File(TestImages.Jpeg.Baseline.ExtendedXmp); diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 8b943194a5..ffdd6f8f2e 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -452,6 +452,7 @@ public static class Issues public const string Issue1530 = "Gif/issues/issue1530.gif"; public const string InvalidColorIndex = "Gif/issues/issue1668_invalidcolorindex.gif"; public const string Issue1962NoColorTable = "Gif/issues/issue1962_tiniest_gif_1st.gif"; + public const string Issue2012EmptyXmp = "Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif"; } public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin, Leo, Ratio4x1, Ratio1x4 }; diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012EmptyXmp_Rgba32_issue2012_Stronghold-Crusader-Extreme-Cover.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012EmptyXmp_Rgba32_issue2012_Stronghold-Crusader-Extreme-Cover.png new file mode 100644 index 0000000000..c646eb8605 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012EmptyXmp_Rgba32_issue2012_Stronghold-Crusader-Extreme-Cover.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:3ba8295d8a4b087d6c19fbad7e97cef7b5ce1a69b9c4c4f79cee6bc77e41f236 +size 62778 diff --git a/tests/Images/Input/Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif b/tests/Images/Input/Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif new file mode 100644 index 0000000000..90f0e0f1c5 --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:97f8fdbabfbd9663bf9940dc33f81edf330b62789d1aa573ae85a520903723e5 +size 77498 From 5214681f4c5f11ff255730f54fb61e1c6f9f1361 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 18 Feb 2022 19:35:30 +0100 Subject: [PATCH 02/23] Add ARM version of adler32 --- src/ImageSharp/Compression/Zlib/Adler32.cs | 192 +++++++++++++++++++-- 1 file changed, 175 insertions(+), 17 deletions(-) diff --git a/src/ImageSharp/Compression/Zlib/Adler32.cs b/src/ImageSharp/Compression/Zlib/Adler32.cs index 7eb3f4516f..0e24aa8e16 100644 --- a/src/ImageSharp/Compression/Zlib/Adler32.cs +++ b/src/ImageSharp/Compression/Zlib/Adler32.cs @@ -6,6 +6,9 @@ #if SUPPORTS_RUNTIME_INTRINSICS using System.Runtime.Intrinsics; using System.Runtime.Intrinsics.X86; +#if NET5_0_OR_GREATER +using System.Runtime.Intrinsics.Arm; +#endif #endif #pragma warning disable IDE0007 // Use implicit type @@ -23,14 +26,16 @@ internal static class Adler32 public const uint SeedValue = 1U; // Largest prime smaller than 65536 - private const uint BASE = 65521; + private const uint Base = 65521; // NMAX is the largest n such that 255n(n+1)/2 + (n+1)(BASE-1) <= 2^32-1 - private const uint NMAX = 5552; + private const uint Nmax = 5552; #if SUPPORTS_RUNTIME_INTRINSICS private const int MinBufferSize = 64; + private const uint ArmBlockSize = 1 << 5; + // The C# compiler emits this as a compile-time constant embedded in the PE file. private static ReadOnlySpan Tap1Tap2 => new byte[] { @@ -67,11 +72,14 @@ public static uint Calculate(uint adler, ReadOnlySpan buffer) { return CalculateSse(adler, buffer); } - - return CalculateScalar(adler, buffer); -#else - return CalculateScalar(adler, buffer); +#if NET5_0_OR_GREATER + if (AdvSimd.IsSupported) + { + return CalculateArm(adler, buffer); + } #endif +#endif + return CalculateScalar(adler, buffer); } // Based on https://github.com/chromium/chromium/blob/master/third_party/zlib/adler32_simd.c @@ -95,7 +103,7 @@ private static unsafe uint CalculateSse(uint adler, ReadOnlySpan buffer) fixed (byte* tapPtr = Tap1Tap2) { index += (int)blocks * BLOCK_SIZE; - var localBufferPtr = bufferPtr; + byte* localBufferPtr = bufferPtr; // _mm_setr_epi8 on x86 Vector128 tap1 = Sse2.LoadVector128((sbyte*)tapPtr); @@ -105,7 +113,7 @@ private static unsafe uint CalculateSse(uint adler, ReadOnlySpan buffer) while (blocks > 0) { - uint n = NMAX / BLOCK_SIZE; /* The NMAX constraint. */ + uint n = Nmax / BLOCK_SIZE; /* The NMAX constraint. */ if (n > blocks) { n = blocks; @@ -158,8 +166,8 @@ private static unsafe uint CalculateSse(uint adler, ReadOnlySpan buffer) s2 = v_s2.ToScalar(); // Reduce. - s1 %= BASE; - s2 %= BASE; + s1 %= Base; + s2 %= Base; } if (length > 0) @@ -192,18 +200,168 @@ private static unsafe uint CalculateSse(uint adler, ReadOnlySpan buffer) s2 += s1 += *localBufferPtr++; } - if (s1 >= BASE) + if (s1 >= Base) { - s1 -= BASE; + s1 -= Base; } - s2 %= BASE; + s2 %= Base; } return s1 | (s2 << 16); } } } + +#if NET5_0_OR_GREATER + + // Based on: https://github.com/chromium/chromium/blob/master/third_party/zlib/adler32_simd.c + [MethodImpl(InliningOptions.HotPath | InliningOptions.ShortMethod)] + private static unsafe uint CalculateArm(uint crc, ReadOnlySpan buffer) + { + // Split Adler-32 into component sums. + uint s1 = crc & 0xFFFF; + uint s2 = (crc >> 16) & 0xFFFF; + int len = buffer.Length; + + // Serially compute s1 & s2, until the data is 16-byte aligned. + int bufferOffset = 0; + if ((bufferOffset & 15) != 0) + { + while ((bufferOffset & 15) != 0) + { + s2 += s1 += buffer[bufferOffset++]; + --len; + + if (s1 >= Base) + { + s1 -= Base; + } + + s2 %= Base; + } + } + + // Process the data in blocks. + long blocks = len / ArmBlockSize; + len -= (int)(blocks * ArmBlockSize); + fixed (byte* bufferPtr = buffer) + { + while (blocks != 0) + { + var n = Nmax / ArmBlockSize; + if (n > blocks) + { + n = (uint)blocks; + } + + blocks -= n; + + // Process n blocks of data. At most nMax data bytes can be + // processed before s2 must be reduced modulo Base. + var vs2 = Vector128.Create(0, 0, 0, s1 * n); + Vector128 vs1 = Vector128.Zero; + Vector128 vColumnSum1 = Vector128.Zero; + Vector128 vColumnSum2 = Vector128.Zero; + Vector128 vColumnSum3 = Vector128.Zero; + Vector128 vColumnSum4 = Vector128.Zero; + + do + { + // Load 32 input bytes. + Vector128 bytes1 = AdvSimd.LoadVector128(bufferPtr + bufferOffset).AsUInt16(); + Vector128 bytes2 = AdvSimd.LoadVector128(bufferPtr + bufferOffset + 16).AsUInt16(); + + // Add previous block byte sum to v_s2. + vs2 = AdvSimd.Add(vs2, vs1); + + // Horizontally add the bytes for s1. + vs1 = AdvSimd.AddPairwiseWideningAndAdd( + vs1.AsUInt32(), + AdvSimd.AddPairwiseWideningAndAdd(AdvSimd.AddPairwiseWidening(bytes1.AsByte()).AsUInt16(), bytes2.AsByte())); + + // Vertically add the bytes for s2. + vColumnSum1 = AdvSimd.AddWideningLower(vColumnSum1, bytes1.GetLower().AsByte()); + vColumnSum2 = AdvSimd.AddWideningLower(vColumnSum2, bytes1.GetUpper().AsByte()); + vColumnSum3 = AdvSimd.AddWideningLower(vColumnSum3, bytes2.GetLower().AsByte()); + vColumnSum4 = AdvSimd.AddWideningLower(vColumnSum4, bytes2.GetUpper().AsByte()); + + bufferOffset += (int)ArmBlockSize; + } while (--n > 0); + + vs2 = AdvSimd.ShiftLeftLogical(vs2, 5); + + // Multiply-add bytes by [ 32, 31, 30, ... ] for s2. + vs2 = AdvSimd.MultiplyWideningLowerAndAdd(vs2, vColumnSum1.GetLower(), Vector64.Create((ushort)32, 31, 30, 29)); + vs2 = AdvSimd.MultiplyWideningLowerAndAdd(vs2, vColumnSum1.GetUpper(), Vector64.Create((ushort)28, 27, 26, 25)); + vs2 = AdvSimd.MultiplyWideningLowerAndAdd(vs2, vColumnSum2.GetLower(), Vector64.Create((ushort)24, 23, 22, 21)); + vs2 = AdvSimd.MultiplyWideningLowerAndAdd(vs2, vColumnSum2.GetUpper(), Vector64.Create((ushort)20, 19, 18, 17)); + vs2 = AdvSimd.MultiplyWideningLowerAndAdd(vs2, vColumnSum3.GetLower(), Vector64.Create((ushort)16, 15, 14, 13)); + vs2 = AdvSimd.MultiplyWideningLowerAndAdd(vs2, vColumnSum3.GetUpper(), Vector64.Create((ushort)12, 11, 10, 9)); + vs2 = AdvSimd.MultiplyWideningLowerAndAdd(vs2, vColumnSum4.GetLower(), Vector64.Create((ushort)8, 7, 6, 5)); + vs2 = AdvSimd.MultiplyWideningLowerAndAdd(vs2, vColumnSum4.GetUpper(), Vector64.Create((ushort)4, 3, 2, 1)); + + // Sum epi32 ints v_s1(s2) and accumulate in s1(s2). + Vector64 sum1 = AdvSimd.AddPairwise(vs1.GetLower(), vs1.GetUpper()); + Vector64 sum2 = AdvSimd.AddPairwise(vs2.GetLower(), vs2.GetUpper()); + Vector64 s1s2 = AdvSimd.AddPairwise(sum1, sum2); + + // Store the results. + s1 = AdvSimd.Extract(s1s2, 0); + s2 = AdvSimd.Extract(s1s2, 1); + + // Reduce. + s1 %= Base; + s2 %= Base; + } + } + + // Handle leftover data. + if (len != 0) + { + if (len >= 16) + { + s2 += s1 += buffer[bufferOffset++]; + s2 += s1 += buffer[bufferOffset++]; + s2 += s1 += buffer[bufferOffset++]; + s2 += s1 += buffer[bufferOffset++]; + + s2 += s1 += buffer[bufferOffset++]; + s2 += s1 += buffer[bufferOffset++]; + s2 += s1 += buffer[bufferOffset++]; + s2 += s1 += buffer[bufferOffset++]; + + s2 += s1 += buffer[bufferOffset++]; + s2 += s1 += buffer[bufferOffset++]; + s2 += s1 += buffer[bufferOffset++]; + s2 += s1 += buffer[bufferOffset++]; + + s2 += s1 += buffer[bufferOffset++]; + s2 += s1 += buffer[bufferOffset++]; + s2 += s1 += buffer[bufferOffset++]; + s2 += s1 += buffer[bufferOffset++]; + + len -= 16; + } + + while (len-- > 0) + { + s2 += s1 += buffer[bufferOffset++]; + } + + if (s1 >= Base) + { + s1 -= Base; + } + + s2 %= Base; + } + + // Return the recombined sums. + return s1 | (s2 << 16); + } + +#endif #endif [MethodImpl(InliningOptions.HotPath | InliningOptions.ShortMethod)] @@ -215,12 +373,12 @@ private static unsafe uint CalculateScalar(uint adler, ReadOnlySpan buffer fixed (byte* bufferPtr = buffer) { - var localBufferPtr = bufferPtr; + byte* localBufferPtr = bufferPtr; uint length = (uint)buffer.Length; while (length > 0) { - k = length < NMAX ? length : NMAX; + k = length < Nmax ? length : Nmax; length -= k; while (k >= 16) @@ -251,8 +409,8 @@ private static unsafe uint CalculateScalar(uint adler, ReadOnlySpan buffer s2 += s1 += *localBufferPtr++; } - s1 %= BASE; - s2 %= BASE; + s1 %= Base; + s2 %= Base; } return (s2 << 16) | s1; From ce429e905f6a8524c936117dea33b1c41a28297e Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 18 Feb 2022 20:14:36 +0100 Subject: [PATCH 03/23] Remove serial computation, does not not make sense here --- src/ImageSharp/Compression/Zlib/Adler32.cs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/ImageSharp/Compression/Zlib/Adler32.cs b/src/ImageSharp/Compression/Zlib/Adler32.cs index 0e24aa8e16..c7c733abf3 100644 --- a/src/ImageSharp/Compression/Zlib/Adler32.cs +++ b/src/ImageSharp/Compression/Zlib/Adler32.cs @@ -224,24 +224,6 @@ private static unsafe uint CalculateArm(uint crc, ReadOnlySpan buffer) uint s2 = (crc >> 16) & 0xFFFF; int len = buffer.Length; - // Serially compute s1 & s2, until the data is 16-byte aligned. - int bufferOffset = 0; - if ((bufferOffset & 15) != 0) - { - while ((bufferOffset & 15) != 0) - { - s2 += s1 += buffer[bufferOffset++]; - --len; - - if (s1 >= Base) - { - s1 -= Base; - } - - s2 %= Base; - } - } - // Process the data in blocks. long blocks = len / ArmBlockSize; len -= (int)(blocks * ArmBlockSize); From c7b62d815547e8a60d72299c469448b6e7810a5f Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 18 Feb 2022 20:16:10 +0100 Subject: [PATCH 04/23] Fix build error --- src/ImageSharp/Compression/Zlib/Adler32.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Compression/Zlib/Adler32.cs b/src/ImageSharp/Compression/Zlib/Adler32.cs index c7c733abf3..96f409f7a8 100644 --- a/src/ImageSharp/Compression/Zlib/Adler32.cs +++ b/src/ImageSharp/Compression/Zlib/Adler32.cs @@ -223,6 +223,7 @@ private static unsafe uint CalculateArm(uint crc, ReadOnlySpan buffer) uint s1 = crc & 0xFFFF; uint s2 = (crc >> 16) & 0xFFFF; int len = buffer.Length; + int bufferOffset = 0; // Process the data in blocks. long blocks = len / ArmBlockSize; @@ -231,7 +232,7 @@ private static unsafe uint CalculateArm(uint crc, ReadOnlySpan buffer) { while (blocks != 0) { - var n = Nmax / ArmBlockSize; + uint n = Nmax / ArmBlockSize; if (n > blocks) { n = (uint)blocks; From 36420de11bed1b6a08dd18008de3b07c91192c45 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 18 Feb 2022 20:22:56 +0100 Subject: [PATCH 05/23] Block size is the same for sse and arm --- src/ImageSharp/Compression/Zlib/Adler32.cs | 26 ++++++++++------------ 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/ImageSharp/Compression/Zlib/Adler32.cs b/src/ImageSharp/Compression/Zlib/Adler32.cs index 96f409f7a8..cf1e70b26d 100644 --- a/src/ImageSharp/Compression/Zlib/Adler32.cs +++ b/src/ImageSharp/Compression/Zlib/Adler32.cs @@ -34,7 +34,8 @@ internal static class Adler32 #if SUPPORTS_RUNTIME_INTRINSICS private const int MinBufferSize = 64; - private const uint ArmBlockSize = 1 << 5; + // Data will be processed in blocks of 32 bytes. + private const int BlockSize = 1 << 5; // The C# compiler emits this as a compile-time constant embedded in the PE file. private static ReadOnlySpan Tap1Tap2 => new byte[] @@ -91,18 +92,16 @@ private static unsafe uint CalculateSse(uint adler, ReadOnlySpan buffer) uint s2 = (adler >> 16) & 0xFFFF; // Process the data in blocks. - const int BLOCK_SIZE = 1 << 5; - uint length = (uint)buffer.Length; - uint blocks = length / BLOCK_SIZE; - length -= blocks * BLOCK_SIZE; + uint blocks = length / BlockSize; + length -= blocks * BlockSize; int index = 0; fixed (byte* bufferPtr = buffer) { fixed (byte* tapPtr = Tap1Tap2) { - index += (int)blocks * BLOCK_SIZE; + index += (int)blocks * BlockSize; byte* localBufferPtr = bufferPtr; // _mm_setr_epi8 on x86 @@ -113,7 +112,7 @@ private static unsafe uint CalculateSse(uint adler, ReadOnlySpan buffer) while (blocks > 0) { - uint n = Nmax / BLOCK_SIZE; /* The NMAX constraint. */ + uint n = Nmax / BlockSize; /* The NMAX constraint. */ if (n > blocks) { n = blocks; @@ -146,7 +145,7 @@ private static unsafe uint CalculateSse(uint adler, ReadOnlySpan buffer) Vector128 mad2 = Ssse3.MultiplyAddAdjacent(bytes2, tap2); v_s2 = Sse2.Add(v_s2, Sse2.MultiplyAddAdjacent(mad2, ones).AsUInt32()); - localBufferPtr += BLOCK_SIZE; + localBufferPtr += BlockSize; } while (--n > 0); @@ -226,13 +225,13 @@ private static unsafe uint CalculateArm(uint crc, ReadOnlySpan buffer) int bufferOffset = 0; // Process the data in blocks. - long blocks = len / ArmBlockSize; - len -= (int)(blocks * ArmBlockSize); + long blocks = len / BlockSize; + len -= (int)(blocks * BlockSize); fixed (byte* bufferPtr = buffer) { while (blocks != 0) { - uint n = Nmax / ArmBlockSize; + uint n = Nmax / BlockSize; if (n > blocks) { n = (uint)blocks; @@ -269,7 +268,7 @@ private static unsafe uint CalculateArm(uint crc, ReadOnlySpan buffer) vColumnSum3 = AdvSimd.AddWideningLower(vColumnSum3, bytes2.GetLower().AsByte()); vColumnSum4 = AdvSimd.AddWideningLower(vColumnSum4, bytes2.GetUpper().AsByte()); - bufferOffset += (int)ArmBlockSize; + bufferOffset += BlockSize; } while (--n > 0); vs2 = AdvSimd.ShiftLeftLogical(vs2, 5); @@ -352,7 +351,6 @@ private static unsafe uint CalculateScalar(uint adler, ReadOnlySpan buffer { uint s1 = adler & 0xFFFF; uint s2 = (adler >> 16) & 0xFFFF; - uint k; fixed (byte* bufferPtr = buffer) { @@ -361,7 +359,7 @@ private static unsafe uint CalculateScalar(uint adler, ReadOnlySpan buffer while (length > 0) { - k = length < Nmax ? length : Nmax; + var k = length < Nmax ? length : Nmax; length -= k; while (k >= 16) From c64078cc17369e92ea83df3ec8687e4d9c41daf3 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 19 Feb 2022 16:58:09 +1100 Subject: [PATCH 06/23] Remove unnecessary throw and optimize write. --- src/ImageSharp/Formats/Gif/GifEncoderCore.cs | 23 +++++++++++-------- .../Sections/GifGraphicControlExtension.cs | 6 ++--- .../GifNetscapeLoopingApplicationExtension.cs | 2 +- .../Sections/GifXmpApplicationExtension.cs | 13 ++--------- 4 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs index e8bb136122..da5b1cb236 100644 --- a/src/ImageSharp/Formats/Gif/GifEncoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifEncoderCore.cs @@ -428,26 +428,31 @@ private void WriteExtension(TGifExtension extension, Stream strea where TGifExtension : struct, IGifExtension { IMemoryOwner owner = null; - Span buffer; + Span extensionBuffer; int extensionSize = extension.ContentLength; - if (extensionSize > this.buffer.Length - 3) + + if (extensionSize == 0) + { + return; + } + else if (extensionSize > this.buffer.Length - 3) { owner = this.memoryAllocator.Allocate(extensionSize + 3); - buffer = owner.GetSpan(); + extensionBuffer = owner.GetSpan(); } else { - buffer = this.buffer; + extensionBuffer = this.buffer; } - buffer[0] = GifConstants.ExtensionIntroducer; - buffer[1] = extension.Label; + extensionBuffer[0] = GifConstants.ExtensionIntroducer; + extensionBuffer[1] = extension.Label; - extension.WriteTo(buffer.Slice(2)); + extension.WriteTo(extensionBuffer.Slice(2)); - buffer[extensionSize + 2] = GifConstants.Terminator; + extensionBuffer[extensionSize + 2] = GifConstants.Terminator; - stream.Write(buffer, 0, extensionSize + 3); + stream.Write(extensionBuffer, 0, extensionSize + 3); owner?.Dispose(); } diff --git a/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs index 801849c9b8..8476336942 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifGraphicControlExtension.cs @@ -71,13 +71,11 @@ public int WriteTo(Span buffer) dest = this; - return 5; + return ((IGifExtension)this).ContentLength; } public static GifGraphicControlExtension Parse(ReadOnlySpan buffer) - { - return MemoryMarshal.Cast(buffer)[0]; - } + => MemoryMarshal.Cast(buffer)[0]; public static byte GetPackedValue(GifDisposalMethod disposalMethod, bool userInputFlag = false, bool transparencyFlag = false) { diff --git a/src/ImageSharp/Formats/Gif/Sections/GifNetscapeLoopingApplicationExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifNetscapeLoopingApplicationExtension.cs index 2c7bed6115..c9e8033dbd 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifNetscapeLoopingApplicationExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifNetscapeLoopingApplicationExtension.cs @@ -40,7 +40,7 @@ public int WriteTo(Span buffer) // 0 means loop indefinitely. Count is set as play n + 1 times. BinaryPrimitives.WriteUInt16LittleEndian(buffer.Slice(14, 2), this.RepeatCount); - return 16; // Length - Introducer + Label + Terminator. + return this.ContentLength; // Length - Introducer + Label + Terminator. } } } diff --git a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs index 00bf4b7a2b..8c396e7fb3 100644 --- a/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs +++ b/src/ImageSharp/Formats/Gif/Sections/GifXmpApplicationExtension.cs @@ -17,7 +17,7 @@ namespace SixLabors.ImageSharp.Formats.Gif // size : 1 // identifier : 11 // magic trailer : 257 - public int ContentLength => this.Data.Length + 269; + public int ContentLength => (this.Data.Length > 0) ? this.Data.Length + 269 : 0; /// /// Gets the raw Data. @@ -50,12 +50,6 @@ public static GifXmpApplicationExtension Read(Stream stream, MemoryAllocator all public int WriteTo(Span buffer) { - int totalSize = this.ContentLength; - if (buffer.Length < totalSize) - { - ThrowInsufficientMemory(); - } - int bytesWritten = 0; buffer[bytesWritten++] = GifConstants.ApplicationBlockSize; @@ -77,7 +71,7 @@ public int WriteTo(Span buffer) buffer[bytesWritten++] = 0x00; - return totalSize; + return this.ContentLength; } private static byte[] ReadXmpData(Stream stream, MemoryAllocator allocator) @@ -100,8 +94,5 @@ private static byte[] ReadXmpData(Stream stream, MemoryAllocator allocator) bytes.WriteByte((byte)b); } } - - private static void ThrowInsufficientMemory() - => throw new InsufficientMemoryException("Unable to write XMP metadata to GIF image"); } } From c129278cae336b997d1de0f90047326e527d9fe2 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Sat, 19 Feb 2022 21:10:15 +1100 Subject: [PATCH 07/23] Don't throw for bad min code, just don't decode indices. --- src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 2 +- src/ImageSharp/Formats/Gif/LzwDecoder.cs | 12 ++++++++---- .../ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs | 12 ++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + .../Issue2012BadMinCode_Rgba32_issue2012_drona1.png | 3 +++ tests/Images/Input/Gif/issues/issue2012_drona1.gif | 3 +++ 6 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png create mode 100644 tests/Images/Input/Gif/issues/issue2012_drona1.gif diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 0a5eba7895..98e012658e 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -378,8 +378,8 @@ private void ReadFrame(ref Image image, ref ImageFrame p } indices = this.Configuration.MemoryAllocator.Allocate2D(this.imageDescriptor.Width, this.imageDescriptor.Height, AllocationOptions.Clean); - this.ReadFrameIndices(indices); + Span rawColorTable = default; if (localColorTable != null) { diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index 1db05bb835..8bb52b6371 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -68,16 +68,20 @@ public LzwDecoder(MemoryAllocator memoryAllocator, BufferedReadStream stream) /// The pixel array to decode to. public void DecodePixels(int dataSize, Buffer2D pixels) { - Guard.MustBeLessThanOrEqualTo(1 << dataSize, MaxStackSize, nameof(dataSize)); + // Calculate the clear code. The value of the clear code is 2 ^ dataSize + int clearCode = 1 << dataSize; + if (clearCode > MaxStackSize) + { + // Don't attempt to decode the frame indices. + // The image is most likely corrupted. + return; + } // The resulting index table length. int width = pixels.Width; int height = pixels.Height; int length = width * height; - // Calculate the clear code. The value of the clear code is 2 ^ dataSize - int clearCode = 1 << dataSize; - int codeSize = dataSize + 1; // Calculate the end code diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index 7ca64bea76..289bf6a4fb 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -271,5 +271,17 @@ public void Issue2012EmptyXmp(TestImageProvider provider) image.DebugSave(provider); image.CompareFirstFrameToReferenceOutput(ImageComparer.Exact, provider); } + + // https://github.com/SixLabors/ImageSharp/issues/2012 + [Theory] + [WithFile(TestImages.Gif.Issues.Issue2012BadMinCode, PixelTypes.Rgba32)] + public void Issue2012BadMinCode(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + using Image image = provider.GetImage(); + + image.DebugSave(provider); + image.CompareFirstFrameToReferenceOutput(ImageComparer.Exact, provider); + } } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index ffdd6f8f2e..6ff1162256 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -453,6 +453,7 @@ public static class Issues public const string InvalidColorIndex = "Gif/issues/issue1668_invalidcolorindex.gif"; public const string Issue1962NoColorTable = "Gif/issues/issue1962_tiniest_gif_1st.gif"; public const string Issue2012EmptyXmp = "Gif/issues/issue2012_Stronghold-Crusader-Extreme-Cover.gif"; + public const string Issue2012BadMinCode = "Gif/issues/issue2012_drona1.gif"; } public static readonly string[] All = { Rings, Giphy, Cheers, Trans, Kumin, Leo, Ratio4x1, Ratio1x4 }; diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png new file mode 100644 index 0000000000..5e23f55284 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:cf4bc93479140b05b25066eb10c33fa9f82c617828a56526d47f5a8c72035ef0 +size 1871 diff --git a/tests/Images/Input/Gif/issues/issue2012_drona1.gif b/tests/Images/Input/Gif/issues/issue2012_drona1.gif new file mode 100644 index 0000000000..803d684874 --- /dev/null +++ b/tests/Images/Input/Gif/issues/issue2012_drona1.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:cbb23b2a19e314969c6da99374ae133d834d76c3f0ab9df4a7edc9334bb065e6 +size 10508 From 892dbe318f03b4b21ecbf71d03989b2985e0db54 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sat, 19 Feb 2022 18:12:14 +0100 Subject: [PATCH 08/23] Throw exception, if palette chunk is missing --- src/ImageSharp/Formats/Png/PngScanlineProcessor.cs | 5 +++++ src/ImageSharp/Formats/Png/PngThrowHelper.cs | 3 +++ 2 files changed, 8 insertions(+) diff --git a/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs b/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs index 58fa5aca82..26bc566d65 100644 --- a/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs +++ b/src/ImageSharp/Formats/Png/PngScanlineProcessor.cs @@ -240,6 +240,11 @@ public static void ProcessPaletteScanline( byte[] paletteAlpha) where TPixel : unmanaged, IPixel { + if (palette.IsEmpty) + { + PngThrowHelper.ThrowMissingPalette(); + } + TPixel pixel = default; ref byte scanlineSpanRef = ref MemoryMarshal.GetReference(scanlineSpan); ref TPixel rowSpanRef = ref MemoryMarshal.GetReference(rowSpan); diff --git a/src/ImageSharp/Formats/Png/PngThrowHelper.cs b/src/ImageSharp/Formats/Png/PngThrowHelper.cs index 8700438bd9..ad733f0105 100644 --- a/src/ImageSharp/Formats/Png/PngThrowHelper.cs +++ b/src/ImageSharp/Formats/Png/PngThrowHelper.cs @@ -21,6 +21,9 @@ public static void ThrowInvalidImageContentException(string errorMessage, Except [MethodImpl(InliningOptions.ColdPath)] public static void ThrowNoData() => throw new InvalidImageContentException("PNG Image does not contain a data chunk"); + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowMissingPalette() => throw new InvalidImageContentException("PNG Image does not contain palette chunk"); + [MethodImpl(InliningOptions.ColdPath)] public static void ThrowInvalidChunkType() => throw new InvalidImageContentException("Invalid PNG data."); From d98171fa54180dec22db9e0e207e803edc4b6c8a Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sat, 19 Feb 2022 18:23:45 +0100 Subject: [PATCH 09/23] Add tests for missing palette chunk --- .../Formats/Png/PngDecoderTests.cs | 16 ++++++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 2 ++ tests/Images/Input/Png/missing_plte.png | 3 +++ tests/Images/Input/Png/missing_plte_2.png | 3 +++ 4 files changed, 24 insertions(+) create mode 100644 tests/Images/Input/Png/missing_plte.png create mode 100644 tests/Images/Input/Png/missing_plte_2.png diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index c29f8c5891..f81a77ca12 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -265,6 +265,22 @@ public void Decode_MissingDataChunk_ThrowsException(TestImageProvider(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + Exception ex = Record.Exception( + () => + { + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + }); + Assert.NotNull(ex); + Assert.Contains("PNG Image does not contain palette chunk", ex.Message); + } + [Theory] [WithFile(TestImages.Png.Bad.BitDepthZero, PixelTypes.Rgba32)] [WithFile(TestImages.Png.Bad.BitDepthThree, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 8b943194a5..d03a601830 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -127,6 +127,8 @@ public static class Bad public const string MissingDataChunk = "Png/xdtn0g01.png"; public const string WrongCrcDataChunk = "Png/xcsn0g01.png"; public const string CorruptedChunk = "Png/big-corrupted-chunk.png"; + public const string MissingPaletteChunk1 = "Png/missing_plte.png"; + public const string MissingPaletteChunk2 = "Png/missing_plte_2.png"; // Zlib errors. public const string ZlibOverflow = "Png/zlib-overflow.png"; diff --git a/tests/Images/Input/Png/missing_plte.png b/tests/Images/Input/Png/missing_plte.png new file mode 100644 index 0000000000..0c24883fbd --- /dev/null +++ b/tests/Images/Input/Png/missing_plte.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:73fd17a394f8258f4767986bc427c0160277819349c937f18cb29044e7549bc8 +size 506 diff --git a/tests/Images/Input/Png/missing_plte_2.png b/tests/Images/Input/Png/missing_plte_2.png new file mode 100644 index 0000000000..8fc6580e53 --- /dev/null +++ b/tests/Images/Input/Png/missing_plte_2.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:797844db61a937c6f31ecb392c8416fbf106017413ba55c6576e0b1fcfc1cf9c +size 597 From d7fec1876c33f1d2b7b53c170a411501879d37c7 Mon Sep 17 00:00:00 2001 From: Brian Popow <38701097+brianpopow@users.noreply.github.com> Date: Sat, 19 Feb 2022 19:30:05 +0100 Subject: [PATCH 10/23] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Günther Foidl --- src/ImageSharp/Compression/Zlib/Adler32.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Compression/Zlib/Adler32.cs b/src/ImageSharp/Compression/Zlib/Adler32.cs index cf1e70b26d..63552cf41e 100644 --- a/src/ImageSharp/Compression/Zlib/Adler32.cs +++ b/src/ImageSharp/Compression/Zlib/Adler32.cs @@ -227,7 +227,7 @@ private static unsafe uint CalculateArm(uint crc, ReadOnlySpan buffer) // Process the data in blocks. long blocks = len / BlockSize; len -= (int)(blocks * BlockSize); - fixed (byte* bufferPtr = buffer) + fixed (byte* bufferPtr = &MemoryMarshal.GetReference(buffer)) { while (blocks != 0) { @@ -241,8 +241,8 @@ private static unsafe uint CalculateArm(uint crc, ReadOnlySpan buffer) // Process n blocks of data. At most nMax data bytes can be // processed before s2 must be reduced modulo Base. - var vs2 = Vector128.Create(0, 0, 0, s1 * n); Vector128 vs1 = Vector128.Zero; + Vector128 vs2 = vs1.WithElement(3, s1 * n); Vector128 vColumnSum1 = Vector128.Zero; Vector128 vColumnSum2 = Vector128.Zero; Vector128 vColumnSum3 = Vector128.Zero; @@ -352,7 +352,7 @@ private static unsafe uint CalculateScalar(uint adler, ReadOnlySpan buffer uint s1 = adler & 0xFFFF; uint s2 = (adler >> 16) & 0xFFFF; - fixed (byte* bufferPtr = buffer) + fixed (byte* bufferPtr = &MemoryMarshal.GetReference(buffer)) { byte* localBufferPtr = bufferPtr; uint length = (uint)buffer.Length; From cfc7847fb843690e798e9ec158e12892147d33af Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Fri, 18 Feb 2022 21:30:58 +0100 Subject: [PATCH 11/23] Use MinBufferSize for ARM --- src/ImageSharp/Compression/Zlib/Adler32.cs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/ImageSharp/Compression/Zlib/Adler32.cs b/src/ImageSharp/Compression/Zlib/Adler32.cs index 63552cf41e..f09bcd45be 100644 --- a/src/ImageSharp/Compression/Zlib/Adler32.cs +++ b/src/ImageSharp/Compression/Zlib/Adler32.cs @@ -74,7 +74,7 @@ public static uint Calculate(uint adler, ReadOnlySpan buffer) return CalculateSse(adler, buffer); } #if NET5_0_OR_GREATER - if (AdvSimd.IsSupported) + if (AdvSimd.IsSupported && buffer.Length >= MinBufferSize) { return CalculateArm(adler, buffer); } @@ -96,12 +96,10 @@ private static unsafe uint CalculateSse(uint adler, ReadOnlySpan buffer) uint blocks = length / BlockSize; length -= blocks * BlockSize; - int index = 0; fixed (byte* bufferPtr = buffer) { fixed (byte* tapPtr = Tap1Tap2) { - index += (int)blocks * BlockSize; byte* localBufferPtr = bufferPtr; // _mm_setr_epi8 on x86 @@ -216,11 +214,11 @@ private static unsafe uint CalculateSse(uint adler, ReadOnlySpan buffer) // Based on: https://github.com/chromium/chromium/blob/master/third_party/zlib/adler32_simd.c [MethodImpl(InliningOptions.HotPath | InliningOptions.ShortMethod)] - private static unsafe uint CalculateArm(uint crc, ReadOnlySpan buffer) + private static unsafe uint CalculateArm(uint adler, ReadOnlySpan buffer) { // Split Adler-32 into component sums. - uint s1 = crc & 0xFFFF; - uint s2 = (crc >> 16) & 0xFFFF; + uint s1 = adler & 0xFFFF; + uint s2 = (adler >> 16) & 0xFFFF; int len = buffer.Length; int bufferOffset = 0; @@ -269,7 +267,8 @@ private static unsafe uint CalculateArm(uint crc, ReadOnlySpan buffer) vColumnSum4 = AdvSimd.AddWideningLower(vColumnSum4, bytes2.GetUpper().AsByte()); bufferOffset += BlockSize; - } while (--n > 0); + } + while (--n > 0); vs2 = AdvSimd.ShiftLeftLogical(vs2, 5); @@ -359,7 +358,7 @@ private static unsafe uint CalculateScalar(uint adler, ReadOnlySpan buffer while (length > 0) { - var k = length < Nmax ? length : Nmax; + uint k = length < Nmax ? length : Nmax; length -= k; while (k >= 16) From 29ddc6053e1a27ab095e87d8baa015f7428c53ef Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sat, 19 Feb 2022 20:31:07 +0100 Subject: [PATCH 12/23] Change error message to "...a palette chunk" --- src/ImageSharp/Formats/Png/PngThrowHelper.cs | 2 +- tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngThrowHelper.cs b/src/ImageSharp/Formats/Png/PngThrowHelper.cs index ad733f0105..ae7d16ec71 100644 --- a/src/ImageSharp/Formats/Png/PngThrowHelper.cs +++ b/src/ImageSharp/Formats/Png/PngThrowHelper.cs @@ -22,7 +22,7 @@ public static void ThrowInvalidImageContentException(string errorMessage, Except public static void ThrowNoData() => throw new InvalidImageContentException("PNG Image does not contain a data chunk"); [MethodImpl(InliningOptions.ColdPath)] - public static void ThrowMissingPalette() => throw new InvalidImageContentException("PNG Image does not contain palette chunk"); + public static void ThrowMissingPalette() => throw new InvalidImageContentException("PNG Image does not contain a palette chunk"); [MethodImpl(InliningOptions.ColdPath)] public static void ThrowInvalidChunkType() => throw new InvalidImageContentException("Invalid PNG data."); diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index f81a77ca12..a92856c730 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -278,7 +278,7 @@ public void Decode_MissingPaletteChunk_ThrowsException(TestImageProvider image.DebugSave(provider); }); Assert.NotNull(ex); - Assert.Contains("PNG Image does not contain palette chunk", ex.Message); + Assert.Contains("PNG Image does not contain a palette chunk", ex.Message); } [Theory] From bef51622cba551e0e3fc128ac328a869bcbde295 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sat, 19 Feb 2022 20:43:49 +0100 Subject: [PATCH 13/23] Add missing using System.Runtime.InteropServices; --- src/ImageSharp/Compression/Zlib/Adler32.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ImageSharp/Compression/Zlib/Adler32.cs b/src/ImageSharp/Compression/Zlib/Adler32.cs index f09bcd45be..2a655a1a29 100644 --- a/src/ImageSharp/Compression/Zlib/Adler32.cs +++ b/src/ImageSharp/Compression/Zlib/Adler32.cs @@ -3,6 +3,7 @@ using System; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; #if SUPPORTS_RUNTIME_INTRINSICS using System.Runtime.Intrinsics; using System.Runtime.Intrinsics.X86; From d62391e9e09729448544613cbbe9c9eebd80e2e8 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 20 Feb 2022 13:50:57 +0100 Subject: [PATCH 14/23] Fix bug in storing the results --- src/ImageSharp/Compression/Zlib/Adler32.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Compression/Zlib/Adler32.cs b/src/ImageSharp/Compression/Zlib/Adler32.cs index 2a655a1a29..0ece3bd229 100644 --- a/src/ImageSharp/Compression/Zlib/Adler32.cs +++ b/src/ImageSharp/Compression/Zlib/Adler32.cs @@ -289,8 +289,8 @@ private static unsafe uint CalculateArm(uint adler, ReadOnlySpan buffer) Vector64 s1s2 = AdvSimd.AddPairwise(sum1, sum2); // Store the results. - s1 = AdvSimd.Extract(s1s2, 0); - s2 = AdvSimd.Extract(s1s2, 1); + s1 += AdvSimd.Extract(s1s2, 0); + s2 += AdvSimd.Extract(s1s2, 1); // Reduce. s1 %= Base; From c3c14e8b4238087a441b2cb38c341e5205347f51 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 20 Feb 2022 14:23:57 +0100 Subject: [PATCH 15/23] Add adler tests with and without intrinsics --- .../Formats/Png/Adler32Tests.cs | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/Adler32Tests.cs b/tests/ImageSharp.Tests/Formats/Png/Adler32Tests.cs index 0886bd84dc..97d9e904e1 100644 --- a/tests/ImageSharp.Tests/Formats/Png/Adler32Tests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/Adler32Tests.cs @@ -3,6 +3,7 @@ using System; using SixLabors.ImageSharp.Compression.Zlib; +using SixLabors.ImageSharp.Tests.TestUtilities; using Xunit; using SharpAdler32 = ICSharpCode.SharpZipLib.Checksum.Adler32; @@ -15,10 +16,7 @@ public class Adler32Tests [InlineData(0)] [InlineData(1)] [InlineData(2)] - public void ReturnsCorrectWhenEmpty(uint input) - { - Assert.Equal(input, Adler32.Calculate(input, default)); - } + public void CalculateAdler_ReturnsCorrectWhenEmpty(uint input) => Assert.Equal(input, Adler32.Calculate(input, default)); [Theory] [InlineData(0)] @@ -28,24 +26,46 @@ public void ReturnsCorrectWhenEmpty(uint input) [InlineData(1024 + 15)] [InlineData(2034)] [InlineData(4096)] - public void MatchesReference(int length) + public void CalculateAdler_MatchesReference(int length) => CalculateAdlerAndCompareToReference(length); + + private static void CalculateAdlerAndCompareToReference(int length) { - var data = GetBuffer(length); + // arrange + byte[] data = GetBuffer(length); var adler = new SharpAdler32(); adler.Update(data); - long expected = adler.Value; + + // act long actual = Adler32.Calculate(data); + // assert Assert.Equal(expected, actual); } private static byte[] GetBuffer(int length) { - var data = new byte[length]; + byte[] data = new byte[length]; new Random(1).NextBytes(data); return data; } + +#if SUPPORTS_RUNTIME_INTRINSICS + [Fact] + public void RunCalculateAdlerTest_WithHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunCalculateAdlerTest, HwIntrinsics.AllowAll); + + [Fact] + public void RunCalculateAdlerTest_WithoutHardwareIntrinsics_Works() => FeatureTestRunner.RunWithHwIntrinsicsFeature(RunCalculateAdlerTest, HwIntrinsics.DisableHWIntrinsic); + + private static void RunCalculateAdlerTest() + { + int[] testData = { 0, 8, 215, 1024, 1024 + 15, 2034, 4096 }; + for (int i = 0; i < testData.Length; i++) + { + CalculateAdlerAndCompareToReference(testData[i]); + } + } +#endif } } From 9179c111e418b5e076e3b2e7f5b0d0604b51fcdb Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Sun, 20 Feb 2022 14:34:28 +0100 Subject: [PATCH 16/23] Add common methods for handling left over for sse and arm --- src/ImageSharp/Compression/Zlib/Adler32.cs | 128 ++++++++------------- 1 file changed, 49 insertions(+), 79 deletions(-) diff --git a/src/ImageSharp/Compression/Zlib/Adler32.cs b/src/ImageSharp/Compression/Zlib/Adler32.cs index 0ece3bd229..648339230c 100644 --- a/src/ImageSharp/Compression/Zlib/Adler32.cs +++ b/src/ImageSharp/Compression/Zlib/Adler32.cs @@ -170,40 +170,7 @@ private static unsafe uint CalculateSse(uint adler, ReadOnlySpan buffer) if (length > 0) { - if (length >= 16) - { - s2 += s1 += localBufferPtr[0]; - s2 += s1 += localBufferPtr[1]; - s2 += s1 += localBufferPtr[2]; - s2 += s1 += localBufferPtr[3]; - s2 += s1 += localBufferPtr[4]; - s2 += s1 += localBufferPtr[5]; - s2 += s1 += localBufferPtr[6]; - s2 += s1 += localBufferPtr[7]; - s2 += s1 += localBufferPtr[8]; - s2 += s1 += localBufferPtr[9]; - s2 += s1 += localBufferPtr[10]; - s2 += s1 += localBufferPtr[11]; - s2 += s1 += localBufferPtr[12]; - s2 += s1 += localBufferPtr[13]; - s2 += s1 += localBufferPtr[14]; - s2 += s1 += localBufferPtr[15]; - - localBufferPtr += 16; - length -= 16; - } - - while (length-- > 0) - { - s2 += s1 += *localBufferPtr++; - } - - if (s1 >= Base) - { - s1 -= Base; - } - - s2 %= Base; + HandleLeftOver(localBufferPtr, length, ref s1, ref s2); } return s1 | (s2 << 16); @@ -211,6 +178,44 @@ private static unsafe uint CalculateSse(uint adler, ReadOnlySpan buffer) } } + private static unsafe void HandleLeftOver(byte* localBufferPtr, uint length, ref uint s1, ref uint s2) + { + if (length >= 16) + { + s2 += s1 += localBufferPtr[0]; + s2 += s1 += localBufferPtr[1]; + s2 += s1 += localBufferPtr[2]; + s2 += s1 += localBufferPtr[3]; + s2 += s1 += localBufferPtr[4]; + s2 += s1 += localBufferPtr[5]; + s2 += s1 += localBufferPtr[6]; + s2 += s1 += localBufferPtr[7]; + s2 += s1 += localBufferPtr[8]; + s2 += s1 += localBufferPtr[9]; + s2 += s1 += localBufferPtr[10]; + s2 += s1 += localBufferPtr[11]; + s2 += s1 += localBufferPtr[12]; + s2 += s1 += localBufferPtr[13]; + s2 += s1 += localBufferPtr[14]; + s2 += s1 += localBufferPtr[15]; + + localBufferPtr += 16; + length -= 16; + } + + while (length-- > 0) + { + s2 += s1 += *localBufferPtr++; + } + + if (s1 >= Base) + { + s1 -= Base; + } + + s2 %= Base; + } + #if NET5_0_OR_GREATER // Based on: https://github.com/chromium/chromium/blob/master/third_party/zlib/adler32_simd.c @@ -220,14 +225,15 @@ private static unsafe uint CalculateArm(uint adler, ReadOnlySpan buffer) // Split Adler-32 into component sums. uint s1 = adler & 0xFFFF; uint s2 = (adler >> 16) & 0xFFFF; - int len = buffer.Length; - int bufferOffset = 0; + uint length = (uint)buffer.Length; // Process the data in blocks. - long blocks = len / BlockSize; - len -= (int)(blocks * BlockSize); + long blocks = length / BlockSize; + length -= (uint)(blocks * BlockSize); fixed (byte* bufferPtr = &MemoryMarshal.GetReference(buffer)) { + byte* localBufferPtr = bufferPtr; + while (blocks != 0) { uint n = Nmax / BlockSize; @@ -250,8 +256,8 @@ private static unsafe uint CalculateArm(uint adler, ReadOnlySpan buffer) do { // Load 32 input bytes. - Vector128 bytes1 = AdvSimd.LoadVector128(bufferPtr + bufferOffset).AsUInt16(); - Vector128 bytes2 = AdvSimd.LoadVector128(bufferPtr + bufferOffset + 16).AsUInt16(); + Vector128 bytes1 = AdvSimd.LoadVector128(localBufferPtr).AsUInt16(); + Vector128 bytes2 = AdvSimd.LoadVector128(localBufferPtr + 0x10).AsUInt16(); // Add previous block byte sum to v_s2. vs2 = AdvSimd.Add(vs2, vs1); @@ -267,7 +273,7 @@ private static unsafe uint CalculateArm(uint adler, ReadOnlySpan buffer) vColumnSum3 = AdvSimd.AddWideningLower(vColumnSum3, bytes2.GetLower().AsByte()); vColumnSum4 = AdvSimd.AddWideningLower(vColumnSum4, bytes2.GetUpper().AsByte()); - bufferOffset += BlockSize; + localBufferPtr += BlockSize; } while (--n > 0); @@ -296,47 +302,11 @@ private static unsafe uint CalculateArm(uint adler, ReadOnlySpan buffer) s1 %= Base; s2 %= Base; } - } - // Handle leftover data. - if (len != 0) - { - if (len >= 16) - { - s2 += s1 += buffer[bufferOffset++]; - s2 += s1 += buffer[bufferOffset++]; - s2 += s1 += buffer[bufferOffset++]; - s2 += s1 += buffer[bufferOffset++]; - - s2 += s1 += buffer[bufferOffset++]; - s2 += s1 += buffer[bufferOffset++]; - s2 += s1 += buffer[bufferOffset++]; - s2 += s1 += buffer[bufferOffset++]; - - s2 += s1 += buffer[bufferOffset++]; - s2 += s1 += buffer[bufferOffset++]; - s2 += s1 += buffer[bufferOffset++]; - s2 += s1 += buffer[bufferOffset++]; - - s2 += s1 += buffer[bufferOffset++]; - s2 += s1 += buffer[bufferOffset++]; - s2 += s1 += buffer[bufferOffset++]; - s2 += s1 += buffer[bufferOffset++]; - - len -= 16; - } - - while (len-- > 0) + if (length != 0) { - s2 += s1 += buffer[bufferOffset++]; + HandleLeftOver(localBufferPtr, length, ref s1, ref s2); } - - if (s1 >= Base) - { - s1 -= Base; - } - - s2 %= Base; } // Return the recombined sums. From 78e5b6c19d45b3083b8160cde0b90cd072bead06 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 21 Feb 2022 18:39:29 +1100 Subject: [PATCH 17/23] Throw for corrupt LZW min code. Add test for deferred clear code --- ImageSharp.sln | 7 +++++ src/ImageSharp/Formats/Gif/GifDecoderCore.cs | 4 +-- src/ImageSharp/Formats/Gif/LzwDecoder.cs | 26 ++++++++++++------- .../Formats/Gif/GifDecoderTests.cs | 17 ++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + ...2012BadMinCode_Rgba32_issue2012_drona1.png | 3 --- ...eferredClearCode_Rgba32_bugzilla-55918.png | 3 +++ .../Input/Gif/issues/bugzilla-55918.gif | 3 +++ 8 files changed, 50 insertions(+), 14 deletions(-) delete mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png create mode 100644 tests/Images/External/ReferenceOutput/GifDecoderTests/IssueDeferredClearCode_Rgba32_bugzilla-55918.png create mode 100644 tests/Images/Input/Gif/issues/bugzilla-55918.gif diff --git a/ImageSharp.sln b/ImageSharp.sln index 17d293b434..5428f3394d 100644 --- a/ImageSharp.sln +++ b/ImageSharp.sln @@ -142,6 +142,13 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Gif", "Gif", "{EE3FB0B3-1C3 EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "issues", "issues", "{BF8DFDC1-CEE5-4A37-B216-D3085360C776}" ProjectSection(SolutionItems) = preProject + tests\Images\Input\Gif\issues\bugzilla-55918.gif = tests\Images\Input\Gif\issues\bugzilla-55918.gif + tests\Images\Input\Gif\issues\issue1505_argumentoutofrange.png = tests\Images\Input\Gif\issues\issue1505_argumentoutofrange.png + tests\Images\Input\Gif\issues\issue1530.gif = tests\Images\Input\Gif\issues\issue1530.gif + tests\Images\Input\Gif\issues\issue1668_invalidcolorindex.gif = tests\Images\Input\Gif\issues\issue1668_invalidcolorindex.gif + tests\Images\Input\Gif\issues\issue1962_tiniest_gif_1st.gif = tests\Images\Input\Gif\issues\issue1962_tiniest_gif_1st.gif + tests\Images\Input\Gif\issues\issue2012_drona1.gif = tests\Images\Input\Gif\issues\issue2012_drona1.gif + tests\Images\Input\Gif\issues\issue2012_Stronghold-Crusader-Extreme-Cover.gif = tests\Images\Input\Gif\issues\issue2012_Stronghold-Crusader-Extreme-Cover.gif tests\Images\Input\Gif\issues\issue403_baddescriptorwidth.gif = tests\Images\Input\Gif\issues\issue403_baddescriptorwidth.gif tests\Images\Input\Gif\issues\issue405_badappextlength252-2.gif = tests\Images\Input\Gif\issues\issue405_badappextlength252-2.gif tests\Images\Input\Gif\issues\issue405_badappextlength252.gif = tests\Images\Input\Gif\issues\issue405_badappextlength252.gif diff --git a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs index 98e012658e..16dca3324f 100644 --- a/src/ImageSharp/Formats/Gif/GifDecoderCore.cs +++ b/src/ImageSharp/Formats/Gif/GifDecoderCore.cs @@ -410,9 +410,9 @@ private void ReadFrame(ref Image image, ref ImageFrame p [MethodImpl(MethodImplOptions.AggressiveInlining)] private void ReadFrameIndices(Buffer2D indices) { - int dataSize = this.stream.ReadByte(); + int minCodeSize = this.stream.ReadByte(); using var lzwDecoder = new LzwDecoder(this.Configuration.MemoryAllocator, this.stream); - lzwDecoder.DecodePixels(dataSize, indices); + lzwDecoder.DecodePixels(minCodeSize, indices); } /// diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index 8bb52b6371..470929c4a1 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -64,17 +64,22 @@ public LzwDecoder(MemoryAllocator memoryAllocator, BufferedReadStream stream) /// /// Decodes and decompresses all pixel indices from the stream. /// - /// Size of the data. + /// Minimum code size of the data. /// The pixel array to decode to. - public void DecodePixels(int dataSize, Buffer2D pixels) + public void DecodePixels(int minCodeSize, Buffer2D pixels) { - // Calculate the clear code. The value of the clear code is 2 ^ dataSize - int clearCode = 1 << dataSize; - if (clearCode > MaxStackSize) + // Calculate the clear code. The value of the clear code is 2 ^ minCodeSize + int clearCode = 1 << minCodeSize; + + // It is possible to specify a larger LZW minimum code size than the palette length in bits + // which may leave a gap in the codes where no colors are assigned. + // http://www.matthewflickinger.com/lab/whatsinagif/lzw_image_data.asp#lzw_compression + if (minCodeSize < 2 || clearCode > MaxStackSize) { // Don't attempt to decode the frame indices. - // The image is most likely corrupted. - return; + // Theoretically we could determine a min code size from the length of the provided + // color palette but we won't bother since the image is most likely corrupted. + ThrowBadMinimumCode(); } // The resulting index table length. @@ -82,7 +87,7 @@ public void DecodePixels(int dataSize, Buffer2D pixels) int height = pixels.Height; int length = width * height; - int codeSize = dataSize + 1; + int codeSize = minCodeSize + 1; // Calculate the end code int endCode = clearCode + 1; @@ -169,7 +174,7 @@ public void DecodePixels(int dataSize, Buffer2D pixels) if (code == clearCode) { // Reset the decoder - codeSize = dataSize + 1; + codeSize = minCodeSize + 1; codeMask = (1 << codeSize) - 1; availableCode = clearCode + 2; oldCode = NullCode; @@ -258,5 +263,8 @@ public void Dispose() this.suffix.Dispose(); this.pixelStack.Dispose(); } + + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowBadMinimumCode() => throw new InvalidImageContentException("Gif Image does not contain a valid LZW minimum code."); } } diff --git a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs index 289bf6a4fb..30bcb72554 100644 --- a/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Gif/GifDecoderTests.cs @@ -277,6 +277,23 @@ public void Issue2012EmptyXmp(TestImageProvider provider) [WithFile(TestImages.Gif.Issues.Issue2012BadMinCode, PixelTypes.Rgba32)] public void Issue2012BadMinCode(TestImageProvider provider) where TPixel : unmanaged, IPixel + { + Exception ex = Record.Exception( + () => + { + using Image image = provider.GetImage(); + image.DebugSave(provider); + }); + + Assert.NotNull(ex); + Assert.Contains("Gif Image does not contain a valid LZW minimum code.", ex.Message); + } + + // https://bugzilla.mozilla.org/show_bug.cgi?id=55918 + [Theory] + [WithFile(TestImages.Gif.Issues.DeferredClearCode, PixelTypes.Rgba32)] + public void IssueDeferredClearCode(TestImageProvider provider) + where TPixel : unmanaged, IPixel { using Image image = provider.GetImage(); diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index 6ff1162256..6fbf584d8d 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -448,6 +448,7 @@ public static class Issues public const string BadAppExtLength = "Gif/issues/issue405_badappextlength252.gif"; public const string BadAppExtLength_2 = "Gif/issues/issue405_badappextlength252-2.gif"; public const string BadDescriptorWidth = "Gif/issues/issue403_baddescriptorwidth.gif"; + public const string DeferredClearCode = "Gif/issues/bugzilla-55918.gif"; public const string Issue1505 = "Gif/issues/issue1505_argumentoutofrange.png"; public const string Issue1530 = "Gif/issues/issue1530.gif"; public const string InvalidColorIndex = "Gif/issues/issue1668_invalidcolorindex.gif"; diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png deleted file mode 100644 index 5e23f55284..0000000000 --- a/tests/Images/External/ReferenceOutput/GifDecoderTests/Issue2012BadMinCode_Rgba32_issue2012_drona1.png +++ /dev/null @@ -1,3 +0,0 @@ -version https://git-lfs.github.com/spec/v1 -oid sha256:cf4bc93479140b05b25066eb10c33fa9f82c617828a56526d47f5a8c72035ef0 -size 1871 diff --git a/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueDeferredClearCode_Rgba32_bugzilla-55918.png b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueDeferredClearCode_Rgba32_bugzilla-55918.png new file mode 100644 index 0000000000..b5769c2c42 --- /dev/null +++ b/tests/Images/External/ReferenceOutput/GifDecoderTests/IssueDeferredClearCode_Rgba32_bugzilla-55918.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:6b33733518b855b25c5e9a1b2f5c93cacf0699a40a459dde795b0ed91a978909 +size 12776 diff --git a/tests/Images/Input/Gif/issues/bugzilla-55918.gif b/tests/Images/Input/Gif/issues/bugzilla-55918.gif new file mode 100644 index 0000000000..929ea67c35 --- /dev/null +++ b/tests/Images/Input/Gif/issues/bugzilla-55918.gif @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d11148669a093c2e39be62bc3482c5863362d28c03c7f26c5a2386d5de28c339 +size 14551 From 85a0ac644d1e51c87b510428c882e51b26aad3e4 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 21 Feb 2022 21:29:13 +1100 Subject: [PATCH 18/23] Use GifThrowHelper --- src/ImageSharp/Formats/Gif/LzwDecoder.cs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/ImageSharp/Formats/Gif/LzwDecoder.cs b/src/ImageSharp/Formats/Gif/LzwDecoder.cs index 470929c4a1..2a07200016 100644 --- a/src/ImageSharp/Formats/Gif/LzwDecoder.cs +++ b/src/ImageSharp/Formats/Gif/LzwDecoder.cs @@ -79,7 +79,7 @@ public void DecodePixels(int minCodeSize, Buffer2D pixels) // Don't attempt to decode the frame indices. // Theoretically we could determine a min code size from the length of the provided // color palette but we won't bother since the image is most likely corrupted. - ThrowBadMinimumCode(); + GifThrowHelper.ThrowInvalidImageContentException("Gif Image does not contain a valid LZW minimum code."); } // The resulting index table length. @@ -263,8 +263,5 @@ public void Dispose() this.suffix.Dispose(); this.pixelStack.Dispose(); } - - [MethodImpl(InliningOptions.ColdPath)] - public static void ThrowBadMinimumCode() => throw new InvalidImageContentException("Gif Image does not contain a valid LZW minimum code."); } } From 0fa6085611e69e5e4eb2827e8293eb747585a034 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 21 Feb 2022 12:10:23 +0100 Subject: [PATCH 19/23] Throw exception, if gamma chunk does not contain enough data --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 10 ++++++++-- src/ImageSharp/Formats/Png/PngThrowHelper.cs | 3 +++ .../Formats/Png/PngDecoderTests.cs | 15 +++++++++++++++ tests/ImageSharp.Tests/TestImages.cs | 1 + tests/Images/Input/Png/length_gama.png | 3 +++ 5 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 tests/Images/Input/Png/length_gama.png diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index f5fc86ee4d..090e1e2b0c 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -429,10 +429,16 @@ private void ReadPhysicalChunk(ImageMetadata metadata, ReadOnlySpan data) /// The metadata to read to. /// The data containing physical data. private void ReadGammaChunk(PngMetadata pngMetadata, ReadOnlySpan data) + { + if (data.Length < 4) + { + PngThrowHelper.ThrowInvalidGamma(); + } - // The value is encoded as a 4-byte unsigned integer, representing gamma times 100000. // For example, a gamma of 1/2.2 would be stored as 45455. - => pngMetadata.Gamma = BinaryPrimitives.ReadUInt32BigEndian(data) * 1e-5F; + // The value is encoded as a 4-byte unsigned integer, representing gamma times 100000. + pngMetadata.Gamma = BinaryPrimitives.ReadUInt32BigEndian(data) * 1e-5F; + } /// /// Initializes the image and various buffers needed for processing diff --git a/src/ImageSharp/Formats/Png/PngThrowHelper.cs b/src/ImageSharp/Formats/Png/PngThrowHelper.cs index ae7d16ec71..07372dae2b 100644 --- a/src/ImageSharp/Formats/Png/PngThrowHelper.cs +++ b/src/ImageSharp/Formats/Png/PngThrowHelper.cs @@ -24,6 +24,9 @@ public static void ThrowInvalidImageContentException(string errorMessage, Except [MethodImpl(InliningOptions.ColdPath)] public static void ThrowMissingPalette() => throw new InvalidImageContentException("PNG Image does not contain a palette chunk"); + [MethodImpl(InliningOptions.ColdPath)] + public static void ThrowInvalidGamma() => throw new InvalidImageContentException("PNG Image does not contain enough data for the gamma chunk"); + [MethodImpl(InliningOptions.ColdPath)] public static void ThrowInvalidChunkType() => throw new InvalidImageContentException("Invalid PNG data."); diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index a92856c730..7fd87258b9 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -281,6 +281,21 @@ public void Decode_MissingPaletteChunk_ThrowsException(TestImageProvider Assert.Contains("PNG Image does not contain a palette chunk", ex.Message); } + [Theory] + [WithFile(TestImages.Png.Bad.InvalidGammaChunk, PixelTypes.Rgba32)] + public void Decode_InvalidGammaChunk_ThrowsException(TestImageProvider provider) + where TPixel : unmanaged, IPixel + { + Exception ex = Record.Exception( + () => + { + using Image image = provider.GetImage(PngDecoder); + image.DebugSave(provider); + }); + Assert.NotNull(ex); + Assert.Contains("PNG Image does not contain enough data for the gamma chunk", ex.Message); + } + [Theory] [WithFile(TestImages.Png.Bad.BitDepthZero, PixelTypes.Rgba32)] [WithFile(TestImages.Png.Bad.BitDepthThree, PixelTypes.Rgba32)] diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index c6bd20a7db..5bce99ce12 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -129,6 +129,7 @@ public static class Bad public const string CorruptedChunk = "Png/big-corrupted-chunk.png"; public const string MissingPaletteChunk1 = "Png/missing_plte.png"; public const string MissingPaletteChunk2 = "Png/missing_plte_2.png"; + public const string InvalidGammaChunk = "Png/length_gama.png"; // Zlib errors. public const string ZlibOverflow = "Png/zlib-overflow.png"; diff --git a/tests/Images/Input/Png/length_gama.png b/tests/Images/Input/Png/length_gama.png new file mode 100644 index 0000000000..caf0fb01d0 --- /dev/null +++ b/tests/Images/Input/Png/length_gama.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:824766b34739727c722e88611d7b55401452c2970cd433f56e5f9f1b36d6950d +size 1285 From d76c40a43a85f3ff82bdbc05f577fe7236095939 Mon Sep 17 00:00:00 2001 From: Brian Popow Date: Mon, 21 Feb 2022 14:39:10 +0100 Subject: [PATCH 20/23] Ignore invalid gamma chunks --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 3 ++- src/ImageSharp/Formats/Png/PngThrowHelper.cs | 3 --- tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs | 5 ++--- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 090e1e2b0c..60437e015e 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -432,7 +432,8 @@ private void ReadGammaChunk(PngMetadata pngMetadata, ReadOnlySpan data) { if (data.Length < 4) { - PngThrowHelper.ThrowInvalidGamma(); + // Ignore invalid gamma chunks. + return; } // For example, a gamma of 1/2.2 would be stored as 45455. diff --git a/src/ImageSharp/Formats/Png/PngThrowHelper.cs b/src/ImageSharp/Formats/Png/PngThrowHelper.cs index 07372dae2b..ae7d16ec71 100644 --- a/src/ImageSharp/Formats/Png/PngThrowHelper.cs +++ b/src/ImageSharp/Formats/Png/PngThrowHelper.cs @@ -24,9 +24,6 @@ public static void ThrowInvalidImageContentException(string errorMessage, Except [MethodImpl(InliningOptions.ColdPath)] public static void ThrowMissingPalette() => throw new InvalidImageContentException("PNG Image does not contain a palette chunk"); - [MethodImpl(InliningOptions.ColdPath)] - public static void ThrowInvalidGamma() => throw new InvalidImageContentException("PNG Image does not contain enough data for the gamma chunk"); - [MethodImpl(InliningOptions.ColdPath)] public static void ThrowInvalidChunkType() => throw new InvalidImageContentException("Invalid PNG data."); diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 7fd87258b9..0af5d99950 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -283,7 +283,7 @@ public void Decode_MissingPaletteChunk_ThrowsException(TestImageProvider [Theory] [WithFile(TestImages.Png.Bad.InvalidGammaChunk, PixelTypes.Rgba32)] - public void Decode_InvalidGammaChunk_ThrowsException(TestImageProvider provider) + public void Decode_InvalidGammaChunk_Ignored(TestImageProvider provider) where TPixel : unmanaged, IPixel { Exception ex = Record.Exception( @@ -292,8 +292,7 @@ public void Decode_InvalidGammaChunk_ThrowsException(TestImageProvider image = provider.GetImage(PngDecoder); image.DebugSave(provider); }); - Assert.NotNull(ex); - Assert.Contains("PNG Image does not contain enough data for the gamma chunk", ex.Message); + Assert.Null(ex); } [Theory] From 854ea5d0962921a8f1831376f77b1bef8a57b75a Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Mon, 21 Feb 2022 23:25:22 +0100 Subject: [PATCH 21/23] workaround for #2001 / https://github.com/dotnet/runtime/issues/65466 --- .../UniformUnmanagedMemoryPoolMemoryAllocator.cs | 13 +++++++++++-- .../UniformUnmanagedPoolMemoryAllocatorTests.cs | 13 +++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 16a3cb73d1..5f591a9bb3 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -74,6 +74,10 @@ internal UniformUnmanagedMemoryPoolMemoryAllocator( this.nonPoolAllocator = new UnmanagedMemoryAllocator(unmanagedBufferSizeInBytes); } + // This delegate allows overriding the method returning the available system memory, + // so we can test our workaround for https://github.com/dotnet/runtime/issues/65466 + internal static Func GetTotalAvailableMemoryBytes { get; set; } = () => GC.GetGCMemoryInfo().TotalAvailableMemoryBytes; + /// protected internal override int GetBufferCapacityInBytes() => this.poolBufferSizeInBytes; @@ -152,8 +156,13 @@ private static long GetDefaultMaxPoolSizeBytes() // https://github.com/dotnet/runtime/issues/55126#issuecomment-876779327 if (Environment.Is64BitProcess || !RuntimeInformation.FrameworkDescription.StartsWith(".NET 5.0")) { - GCMemoryInfo info = GC.GetGCMemoryInfo(); - return info.TotalAvailableMemoryBytes / 8; + long total = GetTotalAvailableMemoryBytes(); + + // Workaround for https://github.com/dotnet/runtime/issues/65466 + if (total > 0) + { + return total / 8; + } } #endif diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 45a7cc278e..0c59e334c5 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -379,5 +379,18 @@ private static void AllocateSingleAndForget(UniformUnmanagedMemoryPoolMemoryAllo g1.GetSpan()[0] = 42; } } + + [Fact] + public void Issue2001_NegativeMemoryReportedByGc() + { + RemoteExecutor.Invoke(RunTest).Dispose(); + + static void RunTest() + { + // Emulate GC.GetGCMemoryInfo() issue https://github.com/dotnet/runtime/issues/65466 + UniformUnmanagedMemoryPoolMemoryAllocator.GetTotalAvailableMemoryBytes = () => -402354176; + _ = MemoryAllocator.Create(); + } + } } } From 5b82c57ce773566da96ef176c0fa1426e70c91fe Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 22 Feb 2022 12:42:36 +1100 Subject: [PATCH 22/23] Add compiler directives --- .../UniformUnmanagedMemoryPoolMemoryAllocator.cs | 7 ++++--- .../Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs | 2 ++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 5f591a9bb3..0da4ff9f8c 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -1,11 +1,10 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. using System; using System.Buffers; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; -using System.Threading; using SixLabors.ImageSharp.Memory.Internals; namespace SixLabors.ImageSharp.Memory @@ -22,7 +21,7 @@ internal sealed class UniformUnmanagedMemoryPoolMemoryAllocator : MemoryAllocato private readonly int poolCapacity; private readonly UniformUnmanagedMemoryPool.TrimSettings trimSettings; - private UniformUnmanagedMemoryPool pool; + private readonly UniformUnmanagedMemoryPool pool; private readonly UnmanagedMemoryAllocator nonPoolAllocator; public UniformUnmanagedMemoryPoolMemoryAllocator(int? maxPoolSizeMegabytes) @@ -74,9 +73,11 @@ internal UniformUnmanagedMemoryPoolMemoryAllocator( this.nonPoolAllocator = new UnmanagedMemoryAllocator(unmanagedBufferSizeInBytes); } +#if NETCOREAPP3_1_OR_GREATER // This delegate allows overriding the method returning the available system memory, // so we can test our workaround for https://github.com/dotnet/runtime/issues/65466 internal static Func GetTotalAvailableMemoryBytes { get; set; } = () => GC.GetGCMemoryInfo().TotalAvailableMemoryBytes; +#endif /// protected internal override int GetBufferCapacityInBytes() => this.poolBufferSizeInBytes; diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 0c59e334c5..414f991f70 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -380,6 +380,7 @@ private static void AllocateSingleAndForget(UniformUnmanagedMemoryPoolMemoryAllo } } +#if NETCOREAPP3_1_OR_GREATER [Fact] public void Issue2001_NegativeMemoryReportedByGc() { @@ -392,5 +393,6 @@ static void RunTest() _ = MemoryAllocator.Create(); } } +#endif } } From de5f661177f4c902a98fd091a406abea92e2ba36 Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 22 Feb 2022 02:53:43 +0100 Subject: [PATCH 23/23] make GetTotalAvailableMemoryBytes conditional --- .../Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs | 2 ++ .../Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 5f591a9bb3..5b72a5804a 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs @@ -74,9 +74,11 @@ internal UniformUnmanagedMemoryPoolMemoryAllocator( this.nonPoolAllocator = new UnmanagedMemoryAllocator(unmanagedBufferSizeInBytes); } +#if NETCOREAPP3_1_OR_GREATER // This delegate allows overriding the method returning the available system memory, // so we can test our workaround for https://github.com/dotnet/runtime/issues/65466 internal static Func GetTotalAvailableMemoryBytes { get; set; } = () => GC.GetGCMemoryInfo().TotalAvailableMemoryBytes; +#endif /// protected internal override int GetBufferCapacityInBytes() => this.poolBufferSizeInBytes; diff --git a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs index 0c59e334c5..414f991f70 100644 --- a/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs +++ b/tests/ImageSharp.Tests/Memory/Allocators/UniformUnmanagedPoolMemoryAllocatorTests.cs @@ -380,6 +380,7 @@ private static void AllocateSingleAndForget(UniformUnmanagedMemoryPoolMemoryAllo } } +#if NETCOREAPP3_1_OR_GREATER [Fact] public void Issue2001_NegativeMemoryReportedByGc() { @@ -392,5 +393,6 @@ static void RunTest() _ = MemoryAllocator.Create(); } } +#endif } }