* [PATCH] Fix PR gdb/19208 - SIGSEV while opening Fortran program compiled with ifort @ 2016-01-04 15:47 Hahnfeld, Jonas 2016-01-06 6:20 ` Joel Brobecker 0 siblings, 1 reply; 5+ messages in thread From: Hahnfeld, Jonas @ 2016-01-04 15:47 UTC (permalink / raw) To: gdb-patches; +Cc: qiyao [-- Attachment #1: Type: text/plain, Size: 1585 bytes --] Hi all, 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 + 2016-01-04 Markus Metzger <markus.t.metzger@intel.com> * btrace.c (btrace_pt_readmem_callback): Do not return in TRY/CATCH. 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: -- Jonas Hahnfeld, MATSE-Auszubildender IT Center Group: High Performance Computing Division: Computational Science and Engineering RWTH Aachen University Seffenter Weg 23 D 52074 Aachen (Germany) Hahnfeld@itc.rwth-aachen.de www.itc.rwth-aachen.de [-- Attachment #2: smime.p7s --] [-- Type: application/pkcs7-signature, Size: 5868 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix PR gdb/19208 - SIGSEV while opening Fortran program compiled with ifort 2016-01-04 15:47 [PATCH] Fix PR gdb/19208 - SIGSEV while opening Fortran program compiled with ifort Hahnfeld, Jonas @ 2016-01-06 6:20 ` Joel Brobecker 2016-01-06 7:42 ` Hahnfeld, Jonas 2016-01-14 7:19 ` Hahnfeld, Jonas 0 siblings, 2 replies; 5+ messages in thread From: Joel Brobecker @ 2016-01-06 6:20 UTC (permalink / raw) To: Hahnfeld, Jonas; +Cc: gdb-patches, qiyao [-- Attachment #1: Type: text/plain, Size: 3680 bytes --] 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 [-- Attachment #2: 0001-GDB-SIGSEGV-opening-a-Fortran-program-compiled-with-.patch --] [-- Type: text/x-diff, Size: 1331 bytes --] 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] Fix PR gdb/19208 - SIGSEV while opening Fortran program compiled with ifort 2016-01-06 6:20 ` Joel Brobecker @ 2016-01-06 7:42 ` Hahnfeld, Jonas 2016-01-14 7:19 ` Hahnfeld, Jonas 1 sibling, 0 replies; 5+ messages in thread From: Hahnfeld, Jonas @ 2016-01-06 7:42 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches, qiyao [-- Attachment #1: Type: text/plain, Size: 4892 bytes --] 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 #2: smime.p7s --] [-- Type: application/pkcs7-signature, Size: 5868 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] Fix PR gdb/19208 - SIGSEV while opening Fortran program compiled with ifort 2016-01-06 6:20 ` Joel Brobecker 2016-01-06 7:42 ` Hahnfeld, Jonas @ 2016-01-14 7:19 ` Hahnfeld, Jonas 2016-01-17 6:18 ` Joel Brobecker 1 sibling, 1 reply; 5+ messages in thread From: Hahnfeld, Jonas @ 2016-01-14 7:19 UTC (permalink / raw) To: Joel Brobecker; +Cc: gdb-patches, qiyao [-- Attachment #1: Type: text/plain, Size: 5481 bytes --] 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 #2: smime.p7s --] [-- Type: application/pkcs7-signature, Size: 5868 bytes --] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Fix PR gdb/19208 - SIGSEV while opening Fortran program compiled with ifort 2016-01-14 7:19 ` Hahnfeld, Jonas @ 2016-01-17 6:18 ` Joel Brobecker 0 siblings, 0 replies; 5+ messages in thread From: Joel Brobecker @ 2016-01-17 6:18 UTC (permalink / raw) To: Hahnfeld, Jonas; +Cc: gdb-patches, qiyao [-- Attachment #1: Type: text/plain, Size: 680 bytes --] > Ping... Attached is what I just pushed. gdb/ChangeLog: PR gdb/19208 * dwarf2read.c (read_partial_die): Do not call set_objfile_main_name if the function has no name. (tested on x86_64-linux, with no Fortran) > Will this also be included in 7.10.2 (if planned?) As of today, there isn't going to be a 7.10.2. We're very close to branching for 7.11, so I don't think it's worth backporting, as the branch will soon be closed. I also meant to review your comments regarding the documentation we have about how to contribute, and haven't had the time. But thank you for those, because they help us spot areas where the info may have bitrotten. -- Joel [-- Attachment #2: 0001-GDB-SIGSEGV-opening-a-Fortran-program-compiled-with-.patch --] [-- Type: text/x-diff, Size: 1854 bytes --] From dc365182240722969f044918ecd3cab00da55502 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/ChangeLog | 7 +++++++ gdb/dwarf2read.c | 3 ++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index fe8fd6c..dbc4b08 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2016-01-17 Jonas Hahnfeld <Hahnfeld@itc.rwth-aachen.de> (tiny change) + + Pushed by Joel Brobecker <brobecker@adacore.com>. + PR gdb/19208 + * dwarf2read.c (read_partial_die): Do not call set_objfile_main_name + if the function has no name. + 2016-01-15 Sandra Loosemore <sandra@codesourcery.com> * charset.c [PHONY_ICONV] (GDB_DEFAULT_HOST_CHARSET): 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-01-17 6:18 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-01-04 15:47 [PATCH] Fix PR gdb/19208 - SIGSEV while opening Fortran program compiled with ifort Hahnfeld, Jonas 2016-01-06 6:20 ` Joel Brobecker 2016-01-06 7:42 ` Hahnfeld, Jonas 2016-01-14 7:19 ` Hahnfeld, Jonas 2016-01-17 6:18 ` Joel Brobecker
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).