[llvm-objcopy][WebAssembly] Initial support for wasm in llvm-objcopy

[dschuff]

Summary

Supports dumping, adding, and removing of named custom sections, but
nothing else yet.

[jhenderson]

llvm/test/tools/llvm-objcopy/Wasm/basic-archive-copy.test

11 ↗

(On Diff #231798)

Nit: too many blank lines.

15 ↗

(On Diff #231798)

'##'

89 ↗

(On Diff #231798)

Use '#' characters here.

llvm/test/tools/llvm-objcopy/Wasm/basic-copy.test

1 ↗

(On Diff #231798)

Same as above.

7–8 ↗

(On Diff #231798)

What's stopping llvm-objcopy using the 5-byte LEBs?

FWIW, in ELF, we don't require a basic copy to produce identical output to the input, because the input might be written in an order style (e.g. the section header table might appear in a different place to "normal", there might be gaps in the object which objcopy compresses etc). Assuming this applies for WASM, you shouldn't try to do a binary comparison, but rather test the key things are all identical.

llvm/tools/llvm-objcopy/Wasm/Object.cpp

49 ↗

(On Diff #231798)

Is there any structure that you can do a sizeof operation on or similar to avoid a literal here (and the need of the comment)?

75 ↗

(On Diff #231798)

Put a blank line before each grouping here. Also, comments need leading capital letters and trailing full stops.

llvm/tools/llvm-objcopy/Wasm/Object.h

60 ↗

(On Diff #231798)

You're already in the llvm namespace. Can't you just do wasm::WasmObjectHeader?

62 ↗

(On Diff #231798)

This line of the comment isn't useful. The names of the functions show this. Please delete.

llvm/tools/llvm-objcopy/Wasm/WasmObjcopy.cpp

39–41 ↗

(On Diff #231798)

Should this be an error? In ELF, an empty section is legitimate, and there's no reason it couldn't generate an empty file in this case.

53 ↗

(On Diff #231798)

This may want to be errc::invalid_argument, since the process hasn't failed to parse the file.

114 ↗

(On Diff #231798)

Copy-paste error?

[dschuff]

• Add test that removed section is gone
• Address round 1 of review comments

[dschuff]

Thanks for the detailed review. I just uploaded a diff to address the comments (other than splitting the patch apart).

llvm/test/tools/llvm-objcopy/Wasm/add-section.test

1 ↗

(On Diff #231798)

This started out as a copy of test/tools/llvm-objcopy/ELF/add-section.test which has a similar structure. Since you've asked to start without this functionality anyway, I can back this test out entirely for now.

llvm/test/tools/llvm-objcopy/Wasm/basic-copy.test

7–8 ↗

(On Diff #231798)

Nothing is stopping it; I just decided to match clang's behavior instead of yaml2obj's (as the comment mentions, they are different). I'll just clarify the comment, since as you say, there's not really any reason to change it.

llvm/tools/llvm-objcopy/Wasm/Object.h

60 ↗

(On Diff #231798)

No:
/s/llvm-upstream/llvm-project/llvm/tools/llvm-objcopy/Wasm/Object.h:60:3: error: no type named 'WasmObjectHeader' in namespace 'llvm::objcopy::wasm'; did you mean '::llvm::wasm::WasmObjectHeader'?

llvm/tools/llvm-objcopy/Wasm/WasmObjcopy.cpp

39–41 ↗

(On Diff #231798)

Actually you're right; I believe custom sections can be empty (Wasm has a distinction between "known" sections whose structure is specified by the standard and "custom" sections which are not interpreted by the VM). Since we only try to support custom sections for now, I'll remove this check.

[dschuff]

• Remove add/remove/dump section support

[sbc100]

Great to see this happening.

I wonder if the directory name should be called just wasm.. like e.g. lld/test/wasm and lld/wasm, We do have a mixture of wasm, 'Wasm` and WebAssembly. I've been thinking of WebAssembly as the target and wasm as the object/container format. So in this case I think just wasm makes the most sense for the directory name.

[dschuff]

Do you mean wasm instead of Wasm?

[dschuff]

• rename Wasm directories to wasm

[sbc100]

llvm/test/tools/llvm-objcopy/Wasm/basic-archive-copy.test

1 ↗

(On Diff #231934)

I looks like objcopy might be the exception here but normally when writing tests in a given format such as this we give them the file extension of the format. So this file should be a .yaml maybe? One of the advantages of this is that you get syntax highlighting by default.

Maybe we should land a rename of the existing tests to follow this convention first?

llvm/tools/llvm-objcopy/Wasm/Object.cpp

57 ↗

(On Diff #231934)

Seems like kind of a shame we have so many different places where we write the object format. This existing ones that I know of are:

1. llvm/lib/MC/WasmObjectWriter.cpp
2. llvm/lib/ObjectYAML/WasmEmitter.cpp
3. lld/wasm/Writer.cpp

So this is added a 4th. But maybe its the same for ELF. I wonder if it would make sense to add a writing capability to lib/Object to unify these. I guess they are all subtly different so maybe it doesn't make sense?

llvm/tools/llvm-objcopy/Wasm/WasmObjcopy.cpp

53 ↗

(On Diff #231934)

Is this assert useful? The contract with the Expected is that that the object pointer is valid if its not an error right? Also "deserialize" is a strange word here.. read or load would make more sense to me).

[dschuff]

llvm/tools/llvm-objcopy/Wasm/Object.cpp

57 ↗

(On Diff #231934)

It is a bit odd that there is a completely independent data model of object files here (the ELF one is particularly complex but I don't have a particular intuition about how much duplication there is for ELF). For wasm (assuming we want to model symbols and relocations against custom sections), my hope was that we could continue to treat the known sections and most custom sections as opaque blobs and hopefully reuse the MC logic for symbols, which would mean that this code wouldn't get too much more complex. Not sure about how much work/duplication symbols would be though.

llvm/tools/llvm-objcopy/Wasm/WasmObjcopy.cpp

53 ↗

(On Diff #231934)

I think if Expected holds a pointer/smart pointer, it can still be null?

[jhenderson]

I looks like objcopy might be the exception here but normally when writing tests in a given format such as this we give them the file extension of the format. So this file should be a .yaml maybe? One of the advantages of this is that you get syntax highlighting by default.

Maybe we should land a rename of the existing tests to follow this convention first?

objcopy isn't the exception. It's common practice for YAML tests throughout the binutils to have .test as an extension, because .yaml isn't in the default set of extensions that lit recognises as a test. I don't object to a mass renaming, but it should probably be done for each of the tools and probably needs to include a change in the default suffixes too (which might cause a problem in some places where a separate YAML file is used as an input...).

llvm/test/tools/llvm-objcopy/wasm/basic-archive-copy.test

2

Maybe rephrase this slightly:

"Test a basic copy of an archive containing a WASM object."

16–17

This is probably not needed: you already have basic-copy.test, which shows that objects are copied correctly, so the comparison of the extracted object is probably sufficient.

43

As mentioned in the now-deleted test, please remove the excessive whitespace, so that the Values in any given block line up neatly, but as close as possible to their tags, i.e.

--- !WASM
FileHeader:
  Version: 0x00000001
Sections:
  - Type: TYPE
    Signatures:
      - Index: 0
        ParamTypes:
          - I32
        ReturnTypes:
          - F32
      - Index: 1
        ParamTypes:
          - I32
          - I64
        ReturnTypes: []
# etc
      - Type:   R_WASM_TABLE_INDEX_SLEB
        Index:  0
        Offset: 0x00000006

74

Not knowing anything really about WASM, how much of the stuff in this output is really necessary for the test? Can, e.g. the Body value here be reduced down to a minimal content?

94–95

You don't need these two lines.

llvm/test/tools/llvm-objcopy/wasm/basic-copy.test

5

Prefer -o to shell redirection.

11

Same comments apply to this YAML as in the other test.

llvm/tools/llvm-objcopy/Wasm/Object.cpp

57 ↗

(On Diff #231934)

I can't comment about WASM, but to some degree, ELF llvm-objcopy needs to have a different internal representation of an Object to that of the library, because the existing Object library object file there doesn't really support mutation. That's largely why things are written the way they were. @jakehehrlich might have more comments on that though.

llvm/tools/llvm-objcopy/wasm/Object.h

66

Can we get rid of this for this version (i.e. add it later when they are needed)?

llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp

39

clang-format?

42

I'd probably just change this comment so that it is future proof "unsupported flag specified", or something like that. Also, we usually use lower-case first letter for error/warning messages.

[dschuff]

• Review 2

llvm/test/tools/llvm-objcopy/wasm/basic-archive-copy.test

43

OK, but is it really worthwhile to deviate from the default output format of obj2yaml and hand-edit everything just so there's less whitespace? It doesn't look any more readable to me.

74

I guess for the purposes of this test it doesn't actually matter whether the function body is valid wasm code or not. I do want to have some known (CODE, TYPE, etc) and linking/reloc sections in the test so that we can ensure that both code that is and is not understood by the object file layer doesn't get modified. That also means that the body has to be long enough that 5-byte relocations against it make sense.

94–95

Removing the readobj test above means removing all of this anyway.

llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp

42

That's more future-proof for me, but much less useful for a user. In order to figure out what *is* supported if someone gets this error, they'd have to keep removing flags until it works. I did update the case though.

[jhenderson]

llvm/test/tools/llvm-objcopy/wasm/basic-archive-copy.test

43

Excessive whitespace for me at least makes it harder to read.

An obj2yaml-generated document is not much better than a canned-object file in most test cases. The vast majority (all?) of our ELF tests use YAML that has been hand-written to make sure it covers the things we care about and nothing more. obj2yaml may well generate significantly more output than is needed to test the different bits that are really being implemented, so whilst it may be useful to provide a basis, the output really should be cut down.

llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp

42

Yeah, I'm not particularly happy with how this is implemented in COFF or Mach-O either. Note that when you do add support for a single switch, you'll need to message, and that will just get unwieldy as more options are added. Perhaps we need to figure out a better method of resolving this, probably with some sort of command-line validation layer somewhere that goes over all switches and sees if they're supported.

[alexander-shaposhnikov]

llvm/tools/llvm-objcopy/wasm/Object.h

2

I'd prefer to follow the structure which we use for other formats (MachO, COFF, ELF), i.e. put the description of the model into Object.h/Object.cpp (the class Object etc), and put the classes Reader and Writer into separate files, even if they are small at the moment, they might grow in the future, it makes things a little bit easier to read.

[dschuff]

• Split Reader/Writer into separate files

[dschuff]

I believe I've addressed all the feedback now.

llvm/tools/llvm-objcopy/wasm/Object.h

2

I don't really mind doing this, but I will note that ELF also has its Readers and Writers in Object.h

[jhenderson]

llvm/test/tools/llvm-objcopy/wasm/basic-archive-copy.test

87–88

Minor nit: add an extra space between Kind/Name and their values to match the Index 1 symbol.

llvm/tools/llvm-objcopy/wasm/Object.h

2

I wouldn't mind somebody changing ELF to split them out too. Having them in the same file always confuses me initially.

llvm/tools/llvm-objcopy/wasm/Writer.cpp

27–29

I think we've been trying to get rid of these sort of non-obvious auto usages within llvm-objcopy. Whilst I can tell that S and Header are likely some kind of section and section header respectively, it's not obvious to me what their type is. Please replace them with the concrete type.

32

Does wasm define a constant for this 0 case here?

36

What is the 5 here? Is there a constant for it somewhere if applicable?

41

Same sort of comment as above. What are the 1 and 5?

53

Does wasm have a concept of endianness that we need to care about here?

56

i -> I

Also, I believe the standard LLVM style is:

for (size_t I = 0, S = SectionHeaders.size(); I < S; ++I)

58

Don't use auto here please. It's not clear what the type is again.

[dschuff]

• Review comments 3

llvm/tools/llvm-objcopy/wasm/Writer.cpp

36

No constant (it's just the max encoded LEB size that can encode 32 bits) but added a comment.

53

No, the spec says that these header values here are both little-endian and we are not trying to interpret them anywhere. All the other values in the format are either 1 byte or LEB-encoded.

[jhenderson]

Only one small pair of questions from me, though absolutely others should approve this too.

llvm/tools/llvm-objcopy/wasm/Writer.cpp

36

Okay, thanks. Why 32-bit though? Also, do we need to handle data that is greater in size than max 32-bits in some way?

[dschuff]

Thanks for all your input!
Who else needs to review this? Can you say explicitly, so we can keep this from becoming one of those reviews that hangs in limbo for a long time without any clear path forward?

[dschuff]

llvm/tools/llvm-objcopy/wasm/Writer.cpp

36

The spec (e.g. for the module format) specifies the size of each kind of field, and also specifies how the values are encoded. In the MVP/1.0 version of wasm everything has 32-bit types (with a few exceptions, e.g. i64/f64 constants; but definitely everything in the module format). A future extension will allow 64-bit pointers and memory sizes and will likely also extend some of the module-level encodings as a result. That would likely be treated by LLVM as a separate architecture and possibly a separate object format variant akin to ELFCLASS64.

[alexander-shaposhnikov]

llvm/tools/llvm-objcopy/wasm/Object.h

22

Sorry for the delays with code review, I'll try to catch up on all the diffs in my backlog soon.
I'm not an expert in WASM format, so please take my comments with a grain of salt.

Okay, what would you say to the following approach ?

struct Section {
   uint8_t Type;
   StringRef Name;
   ArrayRef<uint8_t> Content;
};

If in the future we need to add new sections with some new content - we can use a StringSaver NewSectionsContents
(i.e. it can be a field of the class Object) which would be responsible for holding the corresponding buffers (which are not owned by the input object)).

Looks like in this case the code would be a tiny bit cleaner / less verbose.

[alexander-shaposhnikov]

llvm/tools/llvm-objcopy/wasm/Writer.h

33

if I'm not mistaken this method is not really "finalize",
the main thing what it does - it properly encodes Sections + computes the total size.

What would you say to the following reorganization of this code:

Writer.h:

classWriter {
     ....
     Error write(); 

 private:   
     uint64_t totalSize() const { ... }
 };

Writer.cpp:

uint64_t calculateSerializedSectionSize(const Section &S) {
     uint64_t Size = S.Content.size();
     if (S.SectionType == WASM_SEC_CUSTOM)
          Size += (getULEB128Size(S.Name.size()) + S.Name.size());
     return Size;
 }

 void writeSection(const Section &S, uint8_t* Out) {
      ....
 }

 uint64_t Writer::totalSize() const {
     ....
 }

 Error Writer::write() {
     ....
 }

Side note: it looks like in this case we eliminate SectionData - decrease the number of copies/ memory consumption.

[jhenderson]

In D70930#1782155, @dschuff wrote:

Thanks for all your input!
Who else needs to review this? Can you say explicitly, so we can keep this from becoming one of those reviews that hangs in limbo for a long time without any clear path forward?

I'm mostly just interested in getting someone with wasm experience to review this, to make sure there are no wasm-specific details that we might have missed. I don't really have time unfortunately to try to get to grips with the file format myself, otherwise I'd be happy to.

Also, obviously @alexshap's comments should be addressed.

[sbc100]

Looks fine from a wasm perspective.

This is just the very first step towards a full wasm objcopy but its look like a good start to me. Modulo a few comments inline.

llvm/test/tools/llvm-objcopy/wasm/basic-archive-copy.test

43

I feel like for yaml inputs like this should be the verbatim result of compiling some simple input file (either from .ll or .s) and as such should not be modified in place. Instead we should include instructions on how to regenerate it periodically if/when the yaml format changes.

For yaml output checking I agree there could be argument made for trimming since we often only check for a tiny subset.

llvm/test/tools/llvm-objcopy/wasm/basic-copy.test

14

How was this yaml file generated? Is it the same object as in the other test? Should we instead commit this is a separate input file under: llvm/test/tools/llvm-objcopy/wasm/Input/. At least you should document how to regenerate these inputs.

(I you do you can remove one level of # from these test files too)

llvm/tools/llvm-objcopy/wasm/Writer.cpp

31

Its section type an LEB, or a single byte? You use +1 below we means this should probably be a single byte write, no?

llvm/tools/llvm-objcopy/wasm/Writer.h

33

In the lld codebase we use "finalize" on section to mean exactly that: "properly encodes Sections + computes the total size".

Do you have other other meaning of the work finalize in mind? Perhaps finalizeContents() would be more explicit?

[alexander-shaposhnikov]

llvm/tools/llvm-objcopy/wasm/Writer.h

33

it feels like it's unnecessary to keep around one extra copy (SectionData) (especially if it doesn't contain any fields which need to be computed from the final layout (i.e. global offsets, indices, etc)) + that way it looks simpler to me unless I'm missing smth.

[jhenderson]

llvm/test/tools/llvm-objcopy/wasm/basic-archive-copy.test

43

This sounds like a philosophical difference to me. I'd like to keep our test inputs to the bare minimum that actually test what we care about. This is the approach we take in the ELF objcopy testing, and indeed in most other newer tests on other tools like llvm-readobj (@grimar in particular has gone to a lot of effort to help improve those tests recently along these sort of lines).

If we start going down the approach of just blindly taking whatever obj2yaml produces for an input, many of the tests will be testing things that aren't really anything to do with their purpose.

That all being said, there may be a place for additional tests that do the round-trip obj2yaml, llvm-objcopy, yaml2obj testing, but I don't think these should be the general rule. In this case, I'd actually start from a .ll or .s file and use llvm-mc or whatever to turn them into an object. That way, there is no risk of the YAML and original source drifting apart.

[grimar]

llvm/test/tools/llvm-objcopy/wasm/basic-archive-copy.test

43

I'd like to keep our test inputs to the bare minimum that actually test what we care about. This is the approach we take in the ELF objcopy testing, and indeed in most other newer tests on other tools like llvm-readobj

+1. If we use YAML it makes a lot of sense to keep it minimal. It is much easier to mantain, to extend and add new test cases.

If we start going down the approach of just blindly taking whatever obj2yaml produces for an input, many of the tests will be testing things that aren't really anything to do with their purpose.

Yes, and we saw that many times. For example, having an excessive "AddressAlign" flag might hide the possibility to produce a section that triggers a data type misalignment exception on some platforms for a few sections. It was fixed in llvm-readobj for versioning sections recently. If we did not try to use the minimal YAML descriptions, we would never caught such bugs.
The less YAML instructions we have - the more chances we test exactly what we want to test and not something else.

[dschuff]

• Remove OwnedContents for now, and give Writer::SectionHeader its correct name

llvm/tools/llvm-objcopy/wasm/Object.h

22

This interface (owning the contents) is actually used in D70970, but not here; I'll move it to the next review.
For now I kept the naming ("contentsRef") the same. in the next review we can discuss the current approach (which is meant for moving the contents) vs. your suggested approach which would copy them.

llvm/tools/llvm-objcopy/wasm/Writer.cpp

31

You're right, it's a single byte in the spec; it doesn't make any difference if it's less than 128 and I think we are not 100% consistent in the tools code. But I fixed it here.

llvm/tools/llvm-objcopy/wasm/Writer.h

33

Actually the root problem here is that SectionData is totally misnamed. It's not actually the section payload at all, it's just the header, which does need to be computed from the final layout. (that's why it's a SmallVector). I think I originally had both headers and data and I must have renamed/eliminated the wrong name when I change it.
I just renamed it SectionHeader and it all hopefully makes more sense now.

In light of that, finalize does more than just compute the size, it generates the header as well, and as @sbc100 says, matches the way we use the term in the wasm backend and lld.

[alexander-shaposhnikov]

Thanks for your patience, a few more comments.

llvm/test/tools/llvm-objcopy/wasm/basic-copy.test

7

-o ?

llvm/tools/llvm-objcopy/wasm/Object.h

22

well, for now it seems like it should be a struct (e.g. as in the comment above), because otherwise this looks a bit strange imo. (+ it looks like this "using" and the setter method would not be necessary).

42

since WasmObjectHeader is public i would convert this class into a struct as well / get rid of boilerplate code (getSections/addSections etc.)
So basically I'd keep it as simple & clear as possible, and if we need to add complexity we'll do it when/where it's required (and there it'll be easier to understand / judge what exactly is necessary).

[dschuff]

llvm/tools/llvm-objcopy/wasm/Object.h

42

I understand the concern here (and above) but I'd rather not completely refactor this class now, only to completely rewrite it back tomorrow (since I intend to finish up D70970 as soon as this patch lands). I don't think it makes sense to get rid of addSections given that we'll have removeSections soon, and removeSections is slightly nontrivial. I think the more useful question though is how to handle owned and non-owned section data, since in the next change we'll need to have both. I went ahead and rewrote Section for now, and I made the Object own the data (instead of the section) in the next patch so you can see what you think of that.

[dschuff]

• Simplify Section to a struct

[dschuff]

llvm/test/tools/llvm-objcopy/wasm/basic-copy.test

7

obj2yaml does not support the -o option.

[alexander-shaposhnikov]

llvm/tools/llvm-objcopy/wasm/Object.cpp

22 ↗

(On Diff #234203)

Sections.insert(Sections.end(), NewSections.begin(), NewSections.end());

llvm/tools/llvm-objcopy/wasm/Object.h

23

"public:" is unnecessary

llvm/tools/llvm-objcopy/wasm/Reader.cpp

25

Sections.push_back({WS.Type(), WS.Name, WS.Content});

llvm/tools/llvm-objcopy/wasm/Writer.h

33

I see, thank you for the additional context, the new name makes more sense,
but I'm still kinda curious - what are the advantages of keeping this around vs "writing on the fly" ?
the latter looks simpler + a little bit less code

[alexander-shaposhnikov]

llvm/tools/llvm-objcopy/wasm/Writer.h

33

+ regardless of the matter, i think the code which generates a SectionHeader for a given Section should be put into a helper function + it would be good to add a brief comment explaining the format + a link to the detailed specification (for those who will read this code in the future)

[dschuff]

• Split header generation into separate function

[dschuff]

llvm/tools/llvm-objcopy/wasm/Writer.h

33

I think the reason it's split into a separate step is that because of the LEB128 encodings (ubiquitous in wasm), the size of the section isn't known until the encodings are done. Here it only shows up in header generation where the size includes the encoded length of the section name, but there are other cases (especially when dealing with relocations and instructions) where there are more LEBs that we might prefer to either leave encoded with extra bytes (for rewriting later) or compress to reduce binary size.

[alexander-shaposhnikov]

okay, thank you again for the patience, overall this looks cleaner to me

llvm/tools/llvm-objcopy/wasm/Reader.cpp

27

sorry, I didn't notice this before - but this is one reason why I wasn't a big fan of the proposed interface,
adding a section (or sections) always involves a copy.
For example, if vector<Section> Sections was just a public field of Object
(or Object was a struct (it still can have a method removeSections if necessary))
you would simply do the following:

Obj.Sections.reserve(WasmObj.sections().size());
for (SectionRef Sec : WasobObj.sections()) {
     Sections.push_back(... );
}

Frankly speaking it still feels like addSections(...) doesn't buy us much. (and the same is true for getSections)

[alexander-shaposhnikov]

(+ please, apply clang-format --style=llvm to the diff (if not yet))

[dschuff]

• Make Sections a public member of Object; remove getSections/addSections

[dschuff]

• fix clang-format

[dschuff]

OK, made Sections a public member of Object.

[dschuff]

I think all the feedback has now been addressed.

[merge_guards_bot]

Unit tests: pass. 62116 tests passed, 0 failed and 808 were skipped.

clang-tidy: fail. clang-tidy found 0 errors and 4 warnings.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[dschuff]

• Fix header guard name

[merge_guards_bot]

Unit tests: pass. 62116 tests passed, 0 failed and 808 were skipped.

clang-tidy: fail. clang-tidy found 0 errors and 4 warnings.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[dschuff]

Hm, the bot seems to be using the wrong version of the patch.

[merge_guards_bot]

Unit tests: pass. 62116 tests passed, 0 failed and 808 were skipped.

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

[jhenderson]

I've given this another quick once over, and made a few small comments. I'll try to take a more in-depth look tomorrow or early next week.

llvm/tools/llvm-objcopy/wasm/Writer.cpp

25–26

Missing trailing full stops on these two lines.

32

// static?

llvm/tools/llvm-objcopy/wasm/Writer.h

33–34

Not a big deal, but any particular reason you've called this generateSectionHeader instead of the shorter createSectionHender?

[dschuff]

• Rename generateSectionHeader, clarify comments

llvm/tools/llvm-objcopy/wasm/Writer.cpp

25–26

done (but these are intentionally not complete sentences).

32

This is just the common annotation indicating that generateSectionHeader is a static method. I added a blank line to make it more clear that it goes with the method definition rather than the block comment above.

llvm/tools/llvm-objcopy/wasm/Writer.h

33–34

No particular reason; habit from working on code generators I suppose. I changed it.

[merge_guards_bot]

Unit tests: fail. 62112 tests passed, 5 failed and 807 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[jhenderson]

llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp

24

Is this using needed? You're already inside the wasm namespace.

44

Strictly speaking, basic copying isn't a flag! Perhaps simply "no flags are supported yet" or possibly "no flags are supported yet. Only basic copying is allowed".

llvm/tools/llvm-objcopy/wasm/Writer.cpp

23–31

I wonder whether this comment would make more sense in the header?

32

If it's common practice within other areas of WASM code, then that's fine, but I wasn't aware that it's a wider thing. To me, it initially looked like you'd just accidentally commented it out or something similar.

77

A blank line before this and the similar comment above would nicely break this function up, I think.

[dschuff]

• Rename generateSectionHeader, clarify comments

llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp

44

Done as you suggested, (but I added a full stop at the end ;-).

llvm/tools/llvm-objcopy/wasm/Writer.cpp

23–31

Yeah that makes sense; moved the comment.

32

Looking again, it doesn't seem to be as common in LLVM as in some other codebases I've used; I'll just remove it here.

[merge_guards_bot]

Unit tests: fail. 62112 tests passed, 5 failed and 807 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: fail. clang-tidy found 0 errors and 1 warnings. 0 of them are added as review comments below (why?).

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[dschuff]

• Fix clang-tidy warning

[merge_guards_bot]

Unit tests: fail. 62112 tests passed, 5 failed and 807 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[dschuff]

• remove stray empty line

[merge_guards_bot]

Unit tests: fail. 62112 tests passed, 5 failed and 807 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[jhenderson]

LGTM, with two nits.

llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp

44

Actually, for error messages, we usually don't end with a full stop :-)

The style we try to canonicalise on in the other binutils is lower-case first letter of first sentence, and no trailing full stops.

Don't ask me why that is the style, mind you...

llvm/tools/llvm-objcopy/wasm/Writer.h

33–41

We're a bit hit-and-miss on this, but it's probably good practice to use doxygen-style comments (i.e. '///') for things like this in headers.

[dschuff]

• remove period, use doxygen comments

[merge_guards_bot]

Unit tests: fail. 62112 tests passed, 5 failed and 807 were skipped.

failed: libc++.std/language_support/cmp/cmp_partialord/partialord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongeq/cmp.strongeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_strongord/strongord.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakeq/cmp.weakeq.pass.cpp
failed: libc++.std/language_support/cmp/cmp_weakord/weakord.pass.cpp

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[kamaub]

@dschuff please note this patch causes build failures for PowerPC big endian buildbots with commit rGa928d127a52a
If the solution isn't a simple fix please revert the commit so we may continue testing the rest of the repository

D72723 may show up with some failures in between but here but this has since been resolved
i've attached the builds that broke and the lastest build from those bots:

clang-ppc64be-linux:
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/43577
http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/43643

clang-ppc64be-linux-multistage:
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/23316
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/23336

clang-ppc64be-linux-lnt:
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/34687
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-lnt/builds/34719

[dschuff]

I won't have time to look at it today; i'll just revert and take a look tomorrow.

[dschuff]

Relanded with a fix as rGf2af0607000c
I don't have a BE machine to test on, but I'm *pretty* sure it works :)

[kamaub]

In D70930#1847950, @dschuff wrote:

I won't have time to look at it today; i'll just revert and take a look tomorrow.

Thank you, the buildbots are green again, best of luck addressing the failure.

llvm/include/llvm/Object/Wasm.h

public:
   ArrayRef<wasm::WasmElemSegment> elements() const { return ElemSegments; }
   ArrayRef<WasmSegment> dataSegments() const { return DataSegments; }
   ArrayRef<wasm::WasmFunction> functions() const { return Functions; }
   ArrayRef<wasm::WasmFunctionName> debugNames() const { return DebugNames; }
   uint32_t startFunction() const { return StartFunction; }
   uint32_t getNumImportedGlobals() const { return NumImportedGlobals; }
   uint32_t getNumImportedFunctions() const { return NumImportedFunctions; }
   uint32_t getNumImportedEvents() const { return NumImportedEvents; }
+  uint32_t getNumSections() const { return Sections.size(); }
   void moveSymbolNext(DataRefImpl &Symb) const override;
 
   uint32_t getSymbolFlags(DataRefImpl Symb) const override;
 
   basic_symbol_iterator symbol_begin() const override;
 
   basic_symbol_iterator symbol_end() const override;
   Expected<StringRef> getSymbolName(DataRefImpl Symb) const override;

llvm/test/tools/llvm-objcopy/wasm/basic-archive-copy.test

+## Test a basic copy of an archive containing a wasm object.
+
+# RUN: yaml2obj %s -o %t
+
+## Create an archive and copy it using llvm-objcopy.
+# RUN: rm -f %t.a
+# RUN: llvm-ar crs %t.a %t
+# RUN: cp %t.a %t.copy.a
+# RUN: llvm-objcopy %t.a %t2.a
+## Create another archive from an objcopy-copied object, verify that they match.
+# RUN: llvm-objcopy %t %t2
+# RUN: llvm-ar p %t2.a > %t3
+# RUN: cmp %t2 %t3
+
+## Check that the copied archive has the correct index contents.
+# RUN: llvm-nm --print-armap %t.a | FileCheck --check-prefix=INDEX-TABLE %s
+# RUN: llvm-nm --print-armap %t2.a | FileCheck --check-prefix=INDEX-TABLE %s
+## Verify that llvm-objcopy has not modifed the input.
+# RUN: cmp %t.copy.a %t.a
+
+# INDEX-TABLE: Archive map
+# INDEX-TABLE-NEXT: func1 in
+
+## Do the same with an archive that has no index.
+# RUN: rm -f %t.no.index.a
+# RUN: llvm-ar crS %t.no.index.a %t
+# RUN: llvm-objcopy %t.no.index.a %t2.no.index.a
+# RUN: llvm-ar p %t2.no.index.a > %t4
+
+# RUN: llvm-nm --print-armap %t.no.index.a | FileCheck --check-prefix=NO-INDEX-TABLE %s
+# RUN: llvm-nm --print-armap %t2.no.index.a | FileCheck --check-prefix=NO-INDEX-TABLE %s
+# RUN: cmp %t2 %t4
+
+# NO-INDEX-TABLE-NOT: Archive map
+# NO-INDEX-TABLE-NOT: func1 in
+
+--- !WASM
+FileHeader:
+  Version: 0x00000001
+Sections:
+  - Type: TYPE
+    Signatures:
+      - Index: 0
+        ParamTypes:
+          - I32
+        ReturnTypes:
+          - F32
+      - Index: 1
+        ParamTypes:
+          - I32
+          - I64
+        ReturnTypes:     []
+  - Type: FUNCTION
+    FunctionTypes:
+      - 0
+      - 1
+  - Type: CODE
+    Relocations:
+      - Type:   R_WASM_TABLE_INDEX_SLEB
+        Index:  0
+        Offset: 0x00000002
+      - Type:   R_WASM_FUNCTION_INDEX_LEB
+        Index:  1
+        Offset: 0x0000002
+    Functions:
+      - Index:  0
+        Locals:
+         - Type:  I32
+           Count: 3
+        Body: 010101010B
+      - Index: 1
+        Locals:
+         - Type: I32
+           Count: 1
+        Body: 010101010B
+  - Type: CUSTOM
+    Name:    linking
+    Version: 2
+    SymbolTable:
+      - Index: 0
+        Kind:  FUNCTION
+        Name:  func1
+        Flags: [  ]
+        Function: 0
+      - Index: 1
+        Kind:  FUNCTION
+        Name:  func2
+        Flags: [  ]
+        Function: 1
+...

llvm/test/tools/llvm-objcopy/wasm/basic-copy.test

+## Test that the copied object has the same yaml conversion as the original.
+## The copied object is not bit-identical to the yaml2obj-generated
+## one, as yaml2obj uses 5-byte LEBs for section sizes (unlike objcopy/clang).
+# RUN: yaml2obj %s -o %t.o
+# RUN: llvm-objcopy %t.o %t2.o
+# RUN: obj2yaml %t.o > %t.yaml
+# RUN: obj2yaml %t2.o > %t2.yaml
+# RUN: diff %t.yaml %t2.yaml
+
+--- !WASM
+FileHeader:
+  Version: 0x00000001
+Sections:
+  - Type: TYPE
+    Signatures:
+      - Index: 0
+        ParamTypes:
+          - I32
+        ReturnTypes:
+          - F32
+      - Index: 1
+        ParamTypes:
+          - I32
+          - I64
+        ReturnTypes:     []
+  - Type: FUNCTION
+    FunctionTypes:
+      - 0
+      - 1
+  - Type: CODE
+    Relocations:
+      - Type:   R_WASM_TABLE_INDEX_SLEB
+        Index:  0
+        Offset: 0x00000002
+      - Type:   R_WASM_FUNCTION_INDEX_LEB
+        Index:  1
+        Offset: 0x0000002
+    Functions:
+      - Index:  0
+        Locals:
+         - Type:  I32
+           Count: 3
+        Body: 010101010B
+      - Index: 1
+        Locals:
+         - Type: I32
+           Count: 1
+        Body: 010101010B
+  - Type: CUSTOM
+    Name:    linking
+    Version: 2
+    SymbolTable:
+      - Index: 0
+        Kind:  FUNCTION
+        Name:  func1
+        Flags: [  ]
+        Function: 0
+      - Index: 1
+        Kind: FUNCTION
+        Name: func2
+        Flags: [  ]
+        Function: 1
+...

llvm/tools/llvm-objcopy/CMakeLists.txt

add_llvm_tool(llvm-objcopy
   ELF/ELFConfig.cpp
   ELF/ELFObjcopy.cpp
   ELF/Object.cpp
   MachO/MachOObjcopy.cpp
   MachO/MachOReader.cpp
   MachO/MachOWriter.cpp
   MachO/MachOLayoutBuilder.cpp
   MachO/Object.cpp
+  wasm/Reader.cpp
+  wasm/Writer.cpp
+  wasm/WasmObjcopy.cpp
   DEPENDS
   ObjcopyOptsTableGen
   InstallNameToolOptsTableGen
   StripOptsTableGen
   )
 
 add_llvm_tool_symlink(llvm-install-name-tool llvm-objcopy)
 add_llvm_tool_symlink(llvm-strip llvm-objcopy)
 
 if(LLVM_INSTALL_BINUTILS_SYMLINKS)
   add_llvm_tool_symlink(objcopy llvm-objcopy)
   add_llvm_tool_symlink(strip llvm-objcopy)
 endif()

llvm/tools/llvm-objcopy/llvm-objcopy.cpp

 //===- llvm-objcopy.cpp ---------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm-objcopy.h"
 #include "Buffer.h"
+#include "COFF/COFFObjcopy.h"
 #include "CopyConfig.h"
 #include "ELF/ELFObjcopy.h"
-#include "COFF/COFFObjcopy.h"
 #include "MachO/MachOObjcopy.h"
+#include "wasm/WasmObjcopy.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/ArchiveWriter.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Object/ELFTypes.h"
 #include "llvm/Object/Error.h"
 #include "llvm/Object/MachO.h"
+#include "llvm/Object/Wasm.h"
 #include "llvm/Option/Arg.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ErrorOr.h"
static Error executeObjcopyOnBinary(CopyConfig &Config, object::Binary &In,
   if (auto *ELFBinary = dyn_cast<object::ELFObjectFileBase>(&In)) {
     if (Error E = Config.parseELFConfig())
       return E;
     return elf::executeObjcopyOnBinary(Config, *ELFBinary, Out);
   } else if (auto *COFFBinary = dyn_cast<object::COFFObjectFile>(&In))
     return coff::executeObjcopyOnBinary(Config, *COFFBinary, Out);
   else if (auto *MachOBinary = dyn_cast<object::MachOObjectFile>(&In))
     return macho::executeObjcopyOnBinary(Config, *MachOBinary, Out);
+  else if (auto *WasmBinary = dyn_cast<object::WasmObjectFile>(&In))
+    return objcopy::wasm::executeObjcopyOnBinary(Config, *WasmBinary, Out);
   else
     return createStringError(object_error::invalid_file_type,
                              "unsupported object file format");
 }
 
 static Error executeObjcopyOnArchive(CopyConfig &Config, const Archive &Ar) {
   std::vector<NewArchiveMember> NewArchiveMembers;
   Error Err = Error::success();

llvm/tools/llvm-objcopy/wasm/Object.h

+//===- Object.h -------------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TOOLS_LLVM_OBJCOPY_WASM_OBJECT_H
+#define LLVM_TOOLS_LLVM_OBJCOPY_WASM_OBJECT_H
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Object/Wasm.h"
+#include <vector>
+
+namespace llvm {
+namespace objcopy {
+namespace wasm {
+
+struct Section {
+  // For now, each section is only an opaque binary blob with no distinction
+  // between custom and known sections.
+  uint8_t SectionType;
+  StringRef Name;
+  ArrayRef<uint8_t> Contents;
+};
+
+struct Object {
+  llvm::wasm::WasmObjectHeader Header;
+  // For now don't discriminate between kinds of sections.
+  std::vector<Section> Sections;
+};
+
+} // end namespace wasm
+} // end namespace objcopy
+} // end namespace llvm
+
+#endif // LLVM_TOOLS_LLVM_OBJCOPY_WASM_OBJECT_H

llvm/tools/llvm-objcopy/wasm/Reader.h

+//===- Reader.h -------------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TOOLS_LLVM_OBJCOPY_WASM_READER_H
+#define LLVM_TOOLS_LLVM_OBJCOPY_WASM_READER_H
+
+#include "Object.h"
+
+namespace llvm {
+namespace objcopy {
+namespace wasm {
+
+class Reader {
+public:
+  explicit Reader(const object::WasmObjectFile &O) : WasmObj(O) {}
+  Expected<std::unique_ptr<Object>> create() const;
+
+private:
+  const object::WasmObjectFile &WasmObj;
+};
+
+} // end namespace wasm
+} // end namespace objcopy
+} // end namespace llvm
+
+#endif // LLVM_TOOLS_LLVM_OBJCOPY_WASM_READER_H

llvm/tools/llvm-objcopy/wasm/Reader.cpp

+//===- Reader.cpp ---------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Reader.h"
+
+namespace llvm {
+namespace objcopy {
+namespace wasm {
+
+using namespace object;
+using namespace llvm::wasm;
+
+Expected<std::unique_ptr<Object>> Reader::create() const {
+  auto Obj = std::make_unique<Object>();
+  Obj->Header = WasmObj.getHeader();
+  std::vector<Section> Sections;
+  Obj->Sections.reserve(WasmObj.getNumSections());
+  for (const SectionRef &Sec : WasmObj.sections()) {
+    const WasmSection &WS = WasmObj.getWasmSection(Sec);
+    Obj->Sections.push_back(
+        {static_cast<uint8_t>(WS.Type), WS.Name, WS.Content});
+  }
+  return std::move(Obj);
+}
+
+} // end namespace wasm
+} // end namespace objcopy
+} // end namespace llvm

llvm/tools/llvm-objcopy/wasm/WasmObjcopy.h

+//===- WasmObjcopy.h -------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TOOLS_LLVM_OBJCOPY_WASM_WASMOBJCOPY_H
+#define LLVM_TOOLS_LLVM_OBJCOPY_WASM_WASMOBJCOPY_H
+
+namespace llvm {
+class Error;
+
+namespace object {
+class WasmObjectFile;
+} // end namespace object
+
+namespace objcopy {
+struct CopyConfig;
+class Buffer;
+
+namespace wasm {
+Error executeObjcopyOnBinary(const CopyConfig &Config,
+                             object::WasmObjectFile &In, Buffer &Out);
+
+} // end namespace wasm
+} // end namespace objcopy
+} // end namespace llvm
+
+#endif // LLVM_TOOLS_LLVM_OBJCOPY_WASM_WASMOBJCOPY_H

llvm/tools/llvm-objcopy/wasm/WasmObjcopy.cpp

+//===- WasmObjcopy.cpp ----------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "WasmObjcopy.h"
+#include "Buffer.h"
+#include "CopyConfig.h"
+#include "Object.h"
+#include "Reader.h"
+#include "Writer.h"
+#include "llvm-objcopy.h"
+#include "llvm/Support/Errc.h"
+
+namespace llvm {
+namespace objcopy {
+namespace wasm {
+
+using namespace object;
+
+static Error handleArgs(const CopyConfig &Config, Object &Obj) {
+  if (!Config.AddGnuDebugLink.empty() || !Config.BuildIdLinkDir.empty() ||
+      Config.BuildIdLinkInput || Config.BuildIdLinkOutput ||
+      Config.ExtractPartition || !Config.SplitDWO.empty() ||
+      !Config.SymbolsPrefix.empty() || !Config.AllocSectionsPrefix.empty() ||
+      Config.DiscardMode != DiscardType::None || Config.NewSymbolVisibility ||
+      !Config.SymbolsToAdd.empty() || !Config.RPathToAdd.empty() ||
+      !Config.OnlySection.empty() || !Config.SymbolsToGlobalize.empty() ||
+      !Config.SymbolsToKeep.empty() || !Config.SymbolsToLocalize.empty() ||
+      !Config.SymbolsToRemove.empty() ||
+      !Config.UnneededSymbolsToRemove.empty() ||
+      !Config.SymbolsToWeaken.empty() || !Config.SymbolsToKeepGlobal.empty() ||
+      !Config.SectionsToRename.empty() || !Config.SetSectionAlignment.empty() ||
+      !Config.SetSectionFlags.empty() || !Config.SymbolsToRename.empty() ||
+      !Config.ToRemove.empty() || !Config.DumpSection.empty() ||
+      !Config.AddSection.empty()) {
+    return createStringError(
+        llvm::errc::invalid_argument,
+        "no flags are supported yet, only basic copying is allowed");
+  }
+  return Error::success();
+}
+
+Error executeObjcopyOnBinary(const CopyConfig &Config,
+                             object::WasmObjectFile &In, Buffer &Out) {
+  Reader TheReader(In);
+  Expected<std::unique_ptr<Object>> ObjOrErr = TheReader.create();
+  if (!ObjOrErr)
+    return createFileError(Config.InputFilename, ObjOrErr.takeError());
+  Object *Obj = ObjOrErr->get();
+  assert(Obj && "Unable to deserialize Wasm object");
+  if (Error E = handleArgs(Config, *Obj))
+    return createFileError(Config.InputFilename, std::move(E));
+  Writer TheWriter(*Obj, Out);
+  if (Error E = TheWriter.write())
+    return createFileError(Config.OutputFilename, std::move(E));
+  return Error::success();
+}
+
+} // end namespace wasm
+} // end namespace objcopy
+} // end namespace llvm

llvm/tools/llvm-objcopy/wasm/Writer.h

+//===- Writer.h -------------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_TOOLS_LLVM_OBJCOPY_WASM_WRITER_H
+#define LLVM_TOOLS_LLVM_OBJCOPY_WASM_WRITER_H
+
+#include "Buffer.h"
+#include "Object.h"
+#include <cstdint>
+#include <vector>
+
+namespace llvm {
+namespace objcopy {
+namespace wasm {
+
+class Writer {
+public:
+  Writer(Object &Obj, Buffer &Buf) : Obj(Obj), Buf(Buf) {}
+  Error write();
+
+private:
+  using SectionHeader = SmallVector<char, 8>;
+  Object &Obj;
+  Buffer &Buf;
+  std::vector<SectionHeader> SectionHeaders;
+
+  /// Generate a wasm section section header for S.
+  /// The header consists of
+  /// * A one-byte section ID (aka the section type).
+  /// * The size of the section contents, encoded as ULEB128.
+  /// * If the section is a custom section (type 0) it also has a name, which is
+  ///   encoded as a length-prefixed string. The encoded section size *includes*
+  ///   this string.
+  /// See https://webassembly.github.io/spec/core/binary/modules.html#sections
+  /// Return the header and store the total size in SectionSize.
+  static SectionHeader createSectionHeader(const Section &S,
+                                           size_t &SectionSize);
+  size_t finalize();
+};
+
+} // end namespace wasm
+} // end namespace objcopy
+} // end namespace llvm
+
+#endif // LLVM_TOOLS_LLVM_OBJCOPY_WASM_WRITER_H

llvm/tools/llvm-objcopy/wasm/Writer.cpp

+//===- Writer.cpp ---------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Writer.h"
+
+#include "llvm/BinaryFormat/Wasm.h"
+#include "llvm/Support/LEB128.h"
+#include "llvm/Support/raw_ostream.h"
+
+namespace llvm {
+namespace objcopy {
+namespace wasm {
+
+using namespace object;
+using namespace llvm::wasm;
+
+Writer::SectionHeader Writer::createSectionHeader(const Section &S,
+                                                  size_t &SectionSize) {
+  SectionHeader Header;
+  raw_svector_ostream OS(Header);
+  OS << S.SectionType;
+  bool HasName = S.SectionType == WASM_SEC_CUSTOM;
+  SectionSize = S.Contents.size();
+  if (HasName)
+    SectionSize += getULEB128Size(S.Name.size()) + S.Name.size();
+  // Pad the LEB value out to 5 bytes to make it a predictable size, and
+  // match the behavior of clang.
+  encodeULEB128(SectionSize, OS, 5);
+  if (HasName) {
+    encodeULEB128(S.Name.size(), OS);
+    OS << S.Name;
+  }
+  // Total section size is the content size plus 1 for the section type and
+  // 5 for the LEB-encoded size.
+  SectionSize = SectionSize + 1 + 5;
+  return Header;
+}
+
+size_t Writer::finalize() {
+  size_t ObjectSize = sizeof(WasmMagic) + sizeof(WasmVersion);
+  SectionHeaders.reserve(Obj.Sections.size());
+  // Finalize the headers of each section so we know the total size.
+  for (const Section &S : Obj.Sections) {
+    size_t SectionSize;
+    SectionHeaders.push_back(createSectionHeader(S, SectionSize));
+    ObjectSize += SectionSize;
+  }
+  return ObjectSize;
+}
+
+Error Writer::write() {
+  size_t FileSize = finalize();
+  if (Error E = Buf.allocate(FileSize))
+    return E;
+
+  // Write the header.
+  uint8_t *Ptr = Buf.getBufferStart();
+  Ptr = std::copy(Obj.Header.Magic.begin(), Obj.Header.Magic.end(), Ptr);
+  memcpy(Ptr, &Obj.Header.Version, sizeof(Obj.Header.Version));
+  Ptr += sizeof(Obj.Header.Version);
+
+  // Write each section.
+  for (size_t I = 0, S = SectionHeaders.size(); I < S; ++I) {
+    Ptr = std::copy(SectionHeaders[I].begin(), SectionHeaders[I].end(), Ptr);
+    ArrayRef<uint8_t> Contents = Obj.Sections[I].Contents;
+    Ptr = std::copy(Contents.begin(), Contents.end(), Ptr);
+  }
+  return Buf.commit();
+}
+
+} // end namespace wasm
+} // end namespace objcopy
+} // end namespace llvm