Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BasicCABI.md: mention how function pointers look like #200

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

yamt
Copy link
Contributor

@yamt yamt commented Feb 20, 2023

No description provided.

BasicCABI.md Outdated
@@ -96,6 +96,15 @@ Bitfield type | Witdh *w* | Range
`signed long long` | 1 to 64 | -2<sup>(w-1)</sup> to 2<sup>(w-1)</sup>-1
`long long`, `unsigned long long` | 1 to 64 | 0 to 2<sup>w</sup>-1

**Function pointers**

A pointer to a function is an index into the table 0 of the module.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
A pointer to a function is an index into the table 0 of the module.
A pointer to a function is an index into table 0 of the module.

Copy link
Member

Choose a reason for hiding this comment

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

I think __indirect_function_table might not be table 0. For example, if the module happens to import a table, since import always come first in the index space there is no way for the linker to arrange for __indirect_function_table to be at index 0.

In other words, any documentation like this should probably refer to the table by name on only.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

on runtime (eg. when calling a host function which takes a function pointer), the name is not available, is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i was confused with linker symbol and export name.
i dropped table 0 assumption.


A pointer to a function is an index into the table 0 of the module.
The type of the table is funcref.
The table needs to be exported with the name `__indirect_function_table`
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if this should be a requirement of the base C ABI. IIRC it is what emscripten does, but emscripten is also focused on the JS embedding (and has other conventions such as other exports that are also beyond the scope of this doc). There are also other ways to accomplish the same objective (for example, a module could export a function that takes the function pointer as an argument, and then calls it itself).
Probably it makes sense for conventions used by particular embeddings or particular SDKs/runtimes to be either left out of this doc or put in a separate section. Is this requirement particularly important to the environment that you are using?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are also other ways to accomplish the same objective (for example, a module could export a function that takes the function pointer as an argument, and then calls it itself).

yes. but sometimes it isn't desirable to tweak the api that way. eg. when it's from a standard. or when it's a port from non-wasm platform.

Probably it makes sense for conventions used by particular embeddings or particular SDKs/runtimes to be either left out of this doc or put in a separate section. Is this requirement particularly important to the environment that you are using?

i'm not familiar with emscripten.

my concern is more about wasi: https://github.com/WebAssembly/WASI/blob/main/legacy/application-abi.md#current-unstable-abi

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

Successfully merging this pull request may close these issues.

3 participants