[PATCH] [lld][PECOFF] Read relocation entries.

[Rui Ueyama]

  Sean,

  Thank you for reviewing!


================
Comment at: lib/ReaderWriter/PECOFF/WriterPECOFF.cpp:274-275
@@ +273,4 @@
+    llvm::object::coff_section header;
+    std::memset(header.Name, 0, sizeof(header.Name));
+    std::strncpy(header.Name, sectionName.data(), sizeof(header.Name));
+    header.VirtualSize = 0;
----------------
Sean Silva wrote:
> Try to avoid using unsafe C string manipulation like strncpy. If you really need to use strncpy, please add assertions that ensure the bounds are correct. For example something like `assert(sizeof(header.Name) < sectionName.size())`. Also, is the case where strncpy does not add the trailing NUL acceptable in this situation? Is sectionName NUL terminated?
It's strnpy not strcpy so it will never overrun the buffer. This cannot indeed handle section name longer than 8 byte. If the name is longer than 8 byte, it should be emitted to string table, and a reference to the string table should be emitted to the name field. That's not implemented yet, as we don't have string table yet. Section names are usually equal or shorter than 8 bytes on Windows so we can do that later.

================
Comment at: lib/ReaderWriter/PECOFF/ReaderCOFF.cpp:433
@@ +432,3 @@
+
+  /// Atomize defined symbols.
+  error_code AtomizeDefinedSymbols(SectionToSymbolsT &definedSymbols,
----------------
Sean Silva wrote:
> This comment doesn't help. Just delete it (or improve it).
Removed.

================
Comment at: lib/ReaderWriter/PECOFF/ReaderCOFF.cpp:448
@@ +447,3 @@
+
+      for (COFFDefinedAtom* atom : atoms) {
+        if (!atom->name().empty())
----------------
Sean Silva wrote:
> `*` on the right. I recommend using clang-format.
Applied clang-format.


http://llvm-reviews.chandlerc.com/D976