[lld-macho] Include output filename in UUID hash

[lgrey]

Summary

Similar to what ld64 does here: https://github.com/apple-opensource/ld64/blob/e28c028b20af187a16a7161d89e91868a450cadc/src/ld/OutputFile.cpp#L3724

[keith]

lld/MachO/Writer.cpp

1089

I think this needs to use final_output and not output, the difference being when you use clang to compile a multi-arch binary output is a randomized name, so this breaks hermetic builds

[lgrey]

Use finalOutput

lld/MachO/Writer.cpp

1089

Great catch, thanks! Fixed

[keith]

lld/MachO/Writer.cpp

1089

Can you add a test covering this case to show it's preferred?

[lgrey]

Add test for -final_output

[int3]

lgtm

lld/test/MachO/uuid.s

6

jfyi, the awk isn't super necessary since FileCheck does substring matching by default. but I'm fine if you prefer this for clarity

[lgrey]

lld/test/MachO/uuid.s

6

We need it for cmp to work (but maybe there are better ways to check for string equality? I went this way after I couldn't figure out how to do it purely via FileCheck)

[int3]

lld/test/MachO/uuid.s

6

d'oh, yes, I forgot that llvm-dwarfdump --uuid included the filename in its output. nevermind :)

[aeubanks]

lld/test/MachO/uuid.s

6

just FYI, this is the only test that invokes awk AFAICT and it's breaking our windows buildbot which doesn't have awk
arguably we shouldn't be using gnuwin32 and should upgrade to git bash

I've marked this test as unsupported on windows in 79a9fe6c8afeff727ac60e6c7abe24f02df5a9a4

[lgrey]

lld/test/MachO/uuid.s

6

Is cut available?

lld/MachO/Writer.cpp

 void Writer::writeUuid() {
   TimeTraceScope timeScope("Computing UUID");
 
   ArrayRef<uint8_t> data{buffer->getBufferStart(), buffer->getBufferEnd()};
   unsigned chunkCount = parallel::strategy.compute_thread_count() * 10;
   // Round-up integer division
   size_t chunkSize = (data.size() + chunkCount - 1) / chunkCount;
   std::vector<ArrayRef<uint8_t>> chunks = split(data, chunkSize);
-  std::vector<uint64_t> hashes(chunks.size());
+  // Leave one slot for filename
+  std::vector<uint64_t> hashes(chunks.size() + 1);
   SmallVector<std::shared_future<void>> threadFutures;
   threadFutures.reserve(chunks.size());
   for (size_t i = 0; i < chunks.size(); ++i)
     threadFutures.emplace_back(threadPool.async(
         [&](size_t j) { hashes[j] = xxHash64(chunks[j]); }, i));
   for (std::shared_future<void> &future : threadFutures)
     future.wait();
-
+  // Append the output filename so that identical binaries with different names
+  // don't get the same UUID.
+  hashes[chunks.size()] = xxHash64(sys::path::filename(config->finalOutput));
   uint64_t digest = xxHash64({reinterpret_cast<uint8_t *>(hashes.data()),
                               hashes.size() * sizeof(uint64_t)});
   uuidCommand->writeUuid(digest);
 }
 
 void Writer::writeCodeSignature() {
   if (codeSignatureSection)
     codeSignatureSection->writeHashes(buffer->getBufferStart());

lld/test/MachO/uuid.s

 # REQUIRES: x86
-# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t.o
-# RUN: %lld -lSystem %t.o -o %t
-# RUN: llvm-dwarfdump --uuid %t | FileCheck %s
-# CHECK: 4C4C44{{([[:xdigit:]]{2})}}-5555-{{([[:xdigit:]]{4})}}-A1{{([[:xdigit:]]{2})}}-{{([[:xdigit:]]{12})}} (x86_64)
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %s -o %t/test.o
+# RUN: %lld -lSystem %t/test.o -o %t/a
+# RUN: %lld -lSystem %t/test.o -o %t/b
+# RUN: llvm-dwarfdump --uuid %t/a | awk '{print $2}' > %t/uuida
+# RUN: llvm-dwarfdump --uuid %t/b | awk '{print $2}' > %t/uuidb
+# RUN: FileCheck %s < %t/uuida
+# RUN: FileCheck %s < %t/uuidb
+# RUN: not cmp %t/uuida %t/uuidb
+
+## Ensure -final_output is used for universal binaries, which may be linked with
+## temporary output file names
+# RUN: %lld -lSystem %t/test.o -o %t/c -final_output %t/a
+# RUN: llvm-dwarfdump --uuid %t/c | awk '{print $2}' > %t/uuidc
+# RUN: cmp %t/uuida %t/uuidc
+
+
+# CHECK: 4C4C44{{([[:xdigit:]]{2})}}-5555-{{([[:xdigit:]]{4})}}-A1{{([[:xdigit:]]{2})}}-{{([[:xdigit:]]{12})}}
 
 .globl _main
 _main:
   ret