[PATCH] D33856: [ELF] Define __dso_handle symbol if needed

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

Overall looks fine. I think this is the right approach to define __dso_handle, as it is very easy to do, compared to other solutions.



================
Comment at: ELF/Writer.cpp:913
 
+  // __dso_handle symbol is mandated by C++ ABI with a value which is an
+  // address in one of the object's segments, and as such this symbol has to be
----------------
I'd describe a bit more about the __dso_handle symbol so that the code is more self-contained for the first-time reader of the source code. Specifically, I'd like to mention

 - __dso_handle is passed to __cxa_finalize as a marker to identify each DSO
 - the address of the symbol doesn't matter as long as they are different in different DSOs, so we chose the start address of the DSO

In particular, "... with a value which is an address in one of the object's segment" sounds too formal specification-ish. I'd make it more concrete.



================
Comment at: ELF/Writer.cpp:916
+  // included statically and cannot be a part of a shared library.
+  addOptionalRegular<ELFT>("__dso_handle", Out::ElfHeader, 0, STV_HIDDEN);
+
----------------
Don't you want to do this only when `Config->Shared`?


Repository:
  rL LLVM

https://reviews.llvm.org/D33856