[LLVMdev] Win32 COFF Support

Eli Friedman eli.friedman at gmail.com
Thu May 20 00:28:50 PDT 2010


On Wed, May 19, 2010 at 10:31 PM, Nathan Jeffords
<blunted2night at gmail.com> wrote:
> Hi guys,
> I have attached my patch to support generating win32 COFF object files. I
> would have posted earlier, but my system drive crashed and I had to rebuild
> my system; Luckily, my source code was on a secondary drive. I think this
> would be a good beginning for ongoing support of the COFF object file format
> and was hoping for some feedback as to whether it was commit worthy or what
> was needed to be done to get it their.

The line for commit-worthy is a round of reviews and some tests
showing that basic functionality works.

Minor comments:
1. Is it possible to allocate extra memory with coff::symbol instead
of using std::string and std::vector as class members? The extra
allocations can add up.

2. For the following bit:
+    reinterpret_cast <uint32_t &> (Section->Symbol->Aux [0]) =
Section->Header.SizeOfRawData;
+    reinterpret_cast <uint16_t &> (Section->Symbol->Aux [4]) =
Section->Header.NumberOfRelocations;
+    reinterpret_cast <uint16_t &> (Section->Symbol->Aux [6]) =
Section->Header.PointerToLineNumbers;

This isn't endian-safe; I'd suggest expanding it to 8 assignments.
Same issues with the write_le functions.  We need endian safety not
only so that cross-compiling, for example, from PPC Linux to MingW
works, but also so that tests work cross-platform.

3. For the following bit:
+    typedef std::list <symbol*>  symbols;
+    typedef std::list <section*> sections;

Avoid std::list; see http://llvm.org//docs/ProgrammersManual.html#dss_list .

4. For the following bit:
+    typedef StringMap <coff::symbol *>  name_symbol_map;
+    typedef StringMap <coff::section *> name_section_map;

Unused typedefs?

5. For the following bit (and a few others in the same function):
+    for (i = Sections.begin(), j = Asm.begin();
+         (i != Sections.end()) && (j != Asm.end());
+         i++, j++) {

http://llvm.org//docs/CodingStandards.html#ll_end

6.
+namespace llvm { MCObjectWriter * createWinCOFFObjectWriter
(raw_ostream & OS); }

If you need a function to be visible across files, use a header;
shortcuts like this make the code more difficult to read.

-Eli



More information about the llvm-dev mailing list