[PATCH] D70930: [llvm-objcopy][WebAssembly] Initial support for wasm in llvm-objcopy

[Derek Schuff via Phabricator via llvm-commits]

dschuff added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/wasm/Writer.cpp:35
+      SectionSize += getULEB128Size(S.Name.size()) + S.Name.size();
+    encodeULEB128(SectionSize, OS, 5);
+    if (HasName) {
----------------
jhenderson wrote:
> What is  the `5` here? Is there a constant for it somewhere if applicable?
No constant (it's just the max encoded LEB size that can encode 32 bits) but added a comment.


================
Comment at: llvm/tools/llvm-objcopy/wasm/Writer.cpp:52
+  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);
----------------
jhenderson wrote:
> Does wasm have a concept of endianness that we need to care about here?
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70930/new/

https://reviews.llvm.org/D70930