Open Quuxplusone opened 5 years ago
Attached trace_sched_wakeup-ff17db.c.gz
(911787 bytes, application/gzip): preprocessed kernel/trace/trace_sched_wakeup.c
Attached trace_sched_wakeup-ff17db.sh
(5958 bytes, application/x-shellscript): reproducer sript for sched_wakeup.c
Reduced test case to
#define ftrace_return_address(n) __builtin_return_address(n)
#define CALLER_ADDR1 ftrace_return_address(1)
fn1() { fn2(CALLER_ADDR1); }
Just like GCC, LLVM will not allow __builtin_return_address(N) for any N > 0 in the absence of a backchain (because there is no way to retrieve the return address in higher frames -- except by reading DWARF).
GCC does support builtin_return_address(N) for N > 0 if you also build with backchain (-mbackchain). LLVM doesn't do that even that. This is because originally LLVM didn't support -mbackchain at all. While it now does, we didn't change the builtin_return_address implementation.
If this is important, I think we could add the same support as is in GCC today. As in GCC, it would only work reliably if the whole code base is built with -mbackchain.
I understand that even the kernel doesn't really want the stack size overhead involved with -mbackchain, which is why a while back we implemented a special -mkernel-backchain feature in GCC just for the kernel. This isn't implemented in LLVM at all at the moment.
Why is it a requirement to build the kernel with LLVM? None of the distros do as far as I know ...
(In reply to Ulrich Weigand from comment #3)
> If this is important, I think we could add the same support as is in GCC
> today. As in GCC, it would only work reliably if the whole code base is
> built with -mbackchain.
>
> I understand that even the kernel doesn't really want the stack size
> overhead involved with -mbackchain, which is why a while back we implemented
> a special -mkernel-backchain feature in GCC just for the kernel. This isn't
> implemented in LLVM at all at the moment.
Ok, I see.
> Why is it a requirement to build the kernel with LLVM? None of the distros
> do as far as I know ...
Correct: to my knowledge, there is currently nobody trying to use such a kernel
in practice. My motivation was purely for build testing, and making sure that
the clang binaries I built for creating ARM kernels work across other
architectures as well.
I submitted a kernel patch to avoid the use __builtin_return_address() with
clang, see
https://lore.kernel.org/lkml/20190408212648.2407234-10-arnd@arndb.de/
Including that one in the kernel should be sufficient. However, it would be
helpful to have a more specific error message here (i.e. including the source
code location), and possibly turn it into a warning, to match the gcc behavior
when backchains are disabled. On the reduced test case, gcc only warns
test.c:3:9: warning: unsupported argument to '__builtin_return_address'
fn1() { fn2(CALLER_ADDR1); }
^~~~~~~~~~~~~~~~~
Why is it a requirement to build the kernel with LLVM? None of the distros do as far as I know ...
Kind of a non-starter if there's missing features the kernel depends on.
In general I'm happy to add features to LLVM where it makes sense to support building the kernel. In this particular case it doesn't seem necessary.
I've just talked to Martin and he confirms that the kernel (also on SystemZ) is moving to using ORC for unwinding, which implies that neither -mkernel-backchain nor __builtin_return_address(n) with n>0 will be used any more.
The only remaining issue is that it would indeed be nicer if we got a front-end warning instead of a back-end internal error. Unfortunately that's a deficiency in clang at the moment (__builtin_return_address is handled by platform-independent code that is not aware of platform-specific limitations). I'll see what I can do about that.
trace_sched_wakeup-ff17db.c.gz
(911787 bytes, application/gzip)trace_sched_wakeup-ff17db.sh
(5958 bytes, application/x-shellscript)