Stream: wasi

Topic: Wiggle support for Union/Struct params in fn signatures

Emiliano Lesende (May 26 2020 at 02:26):

It seems I found a roadblock for implementing the sock_connect proposal. I am getting this:

error: proc macro panicked
 --> crates/wasi/src/lib.rs:9:1
  |
9 | wig::define_wasi_struct_for_wiggle!("phases/snapshot/witx/wasi_snapshot_preview1.witx");
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: message: unsupported argument type

Which after some tracking seems to be because of this function signature:

  ;;; Initiate a connection on a socket to the specified address
  ;;; Note: This is similar to `connect` in POSIX
  (@interface func (export "sock_connect")
    ;;; Socket descriptor
    (param $fd $fd)
    ;;; Address of the local socket (ex '/dev/tmp')
    (param $addr $addr)
    (result $error $errno)
  )

$addr is a union of three different structs.

;;; Union of all possible addresses type
(typename $addr
  (union $addr_type
    (field $local $addr_local)
    (field $ip4 $addr_ip4_port)
    (field $ip6 $addr_ip6_port)
  )
)

I'm afraid that I am not knowledgable enough to implement support for that. Can somebody help me out?
@Dan Gohman @Alex Crichton @Pat Hickey @Jakub Konka

Alex Crichton (May 26 2020 at 14:29):

@Emiliano Lesende for know struct-by-value isn't supported, so you'll want to put it behind a pointer indirection

Alex Crichton (May 26 2020 at 14:29):

e.g. pass a pointer to the sockaddr

Emiliano Lesende (May 26 2020 at 14:30):

That would be (@witx pointer $addr) right?

Alex Crichton (May 26 2020 at 14:35):

I believe so yeah

Emiliano Lesende (May 26 2020 at 14:40):

Thank you @Alex Crichton

Pat Hickey (May 26 2020 at 17:30):

it looks like this is a minor issue in the wig crate, which is the adapter between wasmtime and wiggle. I think the fix is: diff --git a/crates/wasi-common/wig/src/wasi.rs b/crates/wasi-common/wig/src/wasi.rs
index 579e6f3ca..80f7f2f90 100644
--- a/crates/wasi-common/wig/src/wasi.rs
+++ b/crates/wasi-common/wig/src/wasi.rs
@@ -155,7 +155,7 @@ pub fn define_struct(args: TokenStream) -> TokenStream {
}

                 witx::Type::Struct(_) | witx::Type::Union(_) => {

• panic!("unsupported argument type")
• add_param(&name, Abi::I32, true);
  }
  }
  }

Pat Hickey (May 26 2020 at 17:30):

wiggle believes that structs and unions are valid to pass "by value" and automatically expects them to be behind a pointer

Pat Hickey (May 26 2020 at 17:31):

so using a param type of $addr should be equivelant to (@witx const_pointer $addr)

Pat Hickey (May 26 2020 at 17:31):

we pass structs by value in other uses of wiggle over in lucet-land, anyway

Pat Hickey (May 26 2020 at 17:33):

this is the idea of the func.core_type() method in witx.

Pat Hickey (May 26 2020 at 17:34):

(also, sorry, zulip didnt show me the rest of this discussion before i replied)

Emiliano Lesende (May 26 2020 at 17:37):

Thanks @Pat Hickey. I already converted the param to a pointer.

Pat Hickey (May 26 2020 at 17:38):

cool. in the long run, we want to get rid of @witx pointer and const_pointer completely

Pat Hickey (May 26 2020 at 17:38):

since that sort of idea isnt really part of interface types

Pat Hickey (May 26 2020 at 17:38):

so its better to avoid them now for everywhere theyre not absolutely necessary

Pat Hickey (May 26 2020 at 17:38):

but if it doesnt hurt and it keeps you moving, then go for it.

Dan Gohman (May 26 2020 at 18:28):

Yeah, for background here, the reason you don't see any examples of pointer-to-aggregate parameters in WASI is that for structs, it's nicer at the wasm level to just pass the individual values as parameters.

Dan Gohman (May 26 2020 at 18:29):

Of course that doesn't work for unions, but until now we haven't had a need to pass a union that wasn't part of an array (which needs to be in memory anyway)

Last updated: Nov 22 2024 at 16:03 UTC