[PATCH] D38239: [ELF] - Define linkerscript symbols early.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/LinkerScript.cpp:128-131
+  // If we reach here and symbol uses PROVIDE() that means symbol satisfies
+  // providing conditions. Since we are calling addSymbol() multiple times and
+  // will define symbol below, we want to disable futher checks.
+  Cmd->Provide = false;
----------------
ruiu wrote:
> This seems like an optimization that you can remove.
I think I can't remove it. It is used for following case. If we have script like:
"PROVIDE_HIDDEN(newsym = __ehdr_start + 5);" (linkerscript/symbol-reserved.s)

When `addSymbol` is called first time to define `newsym` early, `newsym` is undefined, so
`if (Cmd->Provide && (!B || B->isDefined()))` check passes through and we define
dummy `newsym`.

Next time when `addSymbol` called to set proper symbol value, `if` condition will not pass
because symbol is defined. To let it pass I had to drop `Provide` to disable check.


================
Comment at: ELF/Writer.cpp:786-790
+    SymbolBody *Gp = Symtab->find("_gp");
+    if (!Gp || !isa<DefinedRegular>(Gp))
+      ElfSym::MipsGp = Symtab->addAbsolute<ELFT>("_gp", STV_HIDDEN, STB_LOCAL);
+    else
+      ElfSym::MipsGp = dyn_cast<DefinedRegular>(Gp);
----------------
ruiu wrote:
> It is not clear to me why you need to change this.
Because otherwise I would have "duplicate symbol: _gp" error with script like
"SECTIONS { .text : { *(.text) }   _gp = ABSOLUTE(.) + 0x100;   .got  : { *(.got) } }"
I add `_gp` early, so `addAbsolute` would trigger the error without this check.


https://reviews.llvm.org/D38239