Re: [BUG Report] undefined reference to `convert_to_fxsr'

[Miaohe Lin <linmiaohe@huawei.com>]

On 2023/1/6 7:14, Borislav Petkov wrote:
> On Tue, Jan 03, 2023 at 11:05:01AM +0800, Miaohe Lin wrote:
>> Yes, it still reproduces in my working server. It might be the problem of gcc
>> version.
> 
> What is that compiler you're using? Where did you get the package from? Does it
> have some out-of-tree patches in it?

My compiler is gcc 7.3.0:

linux-rtfsc:/home/linmiaohe/linux # gcc --version
gcc (GCC) 7.3.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

linux-rtfsc:/home/linmiaohe/linux #

And I cloned the code from https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git. It's a
clean git repo without applying any out-of-tree patch to it.

> 
>> I think it's because convert_to_fxsr() is only defined when CONFIG_X86_32 or
>> CONFIG_IA32_EMULATION is enabled.
> 
> No, convert_to_fxsr() is part of arch/x86/kernel/fpu/regset.c which is built-in
> unconditionally.
> 
> What happens is this here:
> 
> bool fpu__restore_sig(void __user *buf, int ia32_frame)
> 
> 	...
> 
>         } else {
>                 success = __fpu_restore_sig(buf, buf_fx, ia32_fxstate);
>         }
> 
> That ia32_fxstate is false because
> 
>         ia32_frame &= (IS_ENABLED(CONFIG_X86_32) ||
>                        IS_ENABLED(CONFIG_IA32_EMULATION));
> 
> 
> neither of those config items are set...
> 
>         /*
>          * Only FXSR enabled systems need the FX state quirk.
>          * FRSTOR does not need it and can use the fast path.
>          */
>         if (ia32_frame && use_fxsr()) {
>                 buf_fx = buf + sizeof(struct fregs_state);
>                 size += sizeof(struct fregs_state);
>                 ia32_fxstate = true;
> 		^^^^^^^^^^^^^^^^^^^
> 
> ... so this doesn't happen.
> 
>         }
> 
> Then, in __fpu_restore_sig() you have:
> 
>         if (likely(!ia32_fxstate)) {
>                 /* Restore the FPU registers directly from user memory. */
>                 return restore_fpregs_from_user(buf_fx, user_xfeatures, fx_only,
>                                                 state_size);
>         }
> 
> and since ia32_fxstate is false, we return here, the compiler sees that
> everything behind that code is dead code and eliminates it.

Many thanks for your explanation! It's really helpful. I checked generated asm code
and I found calling to convert_to_fxsr is not eliminated as expected:

objdump -S arch/x86/kernel/fpu/signal.o

 3fc:   e9 00 00 00 00          jmpq   401 <__fpu_restore_sig+0x401>
                        fpregs->xsave.header.xfeatures |= XFEATURE_MASK_FPSSE;
 401:   48 83 8d 40 02 00 00    orq    $0x3,0x240(%rbp)
 408:   03
        convert_to_fxsr(&fpregs->fxsave, &env);
	^^^^^^^^^^^^^^^
 409:   48 8d 74 24 10          lea    0x10(%rsp),%rsi

linux-rtfsc:/home/linmiaohe/linux # objdump -S arch/x86/kernel/fpu/signal.o | grep convert_to_fxsr
	convert_to_fxsr(&fpregs->fxsave, &env);
linux-rtfsc:/home/linmiaohe/linux #

And if I compile the code with below patch, convert_to_fxsr is eliminated:

linux-rtfsc:/home/linmiaohe/linux # objdump -S arch/x86/kernel/fpu/signal.o | grep convert_to_fxsr
linux-rtfsc:/home/linmiaohe/linux #

diff --git a/arch/x86/include/asm/fpu/signal.h b/arch/x86/include/asm/fpu/signal.h
index 611fa41711af..77ea052a8967 100644
--- a/arch/x86/include/asm/fpu/signal.h
+++ b/arch/x86/include/asm/fpu/signal.h
@@ -20,8 +20,14 @@

 extern void convert_from_fxsr(struct user_i387_ia32_struct *env,
                              struct task_struct *tsk);
+#if defined CONFIG_X86_32 || defined CONFIG_IA32_EMULATION
 extern void convert_to_fxsr(struct fxregs_state *fxsave,
                            const struct user_i387_ia32_struct *env);
+#else
+static inline void convert_to_fxsr(struct fxregs_state *fxsave,
+                                  const struct user_i387_ia32_struct *env)
+{}
+#endif

 unsigned long
 fpu__alloc_mathframe(unsigned long sp, int ia32_frame,
-- 
2.27.0

> 
> Your compiler doesn't, apparently.
> 
> It does remove it from regset.c, though, as it sees it is an unused function,
> which leads to this undefined reference.
> 
> So it looks like a funky compiler to me...

It seems another issue with old compiler. Sigh... :(

Thanks,
Miaohe Lin