[PATCH] D60131: [ELF] Change default output section type to SHT_PROGBITS

[Andrew Ng via Phabricator via llvm-commits]

andrewng marked 2 inline comments as done.
andrewng added a comment.

In D60131#1471202 <https://reviews.llvm.org/D60131#1471202>, @ruiu wrote:

> IIUC, you are fixing an issue that a segment whose size is 0 doesn't have a file offset that is congruent modulo page size. Is this correct?
>
> The file offset for an empty segment is not actually significant; we can just set a dummy value. Have you considered that?


No, the actual issue is that a non-empty PT_LOAD segment that starts with an empty symbol only section does not have a file offset that is congruent modulo the page size. As a result, the output is not a valid ELF.



================
Comment at: test/ELF/linkerscript/orphan-phdrs.s:1
+# XFAIL: *
+
----------------
ruiu wrote:
> We shouldn't keep a test that doesn't run at all. Please just remove.
Just to be clear, you would like me to remove this valid test in this patch and then add it back again in the follow up patch D60273? Or should I submit D60273 first, even though it is the change in this patch that necessitates the change in D60273?


================
Comment at: test/ELF/linkerscript/symbol-only-align.test:2-5
+# RUN: echo '.text; \
+# RUN:        ret;  \
+# RUN:       .data; \
+# RUN:       .quad 0' > %t.s
----------------
ruiu wrote:
> nit: You can just write them in a single line.
I deliberately put them on separate lines for readability. Would you prefer a single line?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60131/new/

https://reviews.llvm.org/D60131