A temporary fix Mantis bug #87, which caused all programs compiled with

Estimated hours taken: 12 (mostly in diagnosis)
Branches: main

A temporary fix Mantis bug #87, which caused all programs compiled with
deep profiling to get a sanity check violation when writing out the deep
profiling data.

My detective work tracked the first reported call site that got this violation,
which is in integer.m in the library, where the predicate integer.to_string
calls the function string.++.

(a) String.++ is marked (correctly) as opt_imported.
(b) The test predicate pred_info_is_imported FAILS for opt_imported predicates.
(c) This causes make_rtti_proc_label to put "no" into the pred_is_imported
    field of the rtti_proc_label for string.++.
(d) This rtti_proc_label is put into the element of the call_site_static_data
    list for integer.to_string that corresponds to the call to string.++
    as the rtti_proc_label of the callee.
(e) When the time comes to write out the call_site_static structures,
    layout_out.m invokes make_proc_label_from_rtti to convert this
    rtti_proc_label in the call_site_static_data to a plain, non-RTTI
    proc_label. However, since the current module name ("integer") does not
    match the name of the module that defines the function ("string") AND
    the function is not marked as imported, make_user_proc_label (invoked
    from make_proc_label_from_rtti) believes that this means that this module
    generates code for the callee (as we do for some opt_imported procedures).
(f) For procedures for which we generate target code in a different module
    than their defining module, we generate different labels for them
    than the label for the code for the procedure in its own module.
    We derive the name of a procedure's proc_static structure directly from
    its label, so this difference holds for the names of proc_static structures
    as well as for labels.
(g) This lead to the problem, which was that the call_site_static for this call
    site referred to the proc_static for string.++ defined in integer.c
    (as opposed to string.++ defined in string.c), since integer.c did not
    actually generate code for string.++, there was no such proc_static.
    This should be a link error, but it isn't, due to the requirement on unix
    linkers of being compatible with Fortran's ancient semantics (specifically,
    Fortran's common areas). Instead, the C compiler happily allocates memory
    for the undefined symbol and fills it with zeros as if it were a
    proc_static structure.
(h) The assertion violation in mercury_deep_profiling.c occurs because the
    call_site_static refers to this bogus proc_static, but of course no module
    writes out this proc_static.

It turns out that deep_profiling.m and the code generator both use the same
algorithm to turn the pred_proc_id of string.++ into a label, but the relevant
input to this algorithm, the import_status of string.++, changes between
the invocation of the deep_profiling transformation and the code generator.
The import_status is changed by the dead procedure elimination pass;
it shouldn't be, but it is. The reason why the bug started showing up recently
is my recent diff that moved the dead_procedure elimination pass after the deep
profiling pass.

The temporary fix is to make sure that the deep profiling pass sees the same
status for each procedure as the code generator, by making it invoke the dead
procedure elimination pass itself.

The right fix is to change the representation of import_status, designing it
properly for the first time, but that will take more time. This redesign should
eliminate the need for the dead procedure eliminate pass to change any
statuses.

compiler/deep_profiling.m:
	Make the fix described above.

	Print progress message only with -V, not -v, since this is what
	all the other passes do.

compiler/hlds_out.m:
	When writing out a procedure's deep profiling information, include
	the data structures from which we later construct the procedure's
	proc_static and call_site_static structures.

compiler/code_util.m:
	Replace some if-then-elses with switches.

compiler/proc_label.m:
	Don't export a function that is not used anywhere else.

compiler/type_ctor_info.m:
	Delete a predicate to avoid an ambiguity.
This commit is contained in:
Zoltan Somogyi
2008-11-03 03:08:02 +00:00
parent 58537374bb
commit c58fd846d4
5 changed files with 106 additions and 29 deletions

View File

@@ -108,10 +108,13 @@ make_entry_label(ModuleInfo, PredId, ProcId, Immed) = ProcAddr :-
ProcAddr = make_entry_label_from_rtti(RttiProcLabel, Immed).
make_entry_label_from_rtti(RttiProcLabel, Immed) = ProcAddr :-
( RttiProcLabel ^ proc_is_imported = yes ->
ProcIsImported = RttiProcLabel ^ proc_is_imported,
(
ProcIsImported = yes,
ProcLabel = make_proc_label_from_rtti(RttiProcLabel),
ProcAddr = code_imported_proc(ProcLabel)
;
ProcIsImported = no,
Label = make_local_entry_label_from_rtti(RttiProcLabel, Immed),
ProcAddr = code_label(Label)
).
@@ -129,9 +132,12 @@ make_local_entry_label_from_rtti(RttiProcLabel, Immed) = Label :-
% If we want to define the label or use it to put it into a data
% structure, a label that is usable only within the current C module
% won't do.
( RttiProcLabel ^ proc_is_exported = yes ->
ProcIsExported = RttiProcLabel ^ proc_is_exported,
(
ProcIsExported = yes,
EntryType = entry_label_exported
;
ProcIsExported = no,
EntryType = entry_label_local
),
Label = entry_label(EntryType, ProcLabel)