clang annotation are not removed from binary in release builds

[sbc100]

For example:

$ cat test.c 
[[clang::annotate("foobar")]]
int main() {
  return 0;
}
$ emcc -Oz test.c 
$ llvm-objdump -h a.out.wasm 

a.out.wasm:	file format wasm

Sections:
Idx Name                           Size     VMA      Type
  0 TYPE                           0000000a 0000000a 
  1 FUNCTION                       00000003 00000016 
  2 MEMORY                         00000006 0000001b 
  3 EXPORT                         0000000d 00000023 
  4 CODE                           00000009 00000032 TEXT
  5 llvm.func_attr.annotate.foobar 00000004 0000003d 

Here you can see the foobar custom section in the final binary.

[sbc100]

@brendandahl @kripken IIRC you have been working with annotation stuff recently. What should we be doing here? Presumably there are some annotation that we might want to preserve in terms on the wasm annotation proposal, but we probably don't even want these llvm.func_attr.annotate in their original form like this?

[sbc100]

CC @dschuff

[kripken]

I'm impressed that the annotations get to the final wasm like that...

Yeah, for ones we don't want, we should strip them out. I'm not sure if we should strip them all by default or the reverse, though.

[dschuff]

Yeah it seems tricky because the annotations are put there by the user, and we don't necessarily know what they are for. They can be just arbitrary text, and other than the ones we add ourselves, I don't know that we can be sure they should be in the binary or removed.

[dschuff]

It also brings up the question of whether llvm-strip should remove:

1. annotations sections
2. code metadata sections
3. unknown sections.

Currently it only removes sections it knows about, leaving unknown sections including annotations in the binary.
For sections that just pass optimization info to Binaryen, we probably want to strip those by default. I could imagine you'd wnt to keep code metadata sections by default since they are interpreted by the engine, but I could also I could also imagine a setting where you really want to strip all custom sections. The current strip-all (and a hypothetical "really-strip-all") flag is also compatible with --keep-section which allows stripping everything with user-specified exceptions.

[brendandahl]

Those annotations were added to mark functions that need JSPI wrappers. I don't think anyone is using that yet and even if we do support it, we should probably still strip those custom sections at the end (at least for JSPI).

I have a PR that supports JSPI annotations, but IIRC I still needed to fix binaryen to update the custom section when it changed function indexes. For the JSPI use case, once the section is read by emscripten we can remove it.

Wasn't there something else we also wanted to do with the annotations too?

[kripken]

@brendandahl We are starting to use annotations for intrinsics in binaryen:

WebAssembly/binaryen#8268

Possibly the JSPI annotation should work that way there?

[dschuff]

Regarding strip behavior, here's a proposal: llvm/llvm-project#180246

[sbc100]

Regardless of what we do in llvm-strip we need to come up with a solution in emscripten/binaryen. In emscripten we currently do not rely on llvm-strip for anything but assume wasm-opt is at least as aggressive as strip I believe.

[sbc100]

Hmm, actually do have some usage of llvm-objcopy to strip sections, but its very limits and not in the normal optimizing path. See #26218.

[sbc100]

So I'm thinking the most sensible was to get this removed by default in emscripten builds is to have binaryen remove this at the same point when it removes the producers and target-features sections (since these kind of fall into the same category).