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

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/OutputSections.cpp:764
@@ +763,3 @@
+                              SHF_ALLOC) {
+  // It`s a 4 bytes of header + pointer to the contents of the .eh_frame section
+  // + the number of FDE pointers in the table.
----------------
` -> '

================
Comment at: ELF/OutputSections.cpp:777
@@ +776,3 @@
+EhFrameHdr<ELFT>::getFdePc(uintX_t EhVA, const FdeData &F) {
+
+  const endianness E = ELFT::TargetEndianness;
----------------
Remove this empty line.

================
Comment at: ELF/OutputSections.cpp:844
@@ +843,3 @@
+template <class ELFT>
+void EhFrameHdr<ELFT>::addFde(uint8_t Enc, size_t Off, uint8_t *PCRel) {
+  if (Live && (Enc & 0xF0) == dwarf::DW_EH_PE_datarel) {
----------------
So, if Sec is nonnull but this->Sec can be null,

 - If this->Sec == nullptr, (this->Sec != Sec) can never be true (because Sec is not null), no warning is displayed.
 - If this->Sec != nullptr, (this->Sec != Sec) is true only when somebody tries to assign another one.

But this is a minor point. Leave it as is if you think yours more readable.

================
Comment at: ELF/OutputSections.cpp:1111-1113
@@ +1110,5 @@
+      if (!readByte(D, &FdeEnc))
+        return false;
+      break;
+    case 'P': {
+      uint8_t Enc;
----------------
I'm pretty sure that that is better if this code is just for checking consistency. We generally don't want to read any data unless it is absolutely necessary to link stuff. It is compiler's responsibility to feed consistent file to the linker, so the linker doesn't have to verify all bits. Doing less is important for performance.


http://reviews.llvm.org/D15712