Re: [PATCH v3] Compiler Attributes: don't pollute userspace with macro definitions

2018-12-14 Thread Nick Desaulniers
On Fri, Dec 14, 2018 at 8:06 AM Miguel Ojeda
 wrote:
>
> On Fri, Dec 14, 2018 at 3:16 PM Xiaozhou Liu  
> wrote:
> >
> > Macros 'inline' and '__gnu_inline' used to be defined in compiler-gcc.h,
> > which was (and is) included entirely in (__KERNEL__ && !__ASSEMBLY__).
> > Commit 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
> > exclusive") had those macros exposed to userspace, unintentionally.
> >
> > Then commit a3f8a30f3f00 ("Compiler Attributes: use feature checks
> > instead of version checks") moved '__gnu_inline' back into
> > (__KERNEL__ && !__ASSEMBLY__) and 'inline' was left behind. Since 'inline'
> > depends on '__gnu_inline', compiling error showing "unknown type name
> > ‘__gnu_inline’" will pop up, if userspace somehow includes
> > .
> >
> > Other macros like __must_check, notrace, etc. are in a similar situation.
> > So just move all these macros back into (__KERNEL__ && !__ASSEMBLY__).
> >
> > Note:
> >   1. This patch only affects what userspace sees.
> >   2. __must_check (when !CONFIG_ENABLE_MUST_CHECK) and noinline_for_stack
> >  were once defined in __KERNEL__ only, but we believe that they can
> >  be put into !__ASSEMBLY__ too.
> >
> > Acked-by: Nick Desaulniers 
> > Signed-off-by: Xiaozhou Liu 
>
> Thanks Xiaozhou, picked into compiler-attributes with a slightly
> modified title (since it is not about compiler attributes this time),
> and let's see if there is any problem with it in linux-next.
>
> Nick: I kept your Ack, even if it is a different patch. Let me know if
> you don't want to Ack this version.

Doubled-acked-by: Nick Desaulniers 
(I recognize it's always ambiguous whether to carry forward sign-offs
when a patch set changes)
-- 
Thanks,
~Nick Desaulniers


Re: [PATCH v3] Compiler Attributes: don't pollute userspace with macro definitions

2018-12-14 Thread Miguel Ojeda
On Fri, Dec 14, 2018 at 3:16 PM Xiaozhou Liu  wrote:
>
> Macros 'inline' and '__gnu_inline' used to be defined in compiler-gcc.h,
> which was (and is) included entirely in (__KERNEL__ && !__ASSEMBLY__).
> Commit 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
> exclusive") had those macros exposed to userspace, unintentionally.
>
> Then commit a3f8a30f3f00 ("Compiler Attributes: use feature checks
> instead of version checks") moved '__gnu_inline' back into
> (__KERNEL__ && !__ASSEMBLY__) and 'inline' was left behind. Since 'inline'
> depends on '__gnu_inline', compiling error showing "unknown type name
> ‘__gnu_inline’" will pop up, if userspace somehow includes
> .
>
> Other macros like __must_check, notrace, etc. are in a similar situation.
> So just move all these macros back into (__KERNEL__ && !__ASSEMBLY__).
>
> Note:
>   1. This patch only affects what userspace sees.
>   2. __must_check (when !CONFIG_ENABLE_MUST_CHECK) and noinline_for_stack
>  were once defined in __KERNEL__ only, but we believe that they can
>  be put into !__ASSEMBLY__ too.
>
> Acked-by: Nick Desaulniers 
> Signed-off-by: Xiaozhou Liu 

Thanks Xiaozhou, picked into compiler-attributes with a slightly
modified title (since it is not about compiler attributes this time),
and let's see if there is any problem with it in linux-next.

Nick: I kept your Ack, even if it is a different patch. Let me know if
you don't want to Ack this version.

Cheers,
Miguel


[PATCH v3] Compiler Attributes: don't pollute userspace with macro definitions

2018-12-14 Thread Xiaozhou Liu
Macros 'inline' and '__gnu_inline' used to be defined in compiler-gcc.h,
which was (and is) included entirely in (__KERNEL__ && !__ASSEMBLY__).
Commit 815f0ddb346c ("include/linux/compiler*.h: make compiler-*.h mutually
exclusive") had those macros exposed to userspace, unintentionally.

Then commit a3f8a30f3f00 ("Compiler Attributes: use feature checks
instead of version checks") moved '__gnu_inline' back into
(__KERNEL__ && !__ASSEMBLY__) and 'inline' was left behind. Since 'inline'
depends on '__gnu_inline', compiling error showing "unknown type name
‘__gnu_inline’" will pop up, if userspace somehow includes
.

Other macros like __must_check, notrace, etc. are in a similar situation.
So just move all these macros back into (__KERNEL__ && !__ASSEMBLY__).

Note:
  1. This patch only affects what userspace sees.
  2. __must_check (when !CONFIG_ENABLE_MUST_CHECK) and noinline_for_stack
 were once defined in __KERNEL__ only, but we believe that they can
 be put into !__ASSEMBLY__ too.

Acked-by: Nick Desaulniers 
Signed-off-by: Xiaozhou Liu 
---

v3: move macros into !__ASSEMBLY__ too. (Miguel Ojeda)
v2: update commit message.

 include/linux/compiler_types.h | 108 -
 1 file changed, 54 insertions(+), 54 deletions(-)

diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 4a3f9c09c92d..ba814f18cb4c 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -104,6 +104,60 @@ struct ftrace_likely_data {
unsigned long   constant;
 };
 
+#ifdef CONFIG_ENABLE_MUST_CHECK
+#define __must_check   __attribute__((__warn_unused_result__))
+#else
+#define __must_check
+#endif
+
+#if defined(CC_USING_HOTPATCH)
+#define notrace__attribute__((hotpatch(0, 0)))
+#else
+#define notrace
__attribute__((__no_instrument_function__))
+#endif
+
+/*
+ * it doesn't make sense on ARM (currently the only user of __naked)
+ * to trace naked functions because then mcount is called without
+ * stack and frame pointer being set up and there is no chance to
+ * restore the lr register to the value before mcount was called.
+ */
+#define __naked__attribute__((__naked__)) notrace
+
+#define __compiler_offsetof(a, b)  __builtin_offsetof(a, b)
+
+/*
+ * Force always-inline if the user requests it so via the .config.
+ * GCC does not warn about unused static inline functions for
+ * -Wunused-function.  This turns out to avoid the need for complex #ifdef
+ * directives.  Suppress the warning in clang as well by using "unused"
+ * function attribute, which is redundant but not harmful for gcc.
+ * Prefer gnu_inline, so that extern inline functions do not emit an
+ * externally visible function. This makes extern inline behave as per gnu89
+ * semantics rather than c99. This prevents multiple symbol definition errors
+ * of extern inline functions at link time.
+ * A lot of inline functions can cause havoc with function tracing.
+ * Do not use __always_inline here, since currently it expands to inline again
+ * (which would break users of __always_inline).
+ */
+#if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
+   !defined(CONFIG_OPTIMIZE_INLINING)
+#define inline inline __attribute__((__always_inline__)) __gnu_inline \
+   __maybe_unused notrace
+#else
+#define inline inline__gnu_inline \
+   __maybe_unused notrace
+#endif
+
+#define __inline__ inline
+#define __inline   inline
+
+/*
+ * Rather then using noinline to prevent stack consumption, use
+ * noinline_for_stack instead.  For documentation reasons.
+ */
+#define noinline_for_stack noinline
+
 #endif /* __KERNEL__ */
 
 #endif /* __ASSEMBLY__ */
@@ -161,58 +215,4 @@ struct ftrace_likely_data {
 #define __diag_error(compiler, version, option, comment) \
__diag_ ## compiler(version, error, option)
 
-#ifdef CONFIG_ENABLE_MUST_CHECK
-#define __must_check   __attribute__((__warn_unused_result__))
-#else
-#define __must_check
-#endif
-
-#if defined(CC_USING_HOTPATCH)
-#define notrace__attribute__((hotpatch(0, 0)))
-#else
-#define notrace
__attribute__((__no_instrument_function__))
-#endif
-
-/*
- * it doesn't make sense on ARM (currently the only user of __naked)
- * to trace naked functions because then mcount is called without
- * stack and frame pointer being set up and there is no chance to
- * restore the lr register to the value before mcount was called.
- */
-#define __naked__attribute__((__naked__)) notrace
-
-#define __compiler_offsetof(a, b)  __builtin_offsetof(a, b)
-
-/*
- * Force always-inline if the user requests it so via the .config.
- * GCC does not warn about unused static inline functions for
- * -Wunused-function.  This turns out to avoid the need for complex #ifdef
- * directives.  Suppress the warning in clang as well by