commit 26bc0756f76c5aebacae522dfd3e5154946d65ad
author Tom Sepez <tsepez@chromium.org> Thu Jun 17 01:03:13 2021 +0000
committer Pdfium LUCI CQ <pdfium-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu Jun 17 01:03:13 2021 +0000
tree 25836c38ebf30d9c64c578e86ceafbc16a5e8fad
parent 80bdef6c36cd301d6fd064754fcb0b4acc4f8f01

Fix overly-strong ASSERT in WeightTable::SetStartEnd()

A zero-length range at zero represented by [0, -1]  will be fine so long
as we don't actually try to reference the range. Stop short of switching
over to the simpler [0, 0) representation. Note that `weight_count` is
bounded by earlier size checks and fits in an int.

-- Add another ASSERT to earlier tests.

Bug: chromium:1219974
Change-Id: I26b077d11e54465b861070e887b3efde8f2178da
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/82155
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>

core/fxge/dib/cstretchengine.cpp

diff --git a/core/fxge/dib/cstretchengine.cpp b/core/fxge/dib/cstretchengine.cpp
index 69cb8db..5b2ff3b 100644
--- a/core/fxge/dib/cstretchengine.cpp
+++ b/core/fxge/dib/cstretchengine.cpp
@@ -35,6 +35,9 @@
 // static
 size_t CStretchEngine::PixelWeight::TotalBytesForWeightCount(
     size_t weight_count) {
+  // Always room for one weight even for empty ranges due to declaration
+  // of m_Weights[1] in the header. Don't shrink below this since
+  // CalculateWeights() relies on this later.
   const size_t extra_weights = weight_count > 0 ? weight_count - 1 : 0;
   FX_SAFE_SIZE_T total_bytes = extra_weights;
   total_bytes *= sizeof(m_Weights[0]);
@@ -95,7 +98,8 @@
         src_start = std::max(src_start, src_min);
         src_end = std::min(src_end, src_max - 1);
         pixel_weights.SetStartEnd(src_start, src_end, weight_count);
-        if (pixel_weights.m_SrcStart == pixel_weights.m_SrcEnd) {
+        if (pixel_weights.m_SrcStart >= pixel_weights.m_SrcEnd) {
+          // Always room for one weight per size calculation.
           pixel_weights.m_Weights[0] = kFixedPointOne;
         } else {
           pixel_weights.m_Weights[1] = FixedFromFloat(

core/fxge/dib/cstretchengine.h

diff --git a/core/fxge/dib/cstretchengine.h b/core/fxge/dib/cstretchengine.h
index 5ebebb4..eef7fd0 100644
--- a/core/fxge/dib/cstretchengine.h
+++ b/core/fxge/dib/cstretchengine.h
@@ -38,7 +38,7 @@
     static size_t TotalBytesForWeightCount(size_t weight_count);
 
     void SetStartEnd(int src_start, int src_end, size_t weight_count) {
-      CHECK_LT(static_cast<size_t>(src_end - src_start), weight_count);
+      CHECK_LT(src_end - src_start, static_cast<int>(weight_count));
       m_SrcStart = src_start;
       m_SrcEnd = src_end;
     }
@@ -61,7 +61,7 @@
     }
 
     int m_SrcStart;
-    int m_SrcEnd;           // Note: inclusive.
+    int m_SrcEnd;           // Note: inclusive, [0, -1] for empty range at 0.
     uint32_t m_Weights[1];  // Not really 1, variable size.
   };
 

core/fxge/dib/cstretchengine_unittest.cpp

diff --git a/core/fxge/dib/cstretchengine_unittest.cpp b/core/fxge/dib/cstretchengine_unittest.cpp
index 202db9f..a64024d 100644
--- a/core/fxge/dib/cstretchengine_unittest.cpp
+++ b/core/fxge/dib/cstretchengine_unittest.cpp
@@ -29,8 +29,8 @@
                            const FXDIB_ResampleOptions& options) {
   constexpr uint32_t kExpectedSum = CStretchEngine::kFixedPointOne;
   CStretchEngine::WeightTable table;
-  table.CalculateWeights(dest_width, 0, dest_width, src_width, 0, src_width,
-                         options);
+  ASSERT_TRUE(table.CalculateWeights(dest_width, 0, dest_width, src_width, 0,
+                                     src_width, options));
   for (uint32_t i = 0; i < dest_width; ++i) {
     EXPECT_EQ(kExpectedSum, PixelWeightSum(table.GetPixelWeight(i)))
         << "for { " << src_width << ", " << dest_width << " } at " << i;
@@ -92,3 +92,31 @@
   options.bInterpolateBilinear = true;
   ExecuteStretchTests(options);
 }
+
+TEST(CStretchEngine, ZeroLengthSrc) {
+  FXDIB_ResampleOptions options;
+  CStretchEngine::WeightTable table;
+  ASSERT_TRUE(table.CalculateWeights(100, 0, 100, 0, 0, 0, options));
+}
+
+TEST(CStretchEngine, ZeroLengthSrcNoSmoothing) {
+  FXDIB_ResampleOptions options;
+  options.bNoSmoothing = true;
+  CStretchEngine::WeightTable table;
+  ASSERT_TRUE(table.CalculateWeights(100, 0, 100, 0, 0, 0, options));
+}
+
+TEST(CStretchEngine, ZeroLengthSrcBilinear) {
+  FXDIB_ResampleOptions options;
+  options.bInterpolateBilinear = true;
+  CStretchEngine::WeightTable table;
+  ASSERT_TRUE(table.CalculateWeights(100, 0, 100, 0, 0, 0, options));
+}
+
+TEST(CStretchEngine, ZeroLengthSrcNoSmoothingBilinear) {
+  FXDIB_ResampleOptions options;
+  options.bNoSmoothing = true;
+  options.bInterpolateBilinear = true;
+  CStretchEngine::WeightTable table;
+  ASSERT_TRUE(table.CalculateWeights(100, 0, 100, 0, 0, 0, options));
+}