This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Fix PR gdb/19208 - SIGSEV while opening Fortran program compiled with ifort
- From: Joel Brobecker <brobecker at adacore dot com>
- To: "Hahnfeld, Jonas" <Hahnfeld at itc dot rwth-aachen dot de>
- Cc: "gdb-patches at sourceware dot org" <gdb-patches at sourceware dot org>, "qiyao at gcc dot gnu dot org" <qiyao at gcc dot gnu dot org>
- Date: Wed, 6 Jan 2016 10:19:46 +0400
- Subject: Re: [PATCH] Fix PR gdb/19208 - SIGSEV while opening Fortran program compiled with ifort
- Authentication-results: sourceware.org; auth=none
- References: <233e961324084b0f857047bcf637c655 at rwthex-s2-b dot rwth-ad dot de>
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 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.
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?
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".
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.
> 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
>From 5dc30b5fba761013c064a9b9de6fc39dc2be01e2 Mon Sep 17 00:00:00 2001
From: Jonas Hahnfeld <Hahnfeld@itc.rwth-aachen.de>
Date: Wed, 6 Jan 2016 10:10:39 +0400
Subject: [PATCH] GDB SIGSEGV opening a Fortran program compiled with ifort
This patch fixes a SIGSEGV when trying to open a Fortran program
compiled with ifort (reproduced using version using version 16.0.1.150).
The error can be reproduce with most, if not any program. For instance,
a single file only containing "end", compiled with no additional flag,
suffices.
gdb/ChangeLog:
PR gdb/19208
* dwarf2read.c (read_partial_die): Do not call set_objfile_main_name
if the function has no name.
---
gdb/dwarf2read.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
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:
--
2.5.0