[PATCH] D14833: [ELF] Define symbols "_end" and "end" is referenced.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/Writer.cpp:629-630
@@ +628,4 @@
+  // of .bss section or to the end of .data section if .bss section is absent.
+  // Unfortunately, the order of the sections can be affected by linker script,
+  // so it is hard to predict which section will be last.
+  // So, if this symbol is referenced, we just add the placeholder here
----------------
I don't think it is unfortunate but naturally needed. Any logic to bin sections into segments affects the end position of .bss segment. It cannot be predicted here. (Otherwise we can construct segments right here instead of later.) I'd remove "Unfortunately."

================
Comment at: ELF/Writer.cpp:633
@@ +632,3 @@
+  // and update its value later.
+  DefinedAbsolute<ELFT>::End.setBinding(STB_GLOBAL);
+  if (Symtab.find("_end"))
----------------
Move this to initSymbols in Symbols.cpp.

================
Comment at: ELF/Writer.cpp:640
@@ +639,3 @@
+  // because it is an allowable name for a user symbol.
+  if (dyn_cast_or_null<Undefined<ELFT>>(Symtab.find("end")))
+    Symtab.addAbsoluteSym("end", DefinedAbsolute<ELFT>::End);
----------------
It is a little bit alarming to use dyn_cast_or_null without using its return value, but I don't think we have isa_or_null. I'd probably write this way.

  if (SymbolBody *B = Symtab.find("end"))
    if (isa<Undefined<ELFT>>(B))
      Symtab.addAbsoluteSym...

================
Comment at: ELF/Writer.cpp:641
@@ +640,3 @@
+  if (dyn_cast_or_null<Undefined<ELFT>>(Symtab.find("end")))
+    Symtab.addAbsoluteSym("end", DefinedAbsolute<ELFT>::End);
+
----------------
So we define two symbols, end and _end, but we use only one SymbolBody. Cool.


http://reviews.llvm.org/D14833