[PATCH] D29330: Replace MergeOutputSection with synthetic input section MergeSection.

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 31 14:07:54 PST 2017


ruiu added a comment.

Please update the commit message to describe this patch.



================
Comment at: ELF/InputSection.cpp:127
+    const MergeInputSection<ELFT> *MS = cast<MergeInputSection<ELFT>>(this);
+    return (MS->MergeSec ? MS->MergeSec->OutSecOff : 0 ) + MS->getOffset(Offset);
   }
----------------
s/0 /0/

but probably writing it in three lines would be easier to read.


================
Comment at: ELF/Writer.cpp:153
+static typename ELFT::uint getOutFlags(InputSectionBase<ELFT> *S) {
+  return S->Flags & ~SHF_GROUP & ~SHF_COMPRESSED;
+}
----------------
I think this code has a subtle bug. S->Flags can be uint64_t, but SHF_GROUP and SHF_COMPRESSED are of unsigned int. I think this needs explicit casts.

  return S->Flags & ~(uintX_t)SHF_GROUP & ~(uintX_t)SHF_COMPRESSED;


================
Comment at: ELF/Writer.cpp:157
+// This function scans over V and creates mergeable synthetic sections. It
+// removes MergeInputSections from array and adds new synthetic ones to the end.
+template <class ELFT> static void combineMergableSections() {
----------------
Update this comment -- this function does not add sections to the end.


https://reviews.llvm.org/D29330





More information about the llvm-commits mailing list