Skip to content

[Object][Wasm] Allow parsing of GC types in type and table sections #79235

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

Merged
merged 14 commits into from
Jan 25, 2024

Conversation

dschuff
Copy link
Member

@dschuff dschuff commented Jan 24, 2024

This change allows a WasmObjectFile to be created from a wasm file even if it uses typed funcrefs and GC types.
It does not significantly change how lib/Object models its various internal types (e.g. WasmSignature,
WasmElemSegment), so LLVM does not really "support" or understand such files, but it is sufficient to parse
the type, global and element sections, discarding types that are not understood. This is useful for low-level binary
tools such as nm and objcopy, which use only limited aspects of the binary (such as function definitions) or deal
with sections as opaque blobs.

This is done by allowing WasmValType to have a value of OTHERREF (representing any unmodeled reference
type), and adding a field to WasmSignature indicating it's a placeholder for an unmodeled reference type (since
there is a 1:1 correspondence between WasmSignature objects and types in the type section).
Then the object file parsers for the type and element sections are expanded to parse encoded reference types and
discard any unmodeled fields.

@dschuff dschuff requested a review from sbc100 January 24, 2024 01:19
@@ -259,10 +259,13 @@ class InputFunction : public InputChunk {
file->codeSection->Content.slice(inputSectionOffset, function->Size);
debugName = function->DebugName;
comdat = function->Comdat;
assert(s.Kind != WasmSignature::Placeholder);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we give a more useful error message here?

Perhaps we simply not set isRelocatableObject() for any object file that contains any of these. Then the linker would simply error on them before getting this far?

Copy link
Member Author

Choose a reason for hiding this comment

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

My guess with the assert was that we should never get here, because we would never expect to see a file with a symbol table that also had GC. But yeah, also enforcing it on entry to the linker makes sense. I wonder if a more specific or meaningful error message would make sense (I guess in this case if you tried to link something that just happened to have some unrelated GC types, the link error would be the same as if there were no linking section).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, perhaps another flag in the object file, ("hasGCTypes").. or perhaps the linker to iterate through the types section to make sure it understands them all and error out if it doesn't?

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a more explicit check in createObjectFile. I don't have test because I haven't yet successfully created a file with both GC types and a linking section that wasn't also rejected by the object file parser. But a little redundancy in the checks seems fine.

@dschuff dschuff merged commit 7f409cd into llvm:main Jan 25, 2024
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.

2 participants