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

[George Rimar via llvm-commits]

grimar added inline comments.

================
Comment at: ELF/OutputSections.cpp:767
@@ -764,1 +766,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 and also to
----------------
ruiu wrote:
> Add a space before "--".
Done, comment was moved to class EhFrameHdr definition.

================
Comment at: ELF/OutputSections.cpp:780
@@ +779,3 @@
+                              llvm::ELF::SHF_ALLOC) {
+  // This is initial size without counting FDE`s entries descriptors size.
+  // Its a 4 bytes of header + pointer to the contents of the .eh_frame section
----------------
ruiu wrote:
> ` -> '
Removed that line.

================
Comment at: ELF/OutputSections.cpp:851
@@ +850,3 @@
+void EhFrameHdr<ELFT>::assignEhFrame(EHOutputSection<ELFT> *Sec) {
+  if (this->Sec && this->Sec != Sec) {
+    warning("multiple .eh_frame sections not supported for .eh_frame_hdr");
----------------
ruiu wrote:
> Isn't parameter Sec be non-null? If it is, you can just do
> 
>   if (this->Sec != Sec)
Point to do "if (this->Sec)" check here is to avoid warning and disabling of header for first assignEhFrame call. I only want to warn if one ehframe is assigned already and somebody tries to assign another one.

================
Comment at: ELF/OutputSections.cpp:1122-1124
@@ +1121,5 @@
+    case 'R':
+      if (!readByte(D, &FdeEnc))
+        return false;
+      break;
+    case 'P': {
----------------
ruiu wrote:
> We can exit from this function as soon as we read data, no?
> 
>   return readByte(D, &Cie.FdeEncoding));
I am not sure it is better. At first look it`s like a little optimization of parsing augmentation string but in fact it can hide a potential error when CIE information is corrupted. Early exit can hide warning about that if 'R' is not the latest in a sequence.

================
Comment at: ELF/OutputSections.cpp:1136
@@ +1135,3 @@
+      D = D.slice(EncSize);
+      break;
+    }
----------------
ruiu wrote:
> Ditto
Hmm, I dont think its "Ditto", the propose of the function is to read 1 byte that represents the pointer encoding for the address pointers used in the FDE, which is read in 'R', so we need to read and skip everything we met at least until we read 'R'.


http://reviews.llvm.org/D15712