On Sun, Oct 16, 2016 at 05:16:05PM +0200, Vegard Nossum wrote:
> The test in this loop:
>
> for (b_fw = __start_builtin_fw; b_fw != __end_builtin_fw; b_fw++) {
>
> was getting completely compiled out by my gcc, 7.0.0 20160520. The result
> was that the loop was going beyond the end of the builtin_fw array and
> giving me a page fault when trying to dereference b_fw->name.
>
> This is because __start_builtin_fw and __end_builtin_fw are both declared
> as (separate) arrays, and so gcc conludes that b_fw can never point to
> __end_builtin_fw.
>
> Apparently similar optimizations were observed on NetBSD for GCC 5.4:
> http://mail-index.netbsd.org/netbsd-bugs/2016/06/22/msg047136.html
>
> We can lose the array information about a pointer using
> OPTIMIZER_HIDE_VAR().
>
> Additional points on the design of this interface:
>
> 1) DECLARE_*() follows the kernel convention (you get what you expect,
> more or less)
>
> 2) the real variables defined in the linker script are hidden behind
> some generated names so you don't use them by accident
>
> 3) no need to sprinkle your code with OPTIMIZER_HIDE_VAR() or anything
> else, but you do need to use function calls to access the variables
> (e.g. ext_start(builtin_fw) and ext_end(builtin_fw)).
>
> Reported-by: Jiri Slaby <jslaby@xxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Ming Lei <ming.lei@xxxxxxxxxxxxx>
> Cc: Steven Rostedt <srostedt@xxxxxxxxxx>
> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Vegard Nossum <vegard.nossum@xxxxxxxxxx>
> ---
> include/linux/extarray.h | 65 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 65 insertions(+)
> create mode 100644 include/linux/extarray.h
>
> diff --git a/include/linux/extarray.h b/include/linux/extarray.h
> new file mode 100644
> index 0000000..1185abc
> --- /dev/null
> +++ b/include/linux/extarray.h
> @@ -0,0 +1,65 @@
> +#ifndef LINUX_EXTARRAY_H
> +#define LINUX_EXTARRAY_H
> +
> +#include <linux/compiler.h>
> +
> +/*
> + * A common pattern in the kernel is to put certain objects in a specific
> + * named section and then create variables in the linker script pointing
> + * to the start and the end of this section. These variables are declared
> + * as extern arrays to allow C code to iterate over the list of objects.
> + *
> + * In C, comparing pointers to objects in two different arrays is undefined.
> + * GCC version 7.0 and newer (commit 73447cc5d17) will aggressively optimize
> + * out such comparisons if it can prove that the two pointers point to
> + * different arrays (which is the case when the arrays are declared as two
> + * separate variables). This breaks the typical code used to iterate over
> + * such arrays.
> + *
> + * One way to get around this limitation is to force GCC to lose any array
> + * information about the pointers before we compare them. We can use e.g.
> + * OPTIMIZER_HIDE_VAR() for this.
> + *
> + * This file defines a few helpers to deal with declaring and accessing
> + * such linker-script-defined arrays.
> + */
> +
> +
> +#define DECLARE_EXTARRAY(type, name) \
> + extern type __start_##name[]; \
> + extern type __stop_##name[]; \
"EXTARRAY" kind of gives a good hint as to what is going on, but why not
just spell the thing out, "DECLARE_EXTERNAL_ARRAY()"?
> +#define _ext_start(name, tmp) \
> + ({ \
> + typeof(*__start_##name) *tmp = __start_##name; \
> + OPTIMIZER_HIDE_VAR(tmp); \
> + tmp; \
> + })
> +
> +#define ext_start(name) _ext_start(name, __UNIQUE_ID(ext_start_))
> +
> +#define _ext_end(name, tmp) \
> + ({ \
> + typeof(*__stop_##name) *tmp = __stop_##name; \
> + OPTIMIZER_HIDE_VAR(tmp); \
> + tmp; \
> + })
> +
> +#define ext_end(name) _ext_end(name, __UNIQUE_ID(ext_end_))
> +
> +#define _ext_size(name, tmp1, tmp2) \
> + ({ \
> + typeof(*__start_##name) *tmp1 = __start_##name; \
> + typeof(*__stop_##name) *tmp2 = __stop_##name; \
> + OPTIMIZER_HIDE_VAR(tmp1); \
> + OPTIMIZER_HIDE_VAR(tmp2); \
> + tmp2 - tmp1; \
> + })
> +
> +#define ext_size(name) \
> + _ext_size(name, __UNIQUE_ID(ext_size1_), __UNIQUE_ID(ext_size2_))
> +
> +#define ext_for_each(var, name) \
> + for (var = ext_start(name); var != ext_end(name); var++)
"ext_" is also vague, and hard to know what this is at first glance when
reading code. Again, we have lots of characters, let's use them.
"external_array_for_each()"?
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html