[Object] Deduplicate long archive member names

[Lekensteyn]

Summary

Import libraries as created by llvm-dlltool always use the same archive
member name for every object file (namely, the DLL library name). Ensure
that long names are not repeatedly stored in the string table.

[mstorsjo]

I'm not intimately familiar with the structure of ar files, but the change looks like it makes sense to me at least. Does MSVC link.exe produce import libraries with deduplicated member names as well?

I think the change is possible to apply as such, but it'd be better if the diff was made with more context included (diff -U9999).

[Lekensteyn]

In D55860#1335943, @mstorsjo wrote:

I'm not intimately familiar with the structure of ar files, but the change looks like it makes sense to me at least. Does MSVC link.exe produce import libraries with deduplicated member names as well?

Yes it does deduplicate and use /0 for every archive member File ID.

Additional differences with MSVC which does not appear to have a noticable functional impact:

• LLVM produces string table entries terminated with /\n while MSVC uses \0. The PE Format documentation says: "The strings are null-terminated. Each string begins immediately after the null byte in the previous string."
• MSVC creates an additional Second Linker Member which is omitted by LLVM.

To view the contents you could look at a hexdump or use Wireshark with https://git.lekensteyn.nl/peter/wireshark-notes/tree/lua/file-ar.lua

wireshark -Xlua_script:path/to/file-ar.lua -r your.lib

I think the change is possible to apply as such, but it'd be better if the diff was made with more context included (diff -U9999).

I've used git diff -W to include function context, but if that's not sufficient I'll include the full diff next time :)

[ruiu]

LGTM

lib/Object/ArchiveWriter.cpp

276–277 ↗

(On Diff #178801)

nit: if previous if is enclosed with {}, so should be else.

449 ↗

(On Diff #178801)

I'd think "important" is a bit too strong. Maybe I'd say something like doing this saves space especially for COFF import libraries where all members have the same filename.

[Lekensteyn]

Added braces and updated comment. Thanks for the feedback!

llvm/trunk/lib/Object/ArchiveWriter.cpp

else
       Out << ID;
   } else
     Out << M.MemberName;
   Out << "/\n";
 }
 
 static void printMemberHeader(raw_ostream &Out, uint64_t Pos,
                               raw_ostream &StringTable,
+                              StringMap<uint64_t> &MemberNames,
                               object::Archive::Kind Kind, bool Thin,
                               StringRef ArcName, const NewArchiveMember &M,
                               sys::TimePoint<std::chrono::seconds> ModTime,
                               unsigned Size) {
 
   if (isBSDLike(Kind))
     return printBSDMemberHeader(Out, Pos, M.MemberName, ModTime, M.UID, M.GID,
                                 M.Perms, Size);
   if (!useStringTable(Thin, M.MemberName))
     return printGNUSmallMemberHeader(Out, M.MemberName, ModTime, M.UID, M.GID,
                                      M.Perms, Size);
   Out << '/';
-  uint64_t NamePos = StringTable.tell();
+  uint64_t NamePos;
+  if (Thin) {
+    NamePos = StringTable.tell();
+    addToStringTable(StringTable, ArcName, M, Thin);
+  } else {
+    StringMap<uint64_t>::const_iterator it = MemberNames.find(M.MemberName);
+    if (it == MemberNames.end()) {
+      NamePos = StringTable.tell();
       addToStringTable(StringTable, ArcName, M, Thin);
+      MemberNames[M.MemberName] = NamePos;
+    } else {
+      NamePos = it->second;
+    }
+  }
   printWithSpacePadding(Out, NamePos, 15);
   printRestOfMemberHeader(Out, ModTime, M.UID, M.GID, M.Perms, Size);
 }
 
 namespace {
 struct MemberData {
   std::vector<unsigned> Symbols;
   std::string Header;
computeMemberData(raw_ostream &StringTable, raw_ostream &SymNames,
 
   // This ignores the symbol table, but we only need the value mod 8 and the
   // symbol table is aligned to be a multiple of 8 bytes
   uint64_t Pos = 0;
 
   std::vector<MemberData> Ret;
   bool HasObject = false;
 
+  // Deduplicate long member names in the string table and reuse earlier name
+  // offsets. This especially saves space for COFF Import libraries where all
+  // members have the same name.
+  StringMap<uint64_t> MemberNames;
+
   // UniqueTimestamps is a special case to improve debugging on Darwin:
   //
   // The Darwin linker does not link debug info into the final
   // binary. Instead, it emits entries of type N_OSO in in the output
   // binary's symbol table, containing references to the linked-in
   // object files. Using that reference, the debugger can read the
   // debug data directly from the object files. Alternatively, an
   // invocation of 'dsymutil' will link the debug data from the object
for (const NewArchiveMember &M : NewMembers) {
     StringRef Padding = StringRef(PaddingData, MemberPadding + TailPadding);
 
     sys::TimePoint<std::chrono::seconds> ModTime;
     if (UniqueTimestamps)
       // Increment timestamp for each file of a given name.
       ModTime = sys::toTimePoint(FilenameCount[M.MemberName]++);
     else
       ModTime = M.ModTime;
-    printMemberHeader(Out, Pos, StringTable, Kind, Thin, ArcName, M, ModTime,
-                      Buf.getBufferSize() + MemberPadding);
+    printMemberHeader(Out, Pos, StringTable, MemberNames, Kind, Thin, ArcName,
+                      M, ModTime, Buf.getBufferSize() + MemberPadding);
     Out.flush();
 
     Expected<std::vector<unsigned>> Symbols =
         getSymbols(Buf, SymNames, HasObject);
     if (auto E = Symbols.takeError())
       return std::move(E);
 
     Pos += Header.size() + Data.size() + Padding.size();

llvm/trunk/test/Object/archive-format.test

 RUN: printf bar2 > foo/0123456789abcde
 RUN: printf zed2 > foo/0123456789abcdef
 
 RUN: rm -f %t.a
 RUN: llvm-ar --format=gnu rc %t.a 0123456789abcde 0123456789abcdef foo/0123456789abcde foo/0123456789abcdef
 RUN: cat %t.a | FileCheck -strict-whitespace %s
 
 CHECK:      !<arch>
-CHECK-NEXT: //                                              36        `
-CHECK-NEXT: 0123456789abcdef/
+CHECK-NEXT: //                                              18        `
 CHECK-NEXT: 0123456789abcdef/
 CHECK-NEXT: 0123456789abcde/0           0     0     644     4         `
 CHECK-NEXT: bar.
 CHECK-SAME: /0              0           0     0     644     4         `
 CHECK-NEXT: zed.
 CHECK-SAME: 0123456789abcde/0           0     0     644     4         `
 CHECK-NEXT: bar2
-CHECK-SAME: /18             0           0     0     644     4         `
+CHECK-SAME: /0              0           0     0     644     4         `
 CHECK-NEXT: zed2
 
 RUN: rm -f %t.a
 RUN: llvm-ar --format=bsd rc %t.a 0123456789abcde 0123456789abcdef
 RUN: cat %t.a | FileCheck -strict-whitespace --check-prefix=BSD %s
 
 BSD:      !<arch>
 BSD-NEXT: #1/20           0           0     0     644     24        `