[llvm] 0662430 - [Object] Fix updating darwin archives
Keith Smiley via llvm-commits
llvm-commits at lists.llvm.org
Thu May 19 11:06:27 PDT 2022
Author: Keith Smiley
Date: 2022-05-19T10:56:26-07:00
New Revision: 066243057fc2ae45ae6bbc2f4874ca1f84c3a3ff
URL: https://github.com/llvm/llvm-project/commit/066243057fc2ae45ae6bbc2f4874ca1f84c3a3ff
DIFF: https://github.com/llvm/llvm-project/commit/066243057fc2ae45ae6bbc2f4874ca1f84c3a3ff.diff
LOG: [Object] Fix updating darwin archives
When creating an archive, llvm-ar looks at the host to determine the
archive format to use, on Apple platforms this means it uses the
K_DARWIN format. K_DARWIN is _virtually_ equivalent to K_BSD, expect for
some very slight differences around padding, timestamps in deterministic
mode, and 64 bit formats. When updating an archive using llvm-ar, or
llvm-objcopy, Archive would try to determine the kind, but it was not
possible to get K_DARWIN in the initialization of the archive, because
they're virtually inciting usable from K_BSD, especially since the
slight differences only apply in very specific cases. This leads to
linker failures when the alignment workaround is not applied to an
archive copied with llvm-objcopy. This change teaches Archive to infer
the K_DARWIN type in the cases where it's possible and the first object
in the archive is a macho object. This avoids using the host triple to
determine this to not affect cross compiling.
Ideally we would eliminate the separate K_DARWIN type entirely since
it's not a truly separate archive type, but then we'd have to force the
macho workarounds on the BSD format generally. This might be acceptable
but then it would be unclear how to handle this case without forcing the
K_DARWIN64 format on all BSD users:
```
if (LastOffset >= Sym64Threshold) {
if (Kind == object::Archive::K_DARWIN)
Kind = object::Archive::K_DARWIN64;
else
Kind = object::Archive::K_GNU64;
}
```
The logic used to determine if the object is macho is derived from the
logic llvm-ar uses.
Previous context:
- 111cd669e90e5b2132187d36f8b141b11a671a8b
- 23a76be5adcaa768ba538f8a4514a7afccf61988
Differential Revision: https://reviews.llvm.org/D124895
Added:
llvm/test/tools/llvm-ar/macho-edit.test
llvm/test/tools/llvm-objcopy/MachO/archive-format.test
Modified:
llvm/include/llvm/Object/Archive.h
llvm/include/llvm/Object/ArchiveWriter.h
llvm/lib/ObjCopy/Archive.cpp
llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp
llvm/lib/Object/Archive.cpp
llvm/lib/Object/ArchiveWriter.cpp
llvm/test/tools/llvm-objcopy/MachO/universal-object.test
llvm/tools/llvm-ar/llvm-ar.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/Object/Archive.h b/llvm/include/llvm/Object/Archive.h
index 1f4a274e67fd..a36c9bd6163b 100644
--- a/llvm/include/llvm/Object/Archive.h
+++ b/llvm/include/llvm/Object/Archive.h
@@ -340,6 +340,7 @@ class Archive : public Binary {
Kind kind() const { return (Kind)Format; }
bool isThin() const { return IsThin; }
+ static object::Archive::Kind getDefaultKindForHost();
child_iterator child_begin(Error &Err, bool SkipInternal = true) const;
child_iterator child_end() const;
diff --git a/llvm/include/llvm/Object/ArchiveWriter.h b/llvm/include/llvm/Object/ArchiveWriter.h
index 7eaf13e8fb22..6acab45215da 100644
--- a/llvm/include/llvm/Object/ArchiveWriter.h
+++ b/llvm/include/llvm/Object/ArchiveWriter.h
@@ -26,6 +26,11 @@ struct NewArchiveMember {
NewArchiveMember() = default;
NewArchiveMember(MemoryBufferRef BufRef);
+ // Detect the archive format from the object or bitcode file. This helps
+ // assume the archive format when creating or editing archives in the case
+ // one isn't explicitly set.
+ object::Archive::Kind detectKindFromObject() const;
+
static Expected<NewArchiveMember>
getOldMember(const object::Archive::Child &OldMember, bool Deterministic);
diff --git a/llvm/lib/ObjCopy/Archive.cpp b/llvm/lib/ObjCopy/Archive.cpp
index ef893ccb409c..742ca0b890cf 100644
--- a/llvm/lib/ObjCopy/Archive.cpp
+++ b/llvm/lib/ObjCopy/Archive.cpp
@@ -11,6 +11,7 @@
#include "llvm/ObjCopy/MultiFormatConfig.h"
#include "llvm/ObjCopy/ObjCopy.h"
#include "llvm/Object/Error.h"
+#include "llvm/Object/MachO.h"
#include "llvm/Support/FileOutputBuffer.h"
#include "llvm/Support/SmallVectorMemoryBuffer.h"
@@ -61,6 +62,10 @@ static Error deepWriteArchive(StringRef ArcName,
ArrayRef<NewArchiveMember> NewMembers,
bool WriteSymtab, object::Archive::Kind Kind,
bool Deterministic, bool Thin) {
+ if (Kind == object::Archive::K_BSD && !NewMembers.empty() &&
+ NewMembers.front().detectKindFromObject() == object::Archive::K_DARWIN)
+ Kind = object::Archive::K_DARWIN;
+
if (Error E = writeArchive(ArcName, NewMembers, WriteSymtab, Kind,
Deterministic, Thin))
return createFileError(ArcName, std::move(E));
diff --git a/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp b/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp
index 94e89224b3c8..8ba0c7738932 100644
--- a/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp
+++ b/llvm/lib/ObjCopy/MachO/MachOObjcopy.cpp
@@ -484,9 +484,12 @@ Error objcopy::macho::executeObjcopyOnMachOUniversalBinary(
createNewArchiveMembers(Config, **ArOrErr);
if (!NewArchiveMembersOrErr)
return NewArchiveMembersOrErr.takeError();
+ auto Kind = (*ArOrErr)->kind();
+ if (Kind == object::Archive::K_BSD)
+ Kind = object::Archive::K_DARWIN;
Expected<std::unique_ptr<MemoryBuffer>> OutputBufferOrErr =
writeArchiveToBuffer(*NewArchiveMembersOrErr,
- (*ArOrErr)->hasSymbolTable(), (*ArOrErr)->kind(),
+ (*ArOrErr)->hasSymbolTable(), Kind,
Config.getCommonConfig().DeterministicArchives,
(*ArOrErr)->isThin());
if (!OutputBufferOrErr)
diff --git a/llvm/lib/Object/Archive.cpp b/llvm/lib/Object/Archive.cpp
index 3960971d882c..0b29428f0c00 100644
--- a/llvm/lib/Object/Archive.cpp
+++ b/llvm/lib/Object/Archive.cpp
@@ -22,6 +22,7 @@
#include "llvm/Support/Error.h"
#include "llvm/Support/ErrorOr.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Host.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
@@ -925,6 +926,14 @@ Archive::Archive(MemoryBufferRef Source, Error &Err)
Err = Error::success();
}
+object::Archive::Kind Archive::getDefaultKindForHost() {
+ Triple HostTriple(sys::getProcessTriple());
+ return HostTriple.isOSDarwin()
+ ? object::Archive::K_DARWIN
+ : (HostTriple.isOSAIX() ? object::Archive::K_AIXBIG
+ : object::Archive::K_GNU);
+}
+
Archive::child_iterator Archive::child_begin(Error &Err,
bool SkipInternal) const {
if (isEmpty())
diff --git a/llvm/lib/Object/ArchiveWriter.cpp b/llvm/lib/Object/ArchiveWriter.cpp
index 0ac11224a48c..dbf5052cdac0 100644
--- a/llvm/lib/Object/ArchiveWriter.cpp
+++ b/llvm/lib/Object/ArchiveWriter.cpp
@@ -18,8 +18,11 @@
#include "llvm/IR/LLVMContext.h"
#include "llvm/Object/Archive.h"
#include "llvm/Object/Error.h"
+#include "llvm/Object/IRObjectFile.h"
+#include "llvm/Object/MachO.h"
#include "llvm/Object/ObjectFile.h"
#include "llvm/Object/SymbolicFile.h"
+#include "llvm/Object/XCOFFObjectFile.h"
#include "llvm/Support/Alignment.h"
#include "llvm/Support/EndianStream.h"
#include "llvm/Support/Errc.h"
@@ -44,6 +47,40 @@ NewArchiveMember::NewArchiveMember(MemoryBufferRef BufRef)
: Buf(MemoryBuffer::getMemBuffer(BufRef, false)),
MemberName(BufRef.getBufferIdentifier()) {}
+object::Archive::Kind NewArchiveMember::detectKindFromObject() const {
+ auto MemBufferRef = this->Buf->getMemBufferRef();
+ Expected<std::unique_ptr<object::ObjectFile>> OptionalObject =
+ object::ObjectFile::createObjectFile(MemBufferRef);
+
+ if (OptionalObject)
+ return isa<object::MachOObjectFile>(**OptionalObject)
+ ? object::Archive::K_DARWIN
+ : (isa<object::XCOFFObjectFile>(**OptionalObject)
+ ? object::Archive::K_AIXBIG
+ : object::Archive::K_GNU);
+
+ // Squelch the error in case we had a non-object file.
+ consumeError(OptionalObject.takeError());
+
+ // If we're adding a bitcode file to the archive, detect the Archive kind
+ // based on the target triple.
+ LLVMContext Context;
+ if (identify_magic(MemBufferRef.getBuffer()) == file_magic::bitcode) {
+ if (auto ObjOrErr = object::SymbolicFile::createSymbolicFile(
+ MemBufferRef, file_magic::bitcode, &Context)) {
+ auto &IRObject = cast<object::IRObjectFile>(**ObjOrErr);
+ return Triple(IRObject.getTargetTriple()).isOSDarwin()
+ ? object::Archive::K_DARWIN
+ : object::Archive::K_GNU;
+ } else {
+ // Squelch the error in case this was not a SymbolicFile.
+ consumeError(ObjOrErr.takeError());
+ }
+ }
+
+ return object::Archive::getDefaultKindForHost();
+}
+
Expected<NewArchiveMember>
NewArchiveMember::getOldMember(const object::Archive::Child &OldMember,
bool Deterministic) {
diff --git a/llvm/test/tools/llvm-ar/macho-edit.test b/llvm/test/tools/llvm-ar/macho-edit.test
new file mode 100644
index 000000000000..9bac39053d67
--- /dev/null
+++ b/llvm/test/tools/llvm-ar/macho-edit.test
@@ -0,0 +1,16 @@
+## Make sure the darwin format specifics are preserved when updating archives.
+
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: yaml2obj %p/Inputs/macho.yaml > %t/dup.o
+
+## Create the archive with a duplicate object to ensure that darwin specific
+## incrementing timestamps are used.
+# RUN: llvm-ar --format=darwin crD %t/lib.a %t/dup.o %t/dup.o
+# RUN: cp %t/lib.a %t/lib.copy.a
+
+## Replace an object file in the archive to force a re-write.
+# RUN: llvm-ar crD %t/lib.a %t/dup.o
+# RUN: obj2yaml %t/lib.a | FileCheck --implicit-check-not=LastModified %s
+
+# CHECK: LastModified: '1'
+# CHECK: LastModified: '2'
diff --git a/llvm/test/tools/llvm-objcopy/MachO/archive-format.test b/llvm/test/tools/llvm-objcopy/MachO/archive-format.test
new file mode 100644
index 000000000000..dbbb2c5bf6ce
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/MachO/archive-format.test
@@ -0,0 +1,12 @@
+# REQUIRES: x86-registered-target
+
+## Make sure the darwin format specifics are preserved when updating archives.
+
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: llvm-mc -filetype=obj -triple=x86_64-apple-darwin %p/Inputs/macho.64.s -o %t/dup.o
+# RUN: llvm-ar --format=darwin crD %t/lib.a %t/dup.o %t/dup.o
+# RUN: llvm-objcopy %t/lib.a %t/lib.copy.a
+# RUN: obj2yaml %t/lib.copy.a | FileCheck --implicit-check-not=LastModified %s
+
+# CHECK: LastModified: '1'
+# CHECK: LastModified: '2'
diff --git a/llvm/test/tools/llvm-objcopy/MachO/universal-object.test b/llvm/test/tools/llvm-objcopy/MachO/universal-object.test
index a6146fd56483..b23fecee6c64 100644
--- a/llvm/test/tools/llvm-objcopy/MachO/universal-object.test
+++ b/llvm/test/tools/llvm-objcopy/MachO/universal-object.test
@@ -12,9 +12,9 @@
# RUN: cmp %t.i386 %t.i386.copy
# RUN: cmp %t.x86_64 %t.x86_64.copy
-## Case 2: copy a universal object file containing an archive.
+## Case 2: copy a universal object file containing an archive with darwin specific timestamps.
# RUN: rm -f %t.archive.i386
-# RUN: llvm-ar cr %t.archive.i386 %t.i386
+# RUN: llvm-ar --format=darwin cr %t.archive.i386 %t.i386 %t.i386
# RUN: llvm-lipo %t.archive.i386 %t.x86_64 -create -output %t.universal.containing.archive
# RUN: llvm-objcopy %t.universal.containing.archive %t.universal.containing.archive.copy
# RUN: llvm-lipo %t.universal.containing.archive.copy -archs | FileCheck --check-prefix=VERIFY_ARCHS %s
diff --git a/llvm/tools/llvm-ar/llvm-ar.cpp b/llvm/tools/llvm-ar/llvm-ar.cpp
index 214b6e679698..a93943295cf9 100644
--- a/llvm/tools/llvm-ar/llvm-ar.cpp
+++ b/llvm/tools/llvm-ar/llvm-ar.cpp
@@ -880,48 +880,6 @@ computeNewArchiveMembers(ArchiveOperation Operation,
return Ret;
}
-static object::Archive::Kind getDefaultForHost() {
- Triple HostTriple(sys::getProcessTriple());
- return HostTriple.isOSDarwin()
- ? object::Archive::K_DARWIN
- : (HostTriple.isOSAIX() ? object::Archive::K_AIXBIG
- : object::Archive::K_GNU);
-}
-
-static object::Archive::Kind getKindFromMember(const NewArchiveMember &Member) {
- auto MemBufferRef = Member.Buf->getMemBufferRef();
- Expected<std::unique_ptr<object::ObjectFile>> OptionalObject =
- object::ObjectFile::createObjectFile(MemBufferRef);
-
- if (OptionalObject)
- return isa<object::MachOObjectFile>(**OptionalObject)
- ? object::Archive::K_DARWIN
- : (isa<object::XCOFFObjectFile>(**OptionalObject)
- ? object::Archive::K_AIXBIG
- : object::Archive::K_GNU);
-
- // squelch the error in case we had a non-object file
- consumeError(OptionalObject.takeError());
-
- // If we're adding a bitcode file to the archive, detect the Archive kind
- // based on the target triple.
- LLVMContext Context;
- if (identify_magic(MemBufferRef.getBuffer()) == file_magic::bitcode) {
- if (auto ObjOrErr = object::SymbolicFile::createSymbolicFile(
- MemBufferRef, file_magic::bitcode, &Context)) {
- auto &IRObject = cast<object::IRObjectFile>(**ObjOrErr);
- return Triple(IRObject.getTargetTriple()).isOSDarwin()
- ? object::Archive::K_DARWIN
- : object::Archive::K_GNU;
- } else {
- // Squelch the error in case this was not a SymbolicFile.
- consumeError(ObjOrErr.takeError());
- }
- }
-
- return getDefaultForHost();
-}
-
static void performWriteOperation(ArchiveOperation Operation,
object::Archive *OldArchive,
std::unique_ptr<MemoryBuffer> OldArchiveBuf,
@@ -943,14 +901,23 @@ static void performWriteOperation(ArchiveOperation Operation,
case Default:
if (Thin)
Kind = object::Archive::K_GNU;
- else if (OldArchive)
+ else if (OldArchive) {
Kind = OldArchive->kind();
- else if (NewMembersP)
- Kind = !NewMembersP->empty() ? getKindFromMember(NewMembersP->front())
- : getDefaultForHost();
+ if (Kind == object::Archive::K_BSD) {
+ auto InferredKind = object::Archive::K_BSD;
+ if (NewMembersP && !NewMembersP->empty())
+ InferredKind = NewMembersP->front().detectKindFromObject();
+ else if (!NewMembers.empty())
+ InferredKind = NewMembers.front().detectKindFromObject();
+ if (InferredKind == object::Archive::K_DARWIN)
+ Kind = object::Archive::K_DARWIN;
+ }
+ } else if (NewMembersP)
+ Kind = !NewMembersP->empty() ? NewMembersP->front().detectKindFromObject()
+ : object::Archive::getDefaultKindForHost();
else
- Kind = !NewMembers.empty() ? getKindFromMember(NewMembers.front())
- : getDefaultForHost();
+ Kind = !NewMembers.empty() ? NewMembers.front().detectKindFromObject()
+ : object::Archive::getDefaultKindForHost();
break;
case GNU:
Kind = object::Archive::K_GNU;
More information about the llvm-commits
mailing list