[cpuid-x86.h] Add MSAN annotations to mark memory as initialized

[tkoeppe]

It is valuable for Clang's MSAN to detect uninitialized memory accesses, but MSAN cannot instrument the explicit assembly, so we have to annotate the effects of the assembly manually.

[sthibaul]

MSAN cannot instrument the explicit assembly

It cannot, really? We already explicitly express what values are written through "=x" and "+x"

[tkoeppe]

That doesn't appear to suffice. MSAN has to add instrumentation code, and also it doesn't know what the assembly is doing. I think you should be able to reproduce this quite easily with a small toy example that just sets a variable via assembly and returns if form main, and run that with MSAN.

[sthibaul]

That doesn't appear to suffice.

Ok, but what I am saying is that it is the wrong place to fix this, since the code is already expressing what MSAN needs to know (actually compilers have always needed it so as to properly integrate inline assembly in the rest of their code).

MSAN has to add instrumentation code, and also it doesn't know what the assembly is doing.

It does know what it needs to know: "+x" tells that the snippet both reads & writes the value, and "=x" tells that the snippet writes the value.

[tkoeppe]

Aha, I discussed this with the LLVM team, and it looks like MSAN does recognize outputs. It's only the ebx pointer that needs annotating.

That itself could also be an issue with MSAN, though.

[MaskRay]

Not handling indirect output constraints in inline assembly in a known issue in MemorySanitizer: google/sanitizers#192

llvm/llvm-project#77393 should help this ebx pointer argument (but requires explicit -mllvm -msan-handle-asm-conservative for now).

[bgoglin]

FYI, the CI breakage was unrelated, it passes now.

[tkoeppe]

@bgoglin Thanks! I was hoping that'd be the case :-)

[tkoeppe]

@sthibaul Given that it's unlikely that MSAN will support the ebx case properly out of the box any time soon, would you be willing to reconsider this proposal?

[sthibaul]

@sthibaul Given that it's unlikely that MSAN will support the ebx case properly out of the box any time soon, would you be willing to reconsider this proposal?

Yes, but please add the link to llvm/llvm-project#77393 along the call, so we remember why we still put this and know from which llvm version we will be able to drop it.

[tkoeppe]

@sthibaul That's done. Note, though, that that PR is not going to enable the effect tracking by default, but only in conjunction with a separate flag, so there's still a bit more to do after that PR lands.

[sthibaul]

only in conjunction with a separate flag

AIUI from llvm/llvm-project#77393 having to add the flag will only be temporary

[bgoglin]

To be sure I understand, the hwloc side of the PR is complete, the (temporary) flag doesn't require any change in hwloc itself?

[sthibaul]

yes, the flag is only to be used along -fsanitize=memory

[tkoeppe]

Thank you!