[PATCH] D15712: [ELF] - implemented --eh-frame-hdr command line option.

[Rui Ueyama via llvm-commits]

ruiu accepted this revision.
ruiu added a comment.
This revision is now accepted and ready to land.

LGTM with a few nits.


================
Comment at: ELF/OutputSections.cpp:814-815
@@ +813,4 @@
+
+  // InitialPC -> Offset in .eh_frame,
+  // sorted by InitialPC.
+  std::map<uintX_t, size_t> PcToOffset;
----------------
Write this in one line.

================
Comment at: ELF/OutputSections.cpp:822
@@ +821,3 @@
+    // The first four bytes are an offset to the initial PC value for the FDE.
+    uintX_t InitialPcOff = I.first - VA;
+    write32<E>(Buf, InitialPcOff);
----------------
It is odd that we are using uintX_t here but we write it as a 32-bit integer. Shouldn't we just use uint32_t here?

================
Comment at: ELF/OutputSections.cpp:825
@@ +824,3 @@
+    // The last four bytes are an offset to the FDE data itself.
+    uintX_t FdeDataOff = EhVA + I.second - VA;
+    write32<E>(Buf + 4, FdeDataOff);
----------------
Ditto

================
Comment at: ELF/OutputSections.cpp:1051
@@ +1050,3 @@
+  typedef typename ELFFile<ELFT>::uintX_t uintX_t;
+  unsigned format = Enc & 0x0f;
+  switch (format) {
----------------
format -> Format

================
Comment at: ELF/OutputSections.cpp:1099
@@ +1098,3 @@
+    readByte(D);
+  else if (Version == 3)
+    skipLeb128(D);
----------------
  else if (Version == 3) -> else

because we know that it's always 3.

================
Comment at: ELF/OutputSections.h:438
@@ -435,1 +437,3 @@
 
+// --eh-frame-hdr option tells linker to construct a header for all the
+// .eh_frame sections. This header is placed to a section named .eh_frame_hdr
----------------
Please move this above the definition of `align`.


http://reviews.llvm.org/D15712