[PATCH] D124792: [ifs] Add --strip-size flag

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/docs/CommandGuide/llvm-ifs.rst:198
+.. option:: --strip-size
+ This removes the size from the output ifs and defaults the size of global
+ objects to 1 in stub output. This is useful for shared objects that don't
----------------
It would be nice of this wording was made more consistent with other options in this file.


================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:222
+      if (Size == std::numeric_limits<uint64_t>::max())
+        Size = 1;
+      DynSym.Content.add(DynStr.Content.getOffset(Sym.Name), Size, Bind,
----------------
abrachet wrote:
> jhenderson wrote:
> > Why is `Size` set to 1 and not 0, which would seem the more natural value?
> Linkers would get upset. https://github.com/llvm/llvm-project/blob/main/lld/ELF/Relocations.cpp#L355-L358
But giving a value of 1 means the relocation becomes invalid? It seems to me if you care about linker behaviour around copy relocations, you can't change their size at all.


================
Comment at: llvm/lib/InterfaceStub/IFSHandler.cpp:126
     } else if (Symbol.Type == IFSSymbolType::Func) {
       Symbol.Size = 0;
     } else {
----------------
This is a very weird thing to do in a function about writing YAML. Do you need to do it, and if so, do you need to do it here specifically?


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

https://reviews.llvm.org/D124792