From 79c40c6432daec76883d693924f4e5597517f905 Mon Sep 17 00:00:00 2001
From: Abseil Team <absl-team@google.com>
Date: Thu, 31 Aug 2023 08:37:26 -0700
Subject: Fix incorrect CRC returned by AcceleratedCrcMemcpyEngine when
 kRegions == 1

This bug does not affect any users currently since AcceleratedCrcMemcpyEngine
is never configured with a single region currently.

Before this CL, if the number of regions for the AcceleratedCrcMemcpyEngine was
set to one, the CRC for the sole region would be incorrectly concatenated onto
itself and corrupted.

PiperOrigin-RevId: 561663848
Change-Id: Ibfc596306ab07db906d2e3ecf6eea3f6cb9f1b2b
---
 absl/crc/internal/crc_memcpy_test.cc   |  7 +++++++
 absl/crc/internal/crc_memcpy_x86_64.cc | 28 ++++++++++++++++++----------
 2 files changed, 25 insertions(+), 10 deletions(-)

(limited to 'absl/crc')

diff --git a/absl/crc/internal/crc_memcpy_test.cc b/absl/crc/internal/crc_memcpy_test.cc
index bbdcd205..a4c6a012 100644
--- a/absl/crc/internal/crc_memcpy_test.cc
+++ b/absl/crc/internal/crc_memcpy_test.cc
@@ -107,6 +107,9 @@ TEST_P(x86ParamTest, SmallCorrectnessCheckSourceAlignment) {
         *(base_data + i) =
             static_cast<char>(absl::Uniform<unsigned char>(gen_));
       }
+      SCOPED_TRACE(absl::StrCat("engine=<", GetParam().vector_lanes, ",",
+                                GetParam().integer_lanes, ">, ", "size=", size,
+                                ", source_alignment=", source_alignment));
       absl::crc32c_t initial_crc =
           absl::crc32c_t{absl::Uniform<uint32_t>(gen_)};
       absl::crc32c_t experiment_crc =
@@ -138,6 +141,9 @@ TEST_P(x86ParamTest, SmallCorrectnessCheckDestAlignment) {
         *(base_data + i) =
             static_cast<char>(absl::Uniform<unsigned char>(gen_));
       }
+      SCOPED_TRACE(absl::StrCat("engine=<", GetParam().vector_lanes, ",",
+                                GetParam().integer_lanes, ">, ", "size=", size,
+                                ", destination_alignment=", dest_alignment));
       absl::crc32c_t initial_crc =
           absl::crc32c_t{absl::Uniform<uint32_t>(gen_)};
       absl::crc32c_t experiment_crc =
@@ -161,6 +167,7 @@ INSTANTIATE_TEST_SUITE_P(x86ParamTest, x86ParamTest,
                          ::testing::Values(
                              // Tests for configurations that may occur in prod.
                              TestParams{X86, 3, 0}, TestParams{X86, 1, 2},
+                             TestParams{X86, 1, 0},
                              // Fallback test.
                              TestParams{FALLBACK, 0, 0},
                              // Non Temporal
diff --git a/absl/crc/internal/crc_memcpy_x86_64.cc b/absl/crc/internal/crc_memcpy_x86_64.cc
index c984cf9a..c4ccd472 100644
--- a/absl/crc/internal/crc_memcpy_x86_64.cc
+++ b/absl/crc/internal/crc_memcpy_x86_64.cc
@@ -159,6 +159,7 @@ crc32c_t AcceleratedCrcMemcpyEngine<vec_regions, int_regions>::Compute(
     void* __restrict dst, const void* __restrict src, std::size_t length,
     crc32c_t initial_crc) const {
   constexpr std::size_t kRegions = vec_regions + int_regions;
+  static_assert(kRegions > 0, "Must specify at least one region.");
   constexpr uint32_t kCrcDataXor = uint32_t{0xffffffff};
   constexpr std::size_t kBlockSize = sizeof(__m128i);
   constexpr std::size_t kCopyRoundSize = kRegions * kBlockSize;
@@ -314,6 +315,21 @@ crc32c_t AcceleratedCrcMemcpyEngine<vec_regions, int_regions>::Compute(
   src_bytes += region_size * (kRegions - 1);
   dst_bytes += region_size * (kRegions - 1);
 
+  // Copy and CRC the tail through the XMM registers.
+  std::size_t tail_blocks = tail_size / kBlockSize;
+  LargeTailCopy<0, 1>(&crcs[kRegions - 1], &dst_bytes, &src_bytes, 0,
+                      tail_blocks);
+
+  // Final tail copy for under 16 bytes.
+  crcs[kRegions - 1] =
+      ShortCrcCopy(dst_bytes, src_bytes, tail_size - tail_blocks * kBlockSize,
+                   crcs[kRegions - 1]);
+
+  if (kRegions == 1) {
+    // If there is only one region, finalize and return its CRC.
+    return crc32c_t{static_cast<uint32_t>(crcs[0]) ^ kCrcDataXor};
+  }
+
   // Finalize the first CRCs: XOR the internal CRCs by the XOR mask to undo the
   // XOR done before doing block copy + CRCs.
   for (size_t i = 0; i + 1 < kRegions; i++) {
@@ -326,16 +342,6 @@ crc32c_t AcceleratedCrcMemcpyEngine<vec_regions, int_regions>::Compute(
     full_crc = ConcatCrc32c(full_crc, crcs[i], region_size);
   }
 
-  // Copy and CRC the tail through the XMM registers.
-  std::size_t tail_blocks = tail_size / kBlockSize;
-  LargeTailCopy<0, 1>(&crcs[kRegions - 1], &dst_bytes, &src_bytes, 0,
-                      tail_blocks);
-
-  // Final tail copy for under 16 bytes.
-  crcs[kRegions - 1] =
-      ShortCrcCopy(dst_bytes, src_bytes, tail_size - tail_blocks * kBlockSize,
-                   crcs[kRegions - 1]);
-
   // Finalize and concatenate the final CRC, then return.
   crcs[kRegions - 1] =
       crc32c_t{static_cast<uint32_t>(crcs[kRegions - 1]) ^ kCrcDataXor};
@@ -427,6 +433,8 @@ std::unique_ptr<CrcMemcpyEngine> CrcMemcpy::GetTestEngine(int vector,
     return std::make_unique<AcceleratedCrcMemcpyEngine<3, 0>>();
   } else if (vector == 1 && integer == 2) {
     return std::make_unique<AcceleratedCrcMemcpyEngine<1, 2>>();
+  } else if (vector == 1 && integer == 0) {
+    return std::make_unique<AcceleratedCrcMemcpyEngine<1, 0>>();
   }
   return nullptr;
 }
-- 
cgit v1.2.3