-
Notifications
You must be signed in to change notification settings - Fork 45
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
Add struct chaining docs #469
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,83 @@ | ||
# Struct-Chaining {#StructChaining} | ||
|
||
Struct-chaining is a C API pattern using linked lists and dynamic typing to extend existing structs with new members, while maintaining API and ABI compatibility. For example: | ||
|
||
An extensible struct is statically typed. It is the root of a linked list, containing a pointer to the next struct in the chain: | ||
|
||
```c | ||
typedef struct WGPUMyStructBase { | ||
WGPUChainedStruct * nextInChain; | ||
uint32_t x; | ||
} WGPUMyBaseStruct; | ||
``` | ||
|
||
Each extension struct is a "subtype" of @ref WGPUChainedStruct; that is, its first member is a @ref WGPUChainedStruct, so that it can be safely cast to @ref WGPUChainedStruct. This allows the implementation to read its @ref WGPUChainedStruct::sType, which is some value of @ref WGPUSType ("Struct Type") that dynamically identifies the struct's type. (The `sType` may come from an implementation-specific extension; @ref WGPUSType is an "open" enum.) | ||
|
||
Once the implementation identifies the struct by its `sType`, it casts the pointer back to the appropriate struct type in order to access its contents. Setting `sType` incorrectly (or pointing to any type that isn't a subtype of @ref WGPUChainedStruct) causes undefined behavior. | ||
|
||
```c | ||
typedef enum WGPUSType { | ||
// ... | ||
WGPUSType_MyStructExtension1 = /* ... */, | ||
WGPUSType_MyStructExtension2 = /* ... */, | ||
// ... | ||
} WGPUStype; | ||
|
||
typedef struct WGPUMyStructExtension1 { | ||
WGPUChainedStruct chain; // .chain.sType must be WGPUSType_MyStructExtension1 | ||
uint32_t y; | ||
} WGPUMyStructExtension1; | ||
|
||
typedef struct WGPUMyStructExtension2 { | ||
WGPUChainedStruct chain; // .chain.sType must be WGPUSType_MyStructExtension2 | ||
uint32_t z; | ||
} WGPUMyStructExtension2; | ||
``` | ||
|
||
This is used like so: | ||
|
||
```c | ||
WGPUMyStructExtension2 ext2 = WGPU_MY_STRUCT_EXTENSION2_INIT; | ||
// .chain.sType is already set correctly by the INIT macro. | ||
// .chain.next is set to NULL indicating the end of the chain. | ||
ext2.z = 2; | ||
|
||
WGPUMyStructExtension1 ext1 = WGPU_MY_STRUCT_EXTENSION1_INIT; | ||
// .chain.sType is already set correctly by the INIT macro. | ||
// .chain.next may be set in either of two ways, equivalently: | ||
ext1.chain.next = &ext2.chain; | ||
ext1.chain.next = (WGPUChainedStruct*) &ext2; | ||
ext1.y = 1; | ||
|
||
WGPUMyStructBase base = WGPU_MY_STRUCT_BASE_INIT; | ||
// Note: base structs do not have STypes (they are statically typed). | ||
base.nextInChain = &ext1.chain; | ||
base.x = 0; | ||
``` | ||
|
||
The pointer links in a struct chain are all mutable pointers. Whether the structs in the chain are actually mutated depends on the function they are passed to (whether the struct chain is passed as an input or an output). Some structs (e.g. @ref WGPULimits) may be either an input or an output depending on the function being called. | ||
|
||
## Struct-Chaining Error {#StructChainingError} | ||
|
||
A struct-chaining error occurs if a struct chain is incorrectly constructed (in a detectable way). | ||
They occur if and only if: | ||
|
||
- The `sType` of a struct in the chain is not valid _in the context of the chain root's static type_. | ||
- If this happens, the implementation must not downcast the pointer to access the rest of the struct (even if it would know how to downcast it in other contexts). | ||
- Multiple instances of the same `sType` value are seen in the same struct chain. (Note this also detects and disallows cycles.) | ||
- Implementation-specific extensions also _should_ avoid designs that use unbounded recursion (such as linked lists) in favor of iterative designs (arrays or arrays-of-pointers). This is to avoid stack overflows in struct handling and serialization/deserialization. | ||
|
||
Struct chains which are used in device-timeline validation/operations (e.g. @ref WGPUBufferDescriptor in @ref wgpuDeviceCreateBuffer) have their chain errors surfaced asynchronously, like any other validation error. | ||
|
||
Struct chains which are used in content-timeline operations (e.g. @ref OutStructChainError) have their chain errors surfaced synchronously, like other content-timeline validation errors. | ||
|
||
### Out-Struct-Chain Error {#OutStructChainError} | ||
|
||
Operations which take out-struct-chains (e.g. @ref WGPULimits, in @ref wgpuAdapterGetLimits and @ref wgpuDeviceGetLimits, but not in @ref WGPUDeviceDescriptor) handle struct-chaining errors as follows: | ||
|
||
- The output struct and struct chain is not modified. | ||
lokokung marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- The operation produces a @ref SynchronousError (return value and log message). | ||
|
||
## Ownership | ||
|
||
TODO(#264): Document whether `FreeMembers` functions traverse the chain (see @ref ReturnedWithOwnership). |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,3 +6,4 @@ | |
- \subpage Surfaces | ||
- \subpage SentinelValues | ||
- \subpage Strings | ||
- \subpage StructChaining |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit. To clarify this a bit more (at least w.r.t the fuzzer case), it's not just that the same
sType
can't be in the same chain, but that the samesType
should never appear in a chain or member chain on a root struct. Not sure how to word it clearly here...There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(discussed offline)
The first requirement (sType is valid for the type of the chain root) should be the one that validates that. The only gap is if some implementation adds an extension that can actually recurse, which we probably should not allow them to do, so I'll note that.
We should also guarantee that if the implementation doesn't allow the sType in that context, it will never try to downcast the struct. It's the same as if the sType were totally bogus.
In Dawn's wire, Loko came up with a way to guarantee that invalid chains can be serialized and deserialized without hitting serialization errors: if the client doesn't allow an sType in context, convert it to something like
WGPUSType_DawnInvalidChainedStruct
which indicatesstruct WGPUDawnInvalidChainedStruct { WGPUChainedStruct chain; WGPUSType sTypeForErrorMessage; }
. As long as the client and server are in sync the server can always handle this without a deserialization error, and forward it into dawn-native which will produce a validation error. If the client is out of sync or is malicious (e.g. a fuzzer) then if the server sees an sType that's not valid in context it immediately gives up and fails deserialization.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I started trying to write this down and realized there could be legitimate use cases that pass variable-sized data as a linked list (rather than an array or array of pointers), regardless of whether they're using struct chaining. So I'm going to make this a "should", not a "must".