[Patch, fortran] PR69834 - Collision in derived type hashes

[Paul Richard Thomas <paul dot richard dot thomas at gmail dot com>]

Dear All,

I had the attached patch more or less working at the end of January.
However, there was a regression with submodule_6.f03, which I had
quite a struggle with and only resolved yesterday.

Until now, select type used the hash value to do the type selection
with the inevitable consequence that a collision occurred; albeit a
good number of years after the introduction of OOP. The new testcase
is that of the reporter.

I had developed a fix that used the full, composite string containing
the type name and its module. This works fine but the string length is
such that there is a significant performance hit.

Mikael suggested to use the address of the vtable for type selection
and, apart from the regression mentioned above, this was pretty easy
to get going and causes no measurable performance hit.

The problem with submodule_6.f08 was that of multiple versions of the
vtable for derived type 't_b'. The modifications to
class.c(gfc_find_derived_vtab) provide the solution to this issue and
ensure that the vtable is unique. See the comments in the patch to
understand the mechanism.

I have retained the use of the hash value for intrinsic types, since I
know that there are no collisions there. For classes and derived
types, the addresses of the corresponding vtables are used.
resolve_select_type has been modified accordingly. Note that since
select type is no longer translated into select case, a test for
repeated cases had to be introduced. I retained the original message.
If desired the logic could be broken out into a separate function and
the message modified to reflect the source being select type rather
than select case.

The translation now occurs in two functions in trans-stmt.c. The
implementation is straight forward. Note that I have used a series of
if (condition) {block;goto end_label;} rather than stacked if () {}
else {}'s. This reduces the complexity somewhat and should not lead to
any significant performance problems.

I took the opportunity to eliminate the repeated code chunks that
obtain the symbol for the vtable and then obtain the backend_decl by
packing this into a new function. This constitutes the second block in
the fortran ChangeLog.

Bootstrapped and regtested on FC21/x86_64 - OK for trunk?

It crosses my mind that although this is not a regression, it might be
a good idea to port the patch to 6-branch in a month or two, since it
constitutes a potentially silent gotcha.

Cheers

Paul

2016-10-21  Paul Thomas  <pault@gcc.gnu.org>

    PR fortran/69834
    * class.c (gfc_find_derived_vtab): Obtain the gsymbol for the
    derived type's module. If the gsymbol is present and the top
    level namespace corresponds to a module, use the gsymbol name
    space. In the search to see if the vtable exists, try the gsym
    namespace first.
    * dump-parse-tree (show_code_node): Add explicit dump for the
    select type construct.
    * resolve.c (build_loc_call): New function.
    (resolve_select_type): Add check for repeated type is cases.
    Retain selector expression and use it later instead of expr1.
    Store the address for the vtable in the 'low' expression and
    the hash value in the 'high' expression, for each case. Do not
    call resolve_select.
    * trans.c(trans_code) : Call gfc_trans_select_type.
    * trans-stmt.c (gfc_trans_select_type_cases): New function.
    (gfc_trans_select_type): New function.
    * trans-stmt.h : Add prototype for gfc_trans_select_type.

    Tidy up retrieval of vtable backend decl.
    * trans.h : Add prototype for gfc_get_vtable_decl.
    * trans-array.c (structure_alloc_comps): Use it.
    * trans-decl.c (gfc_get_symbol_decl, gfc_trans_deferred_vars,
    gfc_trans_deferred_vars): The same.
    * trans-expr.c (gfc_get_vtable_decl): New function to obtain
    the vtable symbol and its backend decl for any typespec.
    (gfc_reset_vptr, gfc_conv_derived_to_class,
    gfc_conv_intrinsic_to_class, gfc_trans_class_assign,
    gfc_conv_procedure_call,gfc_trans_subcomponent_assign): Use it.
    trans-intrinsic.c (scalar_transfer, conv_intrinsic_move_alloc):
    The same.
    trans-io.c (transfer_namelist_element): The same.
    trans-stmt.c (gfc_trans_allocate): The same.

2016-10-21  Paul Thomas  <pault@gcc.gnu.org>

    PR fortran/69834
    * gfortran.dg/select_type_36.f03: New test.

Attachment: submit.diff
Description: Text document