Skip to content

Commit

Permalink
[lld/mac] Address review feedback and improve a comment
Browse files Browse the repository at this point in the history
I forgot to move the message() call around as requested in D103428
before committing that change. Move it now.

Also, improve the ordinal uniq'ing comment. I hadn't realized that the
distinct-but-identical files happen with --reproduce and not in general.

No behavior change.

Differential Revision: https://reviews.llvm.org/D103522
  • Loading branch information
nico committed Jun 2, 2021
1 parent f1a0c5d commit 476e4d6
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 9 deletions.
4 changes: 0 additions & 4 deletions lld/MachO/DriverUtils.cpp
Expand Up @@ -214,8 +214,6 @@ DylibFile *macho::loadDylib(MemoryBufferRef mbref, DylibFile *umbrella,
return nullptr;
}
file = make<DylibFile>(**result, umbrella, isBundleLoader);
if (config->printEachFile)
message(toString(file));

// parseReexports() can recursively call loadDylib(). That's fine since
// we wrote DylibFile we just loaded to the loadDylib cache via the `file`
Expand All @@ -231,8 +229,6 @@ DylibFile *macho::loadDylib(MemoryBufferRef mbref, DylibFile *umbrella,
magic == file_magic::macho_executable ||
magic == file_magic::macho_bundle);
file = make<DylibFile>(mbref, umbrella, isBundleLoader);
if (config->printEachFile)
message(toString(file));

// parseLoadCommands() can also recursively call loadDylib(). See comment
// in previous block for why this means we must copy `file` here.
Expand Down
6 changes: 6 additions & 0 deletions lld/MachO/InputFiles.cpp
Expand Up @@ -826,6 +826,9 @@ DylibFile::DylibFile(MemoryBufferRef mb, DylibFile *umbrella,
return;
}

if (config->printEachFile)
message(toString(this));

deadStrippable = hdr->flags & MH_DEAD_STRIPPABLE_DYLIB;

if (!checkCompatibility(this))
Expand Down Expand Up @@ -900,6 +903,9 @@ DylibFile::DylibFile(const InterfaceFile &interface, DylibFile *umbrella,
compatibilityVersion = interface.getCompatibilityVersion().rawValue();
currentVersion = interface.getCurrentVersion().rawValue();

if (config->printEachFile)
message(toString(this));

if (!is_contained(skipPlatformChecks, dylibName) &&
!is_contained(interface.targets(), config->platformInfo.target)) {
error(toString(this) + " is incompatible with " +
Expand Down
14 changes: 9 additions & 5 deletions lld/MachO/Writer.cpp
Expand Up @@ -705,17 +705,21 @@ template <class LP> void Writer::createLoadCommands() {
// Several DylibFiles can have the same installName. Only emit a single
// load command for that installName and give all these DylibFiles the
// same ordinal.
// This can happen if:
// This can happen in several cases:
// - a new framework could change its installName to an older
// framework name via an $ld$ symbol depending on platform_version
// - symlink (eg libpthread.tbd is a symlink to libSystem.tbd)
// - symlinks (for example, libpthread.tbd is a symlink to libSystem.tbd;
// Foo.framework/Foo.tbd is usually a symlink to
// Foo.framework/Versions/Current/Foo.tbd, where
// Foo.framework/Versions/Current is usually a symlink to
// Foo.framework/Versions/A)
// - a framework can be linked both explicitly on the linker
// command line and implicitly as a reexport from a different
// framework. The re-export will usually point to the tbd file
// in Foo.framework/Versions/A/Foo.tbd, while the explicit link will
// usually find Foo.framwork/Foo.tbd. These are usually two identical
// but distinct files (concrete example: CFNetwork.framework, reexported
// from CoreServices.framework).
// usually find Foo.framwork/Foo.tbd. These are usually symlinks,
// but in a --reproduce archive they will be identical but distinct
// files.
// In the first case, *semantically distinct* DylibFiles will have the
// same installName.
int64_t &ordinal = ordinalForInstallName[dylibFile->dylibName];
Expand Down

0 comments on commit 476e4d6

Please sign in to comment.