peter.smith added inline comments.
================
Comment at: llvm/test/CodeGen/AArch64/patchable-function-entry-bti.ll:92
; CHECK-NEXT: .Ltmp{{.*}}:
; CHECK-NEXT: nop
; CHECK-NEXT: .word 3238382334 // 0xc105cafe
----------------
samitolvanen wrote:
> 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.
My concern is more along the lines of how would this function be patched if the
code doing the patching were unaware of the signature. I'm not familiar with
how the kernel uses the nops so it could be that this is won't be a problem in
practice.
With -fsanitize=kcfi it looks like there is no difference to the code doing the
patching as the nops are in the same place with respect to the function entry
point and there is no fall through into the signature.
With -fsanitize=function anything patching the first nop as an instruction
would need to branch over the signature (unless the first entry of the
signature was a branch instruction, but that would mean a target specific
signature), obviously if the nop is patched with data and isn't the entry point
then this doesn't matter. The code doing the patching would also need to know
how to locate the nop ahead of the signature.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D148785/new/
https://reviews.llvm.org/D148785
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
