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

Sam Clegg via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 1 13:39:52 PST 2023


sbc100 accepted this revision.
sbc100 added a comment.
This revision is now accepted and ready to land.

lgtm % some minor nits



================
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
----------------
This seems a little hacky / fragile, but I can't think of a more elegant way to do it..


================
Comment at: lld/wasm/Writer.cpp:1834
   writeSections();
+  writeBuildId();
   if (errorCount())
----------------
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 :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107662



More information about the llvm-commits mailing list