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 6:47 PM, Nathan Jeffords <blunted2night at gmail.com> wrote:> 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. > -NathanCommitted in r114823. - Michael Spencer
thanks On Mon, Sep 27, 2010 at 2:00 AM, Michael Spencer <bigcheesegs at gmail.com>wrote:> On Sun, Sep 26, 2010 at 6:47 PM, Nathan Jeffords > <blunted2night at gmail.com> wrote: > > 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 > > Committed in r114823. > > - Michael Spencer >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20100927/e2b6b669/attachment.html>