[PATCH] D15191: [ELF] Support PHDRS command

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 18 07:07:57 PST 2016


grimar added inline comments.

================
Comment at: ELF/Writer.cpp:1176
@@ +1175,3 @@
+    layoutCustomPhdrs(VA, FileOff);
+  else {
+    // The first phdr entry is PT_PHDR which describes the program header
----------------
Wouldn`t it be better to write something like:

```
 if (hasCustomPhdrs()) {
    layoutCustomPhdrs(VA, FileOff);
    return;
 }

```
And just copy a piece of left logic from below to layoutCustomPhdrs() or even to separate it to another method ?

Also in currect variant bracket is missed:
```
 if (hasCustomPhdrs()) {
```


================
Comment at: ELF/Writer.cpp:1217
@@ +1216,3 @@
+                                ? (uint32_t)PT_LOAD
+                                : getAmdgpuPhdr(Sec);
+          setPhdr(PH, PTType, Flags, FileOff, VA, 0, Target->getPageSize());
----------------
Change is not relative to this patch (fix of formatting).

================
Comment at: ELF/Writer.cpp:1469
@@ -1301,3 +1468,3 @@
        Config->OutputSections)
-    for (StringRef Name : OutSec.second)
+    for (StringRef Name : OutSec.second.InputSections) {
       InputToOutputSection[Name] = OutSec.first;
----------------
There should be brackets I think:

```
for (const std::pair<StringRef, OutputSectionDescription> &OutSec :
       Config->OutputSections) {
...
```


http://reviews.llvm.org/D15191





More information about the llvm-commits mailing list