[PATCH] D39489: [ELF] - Linkerscript: fix issue with multiple output sections definitions.

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

If I understand correctly is that there is no restriction on OutputSection name so something like:

  bar : { *.bar.1 }
  bar : { *.bar.2 }

Would be legal, but not very sensible and a user could just change the name of the second bar. Whereas the case you describe:

  bar : ONLY_IF_RO { *(no_such_section) }
  bar : ONLY_IF_RW { *(foo_aw) }
  sym1 = SIZEOF(bar); 
  sym2 = ADDR(bar);

Is useful as it permits a program to get the [base, limit) of an OutputSection that can contain either RO or RW? This would make the added complexity of the change worthwhile?

If I've understood their purpose correctly; I'm not too sure that the forward declarations are a good idea. Recent ld manuals prohibit forward references for many of the builtin functions.



================
Comment at: ELF/ScriptParser.cpp:974
     StringRef Name = readParenLiteral();
-    OutputSection *Sec = Script->getOrCreateOutputSection(Name);
+    Script->declareOutputSection(Name);
     return [=]() -> ExprValue {
----------------
If I understand correctly this might create a forward reference to some OutputSection Name that we've not seen yet? I don't think that this is allowed in this case: https://sourceware.org/binutils/docs/ld/Builtin-Functions.html
> Return the address (VMA) of the named section. Your script must previously have defined the location of that section. 



================
Comment at: ELF/ScriptParser.cpp:999
     StringRef Name = readParenLiteral();
-    OutputSection *Cmd = Script->getOrCreateOutputSection(Name);
+    Script->declareOutputSection(Name);
     return [=] {
----------------
If this is a forward reference then the manual says we should give an error message:
> If the section has not been allocated when this is evaluated, the linker will report an error.


================
Comment at: ELF/ScriptParser.cpp:1050
     StringRef Name = readParenLiteral();
-    OutputSection *Cmd = Script->getOrCreateOutputSection(Name);
+    Script->declareOutputSection(Name);
     return [=] {
----------------
The docs don't say anything about forward references for this particular builtin. I think that there is a possibility in creating a cycle in address allocation though with something like . = LOADADDR(forward)



================
Comment at: ELF/ScriptParser.cpp:1073
     StringRef Name = readParenLiteral();
-    OutputSection *Cmd = Script->getOrCreateOutputSection(Name);
+    Script->declareOutputSection(Name);
     // Linker script does not create an output section if its content is empty.
----------------
Another case where we are supposed to give an error if there is a forward reference: 
> If the section has not been allocated when this is evaluated, the linker will report an error.


https://reviews.llvm.org/D39489