[PATCH] D21995: [ELF] Implement minimal PHDRS parser and section-to-segment assignment

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/LinkerScript.cpp:276
@@ +275,3 @@
+  int RelroNum = -1;
+  Phdr *Load = nullptr;
+  uintX_t Flags = PF_R;
----------------
I think we do not use int.
Use what you need instead: int32_t I guess.

================
Comment at: ELF/LinkerScript.cpp:307
@@ +306,3 @@
+    case PT_GNU_EH_FRAME:
+      if (!Out<ELFT>::EhFrame->empty() && Out<ELFT>::EhFrameHdr) {
+        ItAdded->H.p_flags = toPhdrFlags(Out<ELFT>::EhFrameHdr->getFlags());
----------------
```
if (Out<ELFT>::EhFrame->empty() || !Out<ELFT>::EhFrameHdr)
  continue;
```

================
Comment at: ELF/LinkerScript.h:18
@@ -17,2 +17,3 @@
 #include "llvm/Support/MemoryBuffer.h"
+#include "Writer.h"
 
----------------
We usually put lld headers before others ones. Move it before #include "lld/Core/LLVM.h" please.

================
Comment at: ELF/Writer.h:29
@@ +28,3 @@
+// Each contains type, access flags and range of output sections that will be
+// placed in it.
+template<class ELFT>
----------------
I think this comment can be 2 lines (did not check though).

================
Comment at: test/ELF/linkerscript-phdrs.s:3
@@ +2,3 @@
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: echo "PHDRS {all PT_LOAD FILEHDR PHDRS;} SECTIONS {. = 0x10000200; .text : {*(.text.*)} :all .foo : {*(.foo.*)} :all .data : {*(.data.*)} :all}" > %t.script
+
----------------
Please format to have max 80 symbols per line (you can refer to other testcases we have for reference)

================
Comment at: test/ELF/linkerscript-phdrs.s:7
@@ +6,3 @@
+# RUN: llvm-readobj -program-headers %t1 | FileCheck %s
+# CHECK: ProgramHeaders [
+# CHECK-NEXT:  ProgramHeader {
----------------
please add few spaces to format accordinly.

================
Comment at: test/ELF/linkerscript-phdrs.s:16
@@ +15,3 @@
+# CHECK-NEXT:    Flags [ (0x7)
+# CHECK-NEXT:      PF_R (0x4)
+# CHECK-NEXT:      PF_W (0x2)
----------------
Usually when we do not need - we dont leave hex flags values check.
I mean we you want write:

```
# CHECK-NEXT:    Flags [
# CHECK-NEXT:      PF_R
# CHECK-NEXT:      PF_W
# CHECK-NEXT:      PF_X
# CHECK-NEXT:    ]
```

================
Comment at: test/ELF/linkerscript-phdrs.s:21
@@ +20,3 @@
+
+
+.global _start
----------------
Remove excessive empty line.


Repository:
  rL LLVM

http://reviews.llvm.org/D21995