__attribute__((naked)) should disable -finstrument-function*

[pcc]

The asm in a naked function may reasonably expect the argument registers to be live, however they may be clobbered by the instrumentation function called by -finstrument-function*. Therefore, we shouldn't insert calls to the instrumentation functions in naked functions.

[pcc]

This is particularly bad with -finstrument-function-entry-bare:

$ cat /tmp/test.c
__attribute__((naked)) void f() {
  __asm__("ret");
}
$ clang -S -o - /tmp/test.c --target=aarch64-none-linux-android29 -finstrument-function-entry-bare
        .text
        .file   "test.c"
        .globl  f                               // -- Begin function f
        .p2align        2
        .type   f,@function
f:                                      // @f
        .cfi_startproc
// %bb.0:                               // %entry
        bl      __cyg_profile_func_enter_bare
        //APP
        ret
        //NO_APP
.Lfunc_end0:
        .size   f, .Lfunc_end0-f
        .cfi_endproc
                                        // -- End function
        .ident  "clang version 17.0.0"
        .section        ".note.GNU-stack","",@progbits

There's no way for f to return to its caller because the bl instruction clobbered the original return address.

[MaskRay]

I have surveyed a few GCC ports that support -finstrument-function: arm, x86, riscv. They don't skip instrumenting __attribute__((naked)) functions. One stance is that compilers should not smart. By using __attribute__((naked)), users should be aware that certain instrumentation options may not work. __attribute__((no_instrument_function)).

If we want to make a change, informing GCC will be a good idea.

[pcc]

One stance is that compilers should not smart. By using __attribute__((naked)), users should be aware that certain instrumentation options may not work.

One counterargument to that is that the developer who added __attribute__((naked)) may not be the same person as the one who added -finstrument-function*. I discovered this bug by doing the latter, and it took quite a bit of debugging effort to track down why AOSP was failing to boot after building the operating system with this option enabled.

I think there's really only one reasonable way to handle the combination of these options, and that's to disable -finstrument-function* for that function, because in general we have no idea how to safely add code to it.

They don't skip instrumenting __attribute__((naked)) functions.

I also noticed that GCC behavior but I think it is a bug, as it violates their own documentation for naked (e.g. https://gcc.gnu.org/onlinedocs/gcc-13.1.0/gcc/x86-Function-Attributes.html) which states:

The specified function will not have prologue/epilogue sequences generated by the compiler.

If we want to make a change, informing GCC will be a good idea.

Makes sense.

[MaskRay]

patch: https://reviews.llvm.org/D149721
GCC discussion: https://gcc.gnu.org/PR109707