[m-rev.] for review: Fix inconsistency in type parameter layout structures.

[Peter Wang]

On Wed, 05 Jun 2019 12:29:39 +0200 (CEST), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> 
> 
> On Wed,  5 Jun 2019 18:02:20 +1000, Peter Wang <novalazy at gmail.com> wrote:
> > Some help with the log message would be appreciated. I don't know if
> > I've named the things correctly.
> 
> I am the person you need, since I wrote that code.
> 
> > The MR_TypeParamLocns_Struct field MR_tp_param_count has type
> > MR_uint_least32_t, but the compiler generates type var vectors (???)
> > in which every value is word sized.
> 
> I am not sure which vector you are talking about; there are so many
> in the debugger RTTI. Is it the MR_tp_param_locns field that immediately
> follows the count? If so, that doesn't hold type vars; it holds descriptions
> of the locations in which the debugger can find the typeinfos describing
> the types that are *bound* to those type vars.

The type parameters vector in the closure layout (I've disabled
use_common_cells to make it clearer):

    static const MR_Integer typevar_vector_2[2] = {
      (MR_Integer) 1,
      (MR_Integer) 33
    };

    static const MR_Box closure_layout_3[8] = {
      NULL,
      ((MR_Box) (&typevar_vector_2)),
      ((MR_Box) ((MR_Integer) 5)),
      ((MR_Box) (&mercury__private_builtin__private_builtin__type_ctor_info_type_info_0)),
      ((MR_Box) ((MR_Integer) 1)),
      ((MR_Box) ((MR_Integer) 1)),
      ((MR_Box) (&mercury__builtin__builtin__type_ctor_info_string_0)),
      ((MR_Box) (&mercury__builtin__builtin__type_ctor_info_string_0))
    };

> 
> > When the vector is cast to
> > MR_TypeParamLocns_Struct, and the MR_tp_param_count field dereferenced,
> > only the lowest 32-bits are read.
> 
> What code is this referring to?

In mercury_deep_copy_body.h line 593, the closure layout is passed to
MR_materialize_closure_type_params which retrieves the number of type
parameters:

        count = tvar_locns->MR_tp_param_count;

(ok, having looked at that again, I think I can write a better log
message tomorrow)

> 
> > On 32-bit or little endian 64-bit
> > machines the value will happen to be correct, but on big endian machines
> > the value will be zero.
> > 
> > This bug caused hard_coded/copy_pred_2.m to fail on a 64-bit big endian
> > machine (AIX/PowerPC).
> > 
> > runtime/mercury_stack_layout.h:
> >     Change type of MR_tp_param_count to MR_Integer.
> > 
> > diff --git a/runtime/mercury_stack_layout.h b/runtime/mercury_stack_layout.h
> > index 3e26e4877..627d30fbd 100644
> > --- a/runtime/mercury_stack_layout.h
> > +++ b/runtime/mercury_stack_layout.h
> > @@ -476,7 +476,7 @@ struct MR_UserEventSpec_Struct {
> >  #define MR_NOT_HIDDEN   0
> >  
> >  struct MR_TypeParamLocns_Struct {
> > -    MR_uint_least32_t       MR_tp_param_count;
> > +    MR_Integer              MR_tp_param_count;
> >      MR_LongLval             MR_tp_param_locns[MR_VARIABLE_SIZED];
> >  };
> 
> I think the right change is to MR_Unsigned, not MR_Integer.

The values are currently always assigned to local variables of type
MR_Integer, so I will leave it as MR_Integer.

Peter