Mercury Bugs - mercury
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0000003mercuryBugpublic2007-09-27 23:332014-07-10 14:01
Reporteranonymous 
Assigned Towangp 
PrioritynormalSeverityminorReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version 
Target VersionFixed in Version 
Summary0000003: deep profiler bug with unify/compare for tuples
DescriptionThe following was orginally reported by Peter Wang on the mercury-bugs list.
-----------------------------------------------------------------

Hi,

Here's a test case for a bug in the deep profiler. I think the problem
is in the unification or comparison of a type T which contains a
(non-singleton) tuple which contains T, e.g.

    :- type node
        ---> node(
                    set({node, resource})
                ).

You need to enable this line in mercury_deep_call_port_body.h to
make the bug more obvious:

  #ifdef MR_DEEP_PROFILING_LOWLEVEL_DEBUG
    /* After we copy it, MR_next_call_site_dynamic is not meaningful; */
    /* zeroing it makes debugging output less cluttered. */
    MR_next_call_site_dynamic = NULL;
  #endif

Unification or comparison involving values of that type will then
segfault, as it tries to dereference MR_next_call_site_dynamic when it
is NULL.

I think what happens is that mercury_unify_compare_body.h merges
together the code for for call and exit ports, with the assumption that
no further calls will be made. However, in the MR_TYPECTOR_REP_TUPLE
case MR_generic_compare/unify will be called for each of the arguments,
thereby entering call/exit ports with the deep profiler invariants been
broken. Uncommenting that line above makes it obvious when that
happens, as MR_next_call_site_dynamic will be clobbered after comparing
the first subtype of the tuple, and not setting it up before comparing
the next argument.

The strange thing is, if the tuple is replaced by a d.u. type then
things seem to work, although the code for tuples and du's seem pretty
much the same. So I may be wrong.

Peter

%-----------------------------------------------------------------------------%

:- module cc.
:- interface.

:- import_module io.

:- pred main(io::di, io::uo) is det.

%-----------------------------------------------------------------------------%
%-----------------------------------------------------------------------------%

:- implementation.

:- import_module set.

%-----------------------------------------------------------------------------%

:- type resource
    ---> uri(string).

:- type tuple(T, U) == {T, U}.
%:- type tuple(T, U) ---> tuple(T, U).

:- func mk(T, U) = tuple(T, U).

mk(T, U) = {T, U}.
%mk(T, U) = tuple(T, U).

:- type node
    ---> node(
                set(tuple(node, resource))
            ).

%-----------------------------------------------------------------------------%

main(!IO) :-
    Child = node(
        set.init
    ),
    A = node(
        set.make_singleton_set(mk(Child, uri("foo")))
    ),
    B = node(
        set.make_singleton_set(mk(Child, uri("bar")))
    ),
    write_string("#################\n", !IO),

% ( unify(A, B) ->
% io.write_string("unify\n", !IO)
% ;
% io.write_string("not unify\n", !IO)
% ),

    compare(R, A, B),
    io.print(R, !IO),
    io.nl(!IO),

    write_string("################# done\n", !IO).


%-----------------------------------------------------------------------------%
% vim: ft=mercury ts=8 sw=4 et wm=0 tw=0
TagsNo tags attached.
Relationships
Attached Files

Notes
(0000731)
wangp   
2014-07-02 11:26   
In mercury_unify_compare_body.h, for case MR_TYPECTOR_REP_TUPLE: there are calls to MR_generic_compare and MR_generic_unify inside a loop (for each type argument). For the first argument, MR_next_call_site_dynamic was set up by the caller. For the second and subsequent arguments, the equivalent of prepare_for_special_call is never called, and so MR_next_call_site_dynamic is either stale or nulled out.

Does that make sense?
(0000732)
pbone   
2014-07-02 11:48   
From what I remember about the deep profiler (it's been a while) Yes, that makes sense.
(0000733)
wangp   
2014-07-03 12:35   
More notes.

mercury_builtin_types.c has this comment:

** The generic unify, compare and compare_rep predicates do different things
** for different kinds of type constructors, but these things all fall into
** one of two categories: either the implementation is entirely in C code,
** or the implementation is a tailcall to a Mercury predicate. In neither
** case do the generic predicates allocate a stack frame, which is why
** the stack traversal component of the procedure layouts won't ever be
** referenced.

The first part is not true for tuple types, I think. (There are also recursive calls to MR_generic_compare/etc. in the compare_representation paths.)

In mercury_unify_compare_body.h the return_unify_answer and return_compare_answer macros call special_pred_call_leave_code which merges together the deep profiling "callcode" and "exitcode", which seems to be another instance of that same assumption. The MR_TYPECTOR_REP_TUPLE case would need to avoid doing that as well.

Since that file is already so hairy, another option I considered is to write the tuple unify/compare predicates in Mercury, and let the compiler transform the predicates in deep profiling grades (seems to work in my test). However that introduces a dependency from the runtime into the library.
(0000737)
wangp   
2014-07-10 14:01   
Finally.

Issue History
Date ModifiedUsernameFieldChange
2007-09-27 23:33anonymousNew Issue
2014-07-02 11:26wangpNote Added: 0000731
2014-07-02 11:48pboneNote Added: 0000732
2014-07-03 12:35wangpNote Added: 0000733
2014-07-10 14:01wangpNote Added: 0000737
2014-07-10 14:01wangpStatusnew => resolved
2014-07-10 14:01wangpResolutionopen => fixed
2014-07-10 14:01wangpAssigned To => wangp