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

[Alexey Lapshin via Phabricator via llvm-commits]

avl added a comment.

In D158124#4651771 <https://reviews.llvm.org/D158124#4651771>, @Alpha wrote:

> 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?

/usr/bin/time -l would be enough:

  /usr/bin/time -l dsymutil clang
    19635666944  maximum resident set size
    ...



================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:1787
+                       AttrInfo, IsLittleEndian);
+    if (FinalAttrSize != 0) {
+      AttributesFixups.push_back(CurAttrFixup);
----------------
Alpha wrote:
> 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.
In that case we probably need to add a flag indicating whether DWARFLinker needs to gather and save relocations info. Searching for relocations and keeping them in some array requires additional resources. Other tools(f.e. llvm-dwarfutil) may save run-time&memory by not doing this thing:

```
class AddressesMap {
  bool needToSaveRelocs();
}
```
and then here:

```
if (FinalAttrSize != 0 && ObjFile.Addresses->needToSaveRelocs()) {
```


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

https://reviews.llvm.org/D158124