[PATCH] D158124: [dsymutil] Add support for mergeable libraries

[Alpha Abdoulaye via Phabricator via llvm-commits]

Alpha marked an inline comment as done.
Alpha added a comment.

In D158124#4651700 <https://reviews.llvm.org/D158124#4651700>, @avl wrote:

> I have a general question - Could you measure difference with run-time memory requirements(f.e. for clang binary) with and without this patch, please?

Is there a specific test suite/benchmark/tool somewhere that you want to be run here, and can point to?



================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:1787
+                       AttrInfo, IsLittleEndian);
+    if (FinalAttrSize != 0) {
+      AttributesFixups.push_back(CurAttrFixup);
----------------
avl wrote:
> Alpha wrote:
> > avl wrote:
> > > it would be good to add check for ObjFile.Addresses->getLibraryInstallName() here to not collect AttributeLinkedOffsetFixup if not necessary:
> > > 
> > > 
> > > ```
> > > std::optional<StringRef> LibraryInstallName = ObjFile.Addresses->getLibraryInstallName();
> > > .....
> > > if (FinalAttrSize != 0 && LibraryInstallName.has_value()) {
> > > ```
> > Hmm, we also need t have the fixups/relocs saved, even if we're dealing with regular object files, as we still need to always save the relocations, regardless of the type of input file.
> Before this patch dsymutil did not store fixup/relocs for regular files. Is that correct that this information would stored for usual files(not mergeable libraries) after this patch?
A mergeable library is a file a regular dylib that contains extra metadata that fundamentally just represents a the original regular object files.
So when let's say an executable links against a mergeable library, in addition to the regular `N_OSO` stab to tell were the symbol might have originated from, there is an additional `N_LIB`  that indicates the library where the symbol is from (so both stabs are present). This is the "only" moment when dsymutil detects the presence of mergeable libraries, and takes can take a different code path.
So in order for dsymutil to be able to work in this case, the `.dSYM` files used as input need the original relocations to be present. So yes, when we emit `.dSYM` files, we need to serialize the relocation info.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158124/new/

https://reviews.llvm.org/D158124