[PATCH] D17601: [ELF] - Define special symbols _etext and _edata

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/Writer.cpp:906
@@ -905,1 +905,3 @@
 
+  auto defineWithAlias = [this](StringRef Name, StringRef Alias, Elf_Sym &Sym) {
+    if (Symtab.find(Name))
----------------
Local variable names need to start with a uppercase letter. Maybe just Define should suffice.

================
Comment at: ELF/Writer.cpp:914-921
@@ -906,11 +913,10 @@
+
   // If the "_end" symbol is referenced, it is expected to point to the address
   // right after the data segment. Usually, this symbol points to the end
   // of .bss section or to the end of .data section if .bss section is absent.
   // We don't know the final address of _end yet, so just add a symbol here,
   // and fix ElfSym<ELFT>::End.st_value later.
-  if (Symtab.find("_end"))
-    Symtab.addAbsolute("_end", ElfSym<ELFT>::End);
-
   // Define "end" as an alias to "_end" if it is used but not defined.
   // We don't want to define that unconditionally because we don't want to
   // break programs that uses "end" as a regular symbol.
+  defineWithAlias("_end", "end", ElfSym<ELFT>::End);
----------------
Need to update this comment.

================
Comment at: ELF/Writer.cpp:1357
@@ -1349,2 +1356,3 @@
 
-    if (Sec->getType() != SHT_NOBITS)
+    const bool HasBits = Sec->getType() != SHT_NOBITS;
+    if (HasBits)
----------------
Can you define the variables at once here?

  bool Alloc = Sec->getFlags() & SHF_ALLOC;
  bool NoBits = Sec->getFlags() & SHF_NOBITS;


================
Comment at: ELF/Writer.cpp:1379-1382
@@ -1367,1 +1378,6 @@
+    // _edata points to the end of the last non SHT_NOBITS section.
+    if (Alloc && !(Sec->getFlags() & SHF_WRITE))
+      ElfSym<ELFT>::Etext.st_value = VA;
+    if (Alloc && HasBits)
+      ElfSym<ELFT>::Edata.st_value = VA;
   }
----------------
Both symbols get the same address. Is this correct?

Is there any way to move this piece of code to fixAbsoluteSymbols? I don't want to add more code to this function.


http://reviews.llvm.org/D17601