[m-dev.] For review: implementation of collect for Opium-M

Erwan Jahier Erwan.Jahier at irisa.fr
Sat Oct 30 00:16:51 AEST 1999


| On 28-Oct-1999, Erwan Jahier <Erwan.Jahier at irisa.fr> wrote:
| > 
| >  browser/debugger_interface.m:
| > +	Define a new function get_collecting_variable_type/2 that retrieves
| > +	the type of a variable. This type is needed in MR_trace_event_external()
| > +	to be able to call MR_make_permanent() on MR_collecting_variable; we
| > +	need to do that to ensure that the memory allocated for it won't be 
| > +	deallocated on backtracking.
| ...
| > +:- pred get_collecting_variable_type(T, type_info).
| > +:- mode get_collecting_variable_type(in, out) is det.
| > +
| > +:- pragma export(get_collecting_variable_type(in, out), 
| > +	"ML_DI_get_collecting_variable_type").
| > +	% This predicate allows mercury_trace_external.c to retrieve the name 
| > +	% of the type_info a collecting variable.
| > +get_collecting_variable_type(CollectVar, Type) :-
| > +	Type = type_of(CollectVar).
| 
| That predicate is not very useful; since the predicate is declared to
| have a polymophic type `T', the exported C function will need to be
| passed a type_info argument, and will just return that same type_info.

that's what I've done

| To make this work, you need the predicate to be declared with a concrete
| type, which probably means it needs to be defined in collect.in, I think.
| 
| > +	case MR_collecting:
| > +		(*send_collect_result_ptr)(
| > +			(Word) MR_collecting_variable, 
| > +			(Word) &MR_debugger_socket_out);
| > +			#if defined(HAVE_DLFCN_H)&&defined(HAVE_DLCLOSE)
| > +				dlclose((void *)handle);
| > +			#endif
| 
| s/&&/ && /

Done.

| Rather than calling dlclose() directly here, it would be better
| to call the Mercury dl__close procedure.  Note that eventually
| the dl__close procedure may need to do additional work which is
| not done by dlclose(), e.g. removing the module's procedures from
| the various tables maintained by the Mercury debugger.
| If you call dl__open to open it, then you should call dl__close
| to close it.

Done.
 
| > @@ -397,16 +450,11 @@
| >  Code *
| >  MR_trace_event_external(MR_Trace_Cmd_Info *cmd, MR_Event_Info *event_info)
| >  {
| > -	static MR_external_debugger_mode_type 
| > -			external_debugger_mode = reading_request;
| >  	static Word	search_data;
| > -	static Word	collecting_variable;
| > -	static Word	(*initialize_ptr)(Word *) = NULL;
| > +	static void	(*initialize_ptr)(Word *) = NULL;
| >  	static Word    	(*filter_ptr)(Integer, Integer, Integer, MR_Trace_Port,
| >  				MR_PredFunc, String, String, String, Integer,
| >  				Integer, Integer, String, Word, Word *) = NULL;
| > -	static Word	(*send_collect_result_ptr)(Word, Word);
| 
| The return types of the filter_ptr and send_collect_result_ptr are still wrong,
| I think -- shouldn't they be `void', not `Word'?

Yes, I did that change, but at every locations. Now it is done everywhere 
(hopefully).

| > +		case MR_collecting:
| > +			/* 
| > +			** XXX Add a another request that takes 
| > +			** arguments into account. We need two kinds 
| > +			** of request in order to not penalize the 
| > +			** performance of collect in the cases where 
| > +			** arguments are not used.
| > +			**  
| > +			** arguments = MR_make_var_list(layout, saved_regs);
| > +			*/
| > +			MR_COLLECT_filter(
| > +				*filter_ptr, 
| > +				(Unsigned) seqno, 
| > +				(Unsigned) depth, 
| > +				(MR_Trace_Port) port,
| > +				layout, 
| > +				path);
| > +			goto done;
| 
| The casts here are unnecessary, I think.

Removed.

| >  			case MR_REQUEST_LINK_COLLECT:
| >  			  {
| >  			        Char	result;
| > +				Word	MR_collecting_variable_type;
| > +				Word	*MR_collecting_variable_type_info=NULL;
| 
| Why do you initialize the type_info to NULL here?
| 
| >  				if (MR_debug_socket) {
| >  					fprintf(stderr, "\nMercury runtime: "
| >  						"REQUEST_LINK_COLLECT\n");
| >  				}
| > +				MR_get_object_file_name(debugger_request,
| > +					    &MR_object_file_name);
| >  				MR_TRACE_CALL_MERCURY(
| >  					ML_DI_link_collect(
| > +			       		    MR_object_file_name,
| >  					    (Word *) &filter_ptr,
| >  					    (Word *) &initialize_ptr,
| >  					    (Word *) &send_collect_result_ptr,
| >  					    (Word *) &handle,
| > -					    (Char *) &result
| > +					    &result
| >  					    ));
| >  				collect_linked = (result == 'y');
| >  				if (collect_linked) {
| >  					MR_send_message_to_socket(
| >  						"link_collect_succeeded");
| > +					MR_TRACE_CALL_MERCURY(
| > +					    ML_DI_get_collecting_variable_type(
| > +						(Word) 
| > +						MR_collecting_variable_type_info,
| > +						MR_collecting_variable,
| > +						(Word *) 
| > +						&MR_collecting_variable_type));
| 
| The cast to (Word *) here is unnecessary.
| In fact the whole call to ML_DI_get_collecting_variable_type
| is unnecessary; given the definition of get_collecting_variable_type
| that you had, that call will just set MR_collecting_variable_type to
| MR_collecting_variable_type_info, so you might as well replace it with
| 
| 	MR_collecting_variable_type = MR_collecting_variable_type_info
|
| Unfortunately the result will still be a NULL pointer, and so



| probably the following call to MR_make_permanent() will crash, if you test
| it in a grade which does not use conservative GC.
| 
| >  					/*
| >  					** In order to perform the collect from
| >  					** the current event, we need to call 
| >  					** filter once here.
| >  					*/
| > +					MR_COLLECT_filter(
| > +						*filter_ptr, 
| > +						(Unsigned) seqno, 
| > +						(Unsigned) depth, 
| > +						(MR_Trace_Port) port,
| > +						layout, 
| > +						path);
| > +					
| 
| The casts there are unnecessary, I think.

Removed.

| > @@ -61,12 +61,12 @@
| >  % for performance reasons; indeed, filter will be called at every events
| >  % so we don't want to pay the price of the maybe variable de-construction
| >  % at each event.
| > -link_collect(Filter, Initialize, SendResult, HandlePtr, Result) -->
| > +link_collect(ObjetFile, Filter, Initialize, SendResult, HandlePtr, Result) -->
| 
| s/ObjetFile/ObjectFile/g

Done.


Here is the new relative diff concerning those comments.


Index: 0.4/debugger_interface.m
--- 0.4/debugger_interface.m Thu, 28 Oct 1999 20:44:31 +0200 jahier (collect/1_debugger_i 1.2 640)
+++ 0.4(w)/debugger_interface.m Fri, 29 Oct 1999 16:11:44 +0200 jahier (collect/1_debugger_i 1.2 640)
@@ -31,7 +31,7 @@
 
 :- implementation.
 :- import_module io, require.
-:- import_module list, bool, std_util.
+:- import_module list, bool, std_util, dl.
 :- import_module interactive_query.
 
 dummy_pred_to_avoid_warning_about_nothing_exported.
@@ -656,6 +656,19 @@
 	% of the type_info a collecting variable.
 get_collecting_variable_type(CollectVar, Type) :-
 	Type = type_of(CollectVar).
+
+
+%------------------------------------------------------------------------------%
+
+:- pred display_close_result(dl__result, io__state, io__state).
+:- mode display_close_result(in, di, uo) is det.
+:- pragma export(display_close_result(in, di, uo), "ML_DI_display_close_result").
+
+display_close_result(ok) --> []. 
+display_close_result(error(String)) -->
+	print(String),
+	nl.
+	
 
 %------------------------------------------------------------------------------%
 
Index: 0.4/dl.m
--- 0.4/dl.m Thu, 28 Oct 1999 12:57:51 +0200 jahier (collect/2_dl.m 1.1 640)
+++ 0.4(w)/dl.m Fri, 29 Oct 1999 14:29:42 +0200 jahier (collect/2_dl.m 1.1 640)
@@ -227,6 +227,7 @@
 	make_aligned_string_copy(ErrorMsg, msg);
 }").
 
+:- pragma export(close(in, out, di, uo), "ML_dl__close").
 close(handle(Handle), Result) -->
 	dlclose(Handle), 
 	dlerror(ErrorMsg),
Index: 0.4/mercury_trace_external.c
--- 0.4/mercury_trace_external.c Fri, 29 Oct 1999 09:00:28 +0200 jahier (collect/3_mercury_tr 1.3 640)
+++ 0.4(w)/mercury_trace_external.c Fri, 29 Oct 1999 16:12:44 +0200 jahier (collect/3_mercury_tr 1.3 640)
@@ -31,6 +31,7 @@
 
 #include "debugger_interface.h"
 #include "collect_lib.h"
+#include "dl.h"
 #include "mercury_deep_copy.h"
 #include "std_util.h"
 
@@ -181,7 +182,7 @@
 			String *objet_file_name_ptr);
 static void	MR_get_variable_name(Word debugger_request, String *var_name_ptr);
 static void	MR_trace_browse_one_external(MR_Var_Spec which_var);
-static void	MR_COLLECT_filter(Word (*filter_ptr)(Integer, Integer, Integer, 
+static void	MR_COLLECT_filter(void (*filter_ptr)(Integer, Integer, Integer, 
 			Word, Word, String, String, String, Integer, Integer, 
 			Integer, String, Word, Word *), Unsigned seqno, 
 			Unsigned depth,	MR_Trace_Port port, 
@@ -429,6 +430,8 @@
 	** one we send the result of the collect activity.
 	*/
 
+	Word	*close_result;
+
 	switch(external_debugger_mode) {
 	case MR_searching:
 	       	MR_send_message_to_socket("forward_move_match_not_found");
@@ -438,8 +441,10 @@
 		(*send_collect_result_ptr)(
 			(Word) MR_collecting_variable, 
 			(Word) &MR_debugger_socket_out);
-			#if defined(HAVE_DLFCN_H)&&defined(HAVE_DLCLOSE)
-				dlclose((void *)handle);
+			#if defined(HAVE_DLFCN_H) && defined(HAVE_DLCLOSE)
+				ML_dl__close((Word) handle, 
+					(Word *) &close_result);
+				ML_DI_display_close_result((Word) close_result);
 			#endif
 		break;
 
@@ -460,9 +465,10 @@
 {
 	static Word	search_data;
 	static void	(*initialize_ptr)(Word *) = NULL;
-	static Word    	(*filter_ptr)(Integer, Integer, Integer, MR_Trace_Port,
+	static void    	(*filter_ptr)(Integer, Integer, Integer, MR_Trace_Port,
 				MR_PredFunc, String, String, String, Integer,
-				Integer, Integer, String, Word, Word *) = NULL;
+				Integer, Integer, String, Word, Word *);
+	static void	(*get_collect_var_type_ptr)(Word *);
 	static bool    	collect_linked = FALSE;
 	Integer		debugger_request_type;
 	Integer		live_var_number;
@@ -532,9 +538,9 @@
 			*/
 			MR_COLLECT_filter(
 				*filter_ptr, 
-				(Unsigned) seqno, 
-				(Unsigned) depth, 
-				(MR_Trace_Port) port,
+				seqno, 
+				depth, 
+				port,
 				layout, 
 				path);
 			goto done;
@@ -757,7 +763,6 @@
 			  {
 			        Char	result;
 				Word	MR_collecting_variable_type;
-				Word	*MR_collecting_variable_type_info=NULL;
 
 				if (MR_debug_socket) {
 					fprintf(stderr, "\nMercury runtime: "
@@ -768,9 +773,10 @@
 				MR_TRACE_CALL_MERCURY(
 					ML_DI_link_collect(
 			       		    MR_object_file_name,
-					    (Word *) &filter_ptr,
-					    (Word *) &initialize_ptr,
-					    (Word *) &send_collect_result_ptr,
+					    (void *) &filter_ptr,
+					    (void *) &initialize_ptr,
+					    (void *) &send_collect_result_ptr,
+					    (void *) &get_collect_var_type_ptr,
 					    (Word *) &handle,
 					    &result
 					    ));
@@ -779,11 +785,7 @@
 					MR_send_message_to_socket(
 						"link_collect_succeeded");
 					MR_TRACE_CALL_MERCURY(
-					    ML_DI_get_collecting_variable_type(
-						(Word) 
-						MR_collecting_variable_type_info,
-						MR_collecting_variable,
-						(Word *) 
+					    (*get_collect_var_type_ptr)(
 						&MR_collecting_variable_type));
 					MR_collecting_variable = 
 					    MR_make_permanent(
@@ -818,9 +820,9 @@
 					*/
 					MR_COLLECT_filter(
 						*filter_ptr, 
-						(Unsigned) seqno, 
-						(Unsigned) depth, 
-						(MR_Trace_Port) port,
+						seqno, 
+						depth, 
+						port,
 						layout, 
 						path);
 					
@@ -1438,7 +1440,7 @@
 ** and dynamically link with the execution.
 */
 static void
-MR_COLLECT_filter(Word (*filter_ptr)(Integer, Integer, Integer, Word, Word, 
+MR_COLLECT_filter(void (*filter_ptr)(Integer, Integer, Integer, Word, Word, 
 	String, String, String, Integer, Integer, Integer, String, Word, Word *), 
        	Unsigned seqno, Unsigned depth, MR_Trace_Port port, 
 	const MR_Stack_Layout_Label *layout, const char *path)

 					
--- /home/terre/d02/lande/jahier/collect_filter4/collect_lib.m Fri Oct 29 09:54:12 1999
+++ collect_lib.m       Fri Oct 29 15:42:28 1999
@@ -44,16 +44,16 @@
 :- import_module io, char.
 
 % dynamically link the collect module; 
-:- pred link_collect(string, c_pointer, c_pointer, c_pointer, c_pointer, char,
-       io__state, io__state).
-:- mode link_collect(in, out, out, out, out, out, di, uo) is det.
+:- pred link_collect(string, c_pointer, c_pointer, c_pointer, c_pointer, 
+       c_pointer, char, io__state, io__state).
+:- mode link_collect(in, out, out, out, out, out, out, di, uo) is det.
 
 %------------------------------------------------------------------------------%
 :- implementation.
 :- import_module int, list, std_util, io, char.
 :- import_module dl.
 
-:- pragma export(link_collect(in, out, out, out, out, out, di, uo), 
+:- pragma export(link_collect(in, out, out, out, out, out, out, di, uo), 
        "ML_DI_link_collect").
 
 % We need Handle to be able to close the shared object (dlclose) later on.
@@ -1438,7 +1438,7 @@
 ** and dynamically link with the execution.
 */
 static void
-MR_COLLECT_filter(Word (*filter_ptr)(Integer, Integer, Integer, Word, Word, 
+MR_COLLECT_filter(void (*filter_ptr)(Integer, Integer, Integer, Word, Word, 
 	String, String, String, Integer, Integer, Integer, String, Word, Word *), 
        	Unsigned seqno, Unsigned depth, MR_Trace_Port port, 
 	const MR_Stack_Layout_Label *layout, const char *path)

@@ -61,18 +61,20 @@
 % for performance reasons; indeed, filter will be called at every events
 % so we don't want to pay the price of the maybe variable de-construction
 % at each event.
-link_collect(ObjetFile, Filter, Initialize, SendResult, HandlePtr, Result) -->
+link_collect(ObjectFile, Filter, Initialize, SendResult, GetCollectType, 
+       HandlePtr, Result) -->
        %
        % Link in the object code for the module `collect' from
        % the file `collect.so'.
        %
-       dl__open(ObjetFile, lazy, local, MaybeHandle),
+       dl__open(ObjectFile, lazy, local, MaybeHandle),
        (
                { MaybeHandle = error(Msg) },
                print("dlopen failed: "), print(Msg), nl,
                { set_to_null_pointer(Initialize) },
                { set_to_null_pointer(Filter) },
                { set_to_null_pointer(SendResult) },
+               { set_to_null_pointer(GetCollectType) },
                { set_to_null_pointer(HandlePtr) },
                { Result = 'n' }
        ;
@@ -84,19 +86,23 @@
                dl__sym(Handle, "ML_COLLECT_initialize", MaybeInitialize),
                dl__sym(Handle, "ML_COLLECT_filter", MaybeFilter),
                dl__sym(Handle, "ML_COLLECT_send_collect_result", MaybeSendResult),
+               dl__sym(Handle, "ML_COLLECT_collecting_variable_type", MaybeType),
                (
                        { MaybeInitialize = ok(Initialize0) },
                        { MaybeFilter = ok(Filter0) },
-                       { MaybeSendResult = ok(SendResult0) }
+                       { MaybeSendResult = ok(SendResult0) },
+                       { MaybeType = ok(Type0) }
                ->
                        { Result = 'y' },
                        { Initialize = Initialize0 },
                        { Filter = Filter0 },
+                       { GetCollectType = Type0 },
                        { SendResult = SendResult0 }
                ;
                        { set_to_null_pointer(Initialize) },
                        { set_to_null_pointer(Filter) },
                        { set_to_null_pointer(SendResult) },
+                       { set_to_null_pointer(GetCollectType) },
                        { Result = 'n' }
                ),
                { Handle = handle(HandlePtr) }


--- collect.in.save     Fri Oct 29 15:53:35 1999
+++ collect.in  Fri Oct 29 15:53:38 1999
@@ -20,6 +20,7 @@
        io__state).  
 :- mode send_collect_result(in, in, di, uo) is det.
 
+:- pred collected_variable_type(type_info::out) is det.
 
 %------------------------------------------------------------------------------%
 :- implementation.
@@ -29,6 +30,8 @@
        acc_in, acc_out), "ML_COLLECT_filter").
 :- pragma export(send_collect_result(in, in, di, uo), 
        "ML_COLLECT_send_collect_result").
+:- pragma export(collected_variable_type(out), 
+       "ML_COLLECT_collecting_variable_type").
 
 :- import_module int, io, std_util.
 
@@ -183,6 +186,9 @@
        io__write(OutputStream, Collected),
        io__print(OutputStream, ".\n"),
        io__flush_output(OutputStream).
+
+:- type collect_result --->
+       collected(collected_type).
 


-- 
R1.


--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------



More information about the developers mailing list