[PATCH] D22625: [ELF] Support PROVIDE and PROVIDE_HIDDEN within SECTIONS {} block

[Eugene Leviant via llvm-commits]

evgeny777 added inline comments.

================
Comment at: ELF/LinkerScript.cpp:459
@@ +458,3 @@
+        else if (Cmd->Provision != SymbolProvision::Create)
+          Cmd->Provision = SymbolProvision::Ignore;
+      }
----------------
ruiu wrote:
> evgeny777 wrote:
> > ruiu wrote:
> > > This doesn't look beautiful. Isn't there any better way?
> > I need some flag, so that I don't override value of provided symbol later in assignAddresses, in case it is already defined somewhere. I'll think how to rewrite it, and open to suggestions.
> How about adding a SymbolBody* to SymbolAssignment class and set it to the return value of addAbsolute?
The problem is that current version overrides symbol value in case it is already defined (addAbsolute is not called) and is not assigned inside PROVIDE() or PROVIDE_HIDDEN(). So in case you specify "end = ." in script, the "end" symbol will be overridden, no matter has it been defined or not.

This was done in one of my previous patches in order to be able to override reserved symbols (end, etext, edata) in linker script, because currently they are always created in addReservedSymbols(). May be this should be fixed first, I don't know.

================
Comment at: ELF/LinkerScript.cpp:799-807
@@ -778,5 +798,11 @@
 std::vector<StringRef> ScriptParser::readSectionsCommandExpr() {
+  int Braces = 0;
   std::vector<StringRef> Expr;
   while (!Error) {
-    StringRef Tok = next();
+    StringRef Tok = peek();
+    Braces += (Tok == "(");
+    Braces -= (Tok == ")");
+    if (Braces < 0)
+      break;
+    next();
     if (Tok == ";")
----------------
grimar wrote:
> evgeny777 wrote:
> > ruiu wrote:
> > > Is this change related to the feature you are adding in this patch?
> > Yes. As I said in summary, this is intended to early terminate the expression reader in case we see extra ')' bracket. Otherwise parser will eat tokens till it finds semicolon
> This should be fixed in separate patch.
May be, but actually this parser improvement is only needed for PROVIDE(). I don't mind making separate review, but not sure if it's really needed. Rui, what's your opinion?


Repository:
  rL LLVM

https://reviews.llvm.org/D22625