[PATCH] D30419: [ELF] - Define __bss_start symbol.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: test/ELF/bss-start.s:16
+
+.global __bss_start
+.text
----------------
ruiu wrote:
> Do you need this .global?
I believe yes. We do not want to add symbol when it is not used, right ?
Declaring it as global adds it as undefined to symbol table and
addOptionalSynthetic() then able to create DefinedSynthetic:

```
addOptionalSynthetic(StringRef Name, OutputSection *Sec,
                     typename ELFT::uint Val, uint8_t StOther = STV_HIDDEN) {
  if (SymbolBody *S = Symtab<ELFT>::X->find(Name))
    if (!S->isInCurrentDSO())
      return cast<DefinedSynthetic>(
          Symtab<ELFT>::X->addSynthetic(Name, Sec, Val, StOther)->body());
return nullptr;
```

Otherwize there will be no symbol in output.


================
Comment at: test/ELF/bss-start.s:20-22
+
+ movl $1, a
+ movl $1, b
----------------
ruiu wrote:
> You can remove this without changing the meaning of this test, no?
First test intention is not only to check that .bss.rel.ro can be used for _bss_start,
but also to show that _bss_start points to the first .bss section if there are multiple in output.
Lines were intentional.


https://reviews.llvm.org/D30419