Details:

 - ❯❯  clang --version
   Debian clang version 19.1.7 (3+build5)
   Target: x86_64-pc-linux-gnu
   Thread model: posix
   InstalledDir: /usr/lib/llvm-19/bin

 - Kernel config:

   https://gist.githubusercontent.com/bjackman/bbfdf4ec2e1dfd0e18657174f0537e2c/raw/a88dcc6567d14c69445e7928a7d5dfc23ca9f619/gistfile0.txt

Note I also get this error:

vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x3b: relocation to !ENDBR: machine_kexec_prepare+0x810

That one's a total mystery to me. I guess it's better to "fix" the SEV
one independently rather than waiting until I know how to fix them both.

Note I also mentioned other similar errors in [0]. Those errors don't
exist in Linus' master and I didn't note down where I saw them. Either
they have since been fixed, or I observed them in Google's internal
codebase where they were instroduced downstream.

This is a successor to [1] but I haven't called it a v2 because it's a
totally different solution. Thanks to Ard for the guidance and
corrections.

[0] https://lore.kernel.org/all/DERNCQGNRITE.139O331ACPKZ9@google.com/

[1] https://lore.kernel.org/all/20251117-b4-sev-gcov-objtool-v1-1-54f7790d54df@google.com/

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
Brendan Jackman (2):
      kasan: mark !__SANITIZE_ADDRESS__ stubs __always_inline
      kcsan: mark !__SANITIZE_THREAD__ stub __always_inline

 include/linux/kasan-checks.h | 4 ++--
 include/linux/kcsan-checks.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
---
base-commit: 67a454e6b1c604555c04501c77b7fedc5d98a779
change-id: 20251208-gcov-inline-noinstr-1550cfee445c

Best regards,
-- 
Brendan Jackman <jackmanb@google.com>

On Mon, 8 Dec 2025 at 02:35, Brendan Jackman <jackmanb@google.com> wrote:
>
> Details:
>
>  - ❯❯  clang --version
>    Debian clang version 19.1.7 (3+build5)
>    Target: x86_64-pc-linux-gnu
>    Thread model: posix
>    InstalledDir: /usr/lib/llvm-19/bin
>
>  - Kernel config:
>
>    https://gist.githubusercontent.com/bjackman/bbfdf4ec2e1dfd0e18657174f0537e2c/raw/a88dcc6567d14c69445e7928a7d5dfc23ca9f619/gistfile0.txt
>
> Note I also get this error:
>
> vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x3b: relocation to !ENDBR: machine_kexec_prepare+0x810
>
> That one's a total mystery to me. I guess it's better to "fix" the SEV
> one independently rather than waiting until I know how to fix them both.
>
> Note I also mentioned other similar errors in [0]. Those errors don't
> exist in Linus' master and I didn't note down where I saw them. Either
> they have since been fixed, or I observed them in Google's internal
> codebase where they were instroduced downstream.
>
> This is a successor to [1] but I haven't called it a v2 because it's a
> totally different solution. Thanks to Ard for the guidance and
> corrections.
>
> [0] https://lore.kernel.org/all/DERNCQGNRITE.139O331ACPKZ9@google.com/
>
> [1] https://lore.kernel.org/all/20251117-b4-sev-gcov-objtool-v1-1-54f7790d54df@google.com/

Why is [1] not the right solution?
The problem is we have lots of "inline" functions, and any one of them
could cause problems in future.

I don't mind turning "inline" into "__always_inline", but it seems
we're playing whack-a-mole here, and just disabling GCOV entirely
would make this noinstr.c file more robust.

> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---
> Brendan Jackman (2):
>       kasan: mark !__SANITIZE_ADDRESS__ stubs __always_inline
>       kcsan: mark !__SANITIZE_THREAD__ stub __always_inline
>
>  include/linux/kasan-checks.h | 4 ++--
>  include/linux/kcsan-checks.h | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> ---
> base-commit: 67a454e6b1c604555c04501c77b7fedc5d98a779
> change-id: 20251208-gcov-inline-noinstr-1550cfee445c
>
> Best regards,
> --
> Brendan Jackman <jackmanb@google.com>
>

On Mon, 8 Dec 2025 at 10:37, Marco Elver <elver@google.com> wrote:
>
> On Mon, 8 Dec 2025 at 02:35, Brendan Jackman <jackmanb@google.com> wrote:
> >
> > Details:
> >
> >  - ❯❯  clang --version
> >    Debian clang version 19.1.7 (3+build5)
> >    Target: x86_64-pc-linux-gnu
> >    Thread model: posix
> >    InstalledDir: /usr/lib/llvm-19/bin
> >
> >  - Kernel config:
> >
> >    https://gist.githubusercontent.com/bjackman/bbfdf4ec2e1dfd0e18657174f0537e2c/raw/a88dcc6567d14c69445e7928a7d5dfc23ca9f619/gistfile0.txt
> >
> > Note I also get this error:
> >
> > vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x3b: relocation to !ENDBR: machine_kexec_prepare+0x810
> >
> > That one's a total mystery to me. I guess it's better to "fix" the SEV
> > one independently rather than waiting until I know how to fix them both.
> >
> > Note I also mentioned other similar errors in [0]. Those errors don't
> > exist in Linus' master and I didn't note down where I saw them. Either
> > they have since been fixed, or I observed them in Google's internal
> > codebase where they were instroduced downstream.
> >
> > This is a successor to [1] but I haven't called it a v2 because it's a
> > totally different solution. Thanks to Ard for the guidance and
> > corrections.
> >
> > [0] https://lore.kernel.org/all/DERNCQGNRITE.139O331ACPKZ9@google.com/
> >
> > [1] https://lore.kernel.org/all/20251117-b4-sev-gcov-objtool-v1-1-54f7790d54df@google.com/
>
> Why is [1] not the right solution?
> The problem is we have lots of "inline" functions, and any one of them
> could cause problems in future.

Perhaps I should qualify: lots of *small* inline functions, including
those stubs.

> I don't mind turning "inline" into "__always_inline", but it seems
> we're playing whack-a-mole here, and just disabling GCOV entirely
> would make this noinstr.c file more robust.

To elaborate: `UBSAN_SANITIZE_noinstr.o := n` and
`K{A,C}SAN_SANITIZE_noinstr.o := n` is already set on this file.
Perhaps adding __always_inline to the stub functions here will be
enough today, but might no longer be in future. If you look at
<linux/instrumented.h>, we also have KMSAN. The KMSAN explicit
instrumentation doesn't appear to be invoked on that file today, but
given it shouldn't, we might consider:

KMSAN_SANITIZE_noinstr.o := n
GCOV_PROFILE_noinstr.o := n

The alternative is to audit the various sanitizer stub functions, and
mark all these "inline" stub functions as "__always_inline". The
changes made in this series are sufficient for the noinstr.c case, but
not complete.

Thanks,
-- Marco

On Mon Dec 8, 2025 at 11:12 AM UTC, Marco Elver wrote:
> On Mon, 8 Dec 2025 at 10:37, Marco Elver <elver@google.com> wrote:
>>
>> On Mon, 8 Dec 2025 at 02:35, Brendan Jackman <jackmanb@google.com> wrote:
>> >
>> > Details:
>> >
>> >  - ❯❯  clang --version
>> >    Debian clang version 19.1.7 (3+build5)
>> >    Target: x86_64-pc-linux-gnu
>> >    Thread model: posix
>> >    InstalledDir: /usr/lib/llvm-19/bin
>> >
>> >  - Kernel config:
>> >
>> >    https://gist.githubusercontent.com/bjackman/bbfdf4ec2e1dfd0e18657174f0537e2c/raw/a88dcc6567d14c69445e7928a7d5dfc23ca9f619/gistfile0.txt
>> >
>> > Note I also get this error:
>> >
>> > vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x3b: relocation to !ENDBR: machine_kexec_prepare+0x810
>> >
>> > That one's a total mystery to me. I guess it's better to "fix" the SEV
>> > one independently rather than waiting until I know how to fix them both.
>> >
>> > Note I also mentioned other similar errors in [0]. Those errors don't
>> > exist in Linus' master and I didn't note down where I saw them. Either
>> > they have since been fixed, or I observed them in Google's internal
>> > codebase where they were instroduced downstream.
>> >
>> > This is a successor to [1] but I haven't called it a v2 because it's a
>> > totally different solution. Thanks to Ard for the guidance and
>> > corrections.
>> >
>> > [0] https://lore.kernel.org/all/DERNCQGNRITE.139O331ACPKZ9@google.com/
>> >
>> > [1] https://lore.kernel.org/all/20251117-b4-sev-gcov-objtool-v1-1-54f7790d54df@google.com/
>>
>> Why is [1] not the right solution?
>> The problem is we have lots of "inline" functions, and any one of them
>> could cause problems in future.
>
> Perhaps I should qualify: lots of *small* inline functions, including
> those stubs.
>
>> I don't mind turning "inline" into "__always_inline", but it seems
>> we're playing whack-a-mole here, and just disabling GCOV entirely
>> would make this noinstr.c file more robust.
>
> To elaborate: `UBSAN_SANITIZE_noinstr.o := n` and
> `K{A,C}SAN_SANITIZE_noinstr.o := n` is already set on this file.
> Perhaps adding __always_inline to the stub functions here will be
> enough today, but might no longer be in future. 

Well you can also see it the other way around: disabling GCOV_PROFILE
might be enough today, but as soon as some other noinstr disables 
__SANITIZE_ADDRESS__ and expects to be able to call instrumented
helpers, that code will be broken too. 

I don't think we can avoid whack-a-mole here. In fact I think the whole
noinstr thing is an inevitable game of whack-a-mole unless we can get a
static anlyzer to find violations at the source level. I suspect there
are loads of violations in the tree that only show up in objtool if you
build in weird configs on a full moon.

One argument in favour of `GCOV_PROFILE_noinstr.o := n` would be: "this
is non-instrumentable code, the issue here is that it is getting
instrumented, so the fix is surely to stop instrumenting it". But, I
don't think that's really true, the issue is not with the
instrumentation but with the out-of-lining. Which highlights another
point: a sufficiently annoying compiler could out-of-line these
stub functions even without GCOV, right?

Still, despite my long-winded arguments I'm not gonna die on this hill,
I would be OK with both ways.

> If you look at
> <linux/instrumented.h>, we also have KMSAN. The KMSAN explicit
> instrumentation doesn't appear to be invoked on that file today, but
> given it shouldn't, we might consider:
>
> KMSAN_SANITIZE_noinstr.o := n
> GCOV_PROFILE_noinstr.o := n

This would make sense to me, although as I hinted above I think it's
sorta orthogonal and we should __always_inline the k[ca]san stubs
regardless.

> The alternative is to audit the various sanitizer stub functions, and
> mark all these "inline" stub functions as "__always_inline". The
> changes made in this series are sufficient for the noinstr.c case, but
> not complete.

Oh, yeah I should have  done __kcsan_{en,di}able_current() too I think.

Are there other stubs you are thinking of? I think we only care about the
!__SANITIZE_*__ stubs - we don't need this for !CONFIG_* stubs, right?
Anything else I'm forgetting?

On Tue, 9 Dec 2025 at 01:05, Brendan Jackman <jackmanb@google.com> wrote:
>
> On Mon Dec 8, 2025 at 11:12 AM UTC, Marco Elver wrote:
> > On Mon, 8 Dec 2025 at 10:37, Marco Elver <elver@google.com> wrote:
> >>
> >> On Mon, 8 Dec 2025 at 02:35, Brendan Jackman <jackmanb@google.com> wrote:
> >> >
> >> > Details:
> >> >
> >> >  - ❯❯  clang --version
> >> >    Debian clang version 19.1.7 (3+build5)
> >> >    Target: x86_64-pc-linux-gnu
> >> >    Thread model: posix
> >> >    InstalledDir: /usr/lib/llvm-19/bin
> >> >
> >> >  - Kernel config:
> >> >
> >> >    https://gist.githubusercontent.com/bjackman/bbfdf4ec2e1dfd0e18657174f0537e2c/raw/a88dcc6567d14c69445e7928a7d5dfc23ca9f619/gistfile0.txt
> >> >
> >> > Note I also get this error:
> >> >
> >> > vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x3b: relocation to !ENDBR: machine_kexec_prepare+0x810
> >> >
> >> > That one's a total mystery to me. I guess it's better to "fix" the SEV
> >> > one independently rather than waiting until I know how to fix them both.
> >> >
> >> > Note I also mentioned other similar errors in [0]. Those errors don't
> >> > exist in Linus' master and I didn't note down where I saw them. Either
> >> > they have since been fixed, or I observed them in Google's internal
> >> > codebase where they were instroduced downstream.
> >> >
> >> > This is a successor to [1] but I haven't called it a v2 because it's a
> >> > totally different solution. Thanks to Ard for the guidance and
> >> > corrections.
> >> >
> >> > [0] https://lore.kernel.org/all/DERNCQGNRITE.139O331ACPKZ9@google.com/
> >> >
> >> > [1] https://lore.kernel.org/all/20251117-b4-sev-gcov-objtool-v1-1-54f7790d54df@google.com/
> >>
> >> Why is [1] not the right solution?
> >> The problem is we have lots of "inline" functions, and any one of them
> >> could cause problems in future.
> >
> > Perhaps I should qualify: lots of *small* inline functions, including
> > those stubs.
> >
> >> I don't mind turning "inline" into "__always_inline", but it seems
> >> we're playing whack-a-mole here, and just disabling GCOV entirely
> >> would make this noinstr.c file more robust.
> >
> > To elaborate: `UBSAN_SANITIZE_noinstr.o := n` and
> > `K{A,C}SAN_SANITIZE_noinstr.o := n` is already set on this file.
> > Perhaps adding __always_inline to the stub functions here will be
> > enough today, but might no longer be in future.
>
> Well you can also see it the other way around: disabling GCOV_PROFILE
> might be enough today, but as soon as some other noinstr disables
> __SANITIZE_ADDRESS__ and expects to be able to call instrumented
> helpers, that code will be broken too.

This itself is a contradiction: a `noinstr` function should not call
instrumented helpers. Normally this all works due to the compiler's
function attributes working as intended for the compiler-inserted
instrumentation, but for explicitly inserted instrumentation it's
obviously not. In otherwise instrumented files with few (not all)
`noinstr` functions, making the stub functions `__always_inline` will
not work, because the preprocessor is applied globally not per
function. In the past, I recall the underlying implementation being
used of e.g. the bitops (arch_foo... or __foo) in `noinstr` functions
to solve that.

The other hammer we have is K[ACM]SAN_SANITIZE_foo.o := n,
GCOV_PROFILE_foo.o := n, and KCOV_INSTRUMENT_foo.o := n.

> I don't think we can avoid whack-a-mole here. In fact I think the whole
> noinstr thing is an inevitable game of whack-a-mole unless we can get a
> static anlyzer to find violations at the source level. I suspect there
> are loads of violations in the tree that only show up in objtool if you
> build in weird configs on a full moon.
>
> One argument in favour of `GCOV_PROFILE_noinstr.o := n` would be: "this
> is non-instrumentable code, the issue here is that it is getting
> instrumented, so the fix is surely to stop instrumenting it". But, I
> don't think that's really true, the issue is not with the
> instrumentation but with the out-of-lining. Which highlights another
> point: a sufficiently annoying compiler could out-of-line these
> stub functions even without GCOV, right?

This would be a compiler bug in my book. Without instrumentation a
"static inline" function with nothing in it not being inlined is an
optimization bug. But those things get caught because it'd have made
someone's system slow.

> Still, despite my long-winded arguments I'm not gonna die on this hill,
> I would be OK with both ways.

To some extent I think doing both to reduce the chance of issues in
future might be what you want. On the other hand, avoiding the
Makefile-level opt-out will help catch more corner cases in future,
which may or may not be helpful outside this noinstr.c file.

> > If you look at
> > <linux/instrumented.h>, we also have KMSAN. The KMSAN explicit
> > instrumentation doesn't appear to be invoked on that file today, but
> > given it shouldn't, we might consider:
> >
> > KMSAN_SANITIZE_noinstr.o := n
> > GCOV_PROFILE_noinstr.o := n
>
> This would make sense to me, although as I hinted above I think it's
> sorta orthogonal and we should __always_inline the k[ca]san stubs
> regardless.
>
> > The alternative is to audit the various sanitizer stub functions, and
> > mark all these "inline" stub functions as "__always_inline". The
> > changes made in this series are sufficient for the noinstr.c case, but
> > not complete.
>
> Oh, yeah I should have  done __kcsan_{en,di}able_current() too I think.
>
> Are there other stubs you are thinking of? I think we only care about the
> !__SANITIZE_*__ stubs - we don't need this for !CONFIG_* stubs, right?
> Anything else I'm forgetting?

Initially, I think !__SANITIZE_* stubs are enough. Well, basically
anything that appears in <linux/instrumented.h>, because all those are
__always_inline, we should make the called functions also
__always_inline.

If you think that'll be enough, and the Makefile-level opt-out is
redundant as a result, let's just do that.

Thanks,
-- Marco

On Tue Dec 9, 2025 at 12:52 AM UTC, Marco Elver wrote:
> On Tue, 9 Dec 2025 at 01:05, Brendan Jackman <jackmanb@google.com> wrote:
>>
>> On Mon Dec 8, 2025 at 11:12 AM UTC, Marco Elver wrote:
>> > On Mon, 8 Dec 2025 at 10:37, Marco Elver <elver@google.com> wrote:
>> >>
>> >> On Mon, 8 Dec 2025 at 02:35, Brendan Jackman <jackmanb@google.com> wrote:
>> >> >
>> >> > Details:
>> >> >
>> >> >  - ❯❯  clang --version
>> >> >    Debian clang version 19.1.7 (3+build5)
>> >> >    Target: x86_64-pc-linux-gnu
>> >> >    Thread model: posix
>> >> >    InstalledDir: /usr/lib/llvm-19/bin
>> >> >
>> >> >  - Kernel config:
>> >> >
>> >> >    https://gist.githubusercontent.com/bjackman/bbfdf4ec2e1dfd0e18657174f0537e2c/raw/a88dcc6567d14c69445e7928a7d5dfc23ca9f619/gistfile0.txt
>> >> >
>> >> > Note I also get this error:
>> >> >
>> >> > vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x3b: relocation to !ENDBR: machine_kexec_prepare+0x810
>> >> >
>> >> > That one's a total mystery to me. I guess it's better to "fix" the SEV
>> >> > one independently rather than waiting until I know how to fix them both.
>> >> >
>> >> > Note I also mentioned other similar errors in [0]. Those errors don't
>> >> > exist in Linus' master and I didn't note down where I saw them. Either
>> >> > they have since been fixed, or I observed them in Google's internal
>> >> > codebase where they were instroduced downstream.
>> >> >
>> >> > This is a successor to [1] but I haven't called it a v2 because it's a
>> >> > totally different solution. Thanks to Ard for the guidance and
>> >> > corrections.
>> >> >
>> >> > [0] https://lore.kernel.org/all/DERNCQGNRITE.139O331ACPKZ9@google.com/
>> >> >
>> >> > [1] https://lore.kernel.org/all/20251117-b4-sev-gcov-objtool-v1-1-54f7790d54df@google.com/
>> >>
>> >> Why is [1] not the right solution?
>> >> The problem is we have lots of "inline" functions, and any one of them
>> >> could cause problems in future.
>> >
>> > Perhaps I should qualify: lots of *small* inline functions, including
>> > those stubs.
>> >
>> >> I don't mind turning "inline" into "__always_inline", but it seems
>> >> we're playing whack-a-mole here, and just disabling GCOV entirely
>> >> would make this noinstr.c file more robust.
>> >
>> > To elaborate: `UBSAN_SANITIZE_noinstr.o := n` and
>> > `K{A,C}SAN_SANITIZE_noinstr.o := n` is already set on this file.
>> > Perhaps adding __always_inline to the stub functions here will be
>> > enough today, but might no longer be in future.
>>
>> Well you can also see it the other way around: disabling GCOV_PROFILE
>> might be enough today, but as soon as some other noinstr disables
>> __SANITIZE_ADDRESS__ and expects to be able to call instrumented
>> helpers, that code will be broken too.
>
> This itself is a contradiction: a `noinstr` function should not call
> instrumented helpers. Normally this all works due to the compiler's
> function attributes working as intended for the compiler-inserted
> instrumentation, but for explicitly inserted instrumentation it's
> obviously not. In otherwise instrumented files with few (not all)
> `noinstr` functions, making the stub functions `__always_inline` will
> not work, because the preprocessor is applied globally not per
> function. In the past, I recall the underlying implementation being
> used of e.g. the bitops (arch_foo... or __foo) in `noinstr` functions
> to solve that.

Sorry I dropped an important word here, I meant to say other noinstr
_files_. I.e. anything else similar to SEV's noinstr.c that is doing
noinstr at the file level.

>> Still, despite my long-winded arguments I'm not gonna die on this hill,
>> I would be OK with both ways.
>
> To some extent I think doing both to reduce the chance of issues in
> future might be what you want. On the other hand, avoiding the
> Makefile-level opt-out will help catch more corner cases in future,
> which may or may not be helpful outside this noinstr.c file.

Cool, then yeah I think I will do both unless anyone shows up to object
to that. Both things ultimately make sense on their own merit and even
if you only need one or the other to make the error go away, I don't
think that actually makes them "redundant".

>> > The alternative is to audit the various sanitizer stub functions, and
>> > mark all these "inline" stub functions as "__always_inline". The
>> > changes made in this series are sufficient for the noinstr.c case, but
>> > not complete.
>>
>> Oh, yeah I should have  done __kcsan_{en,di}able_current() too I think.
>>
>> Are there other stubs you are thinking of? I think we only care about the
>> !__SANITIZE_*__ stubs - we don't need this for !CONFIG_* stubs, right?
>> Anything else I'm forgetting?
>
> Initially, I think !__SANITIZE_* stubs are enough. Well, basically
> anything that appears in <linux/instrumented.h>, because all those are
> __always_inline, we should make the called functions also
> __always_inline.

Ack, thanks for all the input here! I'll probably wait until after LPC
to do a v2.

On Tue, 9 Dec 2025 at 03:25, Brendan Jackman <jackmanb@google.com> wrote:
> On Tue Dec 9, 2025 at 12:52 AM UTC, Marco Elver wrote:
> > On Tue, 9 Dec 2025 at 01:05, Brendan Jackman <jackmanb@google.com> wrote:
> >> On Mon Dec 8, 2025 at 11:12 AM UTC, Marco Elver wrote:
> >> > On Mon, 8 Dec 2025 at 10:37, Marco Elver <elver@google.com> wrote:
> >> >>
> >> >> On Mon, 8 Dec 2025 at 02:35, Brendan Jackman <jackmanb@google.com> wrote:
> >> >> >
> >> >> > Details:
> >> >> >
> >> >> >  - ❯❯  clang --version
> >> >> >    Debian clang version 19.1.7 (3+build5)
> >> >> >    Target: x86_64-pc-linux-gnu
> >> >> >    Thread model: posix
> >> >> >    InstalledDir: /usr/lib/llvm-19/bin
> >> >> >
> >> >> >  - Kernel config:
> >> >> >
> >> >> >    https://gist.githubusercontent.com/bjackman/bbfdf4ec2e1dfd0e18657174f0537e2c/raw/a88dcc6567d14c69445e7928a7d5dfc23ca9f619/gistfile0.txt
> >> >> >
> >> >> > Note I also get this error:
> >> >> >
> >> >> > vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x3b: relocation to !ENDBR: machine_kexec_prepare+0x810
> >> >> >
> >> >> > That one's a total mystery to me. I guess it's better to "fix" the SEV
> >> >> > one independently rather than waiting until I know how to fix them both.
> >> >> >
> >> >> > Note I also mentioned other similar errors in [0]. Those errors don't
> >> >> > exist in Linus' master and I didn't note down where I saw them. Either
> >> >> > they have since been fixed, or I observed them in Google's internal
> >> >> > codebase where they were instroduced downstream.
> >> >> >
> >> >> > This is a successor to [1] but I haven't called it a v2 because it's a
> >> >> > totally different solution. Thanks to Ard for the guidance and
> >> >> > corrections.
> >> >> >
> >> >> > [0] https://lore.kernel.org/all/DERNCQGNRITE.139O331ACPKZ9@google.com/
> >> >> >
> >> >> > [1] https://lore.kernel.org/all/20251117-b4-sev-gcov-objtool-v1-1-54f7790d54df@google.com/
> >> >>
> >> >> Why is [1] not the right solution?
> >> >> The problem is we have lots of "inline" functions, and any one of them
> >> >> could cause problems in future.
> >> >
> >> > Perhaps I should qualify: lots of *small* inline functions, including
> >> > those stubs.
> >> >
> >> >> I don't mind turning "inline" into "__always_inline", but it seems
> >> >> we're playing whack-a-mole here, and just disabling GCOV entirely
> >> >> would make this noinstr.c file more robust.
> >> >
> >> > To elaborate: `UBSAN_SANITIZE_noinstr.o := n` and
> >> > `K{A,C}SAN_SANITIZE_noinstr.o := n` is already set on this file.
> >> > Perhaps adding __always_inline to the stub functions here will be
> >> > enough today, but might no longer be in future.
> >>
> >> Well you can also see it the other way around: disabling GCOV_PROFILE
> >> might be enough today, but as soon as some other noinstr disables
> >> __SANITIZE_ADDRESS__ and expects to be able to call instrumented
> >> helpers, that code will be broken too.
> >
> > This itself is a contradiction: a `noinstr` function should not call
> > instrumented helpers. Normally this all works due to the compiler's
> > function attributes working as intended for the compiler-inserted
> > instrumentation, but for explicitly inserted instrumentation it's
> > obviously not. In otherwise instrumented files with few (not all)
> > `noinstr` functions, making the stub functions `__always_inline` will
> > not work, because the preprocessor is applied globally not per
> > function. In the past, I recall the underlying implementation being
> > used of e.g. the bitops (arch_foo... or __foo) in `noinstr` functions
> > to solve that.
>
> Sorry I dropped an important word here, I meant to say other noinstr
> _files_. I.e. anything else similar to SEV's noinstr.c that is doing
> noinstr at the file level.

Someone at LPC (I couldn't make out who due to technical difficulties)
mentioned that calling explicitly instrumented helpers from noinstr
functions is a general problem.

After that I sat down and finally got around to implement the builtin
that should solve this once and for all, regardless of where it's
called: https://github.com/llvm/llvm-project/pull/172030
What this will allow us to do is to remove the
"K[AC]SAN_SANITIZE_noinstr.o := n" lines from the Makefile, and purely
rely on the noinstr attribute, even in the presence of explicit
instrumentation calls.

It will be a while until it's available and the kernel needs patches
to use it, too, but that should be easy enough. Once it lands in
Clang, it'd be nice if GCC could provide the same.

So for the time being, we still need your patches here to work around
the problem.

(cc Kees)

On Sat, 13 Dec 2025 at 01:11, Marco Elver <elver@google.com> wrote:
>
> On Tue, 9 Dec 2025 at 03:25, Brendan Jackman <jackmanb@google.com> wrote:
> > On Tue Dec 9, 2025 at 12:52 AM UTC, Marco Elver wrote:
> > > On Tue, 9 Dec 2025 at 01:05, Brendan Jackman <jackmanb@google.com> wrote:
> > >> On Mon Dec 8, 2025 at 11:12 AM UTC, Marco Elver wrote:
> > >> > On Mon, 8 Dec 2025 at 10:37, Marco Elver <elver@google.com> wrote:
> > >> >>
> > >> >> On Mon, 8 Dec 2025 at 02:35, Brendan Jackman <jackmanb@google.com> wrote:
> > >> >> >
> > >> >> > Details:
> > >> >> >
> > >> >> >  - ❯❯  clang --version
> > >> >> >    Debian clang version 19.1.7 (3+build5)
> > >> >> >    Target: x86_64-pc-linux-gnu
> > >> >> >    Thread model: posix
> > >> >> >    InstalledDir: /usr/lib/llvm-19/bin
> > >> >> >
> > >> >> >  - Kernel config:
> > >> >> >
> > >> >> >    https://gist.githubusercontent.com/bjackman/bbfdf4ec2e1dfd0e18657174f0537e2c/raw/a88dcc6567d14c69445e7928a7d5dfc23ca9f619/gistfile0.txt
> > >> >> >
> > >> >> > Note I also get this error:
> > >> >> >
> > >> >> > vmlinux.o: warning: objtool: set_ftrace_ops_ro+0x3b: relocation to !ENDBR: machine_kexec_prepare+0x810
> > >> >> >
> > >> >> > That one's a total mystery to me. I guess it's better to "fix" the SEV
> > >> >> > one independently rather than waiting until I know how to fix them both.
> > >> >> >
> > >> >> > Note I also mentioned other similar errors in [0]. Those errors don't
> > >> >> > exist in Linus' master and I didn't note down where I saw them. Either
> > >> >> > they have since been fixed, or I observed them in Google's internal
> > >> >> > codebase where they were instroduced downstream.
> > >> >> >
> > >> >> > This is a successor to [1] but I haven't called it a v2 because it's a
> > >> >> > totally different solution. Thanks to Ard for the guidance and
> > >> >> > corrections.
> > >> >> >
> > >> >> > [0] https://lore.kernel.org/all/DERNCQGNRITE.139O331ACPKZ9@google.com/
> > >> >> >
> > >> >> > [1] https://lore.kernel.org/all/20251117-b4-sev-gcov-objtool-v1-1-54f7790d54df@google.com/
> > >> >>
> > >> >> Why is [1] not the right solution?
> > >> >> The problem is we have lots of "inline" functions, and any one of them
> > >> >> could cause problems in future.
> > >> >
> > >> > Perhaps I should qualify: lots of *small* inline functions, including
> > >> > those stubs.
> > >> >
> > >> >> I don't mind turning "inline" into "__always_inline", but it seems
> > >> >> we're playing whack-a-mole here, and just disabling GCOV entirely
> > >> >> would make this noinstr.c file more robust.
> > >> >
> > >> > To elaborate: `UBSAN_SANITIZE_noinstr.o := n` and
> > >> > `K{A,C}SAN_SANITIZE_noinstr.o := n` is already set on this file.
> > >> > Perhaps adding __always_inline to the stub functions here will be
> > >> > enough today, but might no longer be in future.
> > >>
> > >> Well you can also see it the other way around: disabling GCOV_PROFILE
> > >> might be enough today, but as soon as some other noinstr disables
> > >> __SANITIZE_ADDRESS__ and expects to be able to call instrumented
> > >> helpers, that code will be broken too.
> > >
> > > This itself is a contradiction: a `noinstr` function should not call
> > > instrumented helpers. Normally this all works due to the compiler's
> > > function attributes working as intended for the compiler-inserted
> > > instrumentation, but for explicitly inserted instrumentation it's
> > > obviously not. In otherwise instrumented files with few (not all)
> > > `noinstr` functions, making the stub functions `__always_inline` will
> > > not work, because the preprocessor is applied globally not per
> > > function. In the past, I recall the underlying implementation being
> > > used of e.g. the bitops (arch_foo... or __foo) in `noinstr` functions
> > > to solve that.
> >
> > Sorry I dropped an important word here, I meant to say other noinstr
> > _files_. I.e. anything else similar to SEV's noinstr.c that is doing
> > noinstr at the file level.
>
> Someone at LPC (I couldn't make out who due to technical difficulties)
> mentioned that calling explicitly instrumented helpers from noinstr
> functions is a general problem.
>

That was me.

> After that I sat down and finally got around to implement the builtin
> that should solve this once and for all, regardless of where it's
> called: https://github.com/llvm/llvm-project/pull/172030
> What this will allow us to do is to remove the
> "K[AC]SAN_SANITIZE_noinstr.o := n" lines from the Makefile, and purely
> rely on the noinstr attribute, even in the presence of explicit
> instrumentation calls.
>

Excellent! Thanks for the quick fix. Happy to test and/or look into
the kernel side of this once this lands.

On Sat, Dec 13, 2025 at 08:59:44AM +0900, Ard Biesheuvel wrote:

> > After that I sat down and finally got around to implement the builtin
> > that should solve this once and for all, regardless of where it's
> > called: https://github.com/llvm/llvm-project/pull/172030
> > What this will allow us to do is to remove the
> > "K[AC]SAN_SANITIZE_noinstr.o := n" lines from the Makefile, and purely
> > rely on the noinstr attribute, even in the presence of explicit
> > instrumentation calls.
> >
> 
> Excellent! Thanks for the quick fix. Happy to test and/or look into
> the kernel side of this once this lands.

Well, would not GCC need to grow the same thing and then we must wait
until these versions are the minimum supported versions for sanitizer
builds.

I mean, the extension is nice, but I'm afraid we can't really use it
until much later :/

On Thu, 18 Dec 2025 at 10:51, Peter Zijlstra <peterz@infradead.org> wrote:
> On Sat, Dec 13, 2025 at 08:59:44AM +0900, Ard Biesheuvel wrote:
>
> > > After that I sat down and finally got around to implement the builtin
> > > that should solve this once and for all, regardless of where it's
> > > called: https://github.com/llvm/llvm-project/pull/172030
> > > What this will allow us to do is to remove the
> > > "K[AC]SAN_SANITIZE_noinstr.o := n" lines from the Makefile, and purely
> > > rely on the noinstr attribute, even in the presence of explicit
> > > instrumentation calls.
> > >
> >
> > Excellent! Thanks for the quick fix. Happy to test and/or look into
> > the kernel side of this once this lands.
>
> Well, would not GCC need to grow the same thing and then we must wait
> until these versions are the minimum supported versions for sanitizer
> builds.
>
> I mean, the extension is nice, but I'm afraid we can't really use it
> until much later :/

Unfortunately, yes. But let's try to get the builtin into Clang and
GCC now (for the latter, need to Cc GCC folks to help).

Then we wait for 5 years. :-)

There's a possibility to try and backport it to stable Clang and GCC
versions, but it's a long stretch (extremely unlikely).

Hi!

On Thu, Dec 18, 2025 at 10:56:48AM +0100, Marco Elver wrote:
> On Thu, 18 Dec 2025 at 10:51, Peter Zijlstra <peterz@infradead.org> wrote:
> > On Sat, Dec 13, 2025 at 08:59:44AM +0900, Ard Biesheuvel wrote:
> >
> > > > After that I sat down and finally got around to implement the builtin
> > > > that should solve this once and for all, regardless of where it's
> > > > called: https://github.com/llvm/llvm-project/pull/172030
> > > > What this will allow us to do is to remove the
> > > > "K[AC]SAN_SANITIZE_noinstr.o := n" lines from the Makefile, and purely
> > > > rely on the noinstr attribute, even in the presence of explicit
> > > > instrumentation calls.
> > > >
> > >
> > > Excellent! Thanks for the quick fix. Happy to test and/or look into
> > > the kernel side of this once this lands.
> >
> > Well, would not GCC need to grow the same thing and then we must wait
> > until these versions are the minimum supported versions for sanitizer
> > builds.
> >
> > I mean, the extension is nice, but I'm afraid we can't really use it
> > until much later :/
> 
> Unfortunately, yes. But let's try to get the builtin into Clang and
> GCC now (for the latter, need to Cc GCC folks to help).
> 
> Then we wait for 5 years. :-)
> 
> There's a possibility to try and backport it to stable Clang and GCC
> versions, but it's a long stretch (extremely unlikely).

We (GCC) do not generally want to do backport features; even for
bugfixes the risk/reward ratio comes into the picture.  It *can* be done
if some feature is important enough of course.  If you have to wonder or
ask if your feature is important enough, it is not.

The reason we do not want backports of feature is it increases
maintenance cost a lot, and so, development costs as well.

I guess LLVM has a similar policy, but I of course do not speak for
them.

You might have more success getting the stuff backported to some
distro(s) you care about?  Or get people to use newer compilers more
quickly of course, "five years" before people have it is pretty
ridiculous, two years is at the tail end of things already.


Segher

On Thu, Dec 18, 2025 at 05:58:44AM -0600, Segher Boessenkool wrote:

> You might have more success getting the stuff backported to some
> distro(s) you care about?  Or get people to use newer compilers more
> quickly of course, "five years" before people have it is pretty
> ridiculous, two years is at the tail end of things already.

There is a difference between having and requiring it :/ Our current
minimum compiler version is gcc-8 or clang-15 (IIRC).

On the bright side, I think we can be more aggressively with compiler
versions for debug builds vs regular builds. Not being able to build a
KASAN/UBSAN/whateverSAN kernel isn't too big of a problem (IMO).

Hi!

On Thu, Dec 18, 2025 at 01:18:13PM +0100, Peter Zijlstra wrote:
> On Thu, Dec 18, 2025 at 05:58:44AM -0600, Segher Boessenkool wrote:
> 
> > You might have more success getting the stuff backported to some
> > distro(s) you care about?  Or get people to use newer compilers more
> > quickly of course, "five years" before people have it is pretty
> > ridiculous, two years is at the tail end of things already.
> 
> There is a difference between having and requiring it :/ Our current
> minimum compiler version is gcc-8 or clang-15 (IIRC).

Very much so.  If you have good reasons for requiring it, make sure you
voice that with your backport request!

Nothing we (again, GCC) do is *only* motivated by procedures.  We can do
unusual things in unusual situations.  But you need extraordinary
evidence for why extraordinary things would be needed, of course.  Does
that apply here, you think?

> On the bright side, I think we can be more aggressively with compiler
> versions for debug builds vs regular builds. Not being able to build a
> KASAN/UBSAN/whateverSAN kernel isn't too big of a problem (IMO).

Absolutely.  Just document the feature as needing a recent compiler!


Segher