linux-kernel - Re: [PATCH] staging: rtl8723bs: Fix build error with Clang when inlining is disabled

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 16 Jan 2019 07:42:02 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Nathan Chancellor <natechancellor@...il.com>
Cc:     devel@...verdev.osuosl.org,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] staging: rtl8723bs: Fix build error with Clang when
 inlining is disabled

On Tue, Jan 15, 2019 at 11:03:02PM -0700, Nathan Chancellor wrote:
> When CONFIG_NO_AUTO_INLINE was present in linux-next (which added
> '-fno-inline-functions' to KBUILD_CFLAGS), an allyesconfig build with
> Clang failed at the modpost stage:
> 
> ERROR: "is_broadcast_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko] undefined!
> ERROR: "is_zero_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko] undefined!
> ERROR: "is_multicast_mac_addr" [drivers/staging/rtl8723bs/r8723bs.ko] undefined!
> 
> These functions were marked as extern inline, meaning that if inlining
> doesn't happen, the function will be undefined, as it is above.
> 
> This happens to work with GCC because the '-fno-inline-functions' option
> respects the __inline attribute so all instances of these functions are
> inlined as expected and the definition doesn't actually matter. However,
> with Clang and '-fno-inline-functions', a function has to be marked with
> the __always_inline attribute to be considered for inlining, which none
> of these functions are. Clang tries to find the symbol definition
> elsewhere as it was told and fails, which trickles down to modpost.
> 
> To make sure that this code compiles regardless of compiler and make the
> intention of the code clearer, use 'static __always_inline' to ensure
> that these functions are always inlined. Some alternative solutions
> included 'extern __always_inline' or converting these functions to
> macros (so the preprocessor deals with them) but I would argue this is
> the more "standard" solution.
> 
> Signed-off-by: Nathan Chancellor <natechancellor@...il.com>
> ---
>  drivers/staging/rtl8723bs/include/ieee80211.h | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/include/ieee80211.h b/drivers/staging/rtl8723bs/include/ieee80211.h
> index bcc8dfa8e672..959e822315b5 100644
> --- a/drivers/staging/rtl8723bs/include/ieee80211.h
> +++ b/drivers/staging/rtl8723bs/include/ieee80211.h
> @@ -850,18 +850,18 @@ enum ieee80211_state {
>  #define IP_FMT "%pI4"
>  #define IP_ARG(x) (x)
>  
> -extern __inline int is_multicast_mac_addr(const u8 *addr)
> +static __always_inline int is_multicast_mac_addr(const u8 *addr)

Ick, really?  This is in a .h file, the .c file sees this, so why isn't
clang picking it up?  Worst case it just makes it a "normal" function
and doesn't inline it, right?

How about just replacing "extern" with "static", that should solve this,
adding "__always_inline" everywhere is not going to be fun and doesn't
make any sense.

thanks,

greg k-h