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

Support embed binary file into custom section #2355

Closed
wants to merge 1 commit into from

Conversation

zhuangton
Copy link

Support embed binary file into custom section:
Add (-p + binaryfile name) (-c + cutsom section name) conditional options to insert binary file data into custom fields
Testing method:
Execute the wasmscript.sh script,Complete inserting bin file data into wasm file

Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Can you explain a little more about your use case here?

I guess the text format cannot yet express custom sections (See WebAssembly/design#1153). The final comments on that issue suggest that maybe the annotations proposal can to this?

@@ -139,7 +150,6 @@ int ProgramMain(int argc, char** argv) {
if (Failed(result)) {
WABT_FATAL("unable to read file: %s\n", s_infile);
}

Copy link
Member

Choose a reason for hiding this comment

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

Unrelated whitespace change (revert?)

}
else {
WABT_FATAL("unable to read binary file\n");
}
Copy link
Member

Choose a reason for hiding this comment

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

The above block looks like its indented by 4 instead of 2 spaces

std::vector<uint8_t> custom_data(std::istreambuf_iterator<char>(file), {});
module->customs.emplace_back(Location(0), s_custom_name, custom_data);
}
else {
Copy link
Member

Choose a reason for hiding this comment

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

else goes on the line along with the closing brace above

@@ -0,0 +1,22 @@
#!/bin/bash
Copy link
Member

Choose a reason for hiding this comment

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

Can we give this shell script a better name?

Also, we probably can't have shell scripts in the test setup since we need to be able to run the full suite under windows.

@@ -80,6 +85,12 @@ static void ParseOptions(int argc, char* argv[]) {
parser.AddOption('o', "output", "FILE",
"Output wasm binary file. Use \"-\" to write to stdout.",
[](const char* argument) { s_outfile = argument; });
parser.AddOption('p', "inputfile", "PATHFILE",
"Input binary file. Use \"-\" to get data.",
Copy link
Member

Choose a reason for hiding this comment

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

How about "Input binary for custom section" for the docs and "custom_section_filename" for the name?

"Input binary file. Use \"-\" to get data.",
[](const char* argument) { s_binfile = argument; s_read_binfile = true; });
parser.AddOption('c', "customname", "CUSTOM",
"Input custom section name. Use \"-\" to write custom section.",
Copy link
Member

Choose a reason for hiding this comment

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

How about calling this --append-section? (Since it can only append the end).

What does Use \"-\" to write custom section mean here?

@keithw
Copy link
Member

keithw commented Dec 15, 2023

This seems like useful functionality, but could we do it in a separate command-line program that lets the user add/print/remove a custom section, instead of adding more flags to wat2wasm/wasm2wat?

(I think we want to keep those tools fairly focused on assembly/disassembly if the plan eventually calls for migrating over to the wasm-tools or binaryen codebases -- the ability to edit a module or extract pieces seems like a different thing.)

@sbc100
Copy link
Member

sbc100 commented Dec 15, 2023

I tend to agree. FWIW I think llvm-objcopy can already do this? Is that right @dschuff ?

@dschuff
Copy link
Member

dschuff commented Dec 16, 2023

Yes, llvm-objcopy can add or remove custom sections, to and from files (I'm not sure it can e.g. print them to stdout though).

@zhuangton
Copy link
Author

Thank you for your guidance. I have completed using llvm-objcopy

@zhuangton zhuangton closed this Dec 18, 2023
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.

5 participants