[m-rev.] for review: fix I/O retry bug
Ian MacLarty
maclarty at cs.mu.OZ.AU
Thu Aug 11 22:29:36 AEST 2005
Zoltan, I'm still not entirely sure why MR_trace_find_input_arg sometimes
finds io.state values and sometimes doesn't. Could you clarify that point if
you know the reason?
For review by Zoltan.
Estimated hours taken: 35
Branches: main and 0.12
Fix another bug when retrying across calls which have an I/O state in one of
their polymorphic arguments.
The problem was that I was assuming MR_trace_find_input_arg would never find
the value of an I/O state argument, however sometimes it does (the value is
junk since io.state values are never used, but MR_trace_find_input_arg reports
that it has found a value anyway).
The fix is to check if each argument is an io.state before looking up the
value of the argument.
tests/debugger/Mercury.options:
tests/debugger/Mmakefile:
tests/debugger/poly_io_retry2.exp:
tests/debugger/poly_io_retry2.inp:
tests/debugger/poly_io_retry2.m:
Add a regression test. Previously the two printed test1 atoms produced
different output.
tools/lmc.in:
Allow the compiler to be run under valgrind. This didn't prove very
useful in this case, but I decided to leave the change in in case it
might be useful in the future.
trace/mercury_trace.c:
Fix the bug described above.
Index: tests/debugger/Mercury.options
===================================================================
RCS file: /home/mercury1/repository/tests/debugger/Mercury.options,v
retrieving revision 1.12
diff -u -b -r1.12 Mercury.options
--- tests/debugger/Mercury.options 13 May 2005 13:45:31 -0000 1.12
+++ tests/debugger/Mercury.options 11 Aug 2005 07:13:51 -0000
@@ -12,6 +12,7 @@
MCFLAGS-no_inline_builtins = --no-inline-builtins
MCFLAGS-poly_io_retry = --trace-table-io-all
+MCFLAGS-poly_io_retry2 = --trace-table-io-all
MCFLAGS-queens_rep = --trace rep
MCFLAGS-shallow = --trace shallow
MCFLAGS-tabled_read = --trace-table-io-all
Index: tests/debugger/Mmakefile
===================================================================
RCS file: /home/mercury1/repository/tests/debugger/Mmakefile,v
retrieving revision 1.115
diff -u -b -r1.115 Mmakefile
--- tests/debugger/Mmakefile 26 May 2005 00:17:04 -0000 1.115
+++ tests/debugger/Mmakefile 11 Aug 2005 07:23:32 -0000
@@ -39,6 +39,7 @@
lval_desc_array \
multi_parameter \
poly_io_retry \
+ poly_io_retry2 \
polymorphic_output \
print_goal \
print_table \
@@ -270,6 +271,10 @@
$(MDB_STD) ./poly_io_retry < poly_io_retry.inp \
> poly_io_retry.out 2>&1
+poly_io_retry2.out: poly_io_retry2 poly_io_retry2.inp
+ $(MDB_STD) ./poly_io_retry2 < poly_io_retry2.inp \
+ > poly_io_retry2.out 2>&1
+
# The exception_cmd, exception_vars, polymorphic_output and loopcheck tests
# are supposed to return a non-zero exit status, since they exit by throwing
# an exception. We strip the goal paths from their exception events, since
Index: tests/debugger/poly_io_retry2.exp
===================================================================
RCS file: tests/debugger/poly_io_retry2.exp
diff -N tests/debugger/poly_io_retry2.exp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/debugger/poly_io_retry2.exp 11 Aug 2005 07:16:55 -0000
@@ -0,0 +1,40 @@
+ E1: C1 CALL pred poly_io_retry2.main/2-0 (det) poly_io_retry2.m:13
+mdb> mdb> Contexts will not be printed.
+mdb> echo on
+Command echo enabled.
+mdb> table_io allow
+mdb> table_io start
+I/O tabling started.
+mdb> break test1
+ 0: + stop interface pred poly_io_retry2.test1/5-0 (det)
+mdb> c
+ E2: C2 CALL pred poly_io_retry2.test1/5-0 (det)
+mdb> f
+ E3: C2 EXIT pred poly_io_retry2.test1/5-0 (det)
+mdb> c
+ E4: C3 CALL pred poly_io_retry2.test1/5-0 (det)
+mdb> f
+ E5: C3 EXIT pred poly_io_retry2.test1/5-0 (det)
+mdb> c
+ E6: C4 CALL pred poly_io_retry2.test1/5-0 (det)
+mdb> f
+ E7: C4 EXIT pred poly_io_retry2.test1/5-0 (det)
+mdb> p
+test1(2, 7, 16, _, _)
+mdb> retry 3 -a
+ E8: C5 CALL pred poly_io_retry2.list_foldl2/6-0 (det)
+mdb> c
+ E2: C2 CALL pred poly_io_retry2.test1/5-0 (det)
+mdb> f
+ E3: C2 EXIT pred poly_io_retry2.test1/5-0 (det)
+mdb> c
+ E4: C3 CALL pred poly_io_retry2.test1/5-0 (det)
+mdb> f
+ E5: C3 EXIT pred poly_io_retry2.test1/5-0 (det)
+mdb> c
+ E6: C4 CALL pred poly_io_retry2.test1/5-0 (det)
+mdb> f
+ E7: C4 EXIT pred poly_io_retry2.test1/5-0 (det)
+mdb> p
+test1(2, 7, 16, _, _)
+mdb> quit -y
Index: tests/debugger/poly_io_retry2.inp
===================================================================
RCS file: tests/debugger/poly_io_retry2.inp
diff -N tests/debugger/poly_io_retry2.inp
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/debugger/poly_io_retry2.inp 11 Aug 2005 07:16:16 -0000
@@ -0,0 +1,22 @@
+register --quiet
+context none
+echo on
+table_io allow
+table_io start
+break test1
+c
+f
+c
+f
+c
+f
+p
+retry 3 -a
+c
+f
+c
+f
+c
+f
+p
+quit -y
Index: tests/debugger/poly_io_retry2.m
===================================================================
RCS file: tests/debugger/poly_io_retry2.m
diff -N tests/debugger/poly_io_retry2.m
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/debugger/poly_io_retry2.m 11 Aug 2005 07:13:21 -0000
@@ -0,0 +1,60 @@
+:- module poly_io_retry2.
+
+:- interface.
+
+:- import_module io.
+
+:- pred main(io::di, io::uo) is det.
+
+:- implementation.
+
+:- import_module list, int, std_util.
+
+main(!IO) :-
+ io_set_globals(univ(3), !IO),
+ list_foldl2(test1, [0, 1, 2], 0, _, !IO),
+ nl(!IO).
+
+:- pred test1(int::in, int::in, int::out, io::di, io::uo) is det.
+
+test1(X, Z, Y + X + Z, !IO) :-
+ io_get_globals(U, !IO),
+ ( if univ_to_type(U, Y0) then
+ Y = Y0
+ else
+ Y = 1
+ ),
+ io_set_globals(univ(Y + X + Z), !IO).
+
+:- pred list_foldl2(pred(L, A, A, Z, Z), list(L), A, A, Z, Z).
+:- mode list_foldl2(pred(in, in, out, di, uo) is det,
+ in, in, out, di, uo) is det.
+
+list_foldl2(_, [], !A, !B).
+list_foldl2(P, [H | T], !A, !B) :-
+ call(P, H, !A, !B),
+ list_foldl2(P, T, !A, !B).
+
+:- pred io_get_globals(univ::out, io::di, io::uo) is det.
+
+:- pred io_set_globals(univ::in, io::di, io::uo) is det.
+
+:- pragma foreign_decl("C", "
+ static MR_Word poly_io_retry_test_globals;
+").
+
+:- pragma foreign_proc("C",
+ io_get_globals(Globals::out, IOState0::di, IOState::uo),
+ [will_not_call_mercury, promise_pure, tabled_for_io],
+"
+ Globals = poly_io_retry_test_globals;
+ MR_update_io(IOState0, IOState);
+").
+
+:- pragma foreign_proc("C",
+ io_set_globals(Globals::in, IOState0::di, IOState::uo),
+ [will_not_call_mercury, promise_pure, tabled_for_io],
+"
+ poly_io_retry_test_globals = Globals;
+ MR_update_io(IOState0, IOState);
+").
Index: tools/lmc.in
===================================================================
RCS file: /home/mercury1/repository/mercury/tools/lmc.in,v
retrieving revision 1.6
diff -u -b -r1.6 lmc.in
--- tools/lmc.in 28 Jan 2005 07:12:02 -0000 1.6
+++ tools/lmc.in 11 Aug 2005 07:26:38 -0000
@@ -50,6 +50,10 @@
# execute the Mercury compiler under gdb by setting the environment variable
# MMC_UNDER_GDB to the string "true".
#
+# You can ask this script to run the Mercury compiler under valgrind by
+# setting MMC_UNDER_VALGRIND to "true". Valgrind options can be placed in
+# the environment variable MMC_VALGRIND_OPTIONS.
+#
# If you want to link in the shared versions of the Mercury libraries then set
# the environment variable MMC_USE_SHARED_LIBS to the extension for shared
# libraries on your system. You also still need to pass "--mercury-linkage
@@ -131,6 +135,12 @@
export MERCURY_COMPILER
fi
+if test "$MMC_UNDER_VALGRIND" != ""
+then
+ MERCURY_COMPILER="valgrind $MMC_VALGRIND_OPTIONS $MERCURY_COMPILER"
+ export MERCURY_COMPILER
+fi
+
PATH="$WORKSPACE/scripts:$WORKSPACE/util:$PATH"
export PATH
exec mmc --no-mercury-stdlib-dir --config-file $WORKSPACE/scripts/Mercury.config -I $WORKSPACE/library -I $WORKSPACE/analysis $CDEBUG_FLAGS $C_FLAGS $INIT_FLAGS $LIB_FLAGS $LINK_FLAGS "$@"
Index: trace/mercury_trace.c
===================================================================
RCS file: /home/mercury1/repository/mercury/trace/mercury_trace.c,v
retrieving revision 1.83
diff -u -b -r1.83 mercury_trace.c
--- trace/mercury_trace.c 1 Aug 2005 02:26:25 -0000 1.83
+++ trace/mercury_trace.c 11 Aug 2005 12:26:33 -0000
@@ -536,6 +536,7 @@
MR_bool succeeded;
MR_Word *saved_regs;
MR_bool has_io_state;
+ MR_bool is_io_state;
MR_bool found_io_action_counter;
MR_Unsigned saved_io_action_counter;
#ifdef MR_USE_MINIMAL_MODEL_STACK_COPY
@@ -604,45 +605,48 @@
arg_max = 0;
+ /*
+ ** Check if any of the (non-polymorphic) arguments are of type io.state.
+ ** We check this here and then again for each argument in the for loop
+ ** below, because if the io.state argument is not polymorphic, then it
+ ** would have been removed which means the check in the for loop below
+ ** won't find it. If the argument is polymorphic, then it won't have been
+ ** removed, however the check here won't pick this up since it only uses
+ ** the static argument types. The check in the for loop below will pick up
+ ** polymorphic arguments which have type io.state, because it materializes
+ ** the type of each argument before checking it.
+ */
if (MR_proc_has_io_state_pair(level_layout)) {
has_io_state = MR_TRUE;
- found_io_action_counter = MR_find_saved_io_counter(call_label,
- base_sp, base_curfr, &saved_io_action_counter);
} else {
has_io_state = MR_FALSE;
- /* just to prevent uninitialized variable warnings */
- found_io_action_counter = MR_FALSE;
- saved_io_action_counter = 0;
}
type_params = MR_materialize_type_params_base(return_label_layout,
saved_regs, base_sp, base_curfr);
for (i = 0; i < MR_all_desc_var_count(call_label); i++) {
+ /*
+ ** Check if the argument is of type io.state. We check this before
+ ** calling MR_trace_find_input_arg, since MR_trace_find_input_arg may
+ ** or may not find the value of the io.state argument. We don't care
+ ** if it finds the value of an io.state argument or not, since the
+ ** value is junk and never used. The safety of the retry is addressed
+ ** by the code handling has_io_state after this for loop.
+ */
+ is_io_state = MR_is_io_state(type_params, MR_var_pti(call_label, i));
+
+ if (is_io_state) {
+ has_io_state = MR_TRUE;
+ } else {
arg_value = MR_trace_find_input_arg(return_label_layout,
saved_regs, base_sp, base_curfr,
call_label->MR_sll_var_nums[i], &succeeded);
if (! succeeded) {
- if (! MR_is_io_state(type_params, MR_var_pti(call_label, i))) {
*problem = "Cannot perform retry because the "
"values of some input arguments are missing.";
goto report_problem;
- } else if (! has_io_state) {
- /*
- ** has_io_state would not have been set to true earlier if the
- ** argument is polymorphic.
- */
- has_io_state = MR_TRUE;
- found_io_action_counter = MR_find_saved_io_counter(
- call_label, base_sp, base_curfr, &saved_io_action_counter);
- }
-
- /*
- ** Since values of the I/O states type are not actually used,
- ** we can leave arg_value containing garbage. The safety of
- ** the retry is addressed by the code handling has_io_state below.
- */
}
if (i < MR_long_desc_var_count(call_label)) {
@@ -660,12 +664,15 @@
MR_fatal_error("illegal location for input argument");
}
}
+ }
if (has_io_state) {
MR_Unsigned cur_event_num;
MR_Unsigned retry_event_num;
MR_bool all_actions_tabled;
+ found_io_action_counter = MR_find_saved_io_counter(call_label,
+ base_sp, base_curfr, &saved_io_action_counter);
cur_event_num = event_info->MR_event_number;
/*
--------------------------------------------------------------------------
mercury-reviews mailing list
post: mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------
More information about the reviews
mailing list