From: Mikael Morin <mikael.morin@sfr.fr>
To: Andre Vehreschild <vehre@gmx.de>,
GCC-Patches-ML <gcc-patches@gcc.gnu.org>,
GCC-Fortran-ML <fortran@gcc.gnu.org>
Subject: Re: [Patch, Fortran] Prevent segfault with dump-*-original for implicit class expressions.
Date: Sat, 25 Apr 2015 21:13:00 -0000 [thread overview]
Message-ID: <553C037E.3090006@sfr.fr> (raw)
In-Reply-To: <20150313113339.32440f8c@vepi2>
Hello,
Le 13/03/2015 11:33, Andre Vehreschild a écrit :
> Hi all,
>
> this is another patch preventing a segfault. This time the segfault occurred,
> when -fdump-(fortran|tree)-original was given with the program having an
> implicit class set. The issue is that the _data component is assumed to be
> present in a BT_CLASS w/o checking and trying to access the unlimited
> polymorphic flag there. The patch fixes this by redirecting the access to the
> flag to the correct position whether the _data component is present or not.
>
> Building a testcase for this is difficult for me. May be I am just blocked in
> the head there. The issue occurred when trying to dump the
> (fortran|tree)-original of the testcase gfortran.dg/implicit_class_1.f90. So
> one could argue to add the flag to that testcase, but does it pay in contrast
> to the additional effort each time the testsuite is executed? I have added the
> test now, not being happy with it, but having no clue how to do it better.
>
> Bootstraps and regtests ok on x86_64-linux-gnu/F20.
>
> Ok, for trunk?
>
Comments below:
b/gcc/fortran/symbol.c
> index 32eea21..3379f47 100644
> --- a/gcc/fortran/symbol.c
> +++ b/gcc/fortran/symbol.c
(skipped)
The beginning looks good.
I suggest using 'ts->u.derived->attr.is_class' instead of
'strcmp (ts->u.derived.components->name, "_data") == 0'.
No strong opinion, your choice.
> @@ -4576,13 +4579,14 @@ gfc_type_compatible (gfc_typespec *ts1, gfc_typespec *ts2)
> if (is_derived1 && is_derived2)
> return gfc_compare_derived_types (ts1->u.derived, ts2->u.derived);
>
> - if (is_derived1 && is_class2)
> + if (is_derived1 && is_class2 && ts2->u.derived->components)
> return gfc_compare_derived_types (ts1->u.derived,
> ts2->u.derived->components->ts.u.derived);
> - if (is_class1 && is_derived2)
> + if (is_class1 && is_derived2 && ts1->u.derived->components)
> return gfc_type_is_extension_of (ts1->u.derived->components->ts.u.derived,
> ts2->u.derived);
> - else if (is_class1 && is_class2)
> + else if (is_class1 && is_class2 && ts1->u.derived->components
> + && ts2->u.derived->components)
> return gfc_type_is_extension_of (ts1->u.derived->components->ts.u.derived,
> ts2->u.derived->components->ts.u.derived);
> else
The above change don't seem right.
In the case where the class container is missing, you want to use
"ts->u.derived" instead of "ts->u.derived->components->ts.u.derived",
not skip the procedure call entirely.
> diff --git a/gcc/testsuite/gfortran.dg/implicit_class_1.f90 b/gcc/testsuite/gfortran.dg/implicit_class_1.f90
> index 329f57a..fff1f2b 100644
> --- a/gcc/testsuite/gfortran.dg/implicit_class_1.f90
> +++ b/gcc/testsuite/gfortran.dg/implicit_class_1.f90
> @@ -4,6 +4,10 @@
> !
> ! Contributed by Reinhold Bader <Reinhold.Bader@lrz.de>
>
> +! Add dump-tree-original to check, if the patch preventing a gfortran
> +! segfault is working correctly.
> +! { dg-options "-fdump-tree-original" }
> +
-fdump-tree-original doesn't trigger any bug here.
So use -fdump-fortran-original (I'm not sure the testsuite will like it).
Mikael
next prev parent reply other threads:[~2015-04-25 21:13 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-13 10:33 Andre Vehreschild
2015-04-24 8:32 ` [Ping, Patch, " Andre Vehreschild
2015-04-25 21:13 ` Mikael Morin [this message]
2015-04-28 10:59 ` [Patch, " Andre Vehreschild
2015-04-28 12:06 ` Mikael Morin
2015-04-28 19:43 ` [commited, Patch, " Andre Vehreschild
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=553C037E.3090006@sfr.fr \
--to=mikael.morin@sfr.fr \
--cc=fortran@gcc.gnu.org \
--cc=gcc-patches@gcc.gnu.org \
--cc=vehre@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).