[PATCH] D139069: [lld-macho] Ignored aliases to weak symbols should not retain section data

Jez Ng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 30 21:28:02 PST 2022


int3 marked an inline comment as done.
int3 added inline comments.


================
Comment at: lld/MachO/InputFiles.cpp:624
 
+static bool isIgnoredName(StringRef name) {
+  return name.startswith("l") || name.startswith("L");
----------------
oontvoo wrote:
> super nit ...
will add the comment. honestly I don't think either name is super good heh (I went back and forth before deciding on this). ld64 calls them "ignored labels" which doesn't seem great either because the rest of the code doesn't use the word "label"

but the symbol isn't exactly ignored either :/ it's used at parse time, just immediately dropped as part of the parse...

will mull on this a bit


================
Comment at: lld/MachO/InputFiles.cpp:840-849
+      if (nList[lhs].n_value == nList[rhs].n_value)
+        return !isIgnoredName(getSymName(nList[lhs])) &&
+               isIgnoredName(getSymName(nList[rhs]));
       return nList[lhs].n_value < nList[rhs].n_value;
     });
     for (size_t j = 0; j < symbolIndices.size(); ++j) {
       const uint32_t symIndex = symbolIndices[j];
----------------
oontvoo wrote:
> This seems like we'd be repeating the `strtab + sym.n_strx` a lot ... i wonder if it's worth pre-computing the names once and just use the stored values?
I think it's an extremely cheap computation so probably not worth caching


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139069



More information about the llvm-commits mailing list