ocamldebug segmentation fault

[ghost]

I have found a situation when a segmentation fault occurs during an ocamldebug session.

If I take a simple main.ml program like this:

module Submodule :
sig

  type t
  val value : t
  val pp : Format.formatter -> t -> unit

end = struct

  type t = unit

  let value = ()

  let pp (fmt : Format.formatter) (_ : t) : unit =
    Format.fprintf fmt "DEBUG: Aux.Submodule.pp"

end

let debug () =
  let _value = Submodule.value in
  ()

;;

debug ();

and compile it:

ocamlc -g -o main main.ml

and start a ocamldebug session:

$ ocamldebug main
	OCaml Debugger version 4.09.0

(ocd) run
Loading program... done.
Time: 447
Program exit.
(ocd) break Main.debug
Breakpoint 1 at 158368: file main.ml, line 20, characters 3-39
(ocd) goto 0
Time: 0
Beginning of program.
(ocd) run
Time: 221 - pc: 158368 - module Main
Breakpoint: 1
20   <|b|>let _value = Submodule.value in
(ocd) install_printer Main.Submodule.pp
The debugger does not contain the code for Main.Submodule.pp.
Please load an implementation of Main first.
(ocd) load "main.cmo"
Error during code loading: The module `Main' is already loaded (either by the main program or a previously-dynlinked library)
(ocd) install_printer Main.Submodule.pp
Fatal error: exception End_of_file
Fatal error: exception End_of_file
Segmentation fault

Am I doing something wrong or is this a bug in ocamldebug?

[damiendoligez]

The problem is that ocamldebug itself already has a module called Main, so when you try to dynlink your own main.cmo things go wrong. You can't load pretty-printing modules that have the same name as modules of the ocamldebug sources.

This limitation should be documented, and ocamldebug should give you a better error message (and not attempt loading at all) when you try to load such a module.

[sonologico]

I'd like to pick this one up if no one has done it yet.

[gasche]

@sonologico feel free!

One further option to work on this issue would be to change the sources of ocamldebug to use prefixed modules (OCamldebug_main.ml, etc.; using module aliases module Foo = OCamldebug_foo to make the change non-invasive in the code itself) so that the naming conflict occurs rarely or never.

[sonologico]

@gasche, your suggestion (so that it happens less often) combined with what @damiendoligez suggested (so that ocamldebug handles and reports it better when it does happen) sound like the way to go

[dra27]

Given that ocamldebug is an executable (so uses all its modules), might a pack be better - there’d only be a single module name (OCamlDebugWhyOnEarthDidYouCallYourPrinterThis 🙂) - I think that’d be entirely a Makefile tweak (and with just one obscure symbol, it doesn’t seem necessary to update docs or, possibly, even worry about the error path?!)

[xavierleroy]

Since everyone is proposing their approach, let me add mine :-)

I agree with @damiendoligez that the first thing to do is debug and fix the existing safeguard. In the example shown in the original report, the second install_printer Main.Submodule.pp should fail exactly like the first one, instead of succeeding and crashing.

Later, to lift the name conflicts, yet another possibility is to look at how the toplevel REPL (ocaml) does it and try to take inspiration. ocaml has plenty of internal modules with silly names like Main, yet the #load directive lets you load a module named Main without confusion.

[dra27]

Hah - I'd figured using expunge would be more complicated! Is that not the case?

[xavierleroy]

Maybe packing is simpler, in which case we could consider it for the toplevel REPL as well in replacement of expunge. My point is that ocaml and ocamldebug both need to load user code dynamically, so it's good to look at them in parallel and try to share a solution.

[nojb]

@sonologico feel free!

One further option to work on this issue would be to change the sources of ocamldebug to use prefixed modules (OCamldebug_main.ml, etc.; using module aliases module Foo = OCamldebug_foo to make the change non-invasive in the code itself) so that the naming conflict occurs rarely or never.

FWIW, a variation of this approach is used by dune: one can avoid renaming the sources by using -o to only prefix compiled unit names.

For example, the debugger modules would be compiled to .cm* files prefixed by ocamldebug__exe__, and the sources would be compiled with -open Ocamldebug__exe where ocamldebug__exe.ml contains a list of aliases:

module Main = Ocamldebug__exe__main
(* etc *)

[gasche]

Yes, Dune is implementing the poor-man namespace system by automatically prefixing module with module aliases this way. In this case I suggested doing it manually, because it makes the build system much easier (to retrofit as a Makefile).

Re. other options: I had understood that we were trying to get rid of -pack completely? Expunging sounds like a potential option but it is also more complex and iirc. can create problems (for example other people willing to implement their own toplevel have to find how to fix it on their own). Personally I find prefixing simpler / more transparent, but I'm not the one working on this issue :-)

[dra27]

Sure, but we haven't got rid of -pack yet, so it doesn't rule out using in a case where it's doing something fractionally superior to poor-man's-namespacing. I was under the impression that removing packs would coincide with the additional of namespacing (so there'd hopefully be a seamless transition). I guess either pack or alias-module can profitably be combined with expunge to have 0 exposed names (rather than just a small or single number of obscure exposed names)

[sonologico]

For transparency: I'm done with deadlines that got shifted around and should get back to this tomorrow.

[github-actions]

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.

[sonologico]

This fell from the back of my mind, because the discussion wasn't going anywhere on #9621. Looking at it now, it seems like everyone is ok with -packing even if they have other preferences. That's a tiny change, so I'll do that later today and undraft that PR.

[github-actions]

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.

[lthls]

This should have been fixed by #9621.