[PATCH] D136400: [llvm-ocaml] Migrate from naked pointers to prepare for OCaml 5

[Josh Berdine via Phabricator via llvm-commits]

jberdine added a comment.

> I'm concerned about the diagnostic handler-related functions.
> ...
> Is the diagnostic handler feature safe?

You are right that the diagnostic handler functionality is (already) dangerous.

If a user-specified handler function raises an OCaml exception (or performs a C longjmp, or similar), pending C++ destructors will not be executed, and so the library should be considered to be in an inconsistent state and no `Llvm.` functions should be called again. Protecting against this sort of thing needs to be done in the implementation of the C API, likely by using a handler function that throws a C++ exception and wrapping every function in LLVM-C to catch this exception and then call the user-registered handler routine. This would be an involved change and definitely orthogonal to the present diff.

It is also true that the current OCaml bindings assume that calling the LLVM-C functions does not end up calling into the OCaml runtime. So the current assumption is that any user-registered handler function should not touch the OCaml runtime. This is similar to the constraint on finalizer functions that can be associated with OCaml "Custom" blocks: https://v2.ocaml.org/manual/intfc.html#s:c-custom . It would be good to document this assumption in llvm.mli, but this is an issue that exists before the present diff and is also orthogonal.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136400/new/

https://reviews.llvm.org/D136400