Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

segfault avec ocamlopt -pack #4080

Closed
vicuna opened this issue Aug 8, 2006 · 10 comments
Closed

segfault avec ocamlopt -pack #4080

vicuna opened this issue Aug 8, 2006 · 10 comments

Comments

@vicuna
Copy link

vicuna commented Aug 8, 2006

Original bug ID: 4080
Reporter: @alainfrisch
Assigned to: @mshinwell
Status: assigned (set by @alainfrisch on 2017-03-01T15:49:19Z)
Resolution: reopened
Priority: normal
Severity: minor
Category: compiler driver
Tags: patch
Has duplicate: #5333
Related to: #6537
Monitored by: @protz mehdi "Julien Signoles" @hcarty @alainfrisch

Bug description

Un nouveau bug avec ocamlopt -pack:

buzet ~/bug $ cat sub/a.ml
let f x = x + 1

buzet ~/bug $ cat b.ml
let () = Printf.printf "%i\n" (A.A.f 2)

buzet ~/bug $ make opt
ocamlopt -c -for-pack A sub/a.ml
ocamlopt -pack -o a.cmx -I sub/ sub/a.cmx
ocamlopt -c b.ml
ocamlopt -o b a.cmx b.cmx

buzet ~/bug $ ./b
zsh: segmentation fault ./b

Le lambda-code produit par -pack crée une structure récursive, parce que les
noms globaux A et A ne sont pas distingués. Le premier champ du module packé A, qui devrait pointer sur le module A contenant une fonction, se retrouve en fait à pointer sur le module packé A lui-même.

Note: il y a dans la distribution une instance d'un -pack avec comme nom de module inclus le nom du module crée (c'est Camlp4Top); ça ne pose apparemment pas de problème parce que c'est du bytecode.

File attachments

@vicuna
Copy link
Author

vicuna commented Aug 8, 2006

Comment author: @alainfrisch

Avec l'aide de Nicolas, je propose un correctif en deux coups, qui corrige aussi le problème que je signalais dans le bug 3827 (il faut donner les -I pour retouver les .cmx au moment du -pack, alors qu'on donne déjà leur chemin complet).

Premier coup:

  • changer targetname en "#CURRENT#" dans Ampackager.make_package_object;
  • changer modulename en "#CURRENT#" dans l'appel à Translmod.transl_store_implementation, dans Optcompile.implementation;
  • changer current_unit.ui_name en "#CURRENT#" dans Compilenv.get_global_info.

Ceci a pour effet de faire que le lambda-code généré considère que l'unité courante a le nom #CURRENT#, et c'est au moment de faire le lien entre nom global et unit_infos (en particulier ui_symbol) que l'on détecte ce nom spécial.

Le problème est que dans l'exemple qui j'ai donné pour illustrer le bug, après une première fois où tout se passe bien, il y a un nouveau fichier a.cmx qui apparaît. Donc au moment de compiler de nouveau le lambda-code pour le -pack, Compilenv va chercher le a.cmx dans !load_path. On a beau lui donner le chemin complet pour sub/a.cmx et mettre -I sub, il va quand même chercher a.cmx dans le répertoire courant d'abord. À mon avis c'est dommage (cf #3827): si on donne le chemin pour les .cmx explicitement, il n'y a pas de raison d'aller les chercher ailleurs.

Pour corriger ça, je propose d'ajouter dans Compilenv une table qui garde la trace des .cmx déjà chargés:

let cmx_filenames = Hashtbl.create 17

On rajoute alors dans read_unit_info la ligne:

Hashtbl.add cmx_filenames ui.ui_name filename;

Et on remplace dans get_global_info l'appel à find_in_path_uncap par:

try Hashtbl.find cmx_filenames modname
with Not_found -> find_in_path_uncap !load_path (modname ^ ".cmx")

@vicuna
Copy link
Author

vicuna commented Aug 9, 2006

Comment author: @alainfrisch

Autre solution (implementée dans le patch en pièce jointe): faire la traduction identificateur global -> symbole assembleur à la génération du lambda-code, et non plus à la génération du ulambda-code. Le patch traite aussi le bug #3827; de plus il ajoute un warning lorsqu'ocamlopt ne trouve pas de .cmx pour un module référencé (car ça casse si ce module a été compilé avec -for-pack, et de toute manière, ça empêche les optimisations inter-modules).

C'est un petit peu plus intrusif que l'autre solution (mais peut-être plus robuste).

@vicuna
Copy link
Author

vicuna commented Aug 20, 2010

Comment author: @damiendoligez

Ce bug a ete re-signale sur la Caml List le 2010-05-23 par Goswin von Brederlow. Il existe encore en 3.12.0.

@vicuna
Copy link
Author

vicuna commented Jul 2, 2013

Comment author: @damiendoligez

still present in 4.01+dev

@vicuna
Copy link
Author

vicuna commented Feb 6, 2015

Comment author: @damiendoligez

Maybe a much less intrusive solution would be to simply forbid packing a module in a pack of the same name ?

@vicuna
Copy link
Author

vicuna commented Dec 11, 2015

Comment author: @alainfrisch

We now get a stack overflow, as in #6537.

@vicuna
Copy link
Author

vicuna commented Dec 11, 2015

Comment author: @alainfrisch

Fixed by commit 02b1696 on trunk. The example is still broken on the flambda branch, which might need a slightly different fix.

@vicuna
Copy link
Author

vicuna commented Dec 16, 2015

Comment author: @xavierleroy

The commit was causing miscompilation in Camlp4 and was reverted.

@vicuna
Copy link
Author

vicuna commented Mar 2, 2016

Comment author: @alainfrisch

We forgot to remove the entry in Changes.

@github-actions
Copy link

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants