[PATCH] D18499: [ELF] - Implemented prototype of location counter support.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/LinkerScript.cpp:35-37
@@ +34,5 @@
+
+  // Evaluates the expression given by list of tokens. Fills the Out
+  // with value of expression if succeded. Returns false on error.
+  bool evaluate(std::vector<StringRef> &Tokens, uint64_t *Out) {
+    std::vector<StringRef> RPN = buildRPN(Tokens);
----------------
If there's an error, report that using error() and just return 0.

================
Comment at: ELF/LinkerScript.cpp:85-110
@@ +84,28 @@
+protected:
+  // Builds Reverse Polish Notation using
+  // shunting-yard algorithm by Edsger Dijkstra.
+  // https://en.wikipedia.org/wiki/Shunting-yard_algorithm
+  std::vector<StringRef> buildRPN(std::vector<StringRef> &Tokens) {
+    std::vector<StringRef> OutString;
+    std::list<StringRef> Stack;
+
+    for (StringRef Tok : Tokens) {
+      // If the token is a number, then add it to the output queue.
+      if (!isOperator(Tok)) {
+        OutString.push_back(Tok);
+        continue;
+      }
+
+      while (!Stack.empty()) {
+        StringRef St = Stack.back();
+        OutString.push_back(St);
+        Stack.pop_back();
+      }
+      Stack.push_back(Tok);
+    }
+
+    // Add the remaining stack content to output string.
+    OutString.insert(OutString.end(), Stack.rbegin(), Stack.rend());
+    return OutString;
+  }
+
----------------
Since we don't have to handle operator precedence with '+', please remove this from this patch.

================
Comment at: ELF/Writer.cpp:1271-1273
@@ -1268,3 +1270,5 @@
     if (Flags != NewFlags) {
-      Load = AddHdr(PT_LOAD, NewFlags);
-      Flags = NewFlags;
+      if (Script->Exist) {
+        Load->H.p_flags |= NewFlags;
+      } else {
+        Load = AddHdr(PT_LOAD, NewFlags);
----------------
I don't understand this code. Why did you modify Load->H.p_flags?

================
Comment at: ELF/Writer.cpp:1335-1337
@@ -1328,5 +1334,5 @@
 
 // Visits all headers in PhdrTable and assigns the adresses to
 // the output sections. Also creates common and special headers.
 template <class ELFT> void Writer<ELFT>::assignAddresses() {
   Out<ELFT>::ElfHeader->setSize(sizeof(Elf_Ehdr));
----------------
Please do not add more code this function. Instead, I think you can split it for linker script and non-linker script, because if a linker script is present, all addresses are assigned according to the linker script.


http://reviews.llvm.org/D18499