[WebAssembly] Implement build-id feature

[dschuff]

Summary

Implement the --build-id flag similarly to ELF, and generate a build_id section according to the WebAssembly tool convention specified in https://github.com/WebAssembly/tool-conventions/pull/183

The default style ("fast" aka "tree") hashes the contents of the output and (unlike ELF) generates a v5 UUID based on the hash (using a random namespace).
It also supports generating a random v4 UUID, a sha1 hash, and a user-specified string (as ELF does).

[dschuff]

fix, and rebase

[dschuff]

• Clean up and add test

[dschuff]

• remove stray line

[sbc100]

Great to see this happening!

lld/test/wasm/build-id.s

1 ↗

(On Diff #433561)

lld/test/wasm/lit.local.cfg already takes care of this IIUC

43 ↗

(On Diff #433561)

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.

lld/wasm/Driver.cpp

353 ↗

(On Diff #433561)

Do we want to support the tree alias? I assume this comes from the ELF backend?

lld/wasm/SyntheticSections.cpp

923 ↗

(On Diff #433561)

Do you want to leave these TODOs in here?

[dschuff]

• use start.s as a test instead

lld/test/wasm/build-id.s

43 ↗

(On Diff #433561)

no, it doesn't depend on there being data, I basically just started with the x86 version of this same test. But it has some additional assertions that don't apply to us.
So you're saying this would be a .test file because it would only have RUN lines, and the runs would just assemble start.s? That sounds good.

lld/wasm/Driver.cpp

353 ↗

(On Diff #433561)

yes, this comes from the ELF backend. I think it probably makes sense to support the same options? I would think we'd at least want random UUID, some kind of hash, and user-defined. See also the discussion on the tool-conventions issue though, I'm open to making the implementation different if that makes sense.

lld/wasm/SyntheticSections.cpp

923 ↗

(On Diff #433561)

no, the idea is to remove them once the discussion on tool-conventions is resolved and before committing this.

[dschuff]

Rebase, create valid UUID for fast hash build ID, remove md5

[dschuff]

Correctly rebase against main

[dschuff]

I think this is ready for another review, PTAL.

I'd like to maybe also update objdump and/or other tools to print the build ID in UUID form when applicable, but that can be a followup change.

[sbc100]

lgtm % some minor nits

lld/wasm/SyntheticSections.h

444 ↗

(On Diff #499323)

This seems a little hacky / fragile, but I can't think of a more elegant way to do it..

lld/wasm/Writer.cpp

1834 ↗

(On Diff #499323)

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 :)

[dschuff]

• clean up baked-in buffer size

lld/wasm/SyntheticSections.h

444 ↗

(On Diff #499323)

Ah yes I meant to try to fix that; WDYT of this fix?

lld/wasm/Writer.cpp

1834 ↗

(On Diff #499323)

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.

[sbc100]

lld/wasm/SyntheticSections.h

444 ↗

(On Diff #499323)

Seems better, but maybe add a comment here. What is hashBuf for? Could a better name be used here? isHashLocation? hashPlaceholderPtr?

lld/wasm/Writer.cpp

1834 ↗

(On Diff #499323)

Makes sense

[dschuff]

• add comment and rename hashBuf

[dyung]

@dschuff, I'm not sure if you are aware, but your change fails to build on Windows. You can see the failure on this buildbot:
https://lab.llvm.org/buildbot/#/builders/216/builds/17875

FAILED: tools/lld/wasm/CMakeFiles/lldWasm.dir/Writer.cpp.obj 
C:\bin\ccache.exe C:\PROGRA~2\MICROS~1\2019\BUILDT~1\VC\Tools\MSVC\1429~1.301\bin\HostX64\x64\cl.exe  /nologo /TP -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_LIBCPP_ENABLE_ASSERTIONS -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\lld\wasm -IC:\buildbot-root\llvm-clang-x86_64-sie-win\llvm-project\lld\wasm -IC:\buildbot-root\llvm-clang-x86_64-sie-win\llvm-project\lld\include -Itools\lld\include -Iinclude -IC:\buildbot-root\llvm-clang-x86_64-sie-win\llvm-project\llvm\include /DWIN32 /D_WINDOWS   /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj /permissive- /W4 -wd4141 -wd4146 -wd4244 -wd4267 -wd4291 -wd4351 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4709 -wd5105 -wd4324 -w14062 -we4238 /Gw /MD /O2 /Ob2   -wd4530 -wd4062  /EHs-c- /GR- -UNDEBUG -std:c++17 /showIncludes /Fotools\lld\wasm\CMakeFiles\lldWasm.dir\Writer.cpp.obj /Fdtools\lld\wasm\CMakeFiles\lldWasm.dir\lldWasm.pdb /FS -c C:\buildbot-root\llvm-clang-x86_64-sie-win\llvm-project\lld\wasm\Writer.cpp
C:\buildbot-root\llvm-clang-x86_64-sie-win\llvm-project\lld\wasm\Writer.cpp(263): error C2672: 'std::copy': no matching overloaded function found
C:\buildbot-root\llvm-clang-x86_64-sie-win\llvm-project\lld\wasm\Writer.cpp(263): error C2780: '_FwdIt2 std::copy(_ExPo &&,_FwdIt1,_FwdIt1,_FwdIt2) noexcept': expects 4 arguments - 3 provided
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.29.30133\include\xutility(4463): note: see declaration of 'std::copy'
C:\buildbot-root\llvm-clang-x86_64-sie-win\llvm-project\lld\wasm\Writer.cpp(263): error C2782: '_OutIt std::copy(_InIt,_InIt,_OutIt)': template parameter '_InIt' is ambiguous
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.29.30133\include\xutility(4452): note: see declaration of 'std::copy'
C:\buildbot-root\llvm-clang-x86_64-sie-win\llvm-project\lld\wasm\Writer.cpp(263): note: could be '_Ty *'
        with
        [
            _Ty=uint8_t
        ]
C:\buildbot-root\llvm-clang-x86_64-sie-win\llvm-project\lld\wasm\Writer.cpp(263): note: or       'std::_Array_iterator<_Ty,20>'
        with
        [
            _Ty=uint8_t
        ]
C:\buildbot-root\llvm-clang-x86_64-sie-win\llvm-project\lld\wasm\Writer.cpp(263): error C2784: '_OutIt std::copy(_InIt,_InIt,_OutIt)': could not deduce template argument for '_InIt' from '_Ty *'
        with
        [
            _Ty=uint8_t
        ]
C:\Program Files (x86)\Microsoft Visual Studio\2019\BuildTools\VC\Tools\MSVC\14.29.30133\include\xutility(4452): note: see declaration of 'std::copy'

The failure was masked by an earlier failure which meant the buildbot never got to your code and so the failure didn't show up until the original problem was fixed.

Note this also seems to be the same failure the pre-merge checks hit. Can you take a look and revert if you need time to investigate?

[dschuff]

Sorry about that, will fix.
And I guess I'll have to get into the habit of the pre-merge checks being useful!

lld/wasm/Options.td

multiclass B<string name, string help1, string help2> {
   def NAME: Flag<["--", "-"], name>, HelpText<help1>;
   def no_ # NAME: Flag<["--", "-"], "no-" # name>, HelpText<help2>;
 }
 
 // The following flags are shared with the ELF linker
 def Bsymbolic: F<"Bsymbolic">, HelpText<"Bind defined symbols locally">;
 
 def Bdynamic: F<"Bdynamic">, HelpText<"Link against shared libraries (default)">;
+
 def Bstatic: F<"Bstatic">, HelpText<"Do not link against shared libraries">;
 
 def build_id: F<"build-id">, HelpText<"Alias for --build-id=fast">;
 def build_id_eq: J<"build-id=">, HelpText<"Generate build ID note">,
   MetaVarName<"[fast,sha1,uuid,0x<hexstring>]">;
 
 defm color_diagnostics: B<"color-diagnostics",
   "Alias for --color-diagnostics=always",