Re: linux-next: build failure after merge of the bpf-next tree

Uros Bizjak
2025-03-20T07:49:15Z

On Thu, Mar 20, 2025 at 12:17 AM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Mar 19, 2025 at 12:44 PM Uros Bizjak <ubizjak@gmail.com> wrote:
> >
> > On Wed, Mar 19, 2025 at 7:56 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Wed, Mar 19, 2025 at 9:06 AM Uros Bizjak <ubizjak@gmail.com> wrote:
> > > >
> > > > On Wed, Mar 19, 2025 at 3:55 PM Alexei Starovoitov
> > > > <alexei.starovoitov@gmail.com> wrote:
> > > > >
> > > > > On Wed, Mar 19, 2025 at 7:36 AM Kumar Kartikeya Dwivedi
> > > > > <memxor@gmail.com> wrote:
> > > > > >
> > > > > > > >
> > > > > > > > I've sent a fix [0], but unfortunately I was unable to reproduce the
> > > > > > > > problem with an LLVM >= 19 build, idk why. I will try with GCC >= 14
> > > > > > > > as the patches require to confirm, but based on the error I am 99%
> > > > > > > > sure it will fix the problem.
> > > > > > >
> > > > > > > Probably because __seg_gs has CC_HAS_NAMED_AS depends on CC_IS_GCC.
> > > > > > > Let me give it a go with GCC.
> > > > > > >
> > > > > >
> > > > > > Can confirm now that this fixes it, I just did a build with GCC 14
> > > > > > where Uros's __percpu checks kick in.
> > > > >
> > > > > Great. Thanks for checking and quick fix.
> > > > >
> > > > > btw clang supports it with __attribute__((address_space(256))),
> > > > > so CC_IS_GCC probably should be relaxed.
> > > >
> > > > https://github.com/llvm/llvm-project/issues/93449
> > > >
> > > > needs to be fixed first. Also, the feature has to be thoroughly tested
> > > > (preferably by someone having a deep knowledge of clang) before it is
> > > > enabled by default.
> > >
> > > clang error makes sense to me.
> >
> > It is not an error, but an internal compiler error. This should never happen.
>
> Not quite. llvm backends don't have a good way to explain the error,
> but this is invalid condition.
> Arguably llvm should do a better job in such cases instead of
> printing stack trace.
>
> >
> > > What does it even mean to do addr space cast from percpu to normal address:
> > >
> > > __typeof__(int __seg_gs) const_pcpu_hot;
> > > void *__attribute____UNIQUE_ID___addressable_const_pcpu_hot612 =
> > > (void *)(long)&const_pcpu_hot;
> >
> > Please see [1] for an explanation.
> >
> > [1] https://gcc.gnu.org/onlinedocs/gcc/Named-Address-Spaces.html#x86-Named-Address-Spaces
>
> You didn't answer my question.

Actually, the above link explains and documents the issue:

"... these address spaces are not considered to be subspaces of the
generic (flat) address space. This means that explicit casts are
required to convert pointers between these address spaces and the
generic address space. In practice the application should cast to
uintptr_t and apply the segment base offset that it installed
previously."

IOW, for __seg_gs address space, there exists no (known) offset that
would define it as a subspace of the generic space. It is gs: prefix
that results in segment override that "switches" to __seg_gs address
space. So, to convert the pointer from __seg_gs to (nonsensical!)
generic address space, GCC allows explicit (void *)(uintptr_t) cast
that in effect just strips gs: prefix from the address. You can then
use the pointer as a pointer to a generic space, but you can't use it
to dereference data from __seg_gs address space - this would be
nonsensical, so (__seg_gs void *)(uintptr_t) cast is needed to convert
pointer back to __seg_gs AS.

> As suspected, gcc is producing garbage code.

Nope, this is expected and well documented behavior.

> See:
> https://godbolt.org/z/ozozYY3nv
>
> For
> void *ptr = (void *)(long)&pcpu_hot;
>
> gcc emits
> .quad pcpu_hot
> which is nonsensical, while clang refuses to produce garbage
> and dumps stack.
>
> Sadly, both compilers produce garbage for ret_addr()

No, they are correct. The pointer is explicitly cast to generic
address space and this is what you get.

> and both compilers produce correct code for ret_value().
> At least something.
>
> Uros,
> your percpu code is broken.
> you shouldn't rely on gcc producing garbage.
> Sooner or later gcc will start erroring on it just as clang.

It won't. It is well documented behavior, as documented in [1].
Regarding linux code, you "should not" pass a pointer to generic
address space to dereference percpu data. Currently,
__verify_percpu_ptr() only triggers a warning when sparse checking is
used, but my patchset will now enforce this as a compile-time error
(this was a much sought feature, and it was possible to implement only
recently by using the newly introduced typeof_unqual() operator). Rest
assured, before enabling this feature in linux, plenty of people
unsuccessfully tried to poke a hole in this functionality and long
threads are archived where address space functionality was discussed
to death. ;)

BTW: You can use:

--cut here--
diff --git a/arch/x86/include/asm/percpu.h b/arch/x86/include/asm/percpu.h
index 474d648bca9a..e6a7525c9db9 100644
--- a/arch/x86/include/asm/percpu.h
+++ b/arch/x86/include/asm/percpu.h
@@ -105,6 +105,10 @@
# define __my_cpu_type(var) typeof(var) __percpu_seg_override
# define __my_cpu_ptr(ptr) (__my_cpu_type(*(ptr))*)(__force uintptr_t)(ptr)
# define __my_cpu_var(var) (*__my_cpu_ptr(&(var)))
+
+# if __has_attribute(address_space) && defined(USE_TYPEOF_UNQUAL)
+# define __percpu_qual __attribute__((address_space(3)))
+# endif
#endif

#define __percpu_arg(x) __percpu_prefix "%" #x
--cut here--

to also see clang's address space checks in action on x86 even without
working __seg_gs support (You will need to disable
Wduplicate-decl-specifier warning).

HTH,
Uros.