[Support] Update `MD5` to follow other hashes.

[arames]

Summary

Introduce StringRef final() and StringRef result().

[dexonsmith]

Sorry I missed this before -- I have a few suggestions below to align this better with SHA1 and SHA256.

llvm/include/llvm/Support/MD5.h

81–83

I'm not seeing finalWords() in the other hashers. I suggest not adding it.

85

The other hashers return a StringRef. MD5 should align with that, and also return the raw bytes as they do (instead of a hex string).

Looking at SHA1 and SHA256, they have two methods, result() and final(), the former of which can be called at any time. Here is the implementation of result():

StringRef SHA1::result() {
  auto StateToRestore = InternalState;

  auto Hash = final();

  // Restore the state
  InternalState = StateToRestore;

  // Return pointer to hash (20 characters)
  return Hash;
}

I think if we're adding one we should add both, for consistency.

• Seems we'd need a prep NFC patch that moved its members into an InternalState member.
• MD5::final() can add an MD5Result member, calling final(MD5Result&) on it and returning a StringRef referencing MD5Result::bytes, and then MD5::result() can be implemented exactly as in SHA1.

llvm/lib/Support/MD5.cpp

276

The other hashers do not return a digest, but the raw bytes. MD5 should align with that.

llvm/unittests/Support/MD5Test.cpp

77

I think this should avoid a magic number, and just confirm that the final() return is equal to MD5Result::bytes when calling final(MD5Result&).

[arames]

Address review comments.

[dexonsmith]

LGTM, thanks!

llvm/include/llvm/Support/MD5.h

public:
   void update(ArrayRef<uint8_t> Data);
 
   /// Updates the hash for the StringRef provided.
   void update(StringRef Str);
 
   /// Finishes off the hash and puts the result in result.
   void final(MD5Result &Result);
 
+  /// Finishes off the hash, and returns a reference to the 16-byte hash data.
+  StringRef final();
+
+  /// Finishes off the hash, and returns a reference to the 16-byte hash data.
+  /// This is suitable for getting the MD5 at any time without invalidating the
+  /// internal state, so that more calls can be made into `update`.
+  StringRef result();
+
   /// Translates the bytes in \p Res to a hex string that is
   /// deposited into \p Str. The result will be of length 32.
   static void stringifyResult(MD5Result &Result, SmallString<32> &Str);
 
   /// Computes the hash for a given bytes.
   static std::array<uint8_t, 16> hash(ArrayRef<uint8_t> Data);
 
 private:
   // Any 32-bit or wider unsigned integer data type will do.
   typedef uint32_t MD5_u32plus;
 
   // Internal State
   struct {
     MD5_u32plus a = 0x67452301;
     MD5_u32plus b = 0xefcdab89;
     MD5_u32plus c = 0x98badcfe;
     MD5_u32plus d = 0x10325476;
     MD5_u32plus hi = 0;
     MD5_u32plus lo = 0;
     uint8_t buffer[64];
     MD5_u32plus block[16];
   } InternalState;
 
+  MD5Result Result;
+
   const uint8_t *body(ArrayRef<uint8_t> Data);
 };
 
 inline bool operator==(const MD5::MD5Result &LHS, const MD5::MD5Result &RHS) {
   return LHS.Bytes == RHS.Bytes;
 }
 
 /// Helper to compute and return lower 64 bits of the given string's MD5 hash.

llvm/lib/Support/MD5.cpp

void MD5::final(MD5Result &Result) {
   body(makeArrayRef(InternalState.buffer, 64));
 
   support::endian::write32le(&Result[0], InternalState.a);
   support::endian::write32le(&Result[4], InternalState.b);
   support::endian::write32le(&Result[8], InternalState.c);
   support::endian::write32le(&Result[12], InternalState.d);
 }
 
+StringRef MD5::final() {
+  final(Result);
+  return StringRef(reinterpret_cast<char *>(Result.Bytes.data()),
+                   Result.Bytes.size());
+}
+
+StringRef MD5::result() {
+  auto StateToRestore = InternalState;
+
+  auto Hash = final();
+
+  // Restore the state
+  InternalState = StateToRestore;
+
+  return Hash;
+}
+
 SmallString<32> MD5::MD5Result::digest() const {
   SmallString<32> Str;
   raw_svector_ostream Res(Str);
   for (int i = 0; i < 16; ++i)
     Res << format("%.2x", Bytes[i]);
   return Str;
 }
 

llvm/unittests/Support/MD5Test.cpp

TEST(MD5HashTest, MD5) {
   MD5::MD5Result MD5Res;
   SmallString<32> Res;
   memcpy(MD5Res.Bytes.data(), Vec.data(), Vec.size());
   MD5::stringifyResult(MD5Res, Res);
   EXPECT_EQ(Res, "c3fcd3d76192e4007dfb496cca67e13b");
   EXPECT_EQ(0x3be167ca6c49fb7dULL, MD5Res.high());
   EXPECT_EQ(0x00e49261d7d3fcc3ULL, MD5Res.low());
 }
+
+TEST(MD5Test, FinalAndResultHelpers) {
+  MD5 Hash;
+
+  Hash.update("abcd");
+
+  {
+    MD5 ReferenceHash;
+    ReferenceHash.update("abcd");
+    MD5::MD5Result ReferenceResult;
+    ReferenceHash.final(ReferenceResult);
+    StringRef ExpectedResult =
+        StringRef(reinterpret_cast<char *>(ReferenceResult.Bytes.data()),
+                  ReferenceResult.Bytes.size());
+    EXPECT_EQ(Hash.result(), ExpectedResult);
+  }
+
+  Hash.update("xyz");
+
+  {
+    MD5 ReferenceHash;
+    ReferenceHash.update("abcd");
+    ReferenceHash.update("xyz");
+    MD5::MD5Result ReferenceResult;
+    ReferenceHash.final(ReferenceResult);
+    StringRef ExpectedResult =
+        StringRef(reinterpret_cast<char *>(ReferenceResult.Bytes.data()),
+                  ReferenceResult.Bytes.size());
+    EXPECT_EQ(Hash.final(), ExpectedResult);
+  }
 }
+} // namespace