Should importing or exporting a function count as declaring it for ref.func?

[eqrion]

A function must be 'declared' by use outside of the function section in order to be allowed to be used with ref.func.

I recently discovered that just importing a function doesn't count:

(module
  (import "" "" (func $f))
  (func
    ref.func $f
    drop
  )
)

This code fails to validate in V8, SM, and JSC complaining that $f wasn't pre-declared.

This seems odd to me, and I can't think of any reason we shouldn't consider it valid. But maybe I'm missing something?

I then double checked the spec here, and I believe this is the relevant rule in module validation:

The refs field on C is the set of valid targets for ref.func. If I'm reading it right, that means that both exports and imports aren't included in refs.

SM and V8 treat exporting a function as declaring it, while it looks like JSC does not do that. So there's inconsistent handling on exports.

I personally think that exporting/importing a function should count as declaring it. I'm pretty sure imports/exports and declaring was discussed a long time ago, but can't find anything relevant.

cc @tlively @kmiller68 @rossberg

[tlively]

Looks like this was initially discussed here: WebAssembly/reference-types#76. I didn't see any mentions of import or export sections there, though. I believe the original idea was that only instances of ref.func instructions are counted, but that contradicts the descriptions of refs in the typing context: "References: the list of function indices that occur in the module outside functions and can hence be used to form references inside them." That description would seem to include imported and and exported functions.

Given that the real motivation is allowing engines to emit different prologues and epilogues for indirectly callable functions, we should certainly include imported and exported functions as being referencable; there's no telling whether some other module that may import an exported function will call it indirectly, so the engine has to be prepared for that. I would support updating the spec to reflect this.

[fitzgen]

FWIW, exports do seem to be treated as declared in SM (and Wasmtime; I haven't tested V8 or others):

;; Valid in SM and Wasmtime (and maybe others)
(module
  (func $f (export "f")
    ref.func $f
    drop
  )
)

[eqrion]

@fitzgen Yeah, V8 and SM both treat exports as 'declared', while JSC does not. All of the browsers do not treat imports as 'declared' though.

How does Wasmtime handle imports?

[fitzgen]

How does Wasmtime handle imports?

We reject the OP module and do not treat imported functions as declared in our validator:

$ wasm-tools validate ~/scratch/import-declared.wat 
error: func 1 failed to validate

Caused by:
    0: undeclared function reference (at offset 0x1e)

(Note: wasm-tools validate is the same validator used in Wasmtime)

However, in the engine, after the module has been validated and assuming it was not rejected as invalid, we will mark all imported functions as "escaping" (~= spec's "declared") and therefore generate the same set of trampolines and such for imported functions that we will for functions that are exported or declared in a passive element segment. (And these trampolines are required in both cases.)

---

Anyways, to be clear: I am in favor of clarifying/updating the spec to make both imports and exports explicitly declared.

[rossberg]

Interestingly, the test suite even has a test for exports (see ref_func.wast), so that is an oversight in the spec, which is trivial to fix. (It's somewhat surprising that JSC fails an official test. ;) )

For imports, the story is different, though. The whole mechanism is based on checking which function indices appear where. But function imports have the same status as function definitions: neither has an occurrence of a function index, so naturally neither can contribute to the set of refs. I would prefer not to hack that further.

[bvisness]

Surely it's not much of a "hack" to make something ref-able at the point where it is declared? At the point where its index is added to the address space?

[rossberg]

Well, as currently spec'ed, a declaration doesn't (and never needs to) know "its" index. Technically, indices are never "added" to a name space, they are just that, indices into a given array. That's certainly a technicality that can be changed, but given that the whole check already is such a wart, I would prefer to keep it simple and not grow that wart further if it hardly matters in practice.

Maybe a practical factor to consider is that implicit declaration on imports also creates more complexity for tools like module mergers, which will then have to identify all uses of an import and possibly synthesise new element segments.

[eqrion]

Well, as currently spec'ed, a declaration doesn't (and never needs to) know "its" index. Technically, indices are never "added" to a name space, they are just that, indices into a given array. That's certainly a technicality that can be changed, but given that the whole check already is such a wart, I would prefer to keep it simple and not grow that wart further if it hardly matters in practice.

Maybe a practical factor to consider is that implicit declaration on imports also creates more complexity for tools like module mergers, which will then have to synthesise new element segments.

The main reason for me why I think imports should 'declare' is that anything you import will have been exported from somewhere, and that export must have 'declared' it.

Maybe a practical factor to consider is that implicit declaration on imports also creates more complexity for tools like module mergers, which will then have to identify all uses of an import and possibly synthesise new element segments.

Can you elaborate on this? Are you imagining a tool that removes exports as part of merging two separate modules?

I would imagine the simplest way to solve it would be to not remove an export from a function if it's used with ref.func, which I imagine tools already need to be careful of (but correct me if I'm wrong).

[rossberg]

@eqrion, a merger statically links two (or N) modules by resolving export/import pairs between them and then "concatenating" the module bodies. Beyond that, it shouldn't have to do more than shifting indices accordingly. It could keep resolved exports, but it certainly can't keep resolved imports, which is why imports are the bigger issue.

That said, even for exports, multiple "resolved" exports from different modules (when merging more than two) could happen to have a name clash. Or you don't want to expose what becomes an implementation detail of the combined module. So in fact it would be preferable if exports didn't have implicit extra meaning either (and I'd like to shift away a bit from my earlier statement about exports).

Wrt modules, it always comes down to the same observation: attaching any implicit meaning to imports, exports, or module boundaries conflicts with inter-module composability/transformability, notwithstanding intra-module convenience.

[tlively]

For Binaryen's wasm-merge, at least, this is not a problem. Binaryen always synthesizes declarative element segments during module writing anyway.

I think it would be worth updating the spec to make imports referenceable. It should just be a matter of concatenating the indices 0..<number of imported functions> to the current list of indices, which might be a "hack" but really shouldn't be that complex in the grand scheme of things.

[eqrion]

That said, even for exports, multiple "resolved" exports from different modules (when merging more than two) could happen to have a name clash. Or you don't want to expose what becomes an implementation detail of the combined module. So in fact it would be preferable if exports didn't have implicit extra meaning either (and I'd like to shift away a bit from my earlier statement about exports).

I would be nervous about changing V8/SM's behavior so that 'export' doesn't 'declare' a func (if I'm reading you right). It's possible it's web compatible, but I'd be suspicious it's not. If that's the case, then the question is just what to do about imports.

[rossberg]

@tlively, Binaryen is a big hammer doing substantial rewrites, so not necessarily representative. Do we know the same is true for, say, the likes of wasm-ld? Are there others? I alone hacked together several static wasm linkers in the past (a substrate is this one), and none did any of that. For such tools, it would be a breaking change.

Fundamentally, I still think this is entangling two unrelated features that should remain independent. I am not convinced changing it for imports is worth it, given that nobody ever missed it? From my perspective, it'd be doing more harm than good.

@eqrion, fair enough, although I wish we could try.

[tlively]

Do we know the same is true for, say, the likes of wasm-ld?

AFAIK, wasm-ld has no understanding of declarative element segments. It will make sure any referenced function goes into a synthesized active element segment, which is even more of a hammer than binaryen synthesizing declarative segments.

In general any linker that works today will continue working after we make this change. It might end up putting more functions than necessary into element segments, but that's fine.

[rossberg]

In general any linker that works today will continue working after we make this change.

@tlively, how so? A linker that does not do anything special for imports is correct today, but will become incorrect as soon as we assign additional meaning to imports.

[tlively]

I see, you're worried about the case of merging something like this?

(module $a
  (func $f (export "f"))
)

(module $b
  (func $f (import "a" "f"))
  (func $g (drop (ref.func $f))) ;; proposed to be valid
)

(module $ab
  (func $f)
  (func $g (drop (ref.func $f))) ;; invalid!
)

Indeed, a tool that tries to merge modules as simply as possible would need to be updated. But such a tool cannot in general expect to continue working on new modules without being updated for new features, so I don't think this is a problem. FWIW, I don't know of any such tools.