[m-dev.] for review: line numbers in the debugger (part 1)

[Fergus Henderson]

On 10-Nov-1999, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> Support breakpoints on line numbers that correspond to calls.

Breakpoints are part of it; the other part is adding support to the debugger
for display of line numbers (for *all* event types, not just calls).
This should be mentioned in the log message.

> compiler/code_gen.m:
> compiler/pragma_c_gen.m:
> compiler/trace.m:
> 	Include contexts in the events we generate.

That log message confused me for a few minutes.
Reading that, I wondered why you didn't need to also change
library/exception.m to handle exception events.  But for those
events, it just uses the layout struct for the return label,
so it should work fine.

I think that log message should say include contexts _in the
stack layouts_ for the events that we generate.

> compiler/call_gen.m:
> 	Include contexts in the call LLDS instructions.

Could you explain in a bit more detail why the contexts
need to be added in the call instruction?

> runtime/mercury_stack_trace.[ch]:
> 	Extend the code to print stack traces to also optionally print
> 	contexts.
> 
> 	Add some utility predicates currently used by the debugger that could
> 	also be use for debugging gc or for more detailed stack traces.

> diff -u -b -r1.27 continuation_info.m
> --- continuation_info.m	1999/09/17 05:57:50	1.27
> +++ continuation_info.m	1999/10/15 10:22:52
> @@ -133,11 +136,24 @@
>  	% For labels which correspond to a trace port (part (a) above),
>  	% we record information in the first field. Since trace.m generates
>  	% a unique label for each trace port, this field is never updated
> -	% once it is set in pass 2. For labels which correspond to a call
> -	% return, we record information in the second field during pass 4.
> -	% Since a label can serve as the return label for more than once call,
> -	% this field can be updated (by taking the intersection of the live
> -	% variables) after it is set. Since a call may return to the label
> +	% once it is set in pass 2.
> +	%
> +	% For labels which correspond to redoips (part (b) above), we record
> +	% information in the second fied. Since code_info.m generates

s/fied/field/

> +	% For labels which correspond to a call return (part (c) above),
> +	% we record information in the third field during pass 4. If execution
> +	% tracing is turned on, then jumpopt.m will not redirect call return
> +	% addresses, and thus each label will correspond to at most one call
> +	% return. If execution tracing is turned off, jumpopt.m may redirect
> +	% call return addresses, which means that a label can serve as the
> +	% return label for more than once call. In that case, XXXX
> +	% this field can be updated after it is set. This updating requires
> +	% taking the intersection of the live variables, and 

That comment is unfinished, and the XXXX is unexplained.

> @@ -232,30 +240,33 @@
>  			% into a det epilog.
>  			( CallModel = det ; CallModel = semidet ),
>  			map__search(Procmap, RetLabel, Between0),
> -			PrevInstr = livevals(Livevals) 
> +			PrevInstr = livevals(Livevals),
> +			not set__member(RetLabel, LayoutLabels)

Wow, only the second ever `not' in the Mercury compiler source code ;-)
(Peter Ross wrote the first one, AFAIK, in June this year.)
I've gotten so used to seeing `\+' that `not' looks really wierd! ;-)
But using `not' is probably a very good idea.

> Index: compiler/stack_layout.m
...
> +stack_layout__format_label_table(FileName - LineNoMap, yes(SourceFileVector),
> +		LayoutInfo0, LayoutInfo) :-
...
> +%	ProjectCallees = lambda([LabelInfo::in, CalleeRval::out] is det, (
> +%		LabelInfo = _LineNo - (_Label - IsReturn),
> +%		(
> +%			IsReturn = not_a_return,
> +%			CalleeRval = yes(const(int_const(0)))
> +%		;
> +%			IsReturn = unknown_callee,
> +%			CalleeRval = yes(const(int_const(1)))
> +%		;
> +%			IsReturn = known_callee(Label),
> +%			code_util__extract_proc_label_from_label(Label,
> +%				ProcLabel),
> +%			(
> +%				ProcLabel = proc(ModuleName, _, _, _, _, _)
> +%			;
> +%				ProcLabel = special_proc(ModuleName, _, _,
> +%					_, _, _)
> +%			),
> +%			DataAddr = data_addr(ModuleName, proc_layout(Label)),
> +%			CalleeRval = yes(const(data_addr_const(DataAddr)))
> +%		)
> +%	)),

A comment at the top of this code saying
"see below for why this is commented out" would be helpful.

> +% We do not include the callees vector in the table because it makes references
> +% to the proc layouts of procedures from other modules without knowing whether
> +% those modules were compiled with debugging. This works only if all procedures
> +% always have a proc layout structure, which we don't want to require yet.

What is the callees vector supposed to be used for anyway?

> +stack_layout__update_label_table_2(Label, Context, IsReturn) -->
> +	{ term__context_file(Context, File) },
> +	{ term__context_line(Context, Line) },
> +	stack_layout__get_label_tables(LabelTables0),
> +	{ map__search(LabelTables0, File, LabelTable0) ->
> +		( map__search(LabelTable0, Line, LineInfo0) ->
> +			LineInfo = [Label - IsReturn | LineInfo0],
> +			map__det_update(LabelTable0, Line, LineInfo,
> +				LabelTable),
> +			map__det_update(LabelTables0, File, LabelTable,
> +				LabelTables)
> +		;
> +			LineInfo = [Label - IsReturn],
> +			map__det_insert(LabelTable0, Line, LineInfo,
> +				LabelTable),
> +			map__det_update(LabelTables0, File, LabelTable,
> +				LabelTables)
> +		)
> +	; File = "" ->
> +			% We don't have a valid context for this label,
> +			% so we don't enter it into any tables.
> +		LabelTables = LabelTables0

Should you use `not stack_layout__context_is_valid(Context)' here,
rather than `File = ""'?

[I reviewed everything up to compiler/trace.m.]
-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh>  |  of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3        |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------