[PATCH] D119296: KCFI sanitizer

[Sami Tolvanen via Phabricator via cfe-commits]

samitolvanen added inline comments.


================
Comment at: lld/ELF/Symbols.cpp:553
+  // incompatible declarations for the same function.
+  if (isWeak() && getName().startswith("__kcfi_typeid_") &&
+      cast<Defined>(this)->value != other.value)
----------------
samitolvanen wrote:
> MaskRay wrote:
> > samitolvanen wrote:
> > > MaskRay wrote:
> > > > This change looks like a hack.
> > > I agree, this is not ideal. The request from LKML was to produce a warning when we link object files with different values for these weak symbols. What would be the best way to do this in LLD?
> > I need to read through the code to suggest a solution.
> > 
> > If you can provide additional context like LKML URIs, that'll be great.
> Here's the latest LKML thread: https://lore.kernel.org/lkml/20220513202159.1550547-1-samitolvanen@google.com/
> 
> Rasmus brought up the concern with `__kcfi_typeid_` mismatches between object files here: https://lore.kernel.org/lkml/9bd2db3e-2955-66ba-574e-7976bdd95a8e@rasmusvillemoes.dk/
> 
> He didn't specifically request a warning, but I feel like that would be a reasonable response in this situation.
Dropped the lld changes for now, and will send another patch to improve the error message later.


================
Comment at: llvm/test/CodeGen/AArch64/kcfi-bti.ll:4
+; RUN: llc -mtriple=aarch64-- -stop-after=finalize-isel < %s | FileCheck %s --check-prefixes=MIR,FINAL
+
+; ASM:       .word 12345678
----------------
MaskRay wrote:
> Add a test with `"patchable-function-entry"="2"`
Added the `"patchable-function-entry"="2"` test to `AArch64/kcfi.ll`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119296