[PATCH] D119296: KCFI sanitizer

[Sami Tolvanen via Phabricator via llvm-commits]

samitolvanen added a comment.

In D119296#3733753 <https://reviews.llvm.org/D119296#3733753>, @MaskRay wrote:

> For `STB_WEAK SHN_ABS` `__kcfi_typeid_*`, there is no duplicate definition error. Is this behavior intentional?

Yes, this is by design. Multiple translation units must be able to take the address of the same function declaration, and it must be possible to link them together.

> Note: I don't think we should change lld to recognize some symbol prefix and enforce more rigid diagnostics.
> An error must be implemented with existing ELF features.

Sure, that's reasonable. I don't think we need more diagnostics here as `__kcfi_typeid_` value mismatches are unlikely, but should the need arise, we can figure out the best way to implement them later. Note that this must also work with LTO when linking bitcode files, so we can't rely on ELF features alone.



================
Comment at: llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp:330
+  // comparing the hashes.
+  unsigned ScratchRegs[] = {AArch64::W16, AArch64::W17};
+
----------------
MaskRay wrote:
> const?
This can't be `const` because we assign to this array below in the `for (auto &Reg : ScratchRegs)` loop.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119296