[compiler-rt] [Profile] Dump binary id to raw profiles on Windows. (PR #75618)

[Martin Storsjö via llvm-commits]

mstorsjo wrote:

> > It looks like this breaks the build when using mingw:
> > > C:/a/rust/rust/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/12.2.0/../../../../x86_64-w64-mingw32/bin/ld.exe: C:\a\rust\rust\build\x86_64-pc-windows-gnu\stage2\lib\rustlib\x86_64-pc-windows-gnu\lib\libprofiler_builtins-06ad067b40ce3a0d.rlib(InstrProfilingPlatformWindows.o):InstrProfilingPlatformWindows.c:(.rdata$.refptr.__buildid[.refptr.__buildid]+0x0): undefined reference to `__buildid'
> > 
> > 
> > I don't really get how this code is supposed to work, I don't think extern selectany supports symbols that are not defined?
> 
> I think the idea was that it previously was a weak undefined symbol - if linking with LLD 18, the linker does define it and we can pick up the build ID from there. But while GNU binutils and GCC did come up with the way of emulating ELF weak symbols on top of COFF, they do have a couple of bugs around it, which I presume is what you're running into here.
> 
> I guess that with the fix from #80700, it changes the symbol from a weak undefined into a weak defined. If linking with LLD as opposed to GNU ld, this should hopefully be handled correctly, and it should prefer the `__buildid` that is provided by the linker (as a non-weak symbol) rather than the default dummy uninitialized/zero-initialized `__buildid` here.

Sorry, I hadn't noticed that `COMPILER_RT_WEAK` indeed expands to selectany instead of actual weak attributes.

I tested this case, with the following test snippet, extracted from the code here:
```c
#include <stdio.h>
#include <stdint.h>

__attribute__((selectany)) uint8_t __buildid[16];

int main(int argc, char **argv) {
  if (*__buildid) {
    printf("got build id:");
    for (int i = 0; i < 16; i++)
      printf(" %02x", __buildid[i]);
    printf("\n");
  } else {
    printf("no build id found\n");
  }
  return 0;
}
```
This correctly prints the build ID when built with Clang and linked with LLD. When linked with GNU ld, it links correctly and has no build id. So that bit works as intended.

I have a faint feeling that this is brittle - we have two separate definitions of `__buildid`, one builtin from the linker (which is a regular symbol) and one from the source (which is comdat selectany). So in principle, the linker could be free to pick any of them.

In practice, with the implementation in LLD, these two definitions don't go through the normal comdat conflict resolution, but LLD just overwrites the `__buildid` symbol, whatever it is, with its own definition: https://github.com/llvm/llvm-project/blob/llvmorg-18.1.0-rc1/lld/COFF/Writer.cpp#L1124-L1126

So as long as the LLD implementation does this, we're safe here.

The setup that I had in mind would have been this:
```c
#include <stdio.h>
#include <stdint.h>

__attribute__((weak)) extern uint8_t __buildid[16];

int main(int argc, char **argv) {
  if (__buildid) {
    printf("got build id:");
    for (int i = 0; i < 16; i++)
      printf(" %02x", __buildid[i]);
    printf("\n");
  } else {
    printf("no build id found\n");
  }
  return 0;
} 
```
Note the condition `if (__buildid)`, while it previously checked whether the first ID byte was nonzero `if (*__buildid)` in the former case (and in the compiler-rt source now).

This setup works with Clang+LLD (both new that do define `__buildid` and older ones that don't). If compiled with Clang and linked with GNU ld, it still works as expected, but if compiled with GCC, it fails to link.

So TL;DR, the solution picked here is fine (and probably the least problematic compromise), at least as long as LLD sets the `__buildid` symbol in the way it does.

https://github.com/llvm/llvm-project/pull/75618