This is an archive of the discontinued LLVM Phabricator instance.

[lld-macho] Include output filename in UUID hash
ClosedPublic

Authored by lgrey on Mar 31 2022, 12:04 PM.

Diff Detail

Event Timeline

lgrey created this revision.Mar 31 2022, 12:04 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMar 31 2022, 12:04 PM
lgrey requested review of this revision.Mar 31 2022, 12:04 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2022, 12:04 PM
keith added a subscriber: keith.Mar 31 2022, 12:05 PM
keith added inline comments.
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 updated this revision to Diff 419534.Mar 31 2022, 12:10 PM

Use finalOutput

lld/MachO/Writer.cpp
1089

Great catch, thanks! Fixed

keith added inline comments.Mar 31 2022, 12:25 PM
lld/MachO/Writer.cpp
1089

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

lgrey updated this revision to Diff 419554.Mar 31 2022, 1:18 PM

Add test for -final_output

lgrey marked an inline comment as done.Mar 31 2022, 1:18 PM
int3 accepted this revision.Mar 31 2022, 1:34 PM
int3 added a subscriber: 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

This revision is now accepted and ready to land.Mar 31 2022, 1:34 PM
keith accepted this revision.Mar 31 2022, 1:49 PM
lgrey added inline comments.Mar 31 2022, 2:50 PM
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 added inline comments.Mar 31 2022, 3:06 PM
lld/test/MachO/uuid.s
6

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

This revision was landed with ongoing or failed builds.Mar 31 2022, 3:52 PM
This revision was automatically updated to reflect the committed changes.
aeubanks added inline comments.
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 added inline comments.Apr 1 2022, 3:43 PM
lld/test/MachO/uuid.s
6

Is cut available?