RE: [PATCH] Fix PR gdb/19208 - SIGSEV while opening Fortran program compiled with ifort

["Hahnfeld, Jonas" <Hahnfeld at itc dot rwth-aachen dot de>]

Ping...
Will this also be included in 7.10.2 (if planned?)

> -----Original Message-----
> From: Hahnfeld, Jonas
> Sent: Wednesday, January 06, 2016 8:43 AM
> To: 'Joel Brobecker'
> Cc: gdb-patches@sourceware.org; qiyao@gcc.gnu.org
> Subject: RE: [PATCH] Fix PR gdb/19208 - SIGSEV while opening Fortran
> program compiled with ifort
> 
> Hi,
> 
> first off: Your attached patch works for - sorry that Outlook killed my
> diff...
> 
> > -----Original Message-----
> > From: Joel Brobecker [mailto:brobecker@adacore.com]
> > Sent: Wednesday, January 06, 2016 7:20 AM
> > To: Hahnfeld, Jonas
> > Cc: gdb-patches@sourceware.org; qiyao@gcc.gnu.org
> > Subject: Re: [PATCH] Fix PR gdb/19208 - SIGSEV while opening Fortran
> > program compiled with ifort
> >
> > Hello,
> >
> > A lot of text below, but the summary is the following:
> >
> >   . The patch looks good but I couldn't apply it; I made the change
> >     by hand, but can you confirm that it is correct by testing it
> >     for me with ifort? If confirmed, I will push the change.
> >
> >   . Lots of advice (hence the amount of text). You might want
> >     to take a look at our contribution checklist:
> >     https://sourceware.org/gdb/wiki/ContributionChecklist
> >     It'll help us during patch review, which in turn will help you
> >     get your patches in faster :-)
> 
> Thanks for the pointer, I think this is neither linked at
> https://www.gnu.org/software/gdb/contribute/ nor in
> http://sourceware.org/git/gitweb.cgi?p=binutils-
> gdb.git;a=blob;f=gdb/CONTRIBUTE;hb=HEAD
> 
> >
> > Thanks for the contribution! Now, onto the details of the review...
> >
> > > Find attached a patch that fixes a SIGSEV for me when trying to open a
> > > Fortran program compiled with ifort (using version 16.0.1.150).
> > > The error can be reproduced with a most simple file only containing
"end"
> > > and no additional compiler flags.
> > >
> > > ---
> > > diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 3d8923b..5890f78
> > > 100644
> > > --- a/gdb/ChangeLog
> > > +++ b/gdb/ChangeLog
> > > @@ -1,3 +1,8 @@
> > > +2016-01-04  Jonas Hahnfeld  <Hahnfeld@itc.rwth-aachen.de>
> > > +
> > > +       * dwarf2read.c (read_partial_die): Fix PR gdb/19208 -
> > > +       SIGSEV while opening Fortran program compiled with ifort
> >
> > Just a minor tweak on the ChangeLog entry: we put the PR number at the
> > start of the entry, like this:
> >
> >        PR gdb/19208
> >        * dwarf2read.c (read_partial_die): SIGSEV while opening
> >        Fortran program compiled with ifort.
> 
> Ok, CONTRIBUTE mentions another convention in its last item.
> 
> >
> > In terms of the change's description, there should be a period at the
end of
> > the sentence (added above). I would also describe the change, rather
than
> > what it prevents. Therefore:
> >
> >        PR gdb/19208
> >        * dwarf2read.c (read_partial_die): Do not call
set_objfile_main_name
> >        if the function has no name.
> >
> > The part about the fact that it prevents a SIGSEGV and which compiler
this
> > was tested with is good information for the revision log. We normally
try to
> > put that kind of information in the code as much as we can, but the
added
> > part_die->name check is fairly obvious in this case.
> >
> > In terms of the patch itself, it does not apply to the current HEAD of
the
> > master branch, and I think this is because it got mucked up by your
mailer
> > (line folding, which I tried to fix by hand, and perhaps also
spaces/tabs
> > being
> > altered). Attached is the commit I intend to push, which I tested on
x86_64-
> > linux, but without Fortran support. Can you please let me know if it
works
> > for
> > you?
> 
> See above...
> 
> >
> > In the future, to avoid this kind of issue, what would be nice is for
you to
> > just
> > create a commit on your end, and to either git-email it to us, or at
least
> > attach
> > the result of "git format-patch".
> 
> CONTRIBUTE explicitly mentions `git diff` which I only pasted into my
mailer.
> I just realized that the other mails were complete patches and git
> automatically generates the line `git diff` inside...
> 
> >
> > Other than that, the patch looks good to me. I don't believe you have an
> FSF
> > copyright assignement in place. We can take this patch as "tiny", but if
you
> > think you have other contributions coming, you might want to start the
> > process now (this will also allow us to give you "write after approval"
> > access
> > to the repo, allowing you to push your changes yourself once approved by
> > one of the GDB maintainers.
> 
> Thanks, I was hoping for this being "tiny" and I currently don't plan to
> contribute larger patches. Will return to this point if necessary ;-)
> 
> Greetings
> Jonas
> 
> >
> > > diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index
> > > c410500..1020c12 100644
> > > --- a/gdb/dwarf2read.c
> > > +++ b/gdb/dwarf2read.c
> > > @@ -15936,7 +15936,8 @@ read_partial_die (const struct
> > > die_reader_specs *reader,
> > >              compilers pick up the new representation, we'll support
this
> > >              practice.  */
> > >           if (DW_UNSND (&attr) == DW_CC_program
> > > -             && cu->language == language_fortran)
> > > +             && cu->language == language_fortran
> > > +             && part_die->name != NULL)
> > >             set_objfile_main_name (objfile, part_die->name,
> > > language_fortran);
> > >           break;
> > >         case DW_AT_inline:
> > --
> > Joel

Attachment: smime.p7s
Description: S/MIME cryptographic signature