Structs with string fields get corrupted

[talex5]

Another GC problem. If you change the example like this:

$ git diff
------ examples/triangle.ml
++++++ examples/triangle.ml
@|-303,11 +303,15 ============================================================
 |    let vert_shader = create_shader "vertex" vert
 |
 |    let make_stage stage module' =
+|      let x =
 |        Vkt.Pipeline_shader_stage_create_info.make
 |          ~stage
 |          ~module'
 |          ~name: "main"
 |          ()
+|      in
+|      Gc.full_major ();
+|      x
 |
 |    let frag_stage = make_stage Vkt.Shader_stage_flags.fragment frag_shader
 |    let vert_stage = make_stage Vkt.Shader_stage_flags.vertex vert_shader

then it fails:

$ make test-triangle
...
Success: Creating render pass
Fatal error: exception File "lib/Vk__Subresult.ml", line 100, characters 13-19: Assertion failed

Also, I'm a bit unsure about this

olivine/aster/structured.ml

Lines 450 to 453 in 61ad088

	let keep_field_alive vars acc = function
	(* | Ty.Simple(_,Array _ ) -> acc *)
	| Ty.Simple(f, _ ) -> ex (M.find @@ varname f) vars :: acc
	| _ -> acc

It seems that Olivine structs hold on to simple fields, but not to arrays. Is that safe?

[talex5]

In particular, Ctypes says:

  val string : string typ
  (** A high-level representation of the string type.
      [...]
      The string type representation is suitable for use in function argument
      types such as the following:

          [string @-> returning int]

      where the lifetime of the C-managed storage does not need to extend beyond
      the duration of the function call.  However, it is not suitable for
      use in struct or union fields

          [field s "x" string]

      because it does not provide a way to manage the lifetime of the
      C-managed storage.
  *)

but Olivine generates:

let name = Ctypes.field ctype "name" Ctypes.string

[talex5]

Unions have the same problem, and also don't keep alive non-string pointers either.

Also, the triangle example does:

olivine/examples/triangle.ml

Lines 85 to 86 in 3de63a8

	let extensions =
	A.of_list Ctypes.string ["VK_KHR_surface"; "VK_KHR_xlib_surface" ]

That also frees the strings on GC.

I have some hacky fixes for these problems. Are you interested in getting more PRs? I feel like I've sent too many already...

[Octachron]

Yes, I fear that the memory handling would need to be properly thought and implemented at some point. Notably, not using Ctype.string when we are passing ownership to the C structure.

In principle, I have nothing against PRs. But as you have seen, the project is only half-way from really working.
And my time for hobby project is very fluctuating, in particular it is quite bad near OCaml releases like this summer.

I still like the spec extraction and analysis part, but the code generation part requires a lot of cleanup.

Also I am not sure if I still agree with my past self design of leaving information data structure on the C side. It might be better to do more copy and translate them fully on the OCaml side to avoid ownership issues.

[talex5]

With the fixes on my branch, I have Olivine working quite well with ocaml-wayland (rather than sdl), so I think it's nearly right!

I don't think using OCaml records fixes anything - you still need to generate the C structs and pass them to the C code at some point and the GC needs to be correct there. Also, Vulkan sometimes has you passing in structs to be filled in, which doesn't work if they get created only temporarily, e.g.

		VkPhysicalDeviceDrmPropertiesEXT ext_drm = {
			.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_DRM_PROPERTIES_EXT,
		};
		VkPhysicalDeviceProperties2 props2 = {
			.sType = VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_PROPERTIES_2,
			.pNext = &ext_drm,
		};
		vkGetPhysicalDeviceProperties2(physicalDevices[i], &props2);
		if (ext_drm.renderMajor == major(wayland_dev) && ext_drm.renderMinor == minor(wayland_dev)) {

Should the types of structs and unions should be exposed? Currently Olivine makes them abstract, but that seems to make the Fields module useless (Ctypes.getf requires a ('a, 'b) Ctypes_static.structured).

It's particularly a problem for unions, where there seems to be no other way of getting the value, e.g. Vk__Types__Performance_value_data_intel.ml is generated as:

type t
val view : t Ctypes.typ
val ctype : t Ctypes.typ
module Fields :
sig
  val value_32 : (Vk__builtin__types.Uint_32_t.t, t) Ctypes.field
  val value_64 : (Vk__builtin__types.Uint_64_t.t, t) Ctypes.field
  val value_float : (Vk__builtin__types.Float.t, t) Ctypes.field
  val value_bool : (Vk__builtin__types.Bool_32.t, t) Ctypes.field
  val value_string : (string, t) Ctypes.field
end
val addr : t -> t Ctypes.ptr
val unsafe_make : unit -> t
val array : t list -> t Ctypes.CArray.t
val value_32 : Vk__builtin__types.Uint_32_t.t -> t
val value_64 : Vk__builtin__types.Uint_64_t.t -> t
val value_float : Vk__builtin__types.Float.t -> t
val value_bool : Vk__builtin__types.Bool_32.t -> t
val value_string : string -> t

On my branch I fixed that with:

 let def types (_,kind as tk) name fields =
-  hidden [%stri type mark ]
-  @* item [%stri type t = [%t kind_cstr kind [%type:mark] ] ] [%sigi: type t]
+  item [%stri type mark] [%sigi: type mark]
+  ^:: item [%stri type t = [%t kind_cstr kind [%type:mark] ] ] [%sigi: type t = [%t kind_cstr kind [%type:mark]]]

Is that OK, or did you have some other plan?