public inbox for fortran@gcc.gnu.org
 help / color / mirror / Atom feed
From: Paul Richard Thomas <paul.richard.thomas@gmail.com>
To: Andre Vehreschild <vehre@gmx.de>
Cc: "fortran@gcc.gnu.org" <fortran@gcc.gnu.org>,
	gcc-patches <gcc-patches@gcc.gnu.org>,
	liakhdi@ornl.gov
Subject: Re: [Patch, fortran] PR80850 - Sourced allocate() fails to allocate a pointer
Date: Wed, 01 Nov 2017 09:36:00 -0000	[thread overview]
Message-ID: <CAGkQGiLtatFZx6jgbWh41zNWo-=9fyTGupU1RXqoyV6Wg9wJbA@mail.gmail.com> (raw)
In-Reply-To: <CAGkQGiK4cUsVSL9ZtKG6asLp6RQy6vqBxWy4ida0e=o5gcfwNA@mail.gmail.com>

Committed to 7-branch as revision 254293. I will close the PR now.

Cheers

Paul

On 30 October 2017 at 22:16, Paul Richard Thomas
<paul.richard.thomas@gmail.com> wrote:
> Dear Andre,
>
> Committed to trunk as revision 254244.
>
> In order to debug the code, I was forced to use 7-branch for
> development since there were dependencies that detected the change in
> module number. 7-branch accepted the assignments without casts but I
> was forced to include them in trunk. As advertised the testcase just
> enforces the assignment to the _len field through a tree dump.
>
> 7-branch will come on Wednesday. Tomorrow is full of Halloween fun....
>
> Thanks
>
> Paul
>
>
> On 30 October 2017 at 13:39, Andre Vehreschild <vehre@gmx.de> wrote:
>> Hi Paul,
>>
>> whoopsie, I remember that I inserted the check for _len > 0 in allocate(). So
>> it was me causing the bug. Thanks that you found it. The patch looks good to
>> me. Thanks for the work.
>>
>> - Andre
>>
>> On Mon, 30 Oct 2017 12:20:20 +0000
>> Paul Richard Thomas <paul.richard.thomas@gmail.com> wrote:
>>
>>> Dear All,
>>>
>>> This bug took a silly amount of effort to diagnose but once done, the
>>> fix was obvious.
>>>
>>> The bug is triggered in this function from the reporter's source file
>>> gfc_graph.F90:
>>>
>>>         function GraphIterAppendVertex(this,vertex) result(ierr)
>>> !Appends a new vertex to the graph.
>>>          implicit none
>>>          integer(INTD):: ierr                               !out: error code
>>>          class(graph_iter_t), intent(inout):: this          !inout:
>>> graph iterator
>>>          class(graph_vertex_t), intent(in), target:: vertex !in: new vertex
>>>          type(vert_link_refs_t):: vlr
>>>
>>>          ierr=this%vert_it%append(vertex) !<===== right here!
>>> ....snip....
>>>          return
>>>         end function GraphIterAppendVertex
>>>
>>> 'vertex' is being passed to a class(*) dummy. As you will see from the
>>> attached patch and the ChangeLog, 'vertex' was being cast as unlimited
>>> polymorphic and assigned to the passed actual argument. This left the
>>> _len field indeterminate since it is not present in normal (limited?)
>>> polymorphic objects.
>>>
>>> Further down the way, in stsubs.F90(clone_object) an allocation is
>>> being made using the unlimited version of 'vertex as a source. Since
>>> the size passed to malloc for an unlimited source is, for  _len > 0,
>>> the value of the _len multiplied by the vtable _size, the amount of
>>> memory is also indeterminate and causes the operating system to flag a
>>> failed allocation, pretty much at random.
>>>
>>> The ChangeLog and the patch describe the fix sufficiently well as not
>>> to require further explanation. I will write a testcase that tests the
>>> tree dump for the _len field before committing the patch.
>>>
>>> Bootstraps and regtests on FC23/x86_64 - OK for 7- and 8-branches?
>>>
>>> If I do not receive any contrary comments, I will commit tonight.
>>>
>>> Regards
>>>
>>> Paul
>>>
>>> 2017-10-30  Paul Thomas  <pault@gcc.gnu.org>
>>>
>>>     PR fortran/80850
>>>     * trans_expr.c (gfc_conv_procedure_call): When passing a class
>>>     argument to an unlimited polymorphic dummy, it is wrong to cast
>>>     the passed expression as unlimited, unless it is unlimited. The
>>>     correct way is to assign to each of the fields and set the _len
>>>     field to zero.
>>
>>
>> --
>> Andre Vehreschild * Email: vehre ad gmx dot de
>
>
>
> --
> "If you can't explain it simply, you don't understand it well enough"
> - Albert Einstein



-- 
"If you can't explain it simply, you don't understand it well enough"
- Albert Einstein

      reply	other threads:[~2017-11-01  9:36 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-30 12:20 Paul Richard Thomas
2017-10-30 13:39 ` Andre Vehreschild
2017-10-30 14:23   ` Paul Richard Thomas
2017-10-30 22:16   ` Paul Richard Thomas
2017-11-01  9:36     ` Paul Richard Thomas [this message]

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='CAGkQGiLtatFZx6jgbWh41zNWo-=9fyTGupU1RXqoyV6Wg9wJbA@mail.gmail.com' \
    --to=paul.richard.thomas@gmail.com \
    --cc=fortran@gcc.gnu.org \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=liakhdi@ornl.gov \
    --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).