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

Dynlink support for ocamldebug #6792

Closed
vicuna opened this issue Feb 23, 2015 · 19 comments
Closed

Dynlink support for ocamldebug #6792

vicuna opened this issue Feb 23, 2015 · 19 comments

Comments

@vicuna
Copy link

vicuna commented Feb 23, 2015

Original bug ID: 6792
Reporter: @whitequark
Assigned to: @whitequark
Status: confirmed (set by @damiendoligez on 2015-03-18T14:02:35Z)
Resolution: open
Priority: high
Severity: feature
Target version: undecided
Category: tools (ocaml{lex,yacc,dep,debug,...})
Tags: patch
Has duplicate: #6317
Monitored by: @SkySkimmer @whitequark @gasche

Bug description

Gabriel Scherer has asked me to implement Dynlink support for ocamldebug. I'm attaching the patch. The following is an excerpt from our email exchange describing the capabilities of the patch:

----8<----8<----8<----8<----

$ cat >host.ml
let () = print_endline "hello host"; Dynlink.loadfile "plugin.cma"
$ cat >plugin.ml
let () = print_endline "hello plugin"
$ ocamlc -g -a -o plugin.cma plugin.ml
$ ocamlc -g -o host dynlink.cma host.ml
$ ocamldebug ./host
OCaml Debugger version 4.03.0+dev7-2015-02-08

(ocd) s 22462
Loading program... done.
hello host
hello plugin
Time: 22463 - pc: 1:24 - module Plugin
1 let () = print_endline "hello plugin"<|a|>
(ocd) s 3
Time: 22466 - pc: 0:413204 - module Host
1 let () = print_endline "hello host"; Dynlink.loadfile "plugin.cma"<|a|>
(ocd)

The attached patch (against trunk) implements this feature.
Do note that I broke something that was called "machine interface" in
ocamldebug source; every pc (really position inside a code fragment)
is now prefixed with a number and a colon, e.g. 1:24 above.
The first number signifies the index of the code fragment.

Note that the patch includes a few unrelated bugfixes. Specifically:

  • the changes in meta.c probably should be applied to trunk ASAP.
  • the Debugcom.do_go changes also affect correctness of ocamldebug
    even in trunk.

Note that you cannot set a breakpoint before a module is loaded.
However, a new variable "set break_on_load" (on by default) traps
after dynlink, allowing to set a breakpoint. I.e.:

(ocd) r
Loading program... done.
hello host
Time: 22459 - pc: 0:0
Module(s) Plugin loaded.
(ocd) br @ Plugin 1
Breakpoint 1 at 1:24: file plugin.ml, line 1, characters 10-38
(ocd) r
hello plugin
Time: 22464 - pc: 1:24 - module Plugin
Breakpoint: 1
1 let () = print_endline "hello plugin"<|a|>

Annoyingly, ocamlc doesn't place a debug event at the /start/
of the module initialization code, so you can't break before
the first user code executes.

Interestingly, the patch is completely independent of Dynlink.
It will work for any kind of dynamic code loading, provided that
the debug information is also loaded beforehand. Thus it should
in principle work with toplevel, although the thought of the access
mismanagement inherent to frequently forking a process that is built
around mutating terminal state has kept me from actually trying
to run it.

----8<----8<----8<----8<----

As mentioned, this patch fixes some unrelated minor bugs:

  • in ocamldebug, if you passed a value over max_small_int to "step", then some breakpoints or exceptions could be ignored
  • in meta.c, code fragments coming from Dynlink were registered in caml_code_fragments_table twice, and with different digests, which did not broke closure marshalling because of traversing order, but is still unpleasant.

File attachments

@vicuna
Copy link
Author

vicuna commented May 11, 2015

Comment author: @gasche

the changes in meta.c probably should be applied to trunk ASAP.

Do we need these changes in 4.02 as well?

@vicuna
Copy link
Author

vicuna commented May 11, 2015

Comment author: @whitequark

I think it is a bugfix, so yes, but I do not remember for what exactly it is a bugfix.

@vicuna
Copy link
Author

vicuna commented Jun 24, 2015

Comment author: @damiendoligez

Could it be that the "machine interface" that you broke is the Emacs integration? I'll be very sad if I can't use ocamldebug with Emacs anymore.

@vicuna
Copy link
Author

vicuna commented Jun 24, 2015

Comment author: @whitequark

Yes. You'll have to change that code yourself--it should not involve much--since I've never used emacs.

(By "broke" I mean in the sense of API stability, not in the sense that it does not work anymore.)

@vicuna
Copy link
Author

vicuna commented Nov 23, 2015

Comment author: @damiendoligez

I've looked at the patch and it's a rather impressive piece of work. I'm not knowledgeable enough in the debugger to do a detailed review, but this is a low-risk part of the system anyway, so it'll be good to go if you can get someone else to review it (even superficially).

One stylistic point in C: when p is a pointer, please don't write
if (p) ...
but
if (p != NULL) ...

@vicuna
Copy link
Author

vicuna commented Dec 9, 2015

Comment author: @gasche

I just uploaded a minor patch on top of whitequark's patch that removes a (justified) gcc warning on my machine.

To move forward we would need:

  1. to apply Damien's recommendations of using (foo != NULL) rather
    than implicit boolean conversion

  2. to rebase the patch on the current trunk; the backtrace.c part
    conflicts a bit with your own work on toplevel backtraces, but also
    with Frédéric Bour's refactoring (
    Refactor backtrace #211 )

whitequark and myself are currently out of time to do it; any help to do this would be appreciated.

@vicuna
Copy link
Author

vicuna commented Dec 9, 2015

Comment author: @gasche

Note that whitequark's patch applies cleanly on top of the following commit

commit 22a3f9f
Author: Jacques Garrigue
Date: Thu Feb 19 09:57:41 2015 +0000

Fix #6787

git-svn-id: http://caml.inria.fr/svn/ocaml/trunk@15851 f963ae5c-01c2-4b8c-9fe0-0dff7051ff02

@vicuna
Copy link
Author

vicuna commented Apr 17, 2016

Comment author: @gasche

We haven't had the time to work on this, so I'm postponing to the next version. Any external help to rebase the patch would be appreciated.

@vicuna
Copy link
Author

vicuna commented Apr 3, 2017

Comment author: @ppedrot

Any news on this? I'm afraid I'm not qualified enough to understand the C code and do the rebase myself...

@vicuna
Copy link
Author

vicuna commented Jun 26, 2017

Comment author: mdenes

Is there any hope that the OCaml team may consider integrating this patch soon? As Coq is moving towards a plugin-based architecture, it will soon become critical to be able to debug those.

Who is the ocamldebug maintainer? I could at least try to discuss with him/her to understand what is blocking and see how we can make progress. I don't think anybody from the Coq team has the required knowledge to prepare this patch for integration, unfortunately.

@vicuna
Copy link
Author

vicuna commented Jun 26, 2017

Comment author: @whitequark

There is no dedicated ocamldebug maintainer. The resolution of this issue depends on resolving #6792#c15076, after which it can be packed up as a PR and will likely be accepted once the Emacs interface breakage issue is resolved.

I wrote this over two years ago for no other reason than being asked to. No one exactly rushed to integrate it, and now it bitrotted, so now I don't think I want to work on it again other than by being contracted as a consultant, sorry.

@vicuna
Copy link
Author

vicuna commented Dec 20, 2017

Comment author: @xavierleroy

I had dinner with some Coq folks yesterday and they had tears in their eyes because this PR isn't making progress. whitequark is off duty (but thanks for the initial implementation), so who should resume work on this?

@vicuna
Copy link
Author

vicuna commented Dec 20, 2017

Comment author: @gasche

My best bet is Jacques-Henri; he knows about some parts of the OCaml runtime, and he is almost one of the Coq people this days. Last time I asked him to look at this PR, his answer was "I'm writing my manuscript", he may now be defenseless?

@ppedrot
Copy link
Contributor

ppedrot commented Apr 25, 2019

This is still critical for the Coq folks, if not more pressing, so I'm coming back to the front.

Now that we have a bunch of motivated, hired young people like @gasche, @jhjourdan and myself, can't we build up a special force task on this issue? We can have a remote conference or something.

@jhjourdan
Copy link
Contributor

Alright. I promised that I wold do something about this issue, and I should now have a bit of time. Let's see whether I can do something.

@damiendoligez
Copy link
Member

This looks like something that the ocsf could support. @whitequark are you still interested in getting paid to do the work?

cc @gasche

@whitequark
Copy link
Member

@damiendoligez Sure.

@jhjourdan
Copy link
Contributor

@damiendoligez I don't understand. This is fixed by #8654, right ?

@damiendoligez
Copy link
Member

Oh yes you're right, I'm sorry. Let's close this issue then. Thanks for the reminder.

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

6 participants