Skip to content
This repository has been archived by the owner on Aug 17, 2022. It is now read-only.

add defer-call-export instruction to explainer #63

Merged
merged 4 commits into from
Sep 11, 2019
Merged

Conversation

lukewagner
Copy link
Member

As discussed in #60 and the video call today.

@PoignardAzur
Copy link

PoignardAzur commented Aug 30, 2019

Have you considered the case of nested defer calls? Eg, what happens when the following adapter code is run:

(@interface func (export "foobar")
    defer-call-export "free"
)
(@interface implement (import "foobar")
    defer-call-export "foobar"
)

I assume deferred calls would create a new "scope" with its own LIFO defer stack.

@lukewagner
Copy link
Member Author

As the explainer is currently written, with only defer-call-export, the export that is called is necessarily an export of the core wasm module (just as call-export can only synchronously call an export of the core wasm module), and since core wasm doesn't have the defer-call-export instruction, nested defer calls are not possible.

There's a separate topic that should be discussed in a separate issue of adding a call instruction that allows one adapter function to call another adapter function (in a limited, non-recursive manner). This would raise the question of having a defer-call, which would then raise the nested-defer issue you've raised here. That might be a good reason to avoid having a defer-call.


```wasm
(@interface func (export "greeting") (result string)
call-export "greeting_"
memory-to-string "mem" "free"
dup
memory-to-string "mem"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to imply (with the preceding (memory (export "mem") 1) ) that we are exporting our memory. This is unfortunate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get around this in my polyfill by not re-exporting anything by default. I would say (@interface forward (export "mem")) if I wanted to explicitly export something, but absent that it isn't present in the interface. Here for example we try to print the memories that the modules export, but they're undefined because the exports aren't forwarded.

We talked about hiding-by-default behavior in one of the video meetings, and I think it's a good idea. So, given that, this means we're exporting our memory from the core wasm module to the adapter, but not (necessarily) from the adapter to the interface.

Copy link
Member Author

@lukewagner lukewagner Sep 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with Jacob. In fact, the explainer already explains that memory is not exported from the adapted module (unless it is explicitly re-exported via some as-yet-un-introduced @interface statement).

```wasm
(@interface func (export "greeting") (result string)
call-export "greeting_"
defer-call-export "free"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe that it is better to have a general syntactic structure for performing deferred operations. This defer-call-export instruction seems super specific.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer just defer-call and use helper functions to model generic structure, rather than reimplement blocks (with signatures). I'll try to actually get that written up today.

Copy link
Member Author

@lukewagner lukewagner Sep 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A helper function would be more generic and I think we couldn't do anything finer-grained without introducing fancier closures, which sounds more complicated than we need here. By capturing a call to a function (with no free variables), you only have to capture the call's argument values.

That being said, Olivier's question above is interesting: what happens if you have a defer-call from within a defer-called helper function. There are answers, of course, (basically, what promises do) but it makes defer-calling helper functions more complicated than defer-calling exports and so that makes me want to hear concrete use cases for the increased generality.

Also, in the meantime, the explainer doesn't mention helper functions, so I'd be inclined to start with just defer-call-export and reconsider when helper functions are added.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, in the meantime, the explainer doesn't mention helper functions, so I'd be inclined to start with just defer-call-export and reconsider when helper functions are added.

Agree. More so I'm saying I prefer helper functions in response to the desire for more general syntactic structure here, for most of the reasons you describe (e.g. how to arg.get from a defer'd block?). So rather than make a new generic structure, let's reuse the generic structure that is functions. But that's only relevant if/when we want generic structure in the first place.

Note also that `memory-to-string` has no `free` immediate in this example. While
it would be *possible* to do so, since the caller of the adapter is wasm code,
it's simpler and potentially more efficient to let the caller worry about when
to free the string.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is one of the weakest aspects of C's memory model: lack of structure around who is responsible for free-ing allocated memory. It has to be done by application convention; but that is incompatible with generating adapter code automatically. (Our adapter instructions should not be C or C++ specific.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unique_ptr and shared_ptr might be able to do some of this automatically, but yeah in general this will need to be done manually.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Anything to change here or can I resolve this thread?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also keep in mind that there are use cases where you might not want to dynamically allocate/free anything; eg, maybe you're calling log with a constant string, or a string generated on your linear memory stack.

In practice, the host should worry about dynamic allocation when passing data to/from the callee, not the caller.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but such cases are handled by simply not using any defer-call-export (as in the walkthrough's first section).


```wasm
(@interface func (export "greeting") (result string)
call-export "greeting_"
memory-to-string "mem" "free"
dup
memory-to-string "mem"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get around this in my polyfill by not re-exporting anything by default. I would say (@interface forward (export "mem")) if I wanted to explicitly export something, but absent that it isn't present in the interface. Here for example we try to print the memories that the modules export, but they're undefined because the exports aren't forwarded.

We talked about hiding-by-default behavior in one of the video meetings, and I think it's a good idea. So, given that, this means we're exporting our memory from the core wasm module to the adapter, but not (necessarily) from the adapter to the interface.

```wasm
(@interface func (export "greeting") (result string)
call-export "greeting_"
defer-call-export "free"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer just defer-call and use helper functions to model generic structure, rather than reimplement blocks (with signatures). I'll try to actually get that written up today.

proposals/interface-types/Explainer.md Outdated Show resolved Hide resolved
Note also that `memory-to-string` has no `free` immediate in this example. While
it would be *possible* to do so, since the caller of the adapter is wasm code,
it's simpler and potentially more efficient to let the caller worry about when
to free the string.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unique_ptr and shared_ptr might be able to do some of this automatically, but yeah in general this will need to be done manually.

proposals/interface-types/Explainer.md Show resolved Hide resolved
@fgmccabe
Copy link
Contributor

fgmccabe commented Sep 4, 2019 via email

@lukewagner
Copy link
Member Author

@fgmccabe There seems to be some confusion here, so let me try to be more clear: if an interface adapter custom section is present and the host implements Interface Types, than an export from the core module only shows up as an export from the adapted module if it is explicitly re-exported via the adapter. The adapted module is the only module the outside world sees with the core module an encapsulated implementation detail.

@fgmccabe
Copy link
Contributor

fgmccabe commented Sep 4, 2019 via email

@jgravelle-google
Copy link
Contributor

However, someone can still access the wasm module
ignoring the interface adapters.

How? Walk me through the specific way you're thinking of where you can run a wasm module and ignore the interface adapters.

I can think of a couple ways to do it, I just don't think they're problems. Off the top of my head:

  1. you run two modules in a host that doesn't support interface types.
    1. you polyfill it, and that hides the export, and there's no problem
    2. you don't, so the ABIs don't line up and the module either fails to execute or fails to instantiate
  2. you run a module in a host that supports interface types but does sneaky things with the boundaries. This was already a problem because you don't trust your host, and your host can do much worse things to mess with you
  3. you download a 3rd party module, include it in your application in a host that doesn't support interface types, and subvert the expected interface by providing your own polyfill that doesn't hide exports. This is equivalent to dynamic linking with an undocumented ABI. It's subject to break, but works at the moment. You can read memory in ways the library author didn't intend.

3) is probably the most problematic, but least probable because it relies on a host that ignores the adapters. If the host implements interface types at instantiate time there's no room to do it.

@fgmccabe
Copy link
Contributor

fgmccabe commented Sep 4, 2019 via email

@PoignardAzur
Copy link

PoignardAzur commented Sep 5, 2019

I think there's some confusion here as to what threat model wasm is supposed to guard against.

AFAICT, there is nothing that WebAssembly can (or should) do to protect against misuse by a malicious host. Even if we added a "you are not allowed to use the export directly" boolean field, malicious hosts could always manually edit that field out before compiling (or just plain ingore that field).

My understanding of interface types was that they would provide two types of security:

  • Against accidental type errors.
  • Against untrusted module directly accessing data from other untrusted modules.

Both of those can be enforced as long as a host links adapted modules imports to adapted module exports, and doesn't link bare module imports/exports together. Making sure the host handles these cases correctly doesn't seem like something that could or should be enforced by the language.

EDIT: Never mind. I misunderstood what @fgmccabe was saying.

I think what it comes down to is what is the "default" linking model we want wasm to have.

  • The status quo so far is that nothing is shared between modules unless the host explicitly make it so.

  • The esm-integration proposal assumes that everything a module exports is accessible to other modules.

  • Interface types want some things (eg malloc/free) to be exported to the host, but not to other modules, which conflicts with esm-integration.

I'm not sure how to resolve this. Though I'm thinking even the esm-integration model should probably have some way to differentiate between things you want to export to the host, and things you want to export to importing modules.

@lukewagner
Copy link
Member Author

As a follow-up from some offline discussion: I think the source of the confusion here is resolved by the observation that, once a host has implemented the interface types feature, when an interface adapter custom section is present in a module:

  1. it cannot be optionally ignored; the engine necessarily encapsulates the core module with an adapted module
  2. all core module imports/exports must be explicitly forwarded to show up in the adapted module's imports/exports

(This is briefly covered in the walkthrough which explicitly notes that the greeting_ and mem core exports do not show up in the adapted module's exports.)

As a corollary to (2), adding a single @interface definition will cause all core imports/exports to be hidden by default, requiring explicit re-export (perhaps with some syntactic sugar to explicitly request core imports/exports be present in the adapted module by default).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants