[ObjCopy] Fix type mismatch in writeCodeSignatureData()

[jmroot]

Summary

The result of pointer subtraction is of type ptrdiff_t, which is not necessarily the same underlying type as ssize_t. This can lead to a compilation error since std::min requires both parameters to be the same type.

Fixes: https://github.com/llvm/llvm-project/issues/54846

[jhenderson]

LGTM, but maybe give @alexander-shaposhnikov a chance to comment (i.e. give it a day or two to land this or until he responds).

[drodriguez]

My comment about using size_t instead are not a blocker, just an opinion.

Whenever this is ready to land, if you need us to land this, please comment so and we will submit.

llvm/lib/ObjCopy/MachO/MachOWriter.cpp

522–525

I am sorry for the initial blunder and sorry for breaking this for 32 bits platforms.

I was wondering why we did this original. The StringRef constructor needs size_t for its second argument. BlockSize is already size_t. Because of the while loop check HashReadEnd - CurrHashReadPosition has to be positive. I wonder if the right thing here is casting to size_t.

(ssize_t has a weird range. It is not a "signed size_t", but a size_t that allows -1 to be returned. But in any case, there's no negative numbers being handled here, it is just a ptrdiff_t which we know to be positive).

[alexander-shaposhnikov]

llvm/lib/ObjCopy/MachO/MachOWriter.cpp

522–525

I suspect it might originate from https://github.com/llvm/llvm-project/blob/5ba0a9571b3ee3bc76f65e16549012a440d5a0fb/lld/MachO/SyntheticSections.cpp#L1243
it appears to be safe to cast to size_t

[jmroot]

Now using size_t for the two casts, since that is what the StringRef constructor actually takes.

[alexander-shaposhnikov]

LG, thanks

[jmroot]

Thanks for the reviews. I don't have commit access, so could someone who does please commit at your convenience?

[drodriguez]

In D128117#3600592, @jmroot wrote:

Thanks for the reviews. I don't have commit access, so could someone who does please commit at your convenience?

What username and email would you like for the commit message? See https://llvm.org/docs/Phabricator.html#committing-someone-s-change-from-phabricator for reference of what we need.

[jmroot]

You can use Joshua Root <[e-mail]>, thanks.

llvm/lib/ObjCopy/MachO/MachOWriter.cpp

void MachOWriter::writeCodeSignatureData() {
   memcpy(Id, CodeSignature.OutputFileName.begin(),
          CodeSignature.OutputFileName.size());
   memset(Id + CodeSignature.OutputFileName.size(), 0, FileNamePad);
 
   // Write the hashes.
   uint8_t *CurrHashReadPosition = HashReadStart;
   uint8_t *CurrHashWritePosition = HashWriteStart;
   while (CurrHashReadPosition < HashReadEnd) {
     StringRef Block(reinterpret_cast<char *>(CurrHashReadPosition),
-                    std::min(HashReadEnd - CurrHashReadPosition,
-                             static_cast<ssize_t>(CodeSignature.BlockSize)));
+                    std::min(static_cast<size_t>(HashReadEnd
+                             - CurrHashReadPosition),
+                             static_cast<size_t>(CodeSignature.BlockSize)));
     SHA256 Hasher;
     Hasher.update(Block);
     std::array<uint8_t, 32> Hash = Hasher.final();
     assert(Hash.size() == CodeSignature.HashSize);
     memcpy(CurrHashWritePosition, Hash.data(), CodeSignature.HashSize);
     CurrHashReadPosition += CodeSignature.BlockSize;
     CurrHashWritePosition += CodeSignature.HashSize;
   }