Safety in the WebAssembly C API

[sunfishcode]

The official C API for WebAssembly should strive for safety to the extent that it can, since security is critically important to many WebAssembly use cases.

C is an inherently unsafe language, and it's not practical to defend against all problems through API design alone. However, C APIs can also introduce their own unique forms of undefined behavior, specific to those APIs. This latter category is practical to address, and addressing it helps people using the API from C (even if it doesn't solve every possible problem), and also people using the API through bindings in other languages.

Some of the WebAssembly C API's unsafety would be impractical to make safe. For example, it would be prohibitively expensive to ensure that a C pointer to a linear memory buffer remains valid for the lifetime of the pointer -- see #105 for some discussion. In such cases, it should be the responsibility of the WebAssembly C API to document the conditions under which it has undefined behavior.

The rest of WebAssembly's C APIs should be designed to avoid introducing unique unsafety. For example, wasm_global_set should type-check the global value, and check for immutability, and provide some reasonable behavior when there is an error -- see #131 for more discussion.

Some cases are not immediately obvious. Should wasm_table_set type-check the new value? Should wasm_instance_new require the user to pass in an array bound for the imports array? Should wasm_func_callback_t require bounds arguments for the args and results arrays? We can discuss the pros and cons of different approaches, but ultimately we should either make these safe or document the unsafety.

I acknowledge that this all entails a fair amount of work. What I'm seeking to start with is just consensus that safety is worth pursuing in an official WebAssembly C API, and then we can talk about logistics separately.

[alexcrichton]

I would personally agree that the C API should strive quite hard to not introduce its own forms of undefined behavior beyond what's already inherent to C.

The C API is likely to be the implementation detail of many language's support, for example wasmtime has a .NET embedding based on the C API. These safe languages do not want to actually hit undefined behavior and need to somehow implement the necessary checks for these APIs. It's far more convenient, consistent, and I'd argue safer, for these checks to be implemented in the C API rather than individually in every usage of the C API.

Additionally wasm is often used in contexts where you're executing untrusted code, and in that context it feels like it makes the most sense to ensure that the host implementation is as safe as can be. Inevitably mistakes will be made and you'd prefer to get a crash due to a mistyped wasm_global_set rather than a silent memory corruption that could later be a serious security vulnerability.

I do think that having a safer C API by default may theoretically slow down some use cases. If these use cases are important enough then we can always add unchecked variants of APIs which have fewer safety checks and more words in the documentation about how to use them safely.

[rossberg]

I suggest that we don't conflate two separate considerations: (1) the API specification, and (2) API implementations.

For implementations, I agree it makes perfect sense to provide a best-effort attempt to check various pre-conditions in API functions. That would be similar to assertions. It may depend on the engine what checks are possible and how costly they are, and they may even provide different levels of checking (as many engines do elsewhere).

However, as usual, there may be reasons why developers might want to turn off those checks in production, like often happens with other assertions. They might involve a varying degree of overhead and not all uses of Wasm might be security critical either, or are sandboxed in other ways.

So the API specification should not prescribe checks against incorrect use of the API as part of its semantics. That would be a very un-C thing to do and would just ask implementations to "break" the specification by providing a "production mode" that would not adhere to the spec. There is no benefit to such a situation, given the role of a low-level C API.

@alexcrichton:

These safe languages do not want to actually hit undefined behavior and need to somehow implement the necessary checks for these APIs. It's far more convenient, consistent, and I'd argue safer, for these checks to be implemented in the C API

C is inherently unsafe. A safe-language API on top will always have to do its own thing to some degree. Where possible, however, it should avoid incorrect usage and undefined behaviour by construction. For example, a richer language than C may easily prevent user-side assignment to immutable globals through its type system. A runtime check inside the C API would be unnecessary.

[sunfishcode]

Personally, I still think this is not a natural or principled way to go about incorrect API usage in a language like C. Adding arbitrary one-off "unsafe" functions -- when everything is unsafe anyway -- kind of reenforces that point.

I filed #133 as an example of a pattern we can follow in other parts of the API, in a principled way.

And I filed #132 to discuss how even though C is an unsafe language, it's useful to differentiate between general unsafety in C and unique safety introduced by the API.

I don't think comparison to the JS API is particularly helpful, since that is (1) for a safe language and (2) not designed for efficiently building safer, more high-level API abstractions on top.

I bring up the JS API in part because an implementation of the JS API, or a similar API for any other safe language which doesn't have the ability to propagate wasm's validation through its own type system, on top of the C API today, would need to perform the same checks. And I propose that instead of requiring all such language bindings to have their own implementations of safety-critical bits of WebAssembly semantics buried within otherwise uninteresting binding code, the C API should provide this functionality.

And in #133 I propose a way for tools that don't want the cost and are willing to take responsibility for their own safety can opt out of the extra checking.

[binji]

Perhaps we should provide a "safer" shim around the underlying C-API for these cases? At least in the case of checking mutability in wasm_global_set (and probably others) we can do the check with the functionality provided by the existing API. I agree that users of the API shouldn't have to reimplement the checks themselves, so providing our own library for this could be a nice middle-ground.

[alexcrichton]

I do not personally believe that the C-API as-is is really all that useful to the majority of possible embeddings of wasm. It forces every single embedder which wants to expose a safe interface to implement all manner of checks. This is error-prone, can lead to inconsistencies, and can be very difficult to keep in sync across multiple implementations (as @sunfishcode mentions as well). I suspect the majority of embeddings desire safety. They are, after all, running wasm code and not native code.

@rossberg it sounds like you're saying that all users of the C API should reimplement these runtime type checks and such. You're also mentioning that it's possible for a sufficiently typed implementation to use the low-level semantics the C API currently exposes and still be safe due to the type system of the embedding language. I agree with both of these points and I'm not disputing them. I'm saying this is the wrong default for an official C API for WebAssembly.

Specifically on the speed aspect I don't really believe that the omitted checks in the C API would really amount to an engine being all that much faster in an actual implementation. One pretty speed-critical aspect, calling functions, still seems vastly inefficient compared to the optimal. Parameters and results are all transferred through memory through a wasm_val_t type which even has an extraneous type field. Compared to a native function call which uses the native ABI this is quite a bit slower.

I would personally argue that regardless of speed the official C API of WebAssembly should focus on safety. If there are numbers to back up this claim showing what the actual speed gains are and why it's worth sacrificing the safety of an API for speed, then in that specific case it seems reasonable to have a documented, explicit, escape hatch (as proposed in #133, and as @binji mentions but the other way around, safe defaults with unsafe escape hatches). I do not think that escape hatches should be provided by default in the API, however, until there are motivating use cases. (I would remove the wasm_global_set_unsafe API for now. myself).

[jgravelle-google]

As a drive-by chime-in:

I agree that we want to avoid introducing unsafety in our APIs where reasonable to avoid. But I do think that we want to at least be able to avoid penalizing languages that either don't need or don't want the overhead.

That would be a very un-C thing to do and would just ask implementations to "break" the specification by providing a "production mode" that would not adhere to the spec

The way to cut that knot seems to me me to be having checked and unchecked versions of the functions in question. In general I think wasm should avoid being very opinionated, because we will hopefully have more people targeting it than we could possibly plan for. Having more than one way to do things is a natural way to make that happen I think.

safe defaults with unsafe escape hatches

+1

[rossberg]

Let's take a step back. Of course we can muse about simple checks like mutable globals on a case-by-case basis. But I think we should strive for something more principled.

We can distinguish 3 broad categories of error you can experience with this API:

1. Errors due to C's inherent unsafety, like malformed pointers etc. These don't directly correspond to anything in Wasm.

2. Errors that are invalid uses of Wasm, not defined by Wasm semantics. In Wasm itself, these are precluded by validation.

3. Errors that are the result of exercising legal Wasm semantics. These mostly correspond to traps.

I think we all agree about categories (1) and (3): the latter should be reflected as traps/errors in the API, the former cannot be prevented in C (but higher-level bindings should not leak these undefined behaviours).

The interesting is the middle category, invalid uses. Validation in Wasm is static type-checking. Obviously, this cannot be mirrored in C (though it could in bindings from languages with more expressive type systems).

First thing to note is that category 2 errors are fundamentally different from category 3 errors: category 2 corresponds to bugs in the embedder code, while category 3 is expected behaviour. In that sense, category 2 is closer to category 1. In particular, I don't think buggy use of API should be reflected in special return values -- if you run into such an error you're in an inconsistent state that you cannot reliably recover from. If anything, pre-conditions should hence be assertions that "crash hard".

We potentially have a wide range of possible errors in category 2, checking of which can in turn be put into three rough classes:

2a. Properties that are essentially free to check at runtime -- for example, the attempt to assign to an immutable global.

2b. Properties that are possible to check at runtime, but may have a non-trivial cost -- for example, because it would require the engine to employ a runtime representation that enables full type reflection on every value or object.

2c. Properties that are impossible to check at runtime -- contrary to popular belief, it is not always possible to replace static type checks with dynamic ones, for example, because the properties require non-local knowledge.

Only 2a checks can reasonably be prescribed by the API spec (though impls are of course free to implement others). However, the boundary between 2a and 2b is fuzzy. It heavily depends on implementation choices of any given engine where a specific check falls. Any check based on implementation assumptions that might not hold for all engines should be considered in class 2b. I think that makes most checks end up being in 2b.

If we agree on all this then I suspect that we are ultimately talking about a small set of one-off errors. Not clear whether it's worth making exceptions for those.

[sunfishcode]

I agree with much of your categorization. This aligns with my comments about how it's not practical to make all APIs safe here, about how we can distinguish between C language unsafety and unique API unsafety here, and about how mutating immutable globals is distinct in the wasm spec from runtime errors here.

To be as clear, I am taking a principled approach here. My principle is:

The official C API for WebAssembly should strive for safety to the extent that it can, since security is critically important to many WebAssembly use cases.

As has been acknowledged throughout this discussion, this principle is compatible with having unsafe escape hatches for efficiency.

If you accept this principle, then I'm happy to discuss details, such as how to reporting errors and so on. However, since this principle is so important to my use cases, if you reject it, my next step will be to take this issue to the WebAssembly CG to discuss paths we might take to a WebAssembly C API which does accept this principle.

[alexcrichton]

@rossberg I suspect you don't intend for it to come off this way, but to me it sounds like you're largely ignoring everything else being said in this thread. I, like @sunfishcode, agree with the categories you've laid out, but your post doesn't address the usability concerns of the C API originally brought up which I've echoed directly. I would support @sunfishcode's conclusion above about taking this to the CG too.

I don't think there's anything un-principled about having safety by default in the C API. If a check is free, that's great! If it's expensive, then if it actually matters we'll have an escape hatch. If it's impossible, then that makes the API almost worthless for safe languages anyway and we probably want to design it differently such that the check is possible (and of course have an escape hatch with less context if still necessary).

I suspect that we are ultimately talking about a small set of one-off errors. Not clear whether it's worth making exceptions for those.

This sort of honestly feels like you're belittling concerns brought up in this thread or outright ignoring them. The set of APIs that I would change are:

• wasm_func_callback_t, wasm_func_callback_with_env_t - these would take integer parameters for how big the parameter/result arrays are. A trap would also be generated if values from different wasm_store_t instances were returned from the callback.
• wasm_func_call - this would also take integers for how big the arrays are. An error is also returned if arguments originate from a different store than the func.
• wasm_global_set - this would be fallible if the wrong type is stored or if the global is immutable. Similarly if the value being set comes from a different store, this would fail.
• wasm_table_set - this would fail if the wrong type is provided, or if the value is from the wrong store.
• wasm_table_grow - this would fail for the same reasons as wasm_table_set
• wasm_instance_new - this would take an integer length of the import array

I would disagree that this is a "small set of one-off errors". This is largely about fundamentally changing how you interact with wasm instances and what sorts of errors are expected. I would also argue that this decision is not a local one to belittle that we can just wash our hands of today, but rather the idioms defined now will affect all future APIs too.

[binji]

As a (maybe interesting) side point: when I recently rewrote the wabt interpreter API, I did this for much of the API. See, for example, the Table Object, providing a safe and unsafe API:

  Result Get(u32 offset, Ref* out) const;
  Result Set(Store&, u32 offset, Ref);
  Result Grow(Store&, u32 count, Ref);
  Result Fill(Store&, u32 offset, Ref, u32 size);
  Result Init(Store&, u32 dst_offset, const ElemSegment&, u32 src_offset, u32 size);
  static Result Copy(Store&, Table& dst, u32 dst_offset, const Table& src, u32 src_offset, u32 size);

  // Unsafe API.
  Ref UnsafeGet(u32 offset) const;

I provided the unsafe API mostly as a convenience (for friendlier return types). That said, I don't care much at all about performance.

I'm curious if @jakobkummerow has thoughts w.r.t. this discussion and the V8 implementation of the wasm-c-api.

[rossberg]

@alexcrichton, I didn't mean to discount what you said. Rather, I am trying to get clarity on what we are talking about. In particular, I'd like to clarify that most of the cases you list will in fact fall into class 2b of that categorisation, i.e., they are infeasible to require in all engines:

• Checking whether some object originates from the same store assumes that you can determine the origin of every heap object. That's not a feasible assumption in general, because an engine may not have its objects pointing back to their store, nor may its heap layout allow for simple address range checks.

• Checking whether some value has the right type assumes that the engine is carrying around enough runtime type information in every value to enable reflection on its precise type. Again, that is not a feasible assumption in general. Most engines for typed languages reuse various representations for different values in multiple types, and there is no way to tell them apart without static type information. You could still try to check that something is at least a valid representation in a given type, but then the checks would become inherently implementation dependent.

IOW, the mere ability to perform such checks already involves various hidden assumptions that would impose substantial constraints on the design of an entire engine, and can induce substantial global cost. Neither can simply be circumvented by providing alternative unsafe functions.

The other sort of check occurring on your list is parameter/result arity. I don't understand in what sense passing the arity separately increases safety, since the API still has no way of checking that it matches the actual array size. All it can verify is that it is the same as the function arity, but then it's just checking the equality of two arguments, which are hence redundant. It doesn't say anything about the actual array or whether it's safe to use it. This is classic category 1 unsafety in C that's unfixable.

So if I filter out these 3 sorts of checks from your list then that leaves exactly one item: wasm_global_set checking that the global is mutable. I hope that makes clearer why I was displaying scepticism that it's worth it. :)

[sunfishcode]

@rossberg

I'm pursuing the following principle:

The official C API for WebAssembly should strive for safety to the extent that it can, since security is critically important to many WebAssembly use cases.

The C API requires implementations maintain dynamic information to implement wasm_global_type/wasm_globaltype_content, wasm_table_type/wasm_tabletype_element, wasm_func_type/wasm_func_param_arity/wasm_func_result_arity/wasm_functype_params/wasm_functype_results, and wasm_module_imports, so at least wasm_global_set, wasm_table_set, wasm_func_call, and wasm_instance_new could all check at least that level of type and arity information. (And we can have unsafe versions in case any of this is too expensive in any setting.)

I included "to the extent that it can" in the principle to acknowledge that some aspects of API safety will indeed be impractical to implement on all engines. We can talk about lots of specifics, however I'm pursuing a principled approach, and focusing on the principle first. Beyond the specific size of the set of checks that are practical today, it's also about how we approach API design as we add new features, and about how we communicate with users, bindings maintainers, and engine implementers.

[rossberg]

@sunfishcode, a specification makes a normative requirement. If we agree that some things are impractical on some engines, then the "to the extend that they can" qualifier means that it cannot include those. That's all I'm saying. It seems like we agree?

I hope, then, that my previous comment demonstrated that this necessarily includes almost all the properties that have been suggested so far.

To illustrate, here is a simple example for the problem with store origin checks. Assume we added an i32ref type, like we discussed a few times in the past. And consider that you have a global of type i32ref that you assign a value in the API.

• On a 64 bit machine, any i32ref value would hopefully be unboxed. That means that the engine can never check which store it "comes from".

• On a 32 bit machine, some i32ref value will be boxed. The engine can (maybe) check for those values but not for others.

• Another engine might decide, for whatever reason, never to unbox. Such an engine may be able to reject all cases.

So the ability to make this check is inherently implementation-dependent, and thereby a spec-level requirement to check store origin would not be well-defined.

Similarly, you can construct examples exposing implementation-dependent type confusion, e.g. between an i32ref and an i64ref value. Nor is unboxing the only possible hurdle. Just to be sure: the problem with checking types when assigning tables and globals isn't that the tables/globals don't carry the necessary type information, but that the assigned values don't. That makes requiring such checks not well-defined either.

[sunfishcode]

@rossberg Since it is clear that you aren't listening to what I'm saying, I've filed WebAssembly/meetings#533 to pursue this topic further.

[alexcrichton]

@rossberg I understand what you're saying that imposing some of these checks at a spec level may affect the cost of some engines. I do not think you are listening to me/@sunfishcode and acknowledging the cost of having very little safety in the C API. There's lots of reasons this API is tricky, that's what happens with any standards-based API. With a standards-based API though I don't think it's productive to continually try to sweep others' concerns under a rug because you don't think they're meaningful. This can't really be a productive conversation unless we're all actually acknowledging others' API design constraints.

Are you willing to work with us and our design constraints? So far it does not appear as so because our concerns are batted away quickly with concerns about engines/embeddings/specs/etc. You clearly have your own design constraints that I would be happy to work with as well, but this won't work unless you actually take us seriously.

[jakobkummerow]

Since @binji asked...

I think it is generally unfortunate that when writing software, we so often have to choose either checks or performance. For V8's perspective in this particular case, as implementers of this API, we can and will implement whatever the API is, and whatever performance cost that implies. As @sunfishcode points out, we have a bunch of internal metadata available anyway, and would make sure that more is there if needed.

I think @rossberg's categorization is useful for approaching viable solutions. I am very much sympathetic to C/C++ programmers wishing for facilities to help them catch their own programming errors (often being in that boat myself); at the same time I think it is sad when the price for that must be paid in runtime performance. Compile-time checks are best (but often not possible); #ifdef DEBUG checks are a compromise that's often the least bad option. Does it make sense to extend the API to allow more of them? If Func::call took an integer:

own<Trap> Func::call(const Val params[], int params_count, ...) {
#ifdef DEBUG
  assert(params_count == this->type()->params().size();
#else
  // Silence compiler warning about unused {params_count}
#endif
  ...
}

Then that would:
(1) provide a false sense of security on a developer who never runs tests that would cover this case with a Debug-mode binary
(2) impose a performance cost on calls, potentially causing people to ask "why is this new Wasm replacement so much slower than my previous native code?"
(3) not provide meaningful safety or security benefits, because embedders could do:

Val params[] = {foo, bar, baz};
my_func->call(params, 17, ...);

and an embedder who's careful enough to assert on their end that arraysize(params) == 17 might as well assert that arraysize(params) == my_func->type()->params().size() and hence really doesn't need the API's implementation to waste time repeating that check.

I'm not opposed to adding checks that people think make sense to execute every single time a given code path is taken in a Release-mode binary. Personally, I think the array length case illustrated above is an example where this would not make sense, for the reasons given. I also don't think that the reasoning "WebAssembly provides some security, so surely all WebAssembly users would be happy to sacrifice performance for more security" is particularly convincing; I know that some users of this API are choosing WebAssembly for the portability benefits it provides compared to native binaries, and are hoping that the performance cost they have to pay for that remains as small as possible. Which of course doesn't deny that some users might value bulletproof security at all costs.

[rossberg]

@alexcrichton, @sunfishcode, well, I think we all have our part in not properly listening. ;) Of course, I'm happy to discuss further, as I don't think it's so much the principles we disagree about but rather the conclusions.

But I'm a bit at a loss where exactly we disagree. I haven't seen much acknowledgement of my observation that almost all the checks that have been suggested so far are infeasible as normative requirements. My intention wasn't "batting away" but double-checking seemingly over-optimistic assumptions.

It's valuable to define principles, but for them to be useful you want to ensure that something practical can actually be derived from them. That's what I'm trying to find out. Do you agree that's relevant?

[alexcrichton]

@jakobkummerow so FWIW I think we can have quite a lengthy discussion about safety, checks, performance, etc. I personally work a lot in Rust and have very different opinions than you do about runtime checks and the cost of them, but I continue to believe that for this issue the performance of runtime checks matters only insofar as to guide whether "unchecked" variants of APIs are needed. Regardless of their speed I personally believe that the checks should be the default in the API, and that's the best default for an official wasm C API.

@rossberg to be clear, again, what I'm saying is that this should be a collaborative process of satisfying design constraints. One step of this is listening to what others have to say, but the next is then iterating on concerns. One of the main things I'm saying is that safety is so important that performance and design constraints are secondary concerns. I'm of the opinion that nothing should be sacrificed for safety, and then afterwards everything should be done to make it fast and optimal. I've up to this point, personally at least, not been directly addressing your technical points because it feels like you're unable to talk about the C API from this perspective.

If my opinion is that performance/design is secondary to safety, I don't think it's helpful for me directly address the performance concerns you're bringing up because that's, in my opinion, not the primary goal. Once we can agree to talk about this API as "how can we minimize the design constraint impact and performance impact of safety-by-default" then that's a discussion to have. I do not feel we have reached this point.

It's valuable to define principles, but for them to be useful you want to ensure that something practical can actually be derived from them. That's what I'm trying to find out. Do you agree that's relevant?

This honestly is a baffling statement to me in the sense that it sounds like you're saying that my arguments here follow no principles and have no practical or useful impact on the API. Are you saying that you see no practical or useful impact from safety here? If that's the case then that's a whole separate discussion that would need to happen before we even try to talk about any technical details here.

[tlively]

The official C API for WebAssembly should strive for safety to the extent that it can, since security is critically important to many WebAssembly use cases

I think the only disagreement here is differing interpretations of "the extent that it can" based on differing underlying priorities and assumptions, e.g. how important performance is. Another concern is that "safety" means different things to different people. For example, folks have different opinions on whether passing array arity alongside the array has a safety benefit.

@sunfishcode, could you reframe the principle you'd like to pursue so that it is more specific about the technical thresholds and tradeoffs you'd like to consider in the API design? Hopefully that will bring the disagreement to the fore and we will be able to discuss it without talking past one another.

[alexcrichton]

@tlively I think @rossberg summarized possible kinds of safety/errors here well, and so far no one has disagreed with that categorization. The point of this issue is what to do about the second category, which I (and I believe @sunfishcode) believe should be handled via errors in the API rather than unsafe contracts that must be upheld. I won't speak for @sunfishcode, but I previously elaborated that my belief is that speed/design constraints are secondary to safety.

I've also already laid out what I would like to concretely change and @sunfishcode already made one proposal via PR. I personally do not agree there is simply a technical issue to work through (as I mentioned previously), but rather a discussion needs to first happen about the shape of the API and what is expected from consumers (safety or not).

[lukewagner]

One (dare I say, "principled") approach could be:

• define for each C API a set of preconditions in terms of the C API's existing accessors
• define the C API's default semantics to evaluate preconditions, safely returning a well-defined error if they fail
• if sufficient evidence can be shown that realistic perf is impacted by these checks, add some mechanism for turning off the checks (_unchecked variants or dynamic bool flag... TBD)

By defining the preconditions in terms of existing accessors, we are in theory imposing no additional cross-cutting burden on the underlying implementation.

Having not studied all the individual things we think we can check, though: are all the preconditions we'd like expressible in terms of public accessors? (If not, given that a host may want to do these checks itself, maybe they should be.)

[rossberg]

@lukewagner, that's an interesting way to phrase it.

FWIW, almost none of the suggested checks are expressible in terms of the API itself, because they either are concerned with meta-level correctness (e.g., what virtual store an object resides in, or correct usage of C arrays), or because there is no corresponding functionality in Wasm itself (e.g., type reflection on individual Wasm values). They correspond to operations that are deliberately not defined in Wasm.

Furthermore, all errors in the category under discussion can only arise when the host itself has a bug. Hence it should never try to depend on any possible return values programmatically, other than for going into panic mode right away.

[rossberg]

@alexcrichton:

This honestly is a baffling statement to me in the sense that it sounds like you're saying that my arguments here follow no principles

Sorry if it came across that way, that's not what I meant to say. Rather, what I was getting at is that I saw (and I hope this doesn't come across too badly) a disconnect between the principles that I believe you are striving for and assumptions about what they would realistically imply (from my perspective). That makes me think that there are some fundamentally different assumptions hidden somewhere, and it would further the discussion to identify them.

Your recent comment was helpful clarifying some of this. Where we disagree may mostly be a question of prioritisation. IIUC, you basically seem to say that, by default, safety ought to trump everything else, and you are willing to pay a fairly high price.

But if engines were no longer able to, say, apply standard unboxing techniques (cf my example) then that is not a viable approach to impose on every Wasm engine. It would practically be guaranteed that some engines would skip certain checks or add options to disable them. That's a failure mode for a specification.

And I think this tension is way too fundamental to just treat it is as a secondary concern. That is, we cannot argue about principles only in the abstract without considering those implications at the same time.

[sunfishcode]

@luke

Good idea, I've now posted #134 which takes this apporoach. All safety measures in that patch are performed in terms of existing C API predicates.

@rossberg

#134 illustrates numerous safety checks that can be done in terms of existing API predicates. It's a work in progress, and there's more to do, and the API can evolve, but it hopefully provides a clearer picture of the kinds of safety measures I'm talking about.

I've left cross-store checking out of this PR for now. It's an interesting topic, but not required for the other measures to make sense.

@tlively

I think the only disagreement here is differing interpretations of "the extent that it can" based on differing underlying priorities and assumptions, e.g. how important performance is.

There are some safety measures which will be too expensive for some implementations in some scenarios, and some which will be too expensive for any implementation in any scenario. I'm proposing we provide documented unsafe APIs where appropriate.

Another concern is that "safety" means different things to different people. For example, folks have different opinions on whether passing array arity alongside the array has a safety benefit.

To illustrate how passing in array lengths adds safety, consider the hello example. This allocates
an array by making an assumption about the number of imports defined in a wasm module from a different file. The array length can be easily determined by local and wasm-unaware reasoning, while the module import count can't be.

This is especially common in bindings for higher-level languages too, where any dynamic array type can provide its own size in a simple and wasm-unaware way.

[sunfishcode]

@jakobkummerow In the use case of bindings code for many languages, safety isn't optional and static checking isn't practical. It's better for the wasm-c-api implementations to provide this than requiring all such bindings implementations to have their own copies of this wasm-specific logic.

But for users that want unchecked code in production and help catching bugs in development, it would be straightforward to write wrappers with #ifdef DEBUG-style conditionals on top of what I'm proposing:

#ifndef NDEBUG
#define wasm_global_set_emboldened(store, global, val) \
  (assert(!wasm_global_set(store, global, val)))
#else
#define wasm_global_set_emboldened(store, global, val) \
  ((store), wasm_global_set_unchecked(global, val))
#endif

[jakobkummerow]

@sunfishcode I never said that safety (of the overall system) was optional. I said that in C, requiring a (claimed) size to be passed along with every array in a function signature at best makes it a little harder to write incorrect/unsafe code, but provides no reliable guarantees. The only way to ensure effectiveness of such bounds checks is to use an internally consistent data structure, like a C++ std::vector or a Java array or whatnot, but C doesn't provide any of that.

It's better for the wasm-c-api implementations to provide this than requiring all such bindings implementations to have their own copies

Yes, aside from the concern above, that's what this discussion boils down to: should each user of the Wasm C API have the flexibility and the burden to implement exactly those dynamic checks that that scenario needs (because it can't guarantee them at any other time than runtime), or should implementations of the API instead be required to perform such runtime checks for every user, every time? Flexibility is a pro, burden is a con; which weighs more tends to be contentious (because it tends to depend on the use case).
At the risk of repeating myself, "flexibility" in this case allows both higher performance and increased safety.

for users that want [...], it would be straightforward to write wrappers

Yes, just as it would be straightforward to write wrappers that perform dynamic checks, especially if all preconditions can be expressed via existing predicates, like #134 demonstrates.

As I said before, we'll implement whatever the final consensus will be. In particular, I am not opposed to offering both checked and unchecked versions, as an explicit concession that both use cases are valid. As a suggestion to help find agreement on such a compromise: rather than {(implicitly checked) foo(...) and foo_unchecked(...)} or {foo_checked(...) and (implicitly unchecked) foo(...)}, would it make sense to explicitly label both versions, i.e. have {foo_checked(...) and foo_unchecked(...)}?

[sunfishcode]

Indeed; fortunately, we're not trying to provide watertight safety guarantees here.

We don't see this proposal as taking away any flexibility or performance from anyone. The _unchecked functions are available for use cases where those tradeoffs are appropriate.

If there's popular support for names like foo_checked I'll follow it, but otherwise, in the wasm C API, I think it's best to think of the safe functions as the defaults that most use cases should use most of the time, and the _unchecked functions as things appropriate to highlight with more explicit naming.

[jakobkummerow]

fortunately, we're not trying to provide watertight safety guarantees here.

In what sense is that "fortunate"? The title of the whole issue is "Safety in the API". What, precisely, is "safety without watertight safety guarantees"? When someone asks you "is it safe?", and you have to reply "yeah, well, it's probably safe", doesn't that statement undermine itself?

Maybe this would be a good time to stop using the (fairly abstract) words "safe"/"safety" and use a more concrete description of the proposal instead, like "adding precondition checks to functions" or something like that.

[sunfishcode]

I was responding to your observation that passing sizes with every array wouldn't provide a reliable guarantee. I agree, and I observe that we don't need it to. In my opening comment, I discussed how C is a fundamentally unsafe language, and how we can distinguish between hazards common to all C APIs and those specific to wasm. By passing in an array length instead of relying on a length implied by wasm semantics, we move the hazard from the second category into the first, simplifying bindings to other languages and eliminating a pitfall.

My goal here is broader than just adding some precondition checks. wasm_memory_data and wasm_memory_call both have saftey concerns, not yet addressed in my current PR, that go beyond preconditions. There may be others. I am proposing a new approach to undefined behavior which reflects the needs of many wasm embedding use cases: sandboxing untrusted wasm modules and binding to memory-safe languages.