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

[Derek Schuff via Phabricator via llvm-commits]

dschuff added inline comments.


================
Comment at: lld/wasm/SyntheticSections.h:444
+    SyntheticSection::writeTo(buf);
+    hashBuf = buf + offset + header.size() + 9 /*name string*/ +
+              1 /* hash size */; // FIXME assert these somewhere
----------------
sbc100 wrote:
> This seems a little hacky / fragile, but I can't think of a more elegant way to do it..
Ah yes I meant to try to fix that; WDYT of this fix?


================
Comment at: lld/wasm/Writer.cpp:1834
   writeSections();
+  writeBuildId();
   if (errorCount())
----------------
sbc100 wrote:
> I wonder if this should have a name that lets the reader know its going back an patching an existing location?  Maybe `patchBuildId` ?  But that doesn't convey that this function also calulates the buildid?   "calculateAndPatchBuildId" seems like too much of a mouthful.
> 
> Maybe your existing names is best :)
Yeah, those names are maybe a bit more descriptive but not obviously much better. This is the same name used by the other object formats (and has similar behavior) so consistency seems good in the end.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107662