Hi guys, While trying to get dwarf debugging information to work with Win32 COFF targets, I came across a couple of issues with the current implementation of WinCOFFObjectWriter. Emitting empty section causes debug information to invalid, as the presence of certain debug section implies available information, and emission of labels as symbols confused gdb about the structure of the program. The attached patch allows the WinCOFFObjectWriter to drop empty sections, and label symbols. It converts relocations targeted at symbols into relocations relative to the containing section. I have not run the tests in the '/test/MC/COFF', but have run through the unit-test framework for my compiler and generated some text executables with clang and everything looks to be in order. It may change the output of the '/test/MC/COFF' if the string references are emitted as temporaries. -Nathan -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100926/5fee6e36/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: wincoff.patch Type: application/octet-stream Size: 16085 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100926/5fee6e36/attachment.obj>
On Sun, Sep 26, 2010 at 5:27 PM, Nathan Jeffords <blunted2night at gmail.com> wrote:> Hi guys, > While trying to get dwarf debugging information to work with Win32 COFF > targets, I came across a couple of issues with the current implementation > of WinCOFFObjectWriter. Emitting empty section causes debug information to > invalid, as the presence of certain debug section implies available > information, and emission of labels as symbols confused gdb about the > structure of the program. > The attached patch allows the WinCOFFObjectWriter to drop empty sections, > and label symbols. It converts relocations targeted at symbols into > relocations relative to the containing section. > I have not run the tests in the '/test/MC/COFF', but have run through the > unit-test framework for my compiler and generated some text executables with > clang and everything looks to be in order. It may change the output of > the '/test/MC/COFF' if the string references are emitted as temporaries. > -NathanThe patch looks good, but please follow the LLVM coding conventions. Also, clang emits warnings on all of the signed-unsigned comparisons, and the order of initialization for COFFSymbol::MCData. The test fails, but that's only because it is looking for binary equivalence, just llc the test and run coff-dump to re update it. I ran the test-suite with clang -integrated-as with and without your patch and I got identical results. So fix the coding conventions, warnings, and update the tests and it's good. I don't mind doing this if you would prefer. - Michael Spencer
On Sun, Sep 26, 2010 at 3:28 PM, Michael Spencer <bigcheesegs at gmail.com>wrote:> On Sun, Sep 26, 2010 at 5:27 PM, Nathan Jeffords > <blunted2night at gmail.com> wrote: > > Hi guys, > > While trying to get dwarf debugging information to work with Win32 COFF > > targets, I came across a couple of issues with the current implementation > > of WinCOFFObjectWriter. Emitting empty section causes debug information > to > > invalid, as the presence of certain debug section implies available > > information, and emission of labels as symbols confused gdb about the > > structure of the program. > > The attached patch allows the WinCOFFObjectWriter to drop empty sections, > > and label symbols. It converts relocations targeted at symbols into > > relocations relative to the containing section. > > I have not run the tests in the '/test/MC/COFF', but have run through the > > unit-test framework for my compiler and generated some text executables > with > > clang and everything looks to be in order. It may change the output of > > the '/test/MC/COFF' if the string references are emitted as temporaries. > > -Nathan > > The patch looks good, but please follow the LLVM coding conventions. > Also, clang emits warnings on all of the signed-unsigned comparisons, > and the order of initialization for COFFSymbol::MCData. > > The test fails, but that's only because it is looking for binary > equivalence, just llc the test and run coff-dump to re update it. I > ran the test-suite with clang -integrated-as with and without your > patch and I got identical results. > > So fix the coding conventions, warnings, and update the tests and it's > good. I don't mind doing this if you would prefer. > > - Michael Spencer >If you don't mind then please make the appropriate changes. Thanks. -Nathan -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100926/7d6aff67/attachment.html>
On Sun, Sep 26, 2010 at 5:27 PM, Nathan Jeffords <blunted2night at gmail.com> wrote:> Hi guys, > While trying to get dwarf debugging information to work with Win32 COFF > targets, I came across a couple of issues with the current implementation > of WinCOFFObjectWriter. Emitting empty section causes debug information to > invalid, as the presence of certain debug section implies available > information, and emission of labels as symbols confused gdb about the > structure of the program. > The attached patch allows the WinCOFFObjectWriter to drop empty sections, > and label symbols. It converts relocations targeted at symbols into > relocations relative to the containing section. > I have not run the tests in the '/test/MC/COFF', but have run through the > unit-test framework for my compiler and generated some text executables with > clang and everything looks to be in order. It may change the output of > the '/test/MC/COFF' if the string references are emitted as temporaries. > -Nathanhttp://llvm.org/bugs/show_bug.cgi?id=8248 It seems that there actually is a bug. I thought it was due to something else, but reverting this patch fixes it. Why/when exactly does gdb expect IMAGE_SYM_CLASS_LABEL? - Michael Spencer
On Tue, Sep 28, 2010 at 7:39 PM, Michael Spencer <bigcheesegs at gmail.com> wrote:> On Sun, Sep 26, 2010 at 5:27 PM, Nathan Jeffords > <blunted2night at gmail.com> wrote: >> Hi guys, >> While trying to get dwarf debugging information to work with Win32 COFF >> targets, I came across a couple of issues with the current implementation >> of WinCOFFObjectWriter. Emitting empty section causes debug information to >> invalid, as the presence of certain debug section implies available >> information, and emission of labels as symbols confused gdb about the >> structure of the program. >> The attached patch allows the WinCOFFObjectWriter to drop empty sections, >> and label symbols. It converts relocations targeted at symbols into >> relocations relative to the containing section. >> I have not run the tests in the '/test/MC/COFF', but have run through the >> unit-test framework for my compiler and generated some text executables with >> clang and everything looks to be in order. It may change the output of >> the '/test/MC/COFF' if the string references are emitted as temporaries. >> -Nathan > > http://llvm.org/bugs/show_bug.cgi?id=8248 > > It seems that there actually is a bug. I thought it was due to > something else, but reverting this patch fixes it. Why/when exactly > does gdb expect IMAGE_SYM_CLASS_LABEL? > > - Michael Spencer >I've attached a patch that fixes the above problem while keeping the LABEL storage class on labels. Can you verify that it keeps the behavior you wanted? - Michael Spencer -------------- next part -------------- A non-text attachment was scrubbed... Name: coff-fix-symbol-storage-class.patch Type: application/octet-stream Size: 1524 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100928/2fdcb868/attachment.obj>