[PATCH] D83603: [lld-macho] Support __dso_handle for C++

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: lld/MachO/Symbols.h:121
+// The C++ ABI requires dylibs to pass a pointer to __cxa_atexit which does
+// e.g. cleanup of static global variables. The C++ spec says that the pointer
+// can point to any address in one of the dylib's segments, but in practice
----------------
I don't think the "C++ spec" says anything about this. If anything, it would be the Itanium C++ ABI.


================
Comment at: lld/MachO/Symbols.h:139
+
+  static const constexpr StringRef name = "___dso_handle";
+
----------------
compnerd wrote:
> What do you think of making this ever so slightly more expensive?  I think that we should actually make this computed - the name of the symbol is `__dso_handle` which is decorated with the user label prefix of `_` to give you `___dso_handle`.  We have the information about the user label prefixes in LLVM, why not make that explicit here?  I don't think that the cost is really that high.
FWIW, in general I'm not a fan of the idea of computing string constants like this, as it makes it harder to grep the code for strings like `___dso_handle`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83603