2012-10-24 16:34:18

by Daniel Santos

[permalink] [raw]
Subject: [PATCH v3 0/10] Cleanup & new features for compiler*.h and bug.h


This patch set is a dependency of the generic red-black tree patch set, which
I have now split up into three smaller sets and is based off of linux-next.

The major aim of this patch set is to cleanup compiler-gcc*.h and improve
the manageability of of compiler features at various versions (when they
are broken, etc.), add some needed features to bug.h and clean that up as
well.

compiler-gcc*.h
o Introduce GCC_VERSION - (e.g., gcc 4.7.1 becomes 40701)
o Reorder all features based upon the version introduced (readability)
o Change all version checks to use GCC_VERSION
o Remove redundant __linktime_error

bug.h
o Improve BUILD_BUG_ON(expr) - now generates compile-time error post-gcc-4.4
o Remove duplicate error messages in some cases.
o Introduce BUILD_BUG_ON_MSG and clean up the implementations of the
BUILD_BUG* macros.


2012-10-24 16:39:48

by Daniel Santos

[permalink] [raw]
Subject: [PATCH v3 05/10] compiler{,-gcc4}.h, bug.h: Remove duplicate macros

__linktime_error() does the same thing as __compiletime_error() and is
only used in bug.h. Since the macro defines a function attribute that
will cause a failure at compile-time (not link-time), it makes more
sense to keep __compiletime_error(), which is also neatly mated with
__compiletime_warning().

Signed-off-by: Daniel Santos <[email protected]>
Acked-by: David Rientjes <[email protected]>
---
include/linux/bug.h | 2 +-
include/linux/compiler-gcc4.h | 2 --
include/linux/compiler.h | 3 ---
3 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/include/linux/bug.h b/include/linux/bug.h
index 4bd74d8..a03c3ef 100644
--- a/include/linux/bug.h
+++ b/include/linux/bug.h
@@ -74,7 +74,7 @@ extern int __build_bug_on_failed;
#define BUILD_BUG() \
do { \
extern void __build_bug_failed(void) \
- __linktime_error("BUILD_BUG failed"); \
+ __compiletime_error("BUILD_BUG failed");\
__build_bug_failed(); \
} while (0)

diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index 9755029..7f143ac 100644
--- a/include/linux/compiler-gcc4.h
+++ b/include/linux/compiler-gcc4.h
@@ -33,8 +33,6 @@
the kernel context */
#define __cold __attribute__((__cold__))

-#define __linktime_error(message) __attribute__((__error__(message)))
-
#ifndef __CHECKER__
# define __compiletime_warning(message) __attribute__((warning(message)))
# define __compiletime_error(message) __attribute__((error(message)))
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index b121554..cbf6d9d 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -299,9 +299,6 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
#ifndef __compiletime_error
# define __compiletime_error(message)
#endif
-#ifndef __linktime_error
-# define __linktime_error(message)
-#endif
/*
* Prevent the compiler from merging or refetching accesses. The compiler
* is also forbidden from reordering successive instances of ACCESS_ONCE(),
--
1.7.3.4

2012-10-24 16:40:00

by Daniel Santos

[permalink] [raw]
Subject: [PATCH v3 03/10] compiler-gcc{3,4}.h: Use GCC_VERSION macro

Using GCC_VERSION reduces complexity, is easier to read and is GCC's
recommended mechanism for doing version checks. (Just don't ask me why
they didn't define it in the first place.) This also makes it easy to
merge compiler-gcc{,3,4}.h should somebody want to.

Signed-off-by: Daniel Santos <[email protected]>
Acked-by: David Rientjes <[email protected]>
---
include/linux/compiler-gcc3.h | 8 ++++----
include/linux/compiler-gcc4.h | 14 +++++++-------
2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/include/linux/compiler-gcc3.h b/include/linux/compiler-gcc3.h
index 37d4124..7d89feb 100644
--- a/include/linux/compiler-gcc3.h
+++ b/include/linux/compiler-gcc3.h
@@ -2,22 +2,22 @@
#error "Please don't include <linux/compiler-gcc3.h> directly, include <linux/compiler.h> instead."
#endif

-#if __GNUC_MINOR__ < 2
+#if GCC_VERSION < 30200
# error Sorry, your compiler is too old - please upgrade it.
#endif

-#if __GNUC_MINOR__ >= 3
+#if GCC_VERSION >= 30300
# define __used __attribute__((__used__))
#else
# define __used __attribute__((__unused__))
#endif

-#if __GNUC_MINOR__ >= 4
+#if GCC_VERSION >= 30400
#define __must_check __attribute__((warn_unused_result))
#endif

#ifdef CONFIG_GCOV_KERNEL
-# if __GNUC_MINOR__ < 4
+# if GCC_VERSION < 30400
# error "GCOV profiling support for gcc versions below 3.4 not included"
# endif /* __GNUC_MINOR__ */
#endif /* CONFIG_GCOV_KERNEL */
diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index 8914293..9755029 100644
--- a/include/linux/compiler-gcc4.h
+++ b/include/linux/compiler-gcc4.h
@@ -4,7 +4,7 @@

/* GCC 4.1.[01] miscompiles __weak */
#ifdef __KERNEL__
-# if __GNUC_MINOR__ == 1 && __GNUC_PATCHLEVEL__ <= 1
+# if GCC_VERSION >= 40100 && GCC_VERSION <= 40101
# error Your version of gcc miscompiles the __weak directive
# endif
#endif
@@ -13,11 +13,11 @@
#define __must_check __attribute__((warn_unused_result))
#define __compiler_offsetof(a,b) __builtin_offsetof(a,b)

-#if __GNUC_MINOR__ > 0
+#if GCC_VERSION >= 40100
# define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
#endif

-#if __GNUC_MINOR__ >= 3
+#if GCC_VERSION >= 40300
/* Mark functions as cold. gcc will assume any path leading to a call
to them will be unlikely. This means a lot of manual unlikely()s
are unnecessary now for any paths leading to the usual suspects
@@ -39,9 +39,9 @@
# define __compiletime_warning(message) __attribute__((warning(message)))
# define __compiletime_error(message) __attribute__((error(message)))
#endif /* __CHECKER__ */
-#endif /* __GNUC_MINOR__ >= 3 */
+#endif /* GCC_VERSION >= 40300 */

-#if __GNUC_MINOR__ >= 5
+#if GCC_VERSION >= 40500
/*
* Mark a position in code as unreachable. This can be used to
* suppress control flow warnings after asm blocks that transfer
@@ -56,9 +56,9 @@
/* Mark a function definition as prohibited from being cloned. */
#define __noclone __attribute__((__noclone__))

-#endif /* __GNUC_MINOR__ >= 5 */
+#endif /* GCC_VERSION >= 40500 */

-#if __GNUC_MINOR__ >= 6
+#if GCC_VERSION >= 40600
/*
* Tell the optimizer that something else uses this function or variable.
*/
--
1.7.3.4

2012-10-24 16:40:10

by Daniel Santos

[permalink] [raw]
Subject: [PATCH v3 08/10] bug.h: Fix BUILD_BUG_ON macro in __CHECKER__

When __CHECKER__ is defined, we disable all of the BUILD_BUG.* macros.
However, BUILD_BUG_ON was evaluating to nothing in this case, and we
want (0) since this is a function-like macro that will be followed by a
semicolon.

Signed-off-by: Daniel Santos <[email protected]>
---
include/linux/bug.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/bug.h b/include/linux/bug.h
index f8eae31..1b43ea2 100644
--- a/include/linux/bug.h
+++ b/include/linux/bug.h
@@ -16,7 +16,7 @@ struct pt_regs;
#define BUILD_BUG_ON_NOT_POWER_OF_2(n)
#define BUILD_BUG_ON_ZERO(e) (0)
#define BUILD_BUG_ON_NULL(e) ((void*)0)
-#define BUILD_BUG_ON(condition)
+#define BUILD_BUG_ON(condition) (0)
#define BUILD_BUG() (0)
#else /* __CHECKER__ */

--
1.7.3.4

2012-10-24 16:40:18

by Daniel Santos

[permalink] [raw]
Subject: [PATCH v3 10/10] bug.h: Convert BUILD_BUG{,_ON} to use BUILD_BUG_ON_MSG

Remove duplicate code by converting BUILD_BUG and BUILD_BUG_ON to just
call BUILD_BUG_ON_MSG. This not only reduces source code bloat, but
also prevents the possibility of code being changed for one macro and
not for the other (which was previously the case for BUILD_BUG and
BUILD_BUG_ON).

Signed-off-by: Daniel Santos <[email protected]>
---
include/linux/bug.h | 17 +++--------------
1 files changed, 3 insertions(+), 14 deletions(-)

diff --git a/include/linux/bug.h b/include/linux/bug.h
index f6f81f6..c791d2a 100644
--- a/include/linux/bug.h
+++ b/include/linux/bug.h
@@ -80,14 +80,8 @@ struct pt_regs;
#ifndef __OPTIMIZE__
#define BUILD_BUG_ON(condition) __compiletime_error_fallback(condition)
#else
-#define BUILD_BUG_ON(condition) \
- do { \
- extern void __build_bug_on_failed(void) \
- __compiletime_error("BUILD_BUG_ON failed"); \
- __compiletime_error_fallback(condition); \
- if (condition) \
- __build_bug_on_failed(); \
- } while(0)
+#define BUILD_BUG_ON(condition) \
+ BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
#endif

/**
@@ -97,12 +91,7 @@ struct pt_regs;
* build time, you should use BUILD_BUG to detect if it is
* unexpectedly used.
*/
-#define BUILD_BUG() \
- do { \
- extern void __build_bug_failed(void) \
- __compiletime_error("BUILD_BUG failed");\
- __build_bug_failed(); \
- } while (0)
+#define BUILD_BUG() BUILD_BUG_ON_MSG(1, "BUILD_BUG failed")

#endif /* __CHECKER__ */

--
1.7.3.4

2012-10-24 16:40:25

by Daniel Santos

[permalink] [raw]
Subject: [PATCH v3 07/10] compiler.h, bug.h: Prevent double error messages with BUILD_BUG{,_ON}

Prior to the introduction of __attribute__((error("msg"))) in gcc 4.3,
creating compile-time errors required a little trickery.
BUILD_BUG{,_ON} uses this attribute when available to generate
compile-time errors, but also uses the negative-sized array trick for
older compilers, resulting in two error messages in some cases. The
reason it's "some" cases is that as of gcc 4.4, the negative-sized array
will not create an error in some situations, like inline functions.

This patch replaces the negative-sized array code with the new
__compiletime_error_fallback() macro which expands to the same thing
unless the the error attribute is available, in which case it expands to
do{}while(0), resulting in exactly one compile-time error on all
versions of gcc.

Signed-off-by: Daniel Santos <[email protected]>
---
include/linux/bug.h | 4 ++--
include/linux/compiler.h | 7 +++++++
2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/include/linux/bug.h b/include/linux/bug.h
index 3d4b564..f8eae31 100644
--- a/include/linux/bug.h
+++ b/include/linux/bug.h
@@ -56,13 +56,13 @@ struct pt_regs;
* link-time error, which is harder to track down.
*/
#ifndef __OPTIMIZE__
-#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
+#define BUILD_BUG_ON(condition) __compiletime_error_fallback(condition)
#else
#define BUILD_BUG_ON(condition) \
do { \
extern void __build_bug_on_failed(void) \
__compiletime_error("BUILD_BUG_ON failed"); \
- ((void)sizeof(char[1 - 2*!!(condition)])); \
+ __compiletime_error_fallback(condition); \
if (condition) \
__build_bug_on_failed(); \
} while(0)
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index cbf6d9d..84926f2 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -298,6 +298,13 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);
#endif
#ifndef __compiletime_error
# define __compiletime_error(message)
+# define __compiletime_error_fallback(condition) \
+ do { \
+ ((void)sizeof(char[1 - 2*!!(condition)])); \
+ } while (0)
+#endif
+#ifndef __compiletime_error_fallback
+# define __compiletime_error_fallback(condition) do { } while (0)
#endif
/*
* Prevent the compiler from merging or refetching accesses. The compiler
--
1.7.3.4

2012-10-24 16:40:50

by Daniel Santos

[permalink] [raw]
Subject: [PATCH v3 04/10] bug.h: directly include linux/compiler.h

Currently, we are only including asm/bug.h and then expecting that
linux/compiler.h will eventually be included to define __linktime_error
(used in BUILD_BUG_ON). This patch includes it directly for clarity and
to avoid the possibility of changes in <arch>/*/include/asm/bug.h being
changed or not including linux/compiler.h for some reason. (Later
patches will in this set use more macros defined in compiler*.h.)

Signed-off-by: Daniel Santos <[email protected]>
---
include/linux/bug.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/include/linux/bug.h b/include/linux/bug.h
index aaac4bb..4bd74d8 100644
--- a/include/linux/bug.h
+++ b/include/linux/bug.h
@@ -2,6 +2,7 @@
#define _LINUX_BUG_H

#include <asm/bug.h>
+#include <linux/compiler.h>

enum bug_trap_type {
BUG_TRAP_TYPE_NONE = 0,
--
1.7.3.4

2012-10-24 16:41:41

by Daniel Santos

[permalink] [raw]
Subject: [PATCH v3 01/10] compiler-gcc4.h: Reorder macros based upon gcc ver

This helps to keep the file from getting confusing, removes one
duplicate version check and should encourage future editors to put new
macros where they belong.

Signed-off-by: Daniel Santos <[email protected]>
Acked-by: David Rientjes <[email protected]>
---
include/linux/compiler-gcc4.h | 20 +++++++++++---------
1 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
index 412bc6c..8914293 100644
--- a/include/linux/compiler-gcc4.h
+++ b/include/linux/compiler-gcc4.h
@@ -13,6 +13,10 @@
#define __must_check __attribute__((warn_unused_result))
#define __compiler_offsetof(a,b) __builtin_offsetof(a,b)

+#if __GNUC_MINOR__ > 0
+# define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
+#endif
+
#if __GNUC_MINOR__ >= 3
/* Mark functions as cold. gcc will assume any path leading to a call
to them will be unlikely. This means a lot of manual unlikely()s
@@ -31,6 +35,12 @@

#define __linktime_error(message) __attribute__((__error__(message)))

+#ifndef __CHECKER__
+# define __compiletime_warning(message) __attribute__((warning(message)))
+# define __compiletime_error(message) __attribute__((error(message)))
+#endif /* __CHECKER__ */
+#endif /* __GNUC_MINOR__ >= 3 */
+
#if __GNUC_MINOR__ >= 5
/*
* Mark a position in code as unreachable. This can be used to
@@ -46,8 +56,7 @@
/* Mark a function definition as prohibited from being cloned. */
#define __noclone __attribute__((__noclone__))

-#endif
-#endif
+#endif /* __GNUC_MINOR__ >= 5 */

#if __GNUC_MINOR__ >= 6
/*
@@ -56,10 +65,3 @@
#define __visible __attribute__((externally_visible))
#endif

-#if __GNUC_MINOR__ > 0
-#define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
-#endif
-#if __GNUC_MINOR__ >= 3 && !defined(__CHECKER__)
-#define __compiletime_warning(message) __attribute__((warning(message)))
-#define __compiletime_error(message) __attribute__((error(message)))
-#endif
--
1.7.3.4

2012-10-24 16:41:39

by Daniel Santos

[permalink] [raw]
Subject: [PATCH v3 02/10] compiler-gcc.h: Add gcc-recommended GCC_VERSION macro

Throughout compiler*.h, many version checks are made. These can be
simplified by using the macro that gcc's documentation recommends.
However, my primary reason for adding this is that I need bug-check
macros that are enabled at certain gcc versions and it's cleaner to use
this macro than the tradition method:

if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ => 2)

If you add patch level, it gets this ugly:

if __GNUC__ > 4 || (__GNUC__ == 4 && (__GNUC_MINOR__ > 2 || \
__GNUC_MINOR__ == 2 __GNUC_PATCHLEVEL__ >= 1))

As opposed to:

if GCC_VERSION >= 40201

While having separate headers for gcc 3 & 4 eliminates some of this
verbosity, they can still be cleaned up by this.

See also:
http://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html

Signed-off-by: Daniel Santos <[email protected]>
Acked-by: Borislav Petkov <[email protected]>
Acked-by: David Rientjes <[email protected]>
---
include/linux/compiler-gcc.h | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 6a6d7ae..24545cd 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -5,6 +5,9 @@
/*
* Common definitions for all gcc versions go here.
*/
+#define GCC_VERSION (__GNUC__ * 10000 \
+ + __GNUC_MINOR__ * 100 \
+ + __GNUC_PATCHLEVEL__)


/* Optimization barrier */
--
1.7.3.4

2012-10-24 16:42:14

by Daniel Santos

[permalink] [raw]
Subject: [PATCH v3 06/10] bug.h: Make BUILD_BUG_ON generate compile-time error

Negative sized arrays wont create a compile-time error in some cases
starting with gcc 4.4 (e.g., inlined functions), but gcc 4.3 introduced
the error function attribute that will. This patch modifies
BUILD_BUG_ON to behave like BUILD_BUG already does, using the error
function attribute so that you don't have to build the entire kernel to
discover that you have a problem, and then enjoy trying to track it down
from a link-time error.

Signed-off-by: Daniel Santos <[email protected]>
---
include/linux/bug.h | 24 ++++++++++++++----------
1 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/include/linux/bug.h b/include/linux/bug.h
index a03c3ef..3d4b564 100644
--- a/include/linux/bug.h
+++ b/include/linux/bug.h
@@ -43,24 +43,28 @@ struct pt_regs;
* @condition: the condition which the compiler should know is false.
*
* If you have some code which relies on certain constants being equal, or
- * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
+ * some other compile-time-evaluated condition, you should use BUILD_BUG_ON to
* detect if someone changes it.
*
* The implementation uses gcc's reluctance to create a negative array, but
* gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
- * to inline functions). So as a fallback we use the optimizer; if it can't
- * prove the condition is false, it will cause a link error on the undefined
- * "__build_bug_on_failed". This error message can be harder to track down
- * though, hence the two different methods.
+ * to inline functions). Luckily, in 4.3 they added the "error" function
+ * attribute just for this type of case. Thus, we use a negative sized array
+ * (should always create an error pre-gcc-4.4) and then call an undefined
+ * function with the error attribute (should always creates an error 4.3+). If
+ * for some reason, neither creates a compile-time error, we'll still have a
+ * link-time error, which is harder to track down.
*/
#ifndef __OPTIMIZE__
#define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
#else
-extern int __build_bug_on_failed;
-#define BUILD_BUG_ON(condition) \
- do { \
- ((void)sizeof(char[1 - 2*!!(condition)])); \
- if (condition) __build_bug_on_failed = 1; \
+#define BUILD_BUG_ON(condition) \
+ do { \
+ extern void __build_bug_on_failed(void) \
+ __compiletime_error("BUILD_BUG_ON failed"); \
+ ((void)sizeof(char[1 - 2*!!(condition)])); \
+ if (condition) \
+ __build_bug_on_failed(); \
} while(0)
#endif

--
1.7.3.4

2012-10-24 16:39:47

by Daniel Santos

[permalink] [raw]
Subject: [PATCH v3 09/10] bug.h: Add BUILD_BUG_ON_MSG & _BUILD_BUG_INTERNAL

Add BUILD_BUG_ON_MSG which behaves like BUILD_BUG_ON (with optimizations
enabled), except that it allows you to specify the error message you
want emitted as the third parameter. Under the hood, this relies on
_BUILD_BUG_INTERNAL, which does the actual work and is pretty-much
identical to BUILD_BUG_ON.

Signed-off-by: Daniel Santos <[email protected]>
---
include/linux/bug.h | 22 ++++++++++++++++++++++
1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/include/linux/bug.h b/include/linux/bug.h
index 1b43ea2..f6f81f6 100644
--- a/include/linux/bug.h
+++ b/include/linux/bug.h
@@ -16,6 +16,7 @@ struct pt_regs;
#define BUILD_BUG_ON_NOT_POWER_OF_2(n)
#define BUILD_BUG_ON_ZERO(e) (0)
#define BUILD_BUG_ON_NULL(e) ((void*)0)
+#define BUILD_BUG_ON_MSG(cond, msg) (0)
#define BUILD_BUG_ON(condition) (0)
#define BUILD_BUG() (0)
#else /* __CHECKER__ */
@@ -38,6 +39,27 @@ struct pt_regs;
*/
#define BUILD_BUG_ON_INVALID(e) ((void)(sizeof((__force long)(e))))

+#define __BUILD_BUG_INTERNAL(condition, msg, line) \
+ do { \
+ extern void __build_bug_on_failed_ ## line \
+ (void) __compiletime_error(msg); \
+ __compiletime_error_fallback(condition); \
+ if (condition) \
+ __build_bug_on_failed_ ## line(); \
+ } while (0)
+
+#define _BUILD_BUG_INTERNAL(condition, msg, line) \
+ __BUILD_BUG_INTERNAL(condition, msg, line)
+
+/**
+ * BUILD_BUG_ON_MSG - break compile if a condition is true & emit supplied
+ * error message.
+ * @condition: the condition which the compiler should know is false.
+ *
+ * See BUILD_BUG_ON for description.
+ */
+#define BUILD_BUG_ON_MSG(cond, msg) _BUILD_BUG_INTERNAL(cond, msg, __LINE__)
+
/**
* BUILD_BUG_ON - break compile if a condition is true.
* @condition: the condition which the compiler should know is false.
--
1.7.3.4

2012-10-24 19:05:55

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v3 03/10] compiler-gcc{3,4}.h: Use GCC_VERSION macro

On Wed, Oct 24, 2012 at 11:33:54AM -0500, [email protected] wrote:
> Using GCC_VERSION reduces complexity, is easier to read and is GCC's
> recommended mechanism for doing version checks. (Just don't ask me why
> they didn't define it in the first place.) This also makes it easy to
> merge compiler-gcc{,3,4}.h should somebody want to.
>
> Signed-off-by: Daniel Santos <[email protected]>
> Acked-by: David Rientjes <[email protected]>
> ---
> include/linux/compiler-gcc3.h | 8 ++++----
> include/linux/compiler-gcc4.h | 14 +++++++-------
> 2 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/compiler-gcc3.h b/include/linux/compiler-gcc3.h
> index 37d4124..7d89feb 100644
> --- a/include/linux/compiler-gcc3.h
> +++ b/include/linux/compiler-gcc3.h
> @@ -2,22 +2,22 @@
> #error "Please don't include <linux/compiler-gcc3.h> directly, include <linux/compiler.h> instead."
> #endif
>
> -#if __GNUC_MINOR__ < 2
> +#if GCC_VERSION < 30200
> # error Sorry, your compiler is too old - please upgrade it.
> #endif
>
> -#if __GNUC_MINOR__ >= 3
> +#if GCC_VERSION >= 30300
> # define __used __attribute__((__used__))
> #else
> # define __used __attribute__((__unused__))
> #endif
>
> -#if __GNUC_MINOR__ >= 4
> +#if GCC_VERSION >= 30400
> #define __must_check __attribute__((warn_unused_result))
> #endif
>
> #ifdef CONFIG_GCOV_KERNEL
> -# if __GNUC_MINOR__ < 4
> +# if GCC_VERSION < 30400
> # error "GCOV profiling support for gcc versions below 3.4 not included"
> # endif /* __GNUC_MINOR__ */
> #endif /* CONFIG_GCOV_KERNEL */
> diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
> index 8914293..9755029 100644
> --- a/include/linux/compiler-gcc4.h
> +++ b/include/linux/compiler-gcc4.h
> @@ -4,7 +4,7 @@
>
> /* GCC 4.1.[01] miscompiles __weak */
> #ifdef __KERNEL__
> -# if __GNUC_MINOR__ == 1 && __GNUC_PATCHLEVEL__ <= 1
> +# if GCC_VERSION >= 40100 && GCC_VERSION <= 40101
> # error Your version of gcc miscompiles the __weak directive
> # endif
> #endif
> @@ -13,11 +13,11 @@
> #define __must_check __attribute__((warn_unused_result))
> #define __compiler_offsetof(a,b) __builtin_offsetof(a,b)
>
> -#if __GNUC_MINOR__ > 0
> +#if GCC_VERSION >= 40100

I can't seem to recall so did we explain already why ">= 40100" instead
of "> 40000"?

I mean, it is the same, but still...

Thanks.

--
Regards/Gruss,
Boris.

2012-10-24 19:55:29

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v3 04/10] bug.h: directly include linux/compiler.h

On Wed, Oct 24, 2012 at 11:33:55AM -0500, [email protected] wrote:
> Currently, we are only including asm/bug.h and then expecting that
> linux/compiler.h will eventually be included to define __linktime_error
> (used in BUILD_BUG_ON). This patch includes it directly for clarity and
> to avoid the possibility of changes in <arch>/*/include/asm/bug.h being
> changed or not including linux/compiler.h for some reason. (Later
> patches will in this set use more macros defined in compiler*.h.)
>
> Signed-off-by: Daniel Santos <[email protected]>
> ---
> include/linux/bug.h | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/bug.h b/include/linux/bug.h
> index aaac4bb..4bd74d8 100644
> --- a/include/linux/bug.h
> +++ b/include/linux/bug.h
> @@ -2,6 +2,7 @@
> #define _LINUX_BUG_H
>
> #include <asm/bug.h>
> +#include <linux/compiler.h>
>
> enum bug_trap_type {
> BUG_TRAP_TYPE_NONE = 0,
> --
> 1.7.3.4

Why is this a separate patch and why not add that single line in 6/10
where you define BUILD_BUG_ON?

Thanks.

--
Regards/Gruss,
Boris.

2012-10-24 21:49:12

by Josh Triplett

[permalink] [raw]
Subject: Re: [PATCH v3 03/10] compiler-gcc{3,4}.h: Use GCC_VERSION macro

On Wed, Oct 24, 2012 at 09:05:46PM +0200, Borislav Petkov wrote:
> On Wed, Oct 24, 2012 at 11:33:54AM -0500, [email protected] wrote:
> > Using GCC_VERSION reduces complexity, is easier to read and is GCC's
> > recommended mechanism for doing version checks. (Just don't ask me why
> > they didn't define it in the first place.) This also makes it easy to
> > merge compiler-gcc{,3,4}.h should somebody want to.
> >
> > Signed-off-by: Daniel Santos <[email protected]>
> > Acked-by: David Rientjes <[email protected]>
> > ---
> > include/linux/compiler-gcc3.h | 8 ++++----
> > include/linux/compiler-gcc4.h | 14 +++++++-------
> > 2 files changed, 11 insertions(+), 11 deletions(-)
> >
> > diff --git a/include/linux/compiler-gcc3.h b/include/linux/compiler-gcc3.h
> > index 37d4124..7d89feb 100644
> > --- a/include/linux/compiler-gcc3.h
> > +++ b/include/linux/compiler-gcc3.h
> > @@ -2,22 +2,22 @@
> > #error "Please don't include <linux/compiler-gcc3.h> directly, include <linux/compiler.h> instead."
> > #endif
> >
> > -#if __GNUC_MINOR__ < 2
> > +#if GCC_VERSION < 30200
> > # error Sorry, your compiler is too old - please upgrade it.
> > #endif
> >
> > -#if __GNUC_MINOR__ >= 3
> > +#if GCC_VERSION >= 30300
> > # define __used __attribute__((__used__))
> > #else
> > # define __used __attribute__((__unused__))
> > #endif
> >
> > -#if __GNUC_MINOR__ >= 4
> > +#if GCC_VERSION >= 30400
> > #define __must_check __attribute__((warn_unused_result))
> > #endif
> >
> > #ifdef CONFIG_GCOV_KERNEL
> > -# if __GNUC_MINOR__ < 4
> > +# if GCC_VERSION < 30400
> > # error "GCOV profiling support for gcc versions below 3.4 not included"
> > # endif /* __GNUC_MINOR__ */
> > #endif /* CONFIG_GCOV_KERNEL */
> > diff --git a/include/linux/compiler-gcc4.h b/include/linux/compiler-gcc4.h
> > index 8914293..9755029 100644
> > --- a/include/linux/compiler-gcc4.h
> > +++ b/include/linux/compiler-gcc4.h
> > @@ -4,7 +4,7 @@
> >
> > /* GCC 4.1.[01] miscompiles __weak */
> > #ifdef __KERNEL__
> > -# if __GNUC_MINOR__ == 1 && __GNUC_PATCHLEVEL__ <= 1
> > +# if GCC_VERSION >= 40100 && GCC_VERSION <= 40101
> > # error Your version of gcc miscompiles the __weak directive
> > # endif
> > #endif
> > @@ -13,11 +13,11 @@
> > #define __must_check __attribute__((warn_unused_result))
> > #define __compiler_offsetof(a,b) __builtin_offsetof(a,b)
> >
> > -#if __GNUC_MINOR__ > 0
> > +#if GCC_VERSION >= 40100
>
> I can't seem to recall so did we explain already why ">= 40100" instead
> of "> 40000"?
>
> I mean, it is the same, but still...

__GNUC_MINOR__ > 0 means __GNUC_MINOR__ >= 1, so that would allow 4.1 or
newer. However, GCC_VERSION > 40000 would also allow 4.0.n for n > 0.

- Josh Triplett

2012-10-24 22:34:20

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v3 03/10] compiler-gcc{3,4}.h: Use GCC_VERSION macro

On Wed, Oct 24, 2012 at 02:49:00PM -0700, Josh Triplett wrote:
> > I can't seem to recall so did we explain already why ">= 40100" instead
> > of "> 40000"?
> >
> > I mean, it is the same, but still...
>
> __GNUC_MINOR__ > 0 means __GNUC_MINOR__ >= 1, so that would allow 4.1 or
> newer. However, GCC_VERSION > 40000 would also allow 4.0.n for n > 0.

Ah, sure. In that case

Acked-by: Borislav Petkov <[email protected]>

Thanks.

--
Regards/Gruss,
Boris.

2012-10-25 09:26:25

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v3 05/10] compiler{,-gcc4}.h, bug.h: Remove duplicate macros

On Wed, Oct 24, 2012 at 11:33:56AM -0500, [email protected] wrote:
> __linktime_error() does the same thing as __compiletime_error() and is
> only used in bug.h. Since the macro defines a function attribute that
> will cause a failure at compile-time (not link-time), it makes more
> sense to keep __compiletime_error(), which is also neatly mated with
> __compiletime_warning().
>
> Signed-off-by: Daniel Santos <[email protected]>
> Acked-by: David Rientjes <[email protected]>

Acked-by: Borislav Petkov <[email protected]>

--
Regards/Gruss,
Boris.

2012-10-25 09:33:44

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH v3 06/10] bug.h: Make BUILD_BUG_ON generate compile-time error

On Wed, Oct 24, 2012 at 11:33:57AM -0500, [email protected] wrote:
> Negative sized arrays wont create a compile-time error in some cases
> starting with gcc 4.4 (e.g., inlined functions), but gcc 4.3 introduced
> the error function attribute that will. This patch modifies
> BUILD_BUG_ON to behave like BUILD_BUG already does, using the error
> function attribute so that you don't have to build the entire kernel to
> discover that you have a problem, and then enjoy trying to track it down
> from a link-time error.
>
> Signed-off-by: Daniel Santos <[email protected]>
> ---
> include/linux/bug.h | 24 ++++++++++++++----------
> 1 files changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/bug.h b/include/linux/bug.h
> index a03c3ef..3d4b564 100644
> --- a/include/linux/bug.h
> +++ b/include/linux/bug.h
> @@ -43,24 +43,28 @@ struct pt_regs;
> * @condition: the condition which the compiler should know is false.
> *
> * If you have some code which relies on certain constants being equal, or
> - * other compile-time-evaluated condition, you should use BUILD_BUG_ON to
> + * some other compile-time-evaluated condition, you should use BUILD_BUG_ON to
> * detect if someone changes it.
> *
> * The implementation uses gcc's reluctance to create a negative array, but
> * gcc (as of 4.4) only emits that error for obvious cases (eg. not arguments
> - * to inline functions). So as a fallback we use the optimizer; if it can't
> - * prove the condition is false, it will cause a link error on the undefined
> - * "__build_bug_on_failed". This error message can be harder to track down
> - * though, hence the two different methods.
> + * to inline functions). Luckily, in 4.3 they added the "error" function
> + * attribute just for this type of case. Thus, we use a negative sized array
> + * (should always create an error pre-gcc-4.4) and then call an undefined

.... always create an error on gcc versions older than 4.4)

> + * function with the error attribute (should always creates an error 4.3+). If

..... always create an error on gcc 4.3 and later)

> + * for some reason, neither creates a compile-time error, we'll still have a
> + * link-time error, which is harder to track down.
> */
> #ifndef __OPTIMIZE__
> #define BUILD_BUG_ON(condition) ((void)sizeof(char[1 - 2*!!(condition)]))
> #else
> -extern int __build_bug_on_failed;
> -#define BUILD_BUG_ON(condition) \
> - do { \
> - ((void)sizeof(char[1 - 2*!!(condition)])); \
> - if (condition) __build_bug_on_failed = 1; \
> +#define BUILD_BUG_ON(condition) \
> + do { \
> + extern void __build_bug_on_failed(void) \
> + __compiletime_error("BUILD_BUG_ON failed"); \
> + ((void)sizeof(char[1 - 2*!!(condition)])); \
> + if (condition) \
> + __build_bug_on_failed(); \
> } while(0)
> #endif

With the changes above:

Acked-by: Borislav Petkov <[email protected]>

Thanks.

--
Regards/Gruss,
Boris.

2012-10-28 19:28:58

by Daniel Santos

[permalink] [raw]
Subject: Re: [PATCH v3 04/10] bug.h: directly include linux/compiler.h

On 10/24/2012 02:55 PM, Borislav Petkov wrote:
> On Wed, Oct 24, 2012 at 11:33:55AM -0500, [email protected] wrote:
>> Currently, we are only including asm/bug.h and then expecting that
>> linux/compiler.h will eventually be included to define __linktime_error
>> (used in BUILD_BUG_ON). This patch includes it directly for clarity and
>> to avoid the possibility of changes in <arch>/*/include/asm/bug.h being
>> changed or not including linux/compiler.h for some reason. (Later
>> patches will in this set use more macros defined in compiler*.h.)
>>
>> Signed-off-by: Daniel Santos <[email protected]>
>> ---
>> include/linux/bug.h | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/include/linux/bug.h b/include/linux/bug.h
>> index aaac4bb..4bd74d8 100644
>> --- a/include/linux/bug.h
>> +++ b/include/linux/bug.h
>> @@ -2,6 +2,7 @@
>> #define _LINUX_BUG_H
>>
>> #include <asm/bug.h>
>> +#include <linux/compiler.h>
>>
>> enum bug_trap_type {
>> BUG_TRAP_TYPE_NONE = 0,
>> --
>> 1.7.3.4
> Why is this a separate patch and why not add that single line in 6/10
> where you define BUILD_BUG_ON?
>
> Thanks.
Sorry about that. I think this was originally in another patch with
something else that I moved to another patch set and it ended up by its
self. I'll squash it and resubmit.