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.

Sign up for GitHub

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

Jump to bottom

capi: Import inspection functions #683

Merged
merged 1 commit into from
Jan 26, 2021
Merged

capi: Import inspection functions #683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions include/fizzy/fizzy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,38 @@ typedef struct FizzyExternalGlobal
FizzyGlobalType type;
} FizzyExternalGlobal;

/// External kind.
typedef enum FizzyExternalKind
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please compare with https://github.com/WebAssembly/wasm-c-api/blob/master/include/wasm.h#L254-L260.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What do you mean? They're quite similar except that I use full word for "function" and list them in the order of the spec.

Copy link
Collaborator

Choose a reason for hiding this comment

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

They use uint8_t storage type. So it could be used in C++ directly. Or wrap it in C++ for better control.

typedef uint8_t wasm_externkind_t;
enum wasm_externkind_enum {
  WASM_EXTERN_FUNC,
  WASM_EXTERN_GLOBAL,
  WASM_EXTERN_TABLE,
  WASM_EXTERN_MEMORY,
};

// C++

enum class ExternalKind : wasm_externkind_t {
  func = WASM_EXTERN_FUNC,
  global = WASM_EXTERN_GLOBAL,
  table = WASM_EXTERN_TABLE,
  memory = WASM_EXTERN_MEMORY,
};

This may be done other day, but I'm suggesting exploring the design where some declarations are imported from C to C++ instead of only declaring C equivalents of C++ ones.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, this could have been done for FizzyValueType, too 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

True. We may go old-school in this PR and then consider conversion. This probably requires off-site discussion.

Copy link
Member

Choose a reason for hiding this comment

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

I use full word for "function"

I like this better.

list them in the order of the spec.

So is the wasm-c-api not in order with the spec?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

list them in the order of the spec.

So is the wasm-c-api not in order with the spec?

Yeah, there it's func-global-table-memory.

{
FizzyExternalKindFunction,
FizzyExternalKindTable,
FizzyExternalKindMemory,
FizzyExternalKindGlobal
} FizzyExternalKind;

/// Import description.
///
/// @note Only one member of #desc union corresponding to #kind is defined for each import.
/// @note For the lifetime of the #module and #kind fields, please refer to the description provided
/// by the function used to obtain this structure.
typedef struct FizzyImportDescription
Copy link
Member

@axic axic Jan 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Do we need to mention the lifetime of module/name here? It is explained in fizzy_get_import_description.

Perhaps what we could note here is that the user should refer to the documentation from the function returning this value about lifetimes:

Suggested change
typedef struct FizzyImportDescription
//// For the lifetime of the #module and #kind fields, please refer to the description provided by the function used to obtain this structure.
typedef struct FizzyImportDescription

Is #module the correct way?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, #module and #name worked, but I needed to add individual documentation to the members.

{
/// Import's module name.
const char* module;
Copy link
Member

Choose a reason for hiding this comment

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

These fields are documented in #686. It should be consistent, either document here too, or not in #686.

/// Import name.
const char* name;
/// Import kind.
FizzyExternalKind kind;
/// Import type definition.
union
{
FizzyFunctionType function_type;
FizzyLimits memory_limits;
FizzyLimits table_limits;
FizzyGlobalType global_type;
} desc;
} FizzyImportDescription;

/// Imported function.
typedef struct FizzyImportedFunction
{
Expand Down Expand Up @@ -198,6 +230,23 @@ uint32_t fizzy_get_type_count(const FizzyModule* module);
/// @return Type corresponding to the index.
FizzyFunctionType fizzy_get_type(const FizzyModule* module, uint32_t type_idx);

/// Get number of imports defined in the module.
///
/// @param module Pointer to module. Cannot be NULL.
/// @return Number of imports in the module.
uint32_t fizzy_get_import_count(const FizzyModule* module);

/// Get the import description defined in the module.
///
/// @param module Pointer to module. Cannot be NULL.
/// @param import_idx Import index. Behaviour is undefined if index is not valid according
/// to module definition.
/// @return Type of the import corresponding to the index.
/// FizzyImportDescription::module and FizzyImportDescription::name fields
/// point to the string stored inside the module and are valid as long as
/// module is alive (including after successful instantiation.)
FizzyImportDescription fizzy_get_import_description(const FizzyModule* module, uint32_t import_idx);

/// Get type of the function defined in the module.
///
/// @param module Pointer to module. Cannot be NULL.
Expand Down
43 changes: 43 additions & 0 deletions lib/fizzy/capi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,37 @@ inline std::vector<fizzy::ExternalGlobal> unwrap(
external_globals.begin(), unwrap_external_global_fn);
return external_globals;
}

inline FizzyExternalKind wrap(fizzy::ExternalKind kind) noexcept
{
return static_cast<FizzyExternalKind>(kind);
}

inline FizzyImportDescription wrap(
const fizzy::Import& import, const fizzy::Module& module) noexcept
{
FizzyImportDescription c_import_description;
c_import_description.module = import.module.c_str();
c_import_description.name = import.name.c_str();
c_import_description.kind = wrap(import.kind);
switch (c_import_description.kind)
{
case FizzyExternalKindFunction:
c_import_description.desc.function_type =
wrap(module.typesec[import.desc.function_type_index]);
break;
case FizzyExternalKindTable:
c_import_description.desc.table_limits = wrap(import.desc.table.limits);
break;
case FizzyExternalKindMemory:
c_import_description.desc.memory_limits = wrap(import.desc.memory.limits);
break;
case FizzyExternalKindGlobal:
c_import_description.desc.global_type = wrap(import.desc.global);
Copy link
Member

@axic axic Jan 26, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, shouldn't this be wrap(import.desc.global.type)?

Never mind I see it includes both type and mutability.

break;
}
return c_import_description;
}
} // namespace

extern "C" {
Expand Down Expand Up @@ -339,6 +370,18 @@ FizzyFunctionType fizzy_get_type(const FizzyModule* module, uint32_t type_idx)
return wrap(unwrap(module)->typesec[type_idx]);
}

uint32_t fizzy_get_import_count(const FizzyModule* module)
{
return static_cast<uint32_t>(unwrap(module)->importsec.size());
}

FizzyImportDescription fizzy_get_import_description(
const FizzyModule* c_module, uint32_t import_idx)
{
const auto* module = unwrap(c_module);
return wrap(module->importsec[import_idx], *module);
}

FizzyFunctionType fizzy_get_function_type(const FizzyModule* module, uint32_t func_idx)
{
return wrap(unwrap(module)->get_function_type(func_idx));
Expand Down
140 changes: 140 additions & 0 deletions test/unittests/capi_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1197,6 +1197,146 @@ TEST(capi, get_type)
fizzy_free_module(module_imported_func);
}

TEST(capi, get_import_count)
{
/* wat2wasm
(module)
*/
const auto wasm_empty = from_hex("0061736d01000000");

const auto* module_empty = fizzy_parse(wasm_empty.data(), wasm_empty.size());
ASSERT_NE(module_empty, nullptr);

EXPECT_EQ(fizzy_get_import_count(module_empty), 0);
fizzy_free_module(module_empty);

/* wat2wasm
(func (import "m" "f") (result i32))
(global (import "m" "g") i32)
(table (import "m" "t") 0 anyfunc)
(memory (import "m" "m") 1)
*/
const auto wasm = from_hex(
"0061736d010000000105016000017f021d04016d01660000016d0167037f00016d017401700000016d016d0200"
"01");

const auto* module = fizzy_parse(wasm.data(), wasm.size());
ASSERT_NE(module, nullptr);

EXPECT_EQ(fizzy_get_import_count(module), 4);
fizzy_free_module(module);
}

TEST(capi, get_import_description)
{
/* wat2wasm
(func (import "m" "f1"))
(func (import "m" "f2") (result i32))
(func (import "m" "f3") (param i64))
(func (import "m" "f4") (param f32 f64) (result i64))
(global (import "m" "g1") i32)
(global (import "m" "g2") (mut f64))
(table (import "m" "t") 10 anyfunc)
(memory (import "m" "mem") 1 4)
*/
const auto wasm = from_hex(
"0061736d010000000112046000006000017f60017e0060027d7c017e023f08016d0266310000016d0266320001"
"016d0266330002016d0266340003016d026731037f00016d026732037c01016d01740170000a016d036d656d02"
"010104");

const auto* module = fizzy_parse(wasm.data(), wasm.size());
ASSERT_NE(module, nullptr);
ASSERT_EQ(fizzy_get_import_count(module), 8);

const auto import0 = fizzy_get_import_description(module, 0);
EXPECT_STREQ(import0.module, "m");
EXPECT_STREQ(import0.name, "f1");
EXPECT_EQ(import0.kind, FizzyExternalKindFunction);
EXPECT_EQ(import0.desc.function_type.inputs_size, 0);
EXPECT_EQ(import0.desc.function_type.output, FizzyValueTypeVoid);

const auto import1 = fizzy_get_import_description(module, 1);
EXPECT_STREQ(import1.module, "m");
EXPECT_STREQ(import1.name, "f2");
EXPECT_EQ(import1.kind, FizzyExternalKindFunction);
EXPECT_EQ(import1.desc.function_type.inputs_size, 0);
EXPECT_EQ(import1.desc.function_type.output, FizzyValueTypeI32);

const auto import2 = fizzy_get_import_description(module, 2);
EXPECT_STREQ(import2.module, "m");
EXPECT_STREQ(import2.name, "f3");
EXPECT_EQ(import2.kind, FizzyExternalKindFunction);
ASSERT_EQ(import2.desc.function_type.inputs_size, 1);
EXPECT_EQ(import2.desc.function_type.inputs[0], FizzyValueTypeI64);
EXPECT_EQ(import2.desc.function_type.output, FizzyValueTypeVoid);

const auto import3 = fizzy_get_import_description(module, 3);
EXPECT_STREQ(import3.module, "m");
EXPECT_STREQ(import3.name, "f4");
EXPECT_EQ(import3.kind, FizzyExternalKindFunction);
ASSERT_EQ(import3.desc.function_type.inputs_size, 2);
EXPECT_EQ(import3.desc.function_type.inputs[0], FizzyValueTypeF32);
EXPECT_EQ(import3.desc.function_type.inputs[1], FizzyValueTypeF64);
EXPECT_EQ(import3.desc.function_type.output, FizzyValueTypeI64);

const auto import4 = fizzy_get_import_description(module, 4);
EXPECT_STREQ(import4.module, "m");
EXPECT_STREQ(import4.name, "g1");
EXPECT_EQ(import4.kind, FizzyExternalKindGlobal);
EXPECT_EQ(import4.desc.global_type.value_type, FizzyValueTypeI32);
EXPECT_FALSE(import4.desc.global_type.is_mutable);

const auto import5 = fizzy_get_import_description(module, 5);
EXPECT_STREQ(import5.module, "m");
EXPECT_STREQ(import5.name, "g2");
EXPECT_EQ(import5.kind, FizzyExternalKindGlobal);
EXPECT_EQ(import5.desc.global_type.value_type, FizzyValueTypeF64);
EXPECT_TRUE(import5.desc.global_type.is_mutable);

const auto import6 = fizzy_get_import_description(module, 6);
EXPECT_STREQ(import6.module, "m");
EXPECT_STREQ(import6.name, "t");
EXPECT_EQ(import6.kind, FizzyExternalKindTable);
EXPECT_EQ(import6.desc.table_limits.min, 10);
EXPECT_FALSE(import6.desc.table_limits.has_max);

const auto import7 = fizzy_get_import_description(module, 7);
EXPECT_STREQ(import7.module, "m");
EXPECT_STREQ(import7.name, "mem");
EXPECT_EQ(import7.kind, FizzyExternalKindMemory);
EXPECT_EQ(import7.desc.memory_limits.min, 1);
EXPECT_TRUE(import7.desc.memory_limits.has_max);
EXPECT_EQ(import7.desc.memory_limits.max, 4);

fizzy_free_module(module);
}

TEST(capi, import_name_after_instantiate)
Copy link
Member

Choose a reason for hiding this comment

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

Is this testing the lifetime of the name/namespace pointers?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, checks that it's alive after instantiation, as the documentation claims.

{
/* wat2wasm
(func (import "m" "f1") (result i32))
*/
const auto wasm = from_hex("0061736d010000000105016000017f020801016d0266310000");

const auto* module = fizzy_parse(wasm.data(), wasm.size());
ASSERT_NE(module, nullptr);
ASSERT_EQ(fizzy_get_import_count(module), 1);

const auto import0 = fizzy_get_import_description(module, 0);
EXPECT_STREQ(import0.module, "m");
EXPECT_STREQ(import0.name, "f1");

FizzyExternalFunction host_funcs[] = {{{FizzyValueTypeI32, nullptr, 0}, NullFn, nullptr}};

auto instance = fizzy_instantiate(module, host_funcs, 1, nullptr, nullptr, nullptr, 0);
EXPECT_NE(instance, nullptr);

EXPECT_STREQ(import0.module, "m");
EXPECT_STREQ(import0.name, "f1");

fizzy_free_instance(instance);
}

TEST(capi, get_global_count)
{
/* wat2wasm
Expand Down