D guest bindings generator

[QuantumSegfault]

Implements a d subcrate to support generating bindings for the D programming language.

[QuantumSegfault]

This PR is not yet finished. Just opening a draft early to help interested parties keep track.

[alexcrichton]

Thanks! One thing I'd also recommend as you're implementing things is to add D to this enum as well to hook this into the testing infrastructure. You can then add d to this matrix as appropriate with CI-specific steps to setup the compiler and environment. That'll get codegen/runtime tests working and you'll be able to test out your work by dropping files into the tests/runtime/* folder to implement various components. You'll need to write some orchestration for the test runner to know how to compile D programs (you can draw inspiration from how C is organized) too.

Happy to help answer questions about anything in specific if you have them, and if you have any questions about Rust/idioms/etc feel free to leave a comment here and I can dig in. Otherwise I'll leave this be until you're ready, in which case feel free to ping me and I can take a closer look.

[QuantumSegfault]

@alexcrichton

Things are coming along nicely, but there's one question I have so far.

The codegen tests are excellent for verifying the output of generated imports and types bindings.

Problems arise with the testing of the export wrappers. The way I've implemented exports is using D templates, which are not and cannot be semantically analyzed at all until they have been instantiated. Until an implementation is provided, the wrappers can't be tested.

Is this a problem? This gap in the testing? Or are the runtime tests that will be written enough to compensate?

[alexcrichton]

A good question! If I understand the problem right I believe you're looking for this. The --stubs argument to the Rust generator generates a stub implementation of all exports to ensure that they're codegen'd well. That can also be useful from a "getting started with the generated code" perspective. Would something like that make sense for D, and would that solve the problem too?

[QuantumSegfault]

Yeah! That's pretty much the answer I'm looking for. Something that can create a very basic stub (no impl; just declarations of functions and types) that I can pump into the exports wrapper soley for tests. Thanks!

[QuantumSegfault]

@alexcrichton

I'm still working on getting full test coverage, but I'm ready for this to start getting reviewed.

Support for the new async stuff will be tackled in a future PR. Same with map and implements. This PR brings D support up to the feature set needed for WASIp2.

[alexcrichton]

Integration points all look good to me 👍. I can't speak much to D idioms and I'm not looking too closesly at crates/d/src/lib.rs, but that's what'd be in your purview

[alexcrichton]

For the CI failure, can you publish a wit-bindgen-d crate to crates.io and then invite the wasmtime-publish user? I'll configure the crate through that and remove you as an owner afterwards

[QuantumSegfault]

For the CI failure, can you publish a wit-bindgen-d crate to crates.io and then invite the wasmtime-publish user? I'll configure the crate through that and remove you as an owner afterwards

I'll do that closer to when this is all ready. I might find more small things needing tweaking as I flesh out the tests. No point making more releases than necessary.

[QuantumSegfault]

@alexcrichton

Perhaps you have some opinion or suggestions... (e.g. based on the choices made by other bindings)?

---

Up to now I've been hammering this out trying to make the API as symmetric as possible, and deferring certain aspects of memory management to the user.

Import parameters are "borrowing" (caller takes a constant reference; callee retains ownership of memory), and returns are owning, where the user has to free when they are done. D's scope(exit) (similar to Go's defer) makes it not terrible. But you still have to keep that in mind.

Same applies to exports, where for parameters the API provides you a const "borrow" and frees the memory itself. However you have to move any returned memory to the C heap (witClone is provided to handle a deep copy easily).

To keep signatures consistent for parameters vs. arguments, a thin WitList wrapper has to be used instead of D's native slices (due to ABI difference in ptr & length order). This is a very minor inconvenience, but it's not as idiomatic as it could be.

But once resources come into play, this starts getting more difficult. Right now you have to drop own handles yourself when you are done, and there are no guardrails to prevent use-after-free. And I just realized that borrow handles provided to you via parameters must ALSO be dropped. I've been allowing quietly coercing owns into borrows, but now one has to keep track of where the MyResource.Borrow comes from to know whether or not to drop it. Including now iterating over lists deeply to drop all the handles (so now on top of witFree that does both memory free and handle drops, now you need a specialized one just for parameters).

---

So I'm considering maybe bringing in some RAII (and some other D features) to help make this easier to deal with. Specifically, I'm thinking of making resource handles drop themselves on destroy, and disabling copying, making moves explicit. But moving an own handle out of a parameter requires said parameter to be mutable.

Lists, etc. also have to be aware of this.

So I'm thinking maybe switch things up. Split the types used for parameters from the ones use for returns (which requires duplicating all the record types; making two variants). Parameters can be idiomatic D slices (since lifting lists of lists requires extra copies anyway because of all_bits_valid not being true for pointers), and when there are resources handles in play, I can make them take mutable refs instead.

And returns can be wrapped in their own RAII type that handles freeing all the memory and handles contained (if you don't explicitly move particular buffers or handles out).

---

Switching it now would be setting this back, as I have to rework... most of it. Increases the convenience of the bindings, but increases the complexity of the generator.

But better to do it before merging, rather than breaking things later?

[QuantumSegfault]

After some more thought, I think I'll just keep going with the current model, since I'm so close to having something working available.

I might switch things up later. Need to get more feedback first.

[alexcrichton]

FWIW I personally find WIT bindings more-or-less unusable without automatic resource management (e.g. the C bindings are basically unusable). If it's possible to have a --flag of some sort to enable RAII that might also be a reasonable route to go. Nevertheless no need to worry about breaking changes and such, bindings here regularly break currently and are managed by major versions, and you'd be the one in charge basically saying whether it's reasonable to do so or not. Mostly I want to point out there's no expectation that bindings never break after landing.

[QuantumSegfault]

Yeah...fair point. scope(exit) makes it more paletable in D, but if you do that, you still have to manually clear the handle out after it gets consumed. Still not as succinct as the Rust version.

Once you have the way to opt-in to RAII, I'm not sure there's a point in being able to opt-out (it just adds more complexity to the generator). There could always be escape-hatches created for those who really need custom control over the lifetimes.

---

But good to know. I'm still just rounding out the rest of the tests and make small tweaks along the way. I think most of the hard ones are out of the way. Then after this is merged I'll probably work on the refactoring. Probably before introducing async.

Though I notice I'm going through the tests more thoroughly than some other languages? I'm adding D to every possible test. The only other language that does this consistently is Rust.

[alexcrichton]

For test coverage there's no real reason other than someone hasn't gone through just yet. Don't feel obligated to fill everything out for D, but if you'd like to continue there's also of course no issue with that!

[QuantumSegfault]

Alright, this is just about finished for the initial implementation.

However, I'm having trouble with the two resource_alias tests. Could you help?

I can't figure out why, but specifically for these two tests, the [resource-new] is giving garbage handles.

@wasmImport!("[export]test:resource-alias/e1", "[resource-new]x")
pragma(mangle, "baad")
static private extern(C) uint __import_makeNew(void*);

I get strange values like 1114376 (rather than something more typical, low; like 4), and later down the line it results in traps on unknown handle index 1114376

This is on the test (exporting) side.

[alexcrichton]

Hm ok yeah that's quite fishy. My guess is that this is either a bug in the D toolchain or a bug in wasm-tools. Could you share the test.d compiled core module and/or component? And/or could you share a link of how I could install/run D and build locally on x64 linux?

[QuantumSegfault]

Crate published with wasmtime-publish invited as owner.

[QuantumSegfault]

Hm ok yeah that's quite fishy. My guess is that this is either a bug in the D toolchain or a bug in wasm-tools. Could you share the test.d compiled core module and/or component? And/or could you share a link of how I could install/run D and build locally on x64 linux?

Will do that ASAP. So...closer to 4 PM (1 - 2 hours from now)

[QuantumSegfault]

Hm ok yeah that's quite fishy. My guess is that this is either a bug in the D toolchain or a bug in wasm-tools. Could you share the test.d compiled core module and/or component?

test-d.zip

Contains both the core module and component, as well as the generated D bindings.

And/or could you share a link of how I could install/run D and build locally on x64 linux?

You'll need to install LDC (the LLVM-based D compiler): https://github.com/ldc-developers/ldc

You'll need 1.42 (the latest release).

If ldc2 is in your path, running the wit-bindgen tests with language d should work.

I do:

WASMTIME_BACKTRACE_DETAILS=1 cargo run -- test --artifacts target/artifacts --languages d,rust tests/runtime/resource_alias

Let me know if you need more details.

[alexcrichton]

In that core wasm I see:

  (func $_D3wit4test14resource_alias2e17exports1X6__ctorMFNbNcNiNfkZSQCgQCfQCdQBqQBqQBl (;2;) (type 2) (param i32 i32) (result i32)
    (local i32)
    (local.set 2
      (i32.sub
        (global.get $__stack_pointer)
        (i32.const 16)))
    (i32.store offset=12
      (local.get 2)
      (local.get 1))
    (i32.store
      (local.get 0)
      (i32.load offset=12
        (local.get 2)))
    (return
      (local.get 0))
  )

which is called by the only caller of the imported resource constructor:

    (local.set 7
      (call $__wit_import_:export:test:resource_alias__e1__:resource_new:x
        (i32.load offset=16
          (local.get 2))))
    (drop
      (call $_D3wit4test14resource_alias2e17exports1X6__ctorMFNbNcNiNfkZSQCgQCfQCdQBqQBqQBl
        (i32.add
          (local.get 2)
          (i32.const 8))
        (local.get 7)))

this ctor function looks a bit suspect insofar as it decrements the stack pointer but never increments it. The calling function, $_D3wit4test14resource_alias2e17exports1X__T7makeNewTSQBv5XImplZQuFMDFJQsZvZSQCwQCvQCtQCgQCgQCb, seems to allocate/restore 32 bytes unconditionally. I think the bug might be that the stack is getting misaligned here?

[QuantumSegfault]

Interesting...

I don't think alleged stack imbalance is actually a problem. Said stack pointer is never written back out to the global __stack_pointer. So nothing changes from the caller's point of view.

[QuantumSegfault]

Also, I can cut _D3wit4test14resource_alias2e17exports1X__T7makeNewTSQBv5XImplZQuFMDFJQsZvZSQCwQCvQCtQCgQCgQCb out of the loop and still have the problem.

@wasmImport!("[export]test:resource-alias/e1", "[resource-new]x")
pragma(mangle, "baad")
static private extern(C) uint __import_makeNew(void*);

@witExport("test:resource-alias/e1", "x")
struct XImpl {
    uint val;

    @witExport("test:resource-alias/e1", "[constructor]x")
    static X constructor(uint v) {
        auto h = __import_makeNew(null);
        return *cast(X*)&h;
        //return X.makeNew((out typeof(this) self) {
        //    self.val = v;
        //});
    }
}

Whatever I put in h correctly tracks out. e.g. h = 42 gives unknown handle index 42.

This gives

(import "[export]test:resource-alias/e1" "[resource-new]x" (func $baad (;0;) (type 0)))

...

    (func $__wit_export_test:resource_alias__e1:::constructor:x (;26;) (type 0) (param i32) (result i32)
      (local i32 i32)
      global.get $__stack_pointer
      i32.const 16
      i32.sub
      local.set 1
      local.get 1
      global.set $__stack_pointer
      local.get 1
      local.get 0
      i32.store offset=12
      local.get 1
      local.get 1
      i32.load offset=12
      call $_D4test5XImpl11constructorFkZS3witQBg14resource_alias2e17exports1X
      i32.store offset=4
      local.get 1
      local.get 1
      i32.load offset=4
      i32.store offset=8
      local.get 1
      i32.load offset=8
      local.set 2
      local.get 1
      i32.const 16
      i32.add
      global.set $__stack_pointer
      local.get 2
      return
    )

    (func $_D4test5XImpl11constructorFkZS3witQBg14resource_alias2e17exports1X (;27;) (type 0) (param i32) (result i32)
      (local i32 i32)
      global.get $__stack_pointer
      i32.const 16
      i32.sub
      local.set 1
      local.get 1
      global.set $__stack_pointer
      local.get 1
      local.get 0
      i32.store offset=12
      local.get 1
      i32.const 0
      call $baad
      i32.store offset=8
      local.get 1
      i32.load offset=8
      local.set 2
      local.get 1
      i32.const 16
      i32.add
      global.set $__stack_pointer
      local.get 2
      return
    )

[QuantumSegfault]

With -O3 the above optimizes directly to

(func $__wit_export_test:resource_alias__e1:::constructor:x (;9;) (type 0) (param i32) (result i32)
      i32.const 0
      call $baad
    )

As simple as it gets. At -O0 I consistently see 1114368. With -O3, 1114376.

EDIT: At -Oz it is always 0???

This is really strange.

[alexcrichton]

Said stack pointer is never written back out

Oh, right, yes of course!

---

Ok so I double-checked everything related to wit-component and the component itself and everything checks out. I don't know enough D to understand the syntax/semantics of the generated code. I was curious and threw this at an LLM since I've had some success with debugging in the past with that, and I got this output. I don't understand it myself, but if you're ok wading through some LLM outupt it may be helpful?

Otherwise though my guess is leaning towards a bindings generator issue, I just don't know what :(

[QuantumSegfault]

Yeah, sometimes LLMs be useful. Though it can take some coaxing to reach the actual solution.

T : U[] also matches a static array, and a static array parameter is passed by value.

Not quite. U[] matches a slice, and static arrays are implicitly convertable to slices. It will create a slice of (&ret)[0..ret.length], so the initial witList points to the stack memory. witClone then subsequently malloc's a new chunk of memory for the list, and copies that values out from that stack pointer.

This is not the issue

---

Furthermore, the lists aren't a consideration here. The problem isn't a1 (where the trap occurs), but the input to it in runner, which is the direct output of X.makeNew, which ultimately calls [constructor]x in test.

I've checked, and it is not a problem with some sort of corruption occuring between the components. I can return whatever value I want from the constructor and see invalid handle for the same value.

This is simply, [resource-new] in test giving a bad value. But I just can't figure out WHY. And what's so special about this test that the rest work fine.

[QuantumSegfault]

O_o

The LLM was right.

Using explicit ret[].witList.witClone fixed it.

[QuantumSegfault]

So its nothing wrong with the bindings, I was just using the language wrong. -_-

I guess passing a static array like makes a copy of the array THEN slices it.

Thanks!

[QuantumSegfault]

Okay, it's a touch more complicated than that.

I was right in my initial understanding: passing X[1] to a parameter expecting X[] does NOT make a copy. It will slice in-place, as I expected.

But I messed up my witList template so it was inferring the parameter as X[1] (making a copy) before slicing INSIDE implicitly. I fixed the it so it properly takes T[] (and infers T), so it coerces on the outside.

---

Long story short, it works now! And once CI passes, this should be ready to merge! :D

So yeah. Thanks and sorry about that!

[alexcrichton]

Heh no worries I agree LLM output should be always taken with a very large grain of salt :)

Regardless thanks again for this!

[QuantumSegfault]

The published crate needs to be updated to account for the last minute changes to wit_common.d

Or will this just happen for 0.61.0?

[alexcrichton]

Ah yeah that'll get fixed with 0.61, 0.60 won't actually get used anywhere by the CLI at least. I'll in theory be publishing an 0.61 in the not-too-distant future as well

[QuantumSegfault]

I'll in theory be publishing an 0.61 in the not-too-distant future as well

Nice. I would like to get #1602 finished before then though.