[PATCH] D19190: [ELF] - Implemented PROVIDE linker script command.

[George Rimar via llvm-commits]

grimar marked 2 inline comments as done.

================
Comment at: ELF/LinkerScript.cpp:175
@@ -174,3 +174,3 @@
 static OutputSectionBase<ELFT> *
-findSection(std::vector<OutputSectionBase<ELFT> *> &V, StringRef Name) {
+findSection(ArrayRef<OutputSectionBase<ELFT> *> V, StringRef Name) {
   for (OutputSectionBase<ELFT> *Sec : V)
----------------
rafael wrote:
> Using ArrayRef is a nice independent cleanup. Please commit it first.
Done. r266978

================
Comment at: ELF/LinkerScript.cpp:243
@@ +242,3 @@
+template <class ELFT>
+void LinkerScript::createSymbols(SymbolTable<ELFT> &Symtab,
+                                 ArrayRef<OutputSectionBase<ELFT> *> Sections) {
----------------
rafael wrote:
> This only handles symbols at the start and end of output sections, right?
> 
> I guess that is a reasonable restriction for now. We can add support afterwards for recording an offset.
> 
Yes. And probably we will not need adding anything else here. I never saw in linkerscripts places where
PROVIDE inside output section description was attached to something different than end/start.

================
Comment at: ELF/LinkerScript.cpp:252-253
@@ +251,4 @@
+  // begin and end of the section.
+  StringRef SecName;
+  uintX_t Val;
+  for (SectionsCommand &Cmd : Commands) {
----------------
ruiu wrote:
> Move this inside the for loop to make their scopes as narrow as possible.
I can't do that.
SecName is used to keep what section we currently processing (addSynthetic requires section as argument).
Val value will change and is different for start of the section and end. And since ProvideKind can be 
somewhere between/after ContentKind command, we should have that value already prepared, as
ProvideKind itself does not let know - is it start or end section symbol.


================
Comment at: ELF/LinkerScript.h:48-51
@@ +47,6 @@
+// * SectionKind is a description of output section, like ".data :..."
+// * ContentKind is a content of output section, it helps to distinguish PROVIDE
+//   command before and after section content, first one should have address
+//   equal to address of output section, second points to location after that.
+// * ProvideKind is PROVIDE command.
+enum SectionsCommandKind { ExprKind, SectionKind, ContentKind, ProvideKind };
----------------
ruiu wrote:
> ContentKind is very confusing and seems unnecessary. Why don't you process ProvideKind elements in assignAddresses? You should be able to define symbols using the current VA in the function.
In initial version of this patch, addAbsolute was used, now addSynthetic is. There is no need to save list of symbols and update values in assignAddresses anymore, as they are always attached to begin/end of output section. That is current restriction of the patch discussed above in comments.

Also:
1) Imagine sample script and code:
```
.global _start
_start:
  call used1;
```

```
SECTIONS
{
  .text :
    {
      PROVIDE (used1 = .);
      *(.text)
    }
}
```
If I not define symbols in createSymbols(), I`ll get undefined symbol error before reaching assignAddresses().

2) Imagine another script:

```
echo SECTIONS
{
  .text : 
  { 
    PROVIDE (beginText = .);
    *(.text)
    PROVIDE (endText = .);
    }
}
```

Commands list currently would be:

```
SectionKind
  ProvideKind 
    ContentKind
  ProvideKind
```
So all ProvideKinds that are before ContentKind are attached to the begin of section, others - to the end. ContentKind is used to actually move VA.

Without ContentKind it would be:
```
SectionKind
  ProvideKind 
  ProvideKind
```
How it is possible to know where is start/end provides then ? And when to move VA forward ?


http://reviews.llvm.org/D19190