[PATCH] D107662: [WebAssembly] Implement build-id feature

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added a comment.

Great to see this happening!



================
Comment at: lld/test/wasm/build-id.s:1
+# REQUIRES: wasm
+
----------------
`lld/test/wasm/lit.local.cfg` already takes care of this IIUC


================
Comment at: lld/test/wasm/build-id.s:43
+
+.section .data.foo,"",@
+        .globl  foo
----------------
Why not just use `lld/test/wasm/Inputs/start.s` for this test .. or does it depend on there being some data?

When you could remove all the `--no-entry` flags above, and all the comment markers, and just call this file `.test`.


================
Comment at: lld/wasm/Driver.cpp:353
+// synonym for "sha1" because all our hash functions including
+// -build-id=sha1 are actually tree hashes for performance reasons.
+static std::pair<BuildIdKind, std::vector<uint8_t>>
----------------
Do we want to support the `tree` alias?  I assume this comes from the ELF backend?


================
Comment at: lld/wasm/SyntheticSections.cpp:923
+  // TODO: Remove once we're sure we don't want to encode this as a string.
+  //writeStr(os, std::string(hashSize, ' '), "placeholder");
+}
----------------
Do you want to leave these TODOs in here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107662