[PATCH] D148785: -fsanitize=function: use type hashes instead of RTTI objects

[Sami Tolvanen via Phabricator via cfe-commits]

samitolvanen accepted this revision.
samitolvanen added a comment.
This revision is now accepted and ready to land.

Looks good to me, but maybe worth waiting for someone more familiar with compiler-rt to take a look as well.



================
Comment at: llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll:92
 ; CHECK-NEXT: .Ltmp{{.*}}:
 ; CHECK-NEXT:   nop
 ; CHECK-NEXT:   .word   3238382334  // 0xc105cafe
----------------
peter.smith wrote:
> Assuming the test is the equivalent of `-fpatchable-function-entry=1,1` I think this is the wrong place for the nop, I think it needs to be after the signature and the loads adjusted. For example with -fsanitize=kcfi -fpatchable-function-entries=1,1
> ```
> typedef int Fptr(void);
> 
> int function(void) {
>   return 0;
> }
> 
> int call(Fptr* fp) {
>   return fp();
> }
> ```
> Results in code like:
> ```
>         .word   1670944012                      // @call
>                                         // 0x6398950c
> .Ltmp1:
>         nop
> call:
> .Lfunc_begin1:
>         .cfi_startproc
> // %bb.0:                               // %entry
>         ldur    w16, [x0, #-8]
>         movk    w17, #50598
>         movk    w17, #14001, lsl #16
>         cmp     w16, w17
>         b.eq    .Ltmp2
>         brk     #0x8220
> .Ltmp2:
>         br      x0
> .Lfunc_end1:
> ```
> Note the NOP is after the signature, with the `ldur` having an offset of -8 and not the usual -4. I think you would need to make sure the signature is a branch instruction for each target for this scheme to work.
No, this looks correct to me. Note that in AsmPrinter the type hash is emitted after the patchable-function-prefix nops, while the KCFI type hash is emitted before them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148785