[PATCH v2 0/4] add support for gcov format introduced in gcc 4.7

[PATCH v2 0/4] add support for gcov format introduced in gcc 4.7

[Frantisek Hrbata]

This is an attempt to bring support for modified gcov format in gcc 4.7 to
the kernel. It tries to leverage the existing layout/abstraction, which was
designed keeping in mind that the gcov format could change, but some changes had
to be make. Mostly because the current model does not take into account that
even the core gcov structures, like gcov_info, could change. One part that could
be problematic is the addition of the .init_array section for constructors.

Tested with lcov and seems to be working fine, giving similar results as for the
older format.

v2: Included suggestions/code provided by Peter Oberparleiter. Detailed
    description in patches.

Frantisek Hrbata (4):
  gcov: move gcov structs definitions to a gcc version specific file
  gcov: add support for gcc 4.7 gcov format
  gcov: compile specific gcov implementation based on gcc version
  kernel: add support for init_array constructors

 Documentation/gcov.txt            |   4 +
 include/asm-generic/vmlinux.lds.h |   1 +
 kernel/gcov/Kconfig               |  30 ++
 kernel/gcov/Makefile              |  32 ++-
 kernel/gcov/base.c                |  32 ++-
 kernel/gcov/fs.c                  |  27 +-
 kernel/gcov/gcc_3_4.c             | 115 ++++++++
 kernel/gcov/gcc_4_7.c             | 562 ++++++++++++++++++++++++++++++++++++++
 kernel/gcov/gcov.h                |  65 +----
 kernel/module.c                   |   3 +
 10 files changed, 790 insertions(+), 81 deletions(-)
 create mode 100644 kernel/gcov/gcc_4_7.c

-- 
1.8.3.1

[PATCH v2 1/4] gcov: move gcov structs definitions to a gcc version specific file

[Frantisek Hrbata]

Since also the gcov structures(gcov_info, gcov_fn_info, gcov_ctr_info) can
change between gcc releases, as shown in gcc 4.7, they cannot be defined in a
common header and need to be moved to a specific gcc implemention file. This
also requires to make the gcov_info structure opaque for the common code and to
introduce simple helpers for accessing data inside gcov_info.

Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
---
 kernel/gcov/base.c    |  26 ++++++------
 kernel/gcov/fs.c      |  27 ++++++------
 kernel/gcov/gcc_3_4.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++
 kernel/gcov/gcov.h    |  65 +++++-----------------------
 4 files changed, 153 insertions(+), 80 deletions(-)

diff --git a/kernel/gcov/base.c b/kernel/gcov/base.c
index 9b22d03..912576a 100644
--- a/kernel/gcov/base.c
+++ b/kernel/gcov/base.c
@@ -20,7 +20,6 @@
 #include <linux/mutex.h>
 #include "gcov.h"
 
-static struct gcov_info *gcov_info_head;
 static int gcov_events_enabled;
 static DEFINE_MUTEX(gcov_lock);
 
@@ -34,7 +33,7 @@ void __gcov_init(struct gcov_info *info)
 
 	mutex_lock(&gcov_lock);
 	if (gcov_version == 0) {
-		gcov_version = info->version;
+		gcov_version = gcov_info_version(info);
 		/*
 		 * Printing gcc's version magic may prove useful for debugging
 		 * incompatibility reports.
@@ -45,8 +44,7 @@ void __gcov_init(struct gcov_info *info)
 	 * Add new profiling data structure to list and inform event
 	 * listener.
 	 */
-	info->next = gcov_info_head;
-	gcov_info_head = info;
+	gcov_info_link(info);
 	if (gcov_events_enabled)
 		gcov_event(GCOV_ADD, info);
 	mutex_unlock(&gcov_lock);
@@ -91,13 +89,15 @@ EXPORT_SYMBOL(__gcov_merge_delta);
  */
 void gcov_enable_events(void)
 {
-	struct gcov_info *info;
+	struct gcov_info *info = NULL;
 
 	mutex_lock(&gcov_lock);
 	gcov_events_enabled = 1;
+
 	/* Perform event callback for previously registered entries. */
-	for (info = gcov_info_head; info; info = info->next)
+	while ((info = gcov_info_next(info)))
 		gcov_event(GCOV_ADD, info);
+
 	mutex_unlock(&gcov_lock);
 }
 
@@ -112,25 +112,23 @@ static int gcov_module_notifier(struct notifier_block *nb, unsigned long event,
 				void *data)
 {
 	struct module *mod = data;
-	struct gcov_info *info;
-	struct gcov_info *prev;
+	struct gcov_info *info = NULL;
+	struct gcov_info *prev = NULL;
 
 	if (event != MODULE_STATE_GOING)
 		return NOTIFY_OK;
 	mutex_lock(&gcov_lock);
-	prev = NULL;
+
 	/* Remove entries located in module from linked list. */
-	for (info = gcov_info_head; info; info = info->next) {
+	while ((info = gcov_info_next(info))) {
 		if (within(info, mod->module_core, mod->core_size)) {
-			if (prev)
-				prev->next = info->next;
-			else
-				gcov_info_head = info->next;
+			gcov_info_unlink(prev, info);
 			if (gcov_events_enabled)
 				gcov_event(GCOV_REMOVE, info);
 		} else
 			prev = info;
 	}
+
 	mutex_unlock(&gcov_lock);
 
 	return NOTIFY_OK;
diff --git a/kernel/gcov/fs.c b/kernel/gcov/fs.c
index 9bd0934..27e12ce 100644
--- a/kernel/gcov/fs.c
+++ b/kernel/gcov/fs.c
@@ -242,7 +242,7 @@ static struct gcov_node *get_node_by_name(const char *name)
 
 	list_for_each_entry(node, &all_head, all) {
 		info = get_node_info(node);
-		if (info && (strcmp(info->filename, name) == 0))
+		if (info && (strcmp(gcov_info_filename(info), name) == 0))
 			return node;
 	}
 
@@ -279,7 +279,7 @@ static ssize_t gcov_seq_write(struct file *file, const char __user *addr,
 	seq = file->private_data;
 	info = gcov_iter_get_info(seq->private);
 	mutex_lock(&node_lock);
-	node = get_node_by_name(info->filename);
+	node = get_node_by_name(gcov_info_filename(info));
 	if (node) {
 		/* Reset counts or remove node for unloaded modules. */
 		if (node->num_loaded == 0)
@@ -376,8 +376,9 @@ static void add_links(struct gcov_node *node, struct dentry *parent)
 	if (!node->links)
 		return;
 	for (i = 0; i < num; i++) {
-		target = get_link_target(get_node_info(node)->filename,
-					 &gcov_link[i]);
+		target = get_link_target(
+				gcov_info_filename(get_node_info(node)),
+				&gcov_link[i]);
 		if (!target)
 			goto out_err;
 		basename = strrchr(target, '/');
@@ -576,7 +577,7 @@ static void add_node(struct gcov_info *info)
 	struct gcov_node *parent;
 	struct gcov_node *node;
 
-	filename = kstrdup(info->filename, GFP_KERNEL);
+	filename = kstrdup(gcov_info_filename(info), GFP_KERNEL);
 	if (!filename)
 		return;
 	parent = &root_node;
@@ -631,7 +632,7 @@ static void add_info(struct gcov_node *node, struct gcov_info *info)
 	loaded_info = kcalloc(num + 1, sizeof(struct gcov_info *), GFP_KERNEL);
 	if (!loaded_info) {
 		pr_warning("could not add '%s' (out of memory)\n",
-			   info->filename);
+			   gcov_info_filename(info));
 		return;
 	}
 	memcpy(loaded_info, node->loaded_info,
@@ -645,7 +646,8 @@ static void add_info(struct gcov_node *node, struct gcov_info *info)
 		 */
 		if (!gcov_info_is_compatible(node->unloaded_info, info)) {
 			pr_warning("discarding saved data for %s "
-				   "(incompatible version)\n", info->filename);
+				   "(incompatible version)\n",
+				   gcov_info_filename(info));
 			gcov_info_free(node->unloaded_info);
 			node->unloaded_info = NULL;
 		}
@@ -656,7 +658,7 @@ static void add_info(struct gcov_node *node, struct gcov_info *info)
 		 */
 		if (!gcov_info_is_compatible(node->loaded_info[0], info)) {
 			pr_warning("could not add '%s' (incompatible "
-				   "version)\n", info->filename);
+				   "version)\n", gcov_info_filename(info));
 			kfree(loaded_info);
 			return;
 		}
@@ -692,7 +694,8 @@ static void save_info(struct gcov_node *node, struct gcov_info *info)
 		node->unloaded_info = gcov_info_dup(info);
 		if (!node->unloaded_info) {
 			pr_warning("could not save data for '%s' "
-				   "(out of memory)\n", info->filename);
+				   "(out of memory)\n",
+				   gcov_info_filename(info));
 		}
 	}
 }
@@ -708,7 +711,7 @@ static void remove_info(struct gcov_node *node, struct gcov_info *info)
 	i = get_info_index(node, info);
 	if (i < 0) {
 		pr_warning("could not remove '%s' (not found)\n",
-			   info->filename);
+			   gcov_info_filename(info));
 		return;
 	}
 	if (gcov_persist)
@@ -735,7 +738,7 @@ void gcov_event(enum gcov_action action, struct gcov_info *info)
 	struct gcov_node *node;
 
 	mutex_lock(&node_lock);
-	node = get_node_by_name(info->filename);
+	node = get_node_by_name(gcov_info_filename(info));
 	switch (action) {
 	case GCOV_ADD:
 		if (node)
@@ -748,7 +751,7 @@ void gcov_event(enum gcov_action action, struct gcov_info *info)
 			remove_info(node, info);
 		else {
 			pr_warning("could not remove '%s' (not found)\n",
-				   info->filename);
+				   gcov_info_filename(info));
 		}
 		break;
 	}
diff --git a/kernel/gcov/gcc_3_4.c b/kernel/gcov/gcc_3_4.c
index ae5bb42..27bc88a 100644
--- a/kernel/gcov/gcc_3_4.c
+++ b/kernel/gcov/gcc_3_4.c
@@ -21,6 +21,121 @@
 #include <linux/vmalloc.h>
 #include "gcov.h"
 
+#define GCOV_COUNTERS		5
+
+static struct gcov_info *gcov_info_head;
+
+/**
+ * struct gcov_fn_info - profiling meta data per function
+ * @ident: object file-unique function identifier
+ * @checksum: function checksum
+ * @n_ctrs: number of values per counter type belonging to this function
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time.
+ */
+struct gcov_fn_info {
+	unsigned int ident;
+	unsigned int checksum;
+	unsigned int n_ctrs[0];
+};
+
+/**
+ * struct gcov_ctr_info - profiling data per counter type
+ * @num: number of counter values for this type
+ * @values: array of counter values for this type
+ * @merge: merge function for counter values of this type (unused)
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time with the exception of the values array.
+ */
+struct gcov_ctr_info {
+	unsigned int	num;
+	gcov_type	*values;
+	void		(*merge)(gcov_type *, unsigned int);
+};
+
+/**
+ * struct gcov_info - profiling data per object file
+ * @version: gcov version magic indicating the gcc version used for compilation
+ * @next: list head for a singly-linked list
+ * @stamp: time stamp
+ * @filename: name of the associated gcov data file
+ * @n_functions: number of instrumented functions
+ * @functions: function data
+ * @ctr_mask: mask specifying which counter types are active
+ * @counts: counter data per counter type
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time with the exception of the next pointer.
+ */
+struct gcov_info {
+	unsigned int			version;
+	struct gcov_info		*next;
+	unsigned int			stamp;
+	const char			*filename;
+	unsigned int			n_functions;
+	const struct gcov_fn_info	*functions;
+	unsigned int			ctr_mask;
+	struct gcov_ctr_info		counts[0];
+};
+
+/**
+ * gcov_info_filename - return info filename
+ * @info: profiling data set
+ */
+const char *gcov_info_filename(struct gcov_info *info)
+{
+	return info->filename;
+}
+
+/**
+ * gcov_info_version - return info version
+ * @info: profiling data set
+ */
+unsigned int gcov_info_version(struct gcov_info *info)
+{
+	return info->version;
+}
+
+/**
+ * gcov_info_next - return next profiling data set
+ * @info: profiling data set
+ *
+ * Returns next gcov_info following @info or first gcov_info in the chain if
+ * @info is %NULL.
+ */
+struct gcov_info *gcov_info_next(struct gcov_info *info)
+{
+	if (!info)
+		return gcov_info_head;
+
+	return info->next;
+}
+
+/**
+ * gcov_info_link - link/add profiling data set to the list
+ * @info: profiling data set
+ */
+void gcov_info_link(struct gcov_info *info)
+{
+	info->next = gcov_info_head;
+	gcov_info_head = info;
+}
+
+/**
+ * gcov_info_unlink - unlink/remove profiling data set from the list
+ * @prev: previous profiling data set
+ * @info: profiling data set
+ */
+void gcov_info_unlink(struct gcov_info *prev, struct gcov_info *info)
+{
+	if (prev)
+		prev->next = info->next;
+	else
+		gcov_info_head = info->next;
+}
+
 /* Symbolic links to be created for each profiling data file. */
 const struct gcov_link gcov_link[] = {
 	{ OBJ_TREE, "gcno" },	/* Link to .gcno file in $(objtree). */
diff --git a/kernel/gcov/gcov.h b/kernel/gcov/gcov.h
index 060073e..92c8e22 100644
--- a/kernel/gcov/gcov.h
+++ b/kernel/gcov/gcov.h
@@ -21,7 +21,6 @@
  * gcc and need to be kept as close to the original definition as possible to
  * remain compatible.
  */
-#define GCOV_COUNTERS		5
 #define GCOV_DATA_MAGIC		((unsigned int) 0x67636461)
 #define GCOV_TAG_FUNCTION	((unsigned int) 0x01000000)
 #define GCOV_TAG_COUNTER_BASE	((unsigned int) 0x01a10000)
@@ -34,60 +33,18 @@ typedef long gcov_type;
 typedef long long gcov_type;
 #endif
 
-/**
- * struct gcov_fn_info - profiling meta data per function
- * @ident: object file-unique function identifier
- * @checksum: function checksum
- * @n_ctrs: number of values per counter type belonging to this function
- *
- * This data is generated by gcc during compilation and doesn't change
- * at run-time.
- */
-struct gcov_fn_info {
-	unsigned int ident;
-	unsigned int checksum;
-	unsigned int n_ctrs[0];
-};
-
-/**
- * struct gcov_ctr_info - profiling data per counter type
- * @num: number of counter values for this type
- * @values: array of counter values for this type
- * @merge: merge function for counter values of this type (unused)
- *
- * This data is generated by gcc during compilation and doesn't change
- * at run-time with the exception of the values array.
- */
-struct gcov_ctr_info {
-	unsigned int	num;
-	gcov_type	*values;
-	void		(*merge)(gcov_type *, unsigned int);
-};
+/* Opaque gcov_info. The gcov structures can change as for example in gcc 4.7 so
+ * we cannot use full definition here and they need to be placed in gcc specific
+ * implementation of gcov. This also means no direct access to the members in
+ * generic code and usage of the interface below.*/
+struct gcov_info;
 
-/**
- * struct gcov_info - profiling data per object file
- * @version: gcov version magic indicating the gcc version used for compilation
- * @next: list head for a singly-linked list
- * @stamp: time stamp
- * @filename: name of the associated gcov data file
- * @n_functions: number of instrumented functions
- * @functions: function data
- * @ctr_mask: mask specifying which counter types are active
- * @counts: counter data per counter type
- *
- * This data is generated by gcc during compilation and doesn't change
- * at run-time with the exception of the next pointer.
- */
-struct gcov_info {
-	unsigned int			version;
-	struct gcov_info		*next;
-	unsigned int			stamp;
-	const char			*filename;
-	unsigned int			n_functions;
-	const struct gcov_fn_info	*functions;
-	unsigned int			ctr_mask;
-	struct gcov_ctr_info		counts[0];
-};
+/* Interface to access gcov_info data  */
+const char *gcov_info_filename(struct gcov_info *info);
+unsigned int gcov_info_version(struct gcov_info *info);
+struct gcov_info *gcov_info_next(struct gcov_info *info);
+void gcov_info_link(struct gcov_info *info);
+void gcov_info_unlink(struct gcov_info *prev, struct gcov_info *info);
 
 /* Base interface. */
 enum gcov_action {
-- 
1.8.3.1

[PATCH v2 2/4] gcov: add support for gcc 4.7 gcov format

[Frantisek Hrbata]

The gcov in-memory format changed in gcc 4.7. The biggest change, which
requires this special implementation, is that gcov_info no longer contains
array of counters for each counter type for all functions and gcov_fn_info is
not used for mapping of function's counters to these arrays(offset). Now each
gcov_fn_info contans it's counters, which makes things a little bit easier.

This is heavily based on the previous gcc_3_4.c implementation and patches
provided by Peter Oberparleiter. Specially the buffer gcda implementation for
iterator.

v2: - removed const definition for gcov_fn_info in gcov_info
    - use vmalloc for counter values in gcov_info_dup
    - use iter buffer for gcda

Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
---
 kernel/gcov/base.c    |   6 +
 kernel/gcov/gcc_4_7.c | 562 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 568 insertions(+)
 create mode 100644 kernel/gcov/gcc_4_7.c

diff --git a/kernel/gcov/base.c b/kernel/gcov/base.c
index 912576a..f45b75b 100644
--- a/kernel/gcov/base.c
+++ b/kernel/gcov/base.c
@@ -79,6 +79,12 @@ void __gcov_merge_delta(gcov_type *counters, unsigned int n_counters)
 }
 EXPORT_SYMBOL(__gcov_merge_delta);
 
+void __gcov_merge_ior(gcov_type *counters, unsigned int n_counters)
+{
+	/* Unused. */
+}
+EXPORT_SYMBOL(__gcov_merge_ior);
+
 /**
  * gcov_enable_events - enable event reporting through gcov_event()
  *
diff --git a/kernel/gcov/gcc_4_7.c b/kernel/gcov/gcc_4_7.c
new file mode 100644
index 0000000..d91afee
--- /dev/null
+++ b/kernel/gcov/gcc_4_7.c
@@ -0,0 +1,562 @@
+/*
+ *  This code provides functions to handle gcc's profiling data format
+ *  introduced with gcc 4.7.
+ *
+ *  This file is based heavily on gcc_3_4.c file.
+ *
+ *  For a better understanding, refer to gcc source:
+ *  gcc/gcov-io.h
+ *  libgcc/libgcov.c
+ *
+ *  Uses gcc-internal data definitions.
+ */
+
+#include <linux/errno.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/seq_file.h>
+#include "gcov.h"
+
+#define GCOV_COUNTERS			8
+#define GCOV_TAG_FUNCTION_LENGTH	3
+
+static struct gcov_info *gcov_info_head;
+
+/**
+ * struct gcov_ctr_info - information about counters for a single function
+ * @num: number of counter values for this type
+ * @values: array of counter values for this type
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time with the exception of the values array.
+ */
+struct gcov_ctr_info {
+	unsigned int num;
+	gcov_type *values;
+};
+
+/**
+ * struct gcov_fn_info - profiling meta data per function
+ * @key: comdat key
+ * @ident: unique ident of function
+ * @lineno_checksum: function lineo_checksum
+ * @cfg_checksum: function cfg checksum
+ * @ctrs: instrumented counters
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time.
+ *
+ * Information about a single function.  This uses the trailing array
+ * idiom. The number of counters is determined from the merge pointer
+ * array in gcov_info.  The key is used to detect which of a set of
+ * comdat functions was selected -- it points to the gcov_info object
+ * of the object file containing the selected comdat function.
+ */
+struct gcov_fn_info {
+	const struct gcov_info *key;
+	unsigned int ident;
+	unsigned int lineno_checksum;
+	unsigned int cfg_checksum;
+	struct gcov_ctr_info ctrs[0];
+};
+
+/**
+ * struct gcov_info - profiling data per object file
+ * @version: gcov version magic indicating the gcc version used for compilation
+ * @next: list head for a singly-linked list
+ * @stamp: uniquifying time stamp
+ * @filename: name of the associated gcov data file
+ * @merge: merge functions (null for unused counter type)
+ * @n_functions: number of instrumented functions
+ * @functions: pointer to pointers to function information
+ *
+ * This data is generated by gcc during compilation and doesn't change
+ * at run-time with the exception of the next pointer.
+ */
+struct gcov_info {
+	unsigned int version;
+	struct gcov_info *next;
+	unsigned int stamp;
+	const char *filename;
+	void (*merge[GCOV_COUNTERS])(gcov_type *, unsigned int);
+	unsigned int n_functions;
+	struct gcov_fn_info **functions;
+};
+
+/**
+ * gcov_info_filename - return info filename
+ * @info: profiling data set
+ */
+const char *gcov_info_filename(struct gcov_info *info)
+{
+	return info->filename;
+}
+
+/**
+ * gcov_info_version - return info version
+ * @info: profiling data set
+ */
+unsigned int gcov_info_version(struct gcov_info *info)
+{
+	return info->version;
+}
+
+/**
+ * gcov_info_next - return next profiling data set
+ * @info: profiling data set
+ *
+ * Returns next gcov_info following @info or first gcov_info in the chain if
+ * @info is %NULL.
+ */
+struct gcov_info *gcov_info_next(struct gcov_info *info)
+{
+	if (!info)
+		return gcov_info_head;
+
+	return info->next;
+}
+
+/**
+ * gcov_info_link - link/add profiling data set to the list
+ * @info: profiling data set
+ */
+void gcov_info_link(struct gcov_info *info)
+{
+	info->next = gcov_info_head;
+	gcov_info_head = info;
+}
+
+/**
+ * gcov_info_unlink - unlink/remove profiling data set from the list
+ * @prev: previous profiling data set
+ * @info: profiling data set
+ */
+void gcov_info_unlink(struct gcov_info *prev, struct gcov_info *info)
+{
+	if (prev)
+		prev->next = info->next;
+	else
+		gcov_info_head = info->next;
+}
+
+/* Symbolic links to be created for each profiling data file. */
+const struct gcov_link gcov_link[] = {
+	{ OBJ_TREE, "gcno" },	/* Link to .gcno file in $(objtree). */
+	{ 0, NULL},
+};
+
+/*
+ * Determine whether a counter is active. Doesn't change at run-time.
+ */
+static int counter_active(struct gcov_info *info, unsigned int type)
+{
+	return info->merge[type] ? 1 : 0;
+}
+
+/* Determine number of active counters. Based on gcc magic. */
+static unsigned int num_counter_active(struct gcov_info *info)
+{
+	unsigned int i;
+	unsigned int result = 0;
+
+	for (i = 0; i < GCOV_COUNTERS; i++) {
+		if (counter_active(info, i))
+			result++;
+	}
+	return result;
+}
+
+/**
+ * gcov_info_reset - reset profiling data to zero
+ * @info: profiling data set
+ */
+void gcov_info_reset(struct gcov_info *info)
+{
+	struct gcov_ctr_info *ci_ptr;
+	unsigned int fi_idx;
+	unsigned int ct_idx;
+
+	for (fi_idx = 0; fi_idx < info->n_functions; fi_idx++)
+	{
+		ci_ptr = info->functions[fi_idx]->ctrs;
+
+		for (ct_idx = 0; ct_idx < GCOV_COUNTERS; ct_idx++) {
+			if (!counter_active(info, ct_idx))
+				continue;
+
+			memset(ci_ptr->values, 0,
+					sizeof(gcov_type) * ci_ptr->num);
+			ci_ptr++;
+		}
+	}
+}
+
+/**
+ * gcov_info_is_compatible - check if profiling data can be added
+ * @info1: first profiling data set
+ * @info2: second profiling data set
+ *
+ * Returns non-zero if profiling data can be added, zero otherwise.
+ */
+int gcov_info_is_compatible(struct gcov_info *info1, struct gcov_info *info2)
+{
+	return (info1->stamp == info2->stamp);
+}
+
+/**
+ * gcov_info_add - add up profiling data
+ * @dest: profiling data set to which data is added
+ * @source: profiling data set which is added
+ *
+ * Adds profiling counts of @source to @dest.
+ */
+void gcov_info_add(struct gcov_info *dst, struct gcov_info *src)
+{
+	struct gcov_ctr_info *dci_ptr;
+	struct gcov_ctr_info *sci_ptr;
+	unsigned int fi_idx;
+	unsigned int ct_idx;
+	unsigned int val_idx;
+
+	for (fi_idx = 0; fi_idx < src->n_functions; fi_idx++)
+	{
+		dci_ptr = dst->functions[fi_idx]->ctrs;
+		sci_ptr = src->functions[fi_idx]->ctrs;
+
+		for (ct_idx = 0; ct_idx < GCOV_COUNTERS; ct_idx++) {
+			if (!counter_active(src, ct_idx))
+				continue;
+
+			for (val_idx = 0; val_idx < sci_ptr->num; val_idx++)
+				dci_ptr->values[val_idx] +=
+					sci_ptr->values[val_idx];
+
+			dci_ptr++;
+			sci_ptr++;
+		}
+	}
+}
+
+/**
+ * gcov_info_dup - duplicate profiling data set
+ * @info: profiling data set to duplicate
+ *
+ * Return newly allocated duplicate on success, %NULL on error.
+ */
+struct gcov_info *gcov_info_dup(struct gcov_info *info)
+{
+	struct gcov_info *dup;
+	struct gcov_ctr_info *dci_ptr; /* dst counter info */
+	struct gcov_ctr_info *sci_ptr; /* src counter info */
+	unsigned int active;
+	unsigned int fi_idx; /* function info idx */
+	unsigned int ct_idx; /* counter type idx */
+	size_t fi_size; /* function info size */
+	size_t cv_size; /* counter values size */
+
+	dup = kmalloc(sizeof(struct gcov_info), GFP_KERNEL);
+	if (!dup)
+		return NULL;
+
+	*dup = *info;
+	dup->next = NULL;
+	dup->filename = NULL;
+	dup->functions = NULL;
+
+	dup->filename = kstrdup(info->filename, GFP_KERNEL);
+	if (!dup->filename)
+		goto err_free;
+
+	dup->functions = kzalloc(sizeof(struct gcov_fn_info *) *
+			info->n_functions, GFP_KERNEL);
+	if (!dup->functions)
+		goto err_free;
+
+	active = num_counter_active(info);
+	fi_size = sizeof(struct gcov_fn_info);
+	fi_size += sizeof(struct gcov_ctr_info) * active;
+
+	for (fi_idx = 0; fi_idx < info->n_functions; fi_idx++) {
+		dup->functions[fi_idx] = kzalloc(fi_size, GFP_KERNEL);
+		if (!dup->functions[fi_idx])
+			goto err_free;
+
+		*(dup->functions[fi_idx]) = *(info->functions[fi_idx]);
+
+		sci_ptr = info->functions[fi_idx]->ctrs;
+		dci_ptr = dup->functions[fi_idx]->ctrs;
+
+		for (ct_idx = 0; ct_idx < active; ct_idx++) {
+
+			cv_size = sizeof(gcov_type) * sci_ptr->num;
+
+			dci_ptr->values = vmalloc(cv_size);
+
+			if (!dci_ptr->values)
+				goto err_free;
+
+			dci_ptr->num = sci_ptr->num;
+			memcpy(dci_ptr->values, sci_ptr->values, cv_size);
+
+			sci_ptr++;
+			dci_ptr++;
+		}
+	}
+
+	return dup;
+err_free:
+	gcov_info_free(dup);
+	return NULL;
+}
+
+/**
+ * gcov_info_free - release memory for profiling data set duplicate
+ * @info: profiling data set duplicate to free
+ */
+void gcov_info_free(struct gcov_info *info)
+{
+	unsigned int active;
+	unsigned int fi_idx;
+	unsigned int ct_idx;
+	struct gcov_ctr_info *ci_ptr;
+
+	if (!info->functions)
+		goto free_info;
+
+	active = num_counter_active(info);
+
+	for (fi_idx = 0; fi_idx < info->n_functions; fi_idx++) {
+		if (!info->functions[fi_idx])
+			continue;
+
+		ci_ptr = info->functions[fi_idx]->ctrs;
+
+		for (ct_idx = 0; ct_idx < active; ct_idx++, ci_ptr++)
+			vfree(ci_ptr->values);
+
+		kfree(info->functions[fi_idx]);
+	}
+
+free_info:
+	kfree(info->functions);
+	kfree(info->filename);
+	kfree(info);
+}
+
+#define ITER_STRIDE	PAGE_SIZE
+
+/**
+ * struct gcov_iterator - specifies current file position in logical records
+ * @info: associated profiling data
+ * @buffer: buffer containing file data
+ * @size: size of buffer
+ * @pos: current position in file
+ */
+struct gcov_iterator {
+	struct gcov_info *info;
+	void *buffer;
+	size_t size;
+	loff_t pos;
+};
+
+/**
+ * store_gcov_u32 - store 32 bit number in gcov format to buffer
+ * @buffer: target buffer or NULL
+ * @off: offset into the buffer
+ * @v: value to be stored
+ *
+ * Number format defined by gcc: numbers are recorded in the 32 bit
+ * unsigned binary form of the endianness of the machine generating the
+ * file. Returns the number of bytes stored. If @buffer is %NULL, doesn't
+ * store anything.
+ */
+static size_t store_gcov_u32(void *buffer, size_t off, u32 v)
+{
+	u32 *data;
+
+	if (buffer) {
+		data = buffer + off;
+		*data = v;
+	}
+
+	return sizeof(*data);
+}
+
+/**
+ * store_gcov_u64 - store 64 bit number in gcov format to buffer
+ * @buffer: target buffer or NULL
+ * @off: offset into the buffer
+ * @v: value to be stored
+ *
+ * Number format defined by gcc: numbers are recorded in the 32 bit
+ * unsigned binary form of the endianness of the machine generating the
+ * file. 64 bit numbers are stored as two 32 bit numbers, the low part
+ * first. Returns the number of bytes stored. If @buffer is %NULL, doesn't store
+ * anything.
+ */
+static size_t store_gcov_u64(void *buffer, size_t off, u64 v)
+{
+	u32 *data;
+
+	if (buffer) {
+		data = buffer + off;
+
+		data[0] = (v & 0xffffffffUL);
+		data[1] = (v >> 32);
+	}
+
+	return sizeof(*data) * 2;
+}
+
+/**
+ * convert_to_gcda - convert profiling data set to gcda file format
+ * @buffer: the buffer to store file data or %NULL if no data should be stored
+ * @info: profiling data set to be converted
+ *
+ * Returns the number of bytes that were/would have been stored into the buffer.
+ */
+static size_t convert_to_gcda(char *buffer, struct gcov_info *info)
+{
+	struct gcov_fn_info *fi_ptr;
+	struct gcov_ctr_info *ci_ptr;
+	unsigned int fi_idx;
+	unsigned int ct_idx;
+	unsigned int cv_idx;
+	size_t pos = 0;
+
+	/* File header. */
+	pos += store_gcov_u32(buffer, pos, GCOV_DATA_MAGIC);
+	pos += store_gcov_u32(buffer, pos, info->version);
+	pos += store_gcov_u32(buffer, pos, info->stamp);
+
+	for (fi_idx = 0; fi_idx < info->n_functions; fi_idx++) {
+		fi_ptr = info->functions[fi_idx];
+
+		/* Function record. */
+		pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION);
+		pos += store_gcov_u32(buffer, pos, GCOV_TAG_FUNCTION_LENGTH);
+		pos += store_gcov_u32(buffer, pos, fi_ptr->ident);
+		pos += store_gcov_u32(buffer, pos, fi_ptr->lineno_checksum);
+		pos += store_gcov_u32(buffer, pos, fi_ptr->cfg_checksum);
+
+		ci_ptr = fi_ptr->ctrs;
+
+		for (ct_idx = 0; ct_idx < GCOV_COUNTERS; ct_idx++) {
+			if (!counter_active(info, ct_idx))
+				continue;
+
+			/* Counter record. */
+			pos += store_gcov_u32(buffer, pos,
+					      GCOV_TAG_FOR_COUNTER(ct_idx));
+			pos += store_gcov_u32(buffer, pos, ci_ptr->num * 2);
+
+			for (cv_idx = 0; cv_idx < ci_ptr->num; cv_idx++) {
+				pos += store_gcov_u64(buffer, pos,
+						      ci_ptr->values[cv_idx]);
+			}
+
+			ci_ptr++;
+		}
+	}
+
+	return pos;
+}
+
+/**
+ * gcov_iter_new - allocate and initialize profiling data iterator
+ * @info: profiling data set to be iterated
+ *
+ * Return file iterator on success, %NULL otherwise.
+ */
+struct gcov_iterator *gcov_iter_new(struct gcov_info *info)
+{
+	struct gcov_iterator *iter;
+
+	iter = kzalloc(sizeof(struct gcov_iterator), GFP_KERNEL);
+	if (!iter)
+		goto err_free;
+
+	iter->info = info;
+	/* Dry-run to get the actual buffer size. */
+	iter->size = convert_to_gcda(NULL, info);
+	iter->buffer = vmalloc(iter->size);
+	if (!iter->buffer)
+		goto err_free;
+
+	convert_to_gcda(iter->buffer, info);
+
+	return iter;
+
+err_free:
+	kfree(iter);
+	return NULL;
+}
+
+
+/**
+ * gcov_iter_get_info - return profiling data set for given file iterator
+ * @iter: file iterator
+ */
+void gcov_iter_free(struct gcov_iterator *iter)
+{
+	vfree(iter->buffer);
+	kfree(iter);
+}
+
+/**
+ * gcov_iter_get_info - return profiling data set for given file iterator
+ * @iter: file iterator
+ */
+struct gcov_info *gcov_iter_get_info(struct gcov_iterator *iter)
+{
+	return iter->info;
+}
+
+/**
+ * gcov_iter_start - reset file iterator to starting position
+ * @iter: file iterator
+ */
+void gcov_iter_start(struct gcov_iterator *iter)
+{
+	iter->pos = 0;
+}
+
+/**
+ * gcov_iter_next - advance file iterator to next logical record
+ * @iter: file iterator
+ *
+ * Return zero if new position is valid, non-zero if iterator has reached end.
+ */
+int gcov_iter_next(struct gcov_iterator *iter)
+{
+	if (iter->pos < iter->size)
+		iter->pos += ITER_STRIDE;
+
+	if (iter->pos >= iter->size)
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
+ * gcov_iter_write - write data for current pos to seq_file
+ * @iter: file iterator
+ * @seq: seq_file handle
+ *
+ * Return zero on success, non-zero otherwise.
+ */
+int gcov_iter_write(struct gcov_iterator *iter, struct seq_file *seq)
+{
+	size_t len;
+
+	if (iter->pos >= iter->size)
+		return -EINVAL;
+
+	len = ITER_STRIDE;
+	if (iter->pos + len > iter->size)
+		len = iter->size - iter->pos;
+
+	seq_write(seq, iter->buffer + iter->pos, len);
+
+	return 0;
+}
-- 
1.8.3.1

[PATCH v2 3/4] gcov: compile specific gcov implementation based on gcc version

[Frantisek Hrbata]

Compile the correct gcov implementation file for the specific gcc version.

v2: - added possibility to explicitly select gcc version during configuration,
      this is based on code provided by Peter Oberparleiter.
    - added a note about gcov format selection to gcov.txt

Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
---
 Documentation/gcov.txt |  4 ++++
 kernel/gcov/Kconfig    | 30 ++++++++++++++++++++++++++++++
 kernel/gcov/Makefile   | 32 +++++++++++++++++++++++++++++++-
 3 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/Documentation/gcov.txt b/Documentation/gcov.txt
index e7ca647..7b72778 100644
--- a/Documentation/gcov.txt
+++ b/Documentation/gcov.txt
@@ -50,6 +50,10 @@ Configure the kernel with:
         CONFIG_DEBUG_FS=y
         CONFIG_GCOV_KERNEL=y
 
+select the gcc's gcov format, default is autodetect based on gcc version:
+
+        CONFIG_GCOV_FORMAT_AUTODETECT=y
+
 and to get coverage data for the entire kernel:
 
         CONFIG_GCOV_PROFILE_ALL=y
diff --git a/kernel/gcov/Kconfig b/kernel/gcov/Kconfig
index d4da55d..d04ce8a 100644
--- a/kernel/gcov/Kconfig
+++ b/kernel/gcov/Kconfig
@@ -46,4 +46,34 @@ config GCOV_PROFILE_ALL
 	larger and run slower. Also be sure to exclude files from profiling
 	which are not linked to the kernel image to prevent linker errors.
 
+choice
+	prompt "Specify GCOV format"
+	depends on GCOV_KERNEL
+	default GCOV_FORMAT_AUTODETECT
+	---help---
+	The gcov format is usually determined by the GCC version, but there are
+	exceptions where format changes are integrated in lower-version GCCs.
+	In such a case use this option to adjust the format used in the kernel
+	accordingly.
+
+	If unsure, choose "Autodetect".
+
+config GCOV_FORMAT_AUTODETECT
+	bool "Autodetect"
+	---help---
+	Select this option to use the format that corresponds to your GCC
+	version.
+
+config GCOV_FORMAT_3_4
+	bool "GCC 3.4 format"
+	---help---
+	Select this option to use the format defined by GCC 3.4.
+
+config GCOV_FORMAT_4_7
+	bool "GCC 4.7 format"
+	---help---
+	Select this option to use the format defined by GCC 4.7.
+
+endchoice
+
 endmenu
diff --git a/kernel/gcov/Makefile b/kernel/gcov/Makefile
index e97ca59..52aa7e8 100644
--- a/kernel/gcov/Makefile
+++ b/kernel/gcov/Makefile
@@ -1,3 +1,33 @@
 ccflags-y := -DSRCTREE='"$(srctree)"' -DOBJTREE='"$(objtree)"'
 
-obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o gcc_3_4.o
+# if-lt
+# Usage VAR := $(call if-lt, $(a), $(b))
+# Returns 1 if (a < b)
+if-lt = $(shell [ $(1) -lt $(2) ] && echo 1)
+
+ifeq ($(CONFIG_GCOV_FORMAT_3_4),y)
+  cc-ver := 0304
+else ifeq ($(CONFIG_GCOV_FORMAT_4_7),y)
+  cc-ver := 0407
+else
+# Use cc-version if available, otherwise set 0
+#
+# scripts/Kbuild.include, which contains cc-version function, is not included
+# during make clean "make -f scripts/Makefile.clean obj=kernel/gcov"
+# Meaning cc-ver is empty causing if-lt test to fail with
+# "/bin/sh: line 0: [: -lt: unary operator expected" error mesage.
+# This has no affect on the clean phase, but the error message could be
+# confusing/annoying. So this dummy workaround sets cc-ver to zero if cc-version
+# is not available. We can probably move if-lt to Kbuild.include, so it's also
+# not defined during clean or to include Kbuild.include in
+# scripts/Makefile.clean. But the following workaround seems least invasive.
+  cc-ver := $(if $(call cc-version),$(call cc-version),0)
+endif
+
+obj-$(CONFIG_GCOV_KERNEL) := base.o fs.o
+
+ifeq ($(call if-lt, $(cc-ver), 0407),1)
+  obj-$(CONFIG_GCOV_KERNEL) += gcc_3_4.o
+else
+  obj-$(CONFIG_GCOV_KERNEL) += gcc_4_7.o
+endif
-- 
1.8.3.1

[PATCH v2 4/4] kernel: add support for init_array constructors

[Frantisek Hrbata]

This adds the .init_array section as yet another section with constructors. This
is needed because gcc could add __gcov_init calls to .init_array or .ctors
section, depending on gcc version.

v2: - reuse mod->ctors for .init_array section for modules, because gcc uses
      .ctors or .init_array, but not both at the same time

Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
---
 include/asm-generic/vmlinux.lds.h | 1 +
 kernel/module.c                   | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 69732d2..c55d8d9 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -468,6 +468,7 @@
 #define KERNEL_CTORS()	. = ALIGN(8);			   \
 			VMLINUX_SYMBOL(__ctors_start) = .; \
 			*(.ctors)			   \
+			*(.init_array)			   \
 			VMLINUX_SYMBOL(__ctors_end) = .;
 #else
 #define KERNEL_CTORS()
diff --git a/kernel/module.c b/kernel/module.c
index 2069158..bbbd953 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2760,6 +2760,9 @@ static void find_module_sections(struct module *mod, struct load_info *info)
 #ifdef CONFIG_CONSTRUCTORS
 	mod->ctors = section_objs(info, ".ctors",
 				  sizeof(*mod->ctors), &mod->num_ctors);
+	if (!mod->ctors)
+		mod->ctors = section_objs(info, ".init_array",
+				sizeof(*mod->ctors), &mod->num_ctors);
 #endif
 
 #ifdef CONFIG_TRACEPOINTS
-- 
1.8.3.1

Re: [PATCH v2 4/4] kernel: add support for init_array constructors

[Rusty Russell]

Frantisek Hrbata <fhrbata@redhat.com> writes:
> This adds the .init_array section as yet another section with constructors. This
> is needed because gcc could add __gcov_init calls to .init_array or .ctors
> section, depending on gcc version.
>
> v2: - reuse mod->ctors for .init_array section for modules, because gcc uses
>       .ctors or .init_array, but not both at the same time
>
> Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>

Might be nice to document which gcc version changed this, so people can
choose whether to cherry-pick this change?

Acked-by: Rusty Russell <rusty@rustcorp.com.au>

> ---
>  include/asm-generic/vmlinux.lds.h | 1 +
>  kernel/module.c                   | 3 +++
>  2 files changed, 4 insertions(+)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 69732d2..c55d8d9 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -468,6 +468,7 @@
>  #define KERNEL_CTORS()	. = ALIGN(8);			   \
>  			VMLINUX_SYMBOL(__ctors_start) = .; \
>  			*(.ctors)			   \
> +			*(.init_array)			   \
>  			VMLINUX_SYMBOL(__ctors_end) = .;
>  #else
>  #define KERNEL_CTORS()
> diff --git a/kernel/module.c b/kernel/module.c
> index 2069158..bbbd953 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2760,6 +2760,9 @@ static void find_module_sections(struct module *mod, struct load_info *info)
>  #ifdef CONFIG_CONSTRUCTORS
>  	mod->ctors = section_objs(info, ".ctors",
>  				  sizeof(*mod->ctors), &mod->num_ctors);
> +	if (!mod->ctors)
> +		mod->ctors = section_objs(info, ".init_array",
> +				sizeof(*mod->ctors), &mod->num_ctors);
>  #endif
>  
>  #ifdef CONFIG_TRACEPOINTS
> -- 
> 1.8.3.1

Re: [PATCH v2 4/4] kernel: add support for init_array constructors

[Frantisek Hrbata]

On Fri, Sep 06, 2013 at 11:43:08AM +0930, Rusty Russell wrote:
> Frantisek Hrbata <fhrbata@redhat.com> writes:
> > This adds the .init_array section as yet another section with constructors. This
> > is needed because gcc could add __gcov_init calls to .init_array or .ctors
> > section, depending on gcc version.
> >
> > v2: - reuse mod->ctors for .init_array section for modules, because gcc uses
> >       .ctors or .init_array, but not both at the same time
> >
> > Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
> 
> Might be nice to document which gcc version changed this, so people can
> choose whether to cherry-pick this change?

Thank you for pointing this out. As per gcc git this was introduced by commit
ef1da80 and released in 4.7 version.

$ git describe --contains ef1da80
gcc-4_7_0-release~4358

Do you want me to post v3 with this info included in the descrition?

> 
> Acked-by: Rusty Russell <rusty@rustcorp.com.au>

Many thanks

> 
> > ---
> >  include/asm-generic/vmlinux.lds.h | 1 +
> >  kernel/module.c                   | 3 +++
> >  2 files changed, 4 insertions(+)
> >
> > diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> > index 69732d2..c55d8d9 100644
> > --- a/include/asm-generic/vmlinux.lds.h
> > +++ b/include/asm-generic/vmlinux.lds.h
> > @@ -468,6 +468,7 @@
> >  #define KERNEL_CTORS()	. = ALIGN(8);			   \
> >  			VMLINUX_SYMBOL(__ctors_start) = .; \
> >  			*(.ctors)			   \
> > +			*(.init_array)			   \
> >  			VMLINUX_SYMBOL(__ctors_end) = .;
> >  #else
> >  #define KERNEL_CTORS()
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 2069158..bbbd953 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -2760,6 +2760,9 @@ static void find_module_sections(struct module *mod, struct load_info *info)
> >  #ifdef CONFIG_CONSTRUCTORS
> >  	mod->ctors = section_objs(info, ".ctors",
> >  				  sizeof(*mod->ctors), &mod->num_ctors);
> > +	if (!mod->ctors)
> > +		mod->ctors = section_objs(info, ".init_array",
> > +				sizeof(*mod->ctors), &mod->num_ctors);
> >  #endif
> >  
> >  #ifdef CONFIG_TRACEPOINTS
> > -- 
> > 1.8.3.1

-- 
Frantisek Hrbata

Re: [PATCH v2 4/4] kernel: add support for init_array constructors

[Kyle McMartin]

On Fri, Sep 06, 2013 at 07:51:18PM +0200, Frantisek Hrbata wrote:
> > > v2: - reuse mod->ctors for .init_array section for modules, because gcc uses
> > >       .ctors or .init_array, but not both at the same time
> > >
> > > Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
> > 
> > Might be nice to document which gcc version changed this, so people can
> > choose whether to cherry-pick this change?
> 
> Thank you for pointing this out. As per gcc git this was introduced by commit
> ef1da80 and released in 4.7 version.
> 
> $ git describe --contains ef1da80
> gcc-4_7_0-release~4358
> 
> Do you want me to post v3 with this info included in the descrition?
> 

It actually depends on the combination of binutils/ld and gcc you use, not
simply which gcc version you use. :/

--Kyle

Re: [PATCH v2 4/4] kernel: add support for init_array constructors

[Rusty Russell]

Kyle McMartin <kyle@infradead.org> writes:
> On Fri, Sep 06, 2013 at 07:51:18PM +0200, Frantisek Hrbata wrote:
>> > > v2: - reuse mod->ctors for .init_array section for modules, because gcc uses
>> > >       .ctors or .init_array, but not both at the same time
>> > >
>> > > Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
>> > 
>> > Might be nice to document which gcc version changed this, so people can
>> > choose whether to cherry-pick this change?
>> 
>> Thank you for pointing this out. As per gcc git this was introduced by commit
>> ef1da80 and released in 4.7 version.
>> 
>> $ git describe --contains ef1da80
>> gcc-4_7_0-release~4358
>> 
>> Do you want me to post v3 with this info included in the descrition?
>> 
>
> It actually depends on the combination of binutils/ld and gcc you use, not
> simply which gcc version you use. :/

Indeed, and seems it was binutils 20110507 which actually handled it
properly.

AFAICT it's theoretically possible to have .ctors and .init_array in a
module.  Unlikely, but the patch should check for both and refuse to
load the module in that case.  Otherwise weird things would happen.

Cheers,
Rusty.

Re: [PATCH v2 4/4] kernel: add support for init_array constructors

[Frantisek Hrbata]

On Mon, Sep 09, 2013 at 10:44:03AM +0930, Rusty Russell wrote:
> Kyle McMartin <kyle@infradead.org> writes:
> > On Fri, Sep 06, 2013 at 07:51:18PM +0200, Frantisek Hrbata wrote:
> >> > > v2: - reuse mod->ctors for .init_array section for modules, because gcc uses
> >> > >       .ctors or .init_array, but not both at the same time
> >> > >
> >> > > Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
> >> > 
> >> > Might be nice to document which gcc version changed this, so people can
> >> > choose whether to cherry-pick this change?
> >> 
> >> Thank you for pointing this out. As per gcc git this was introduced by commit
> >> ef1da80 and released in 4.7 version.
> >> 
> >> $ git describe --contains ef1da80
> >> gcc-4_7_0-release~4358
> >> 
> >> Do you want me to post v3 with this info included in the descrition?
> >> 
> >
> > It actually depends on the combination of binutils/ld and gcc you use, not
> > simply which gcc version you use. :/
> 
> Indeed, and seems it was binutils 20110507 which actually handled it
> properly.
> 
> AFAICT it's theoretically possible to have .ctors and .init_array in a
> module.  Unlikely, but the patch should check for both and refuse to
> load the module in that case.  Otherwise weird things would happen.

I'm not sure if coexistence of .ctors and .init_array sections should result in
denial of module, but I for sure know nothing about this :). Could you maybe
privide one example of the "weird thing"?

Anyway many thanks for taking time to look at this. Below is my attepmt to
implement the check you proposed. 

untested/uncompiled

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 83e2c31..bc2121f 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -473,6 +473,7 @@
 #define KERNEL_CTORS()	. = ALIGN(8);			   \
 			VMLINUX_SYMBOL(__ctors_start) = .; \
 			*(.ctors)			   \
+			*(.init_array)			   \
 			VMLINUX_SYMBOL(__ctors_end) = .;
 #else
 #define KERNEL_CTORS()
diff --git a/include/linux/module.h b/include/linux/module.h
index 05f2447..e775b41 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -380,6 +380,8 @@ struct module
 	/* Constructor functions. */
 	ctor_fn_t *ctors;
 	unsigned int num_ctors;
+	ctor_fn_t *init_array;
+	unsigned int num_init_array;
 #endif
 };
 #ifndef MODULE_ARCH_INIT
diff --git a/kernel/module.c b/kernel/module.c
index dc58274..831b92d 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2768,6 +2768,9 @@ static void find_module_sections(struct module *mod, struct load_info *info)
 #ifdef CONFIG_CONSTRUCTORS
 	mod->ctors = section_objs(info, ".ctors",
 				  sizeof(*mod->ctors), &mod->num_ctors);
+	mod->init_array = section_objs(info, ".init_array",
+				  sizeof(*mod->init_array),
+				  &mod->num_init_array);
 #endif
 
 #ifdef CONFIG_TRACEPOINTS
@@ -2808,6 +2811,18 @@ static void find_module_sections(struct module *mod, struct load_info *info)
 				   sizeof(*info->debug), &info->num_debug);
 }
 
+static void check_module_sections(struct module *mod)
+{
+#ifdef CONFIG_CONSTRUCTORS
+	if (mod->ctors && mod->init_array) {
+		pr_err("%s: .ctors and .init_array sections mishmash\n",
+				mod->name);
+		return -EINVAL;
+	}
+#endif
+	return 0;
+}
+
 static int move_module(struct module *mod, struct load_info *info)
 {
 	int i;
@@ -3032,6 +3047,9 @@ static void do_mod_ctors(struct module *mod)
 
 	for (i = 0; i < mod->num_ctors; i++)
 		mod->ctors[i]();
+
+	for (i = 0; i < mod->num_init_array; i++)
+		mod->init_array[i]();
 #endif
 }
 
@@ -3265,6 +3283,10 @@ static int load_module(struct load_info *info, const char __user *uargs,
 	 * find optional sections. */
 	find_module_sections(mod, info);
 
+	err = check_module_sections(mod);
+	if (err)
+		goto free_unload;
+
 	err = check_module_license_and_versions(mod);
 	if (err)
 		goto free_unload;
-- 
1.8.3.1

> 
> Cheers,
> Rusty.

-- 
Frantisek Hrbata

Re: [PATCH v2 4/4] kernel: add support for init_array constructors

[Kyle McMartin]

On Mon, Sep 09, 2013 at 06:28:14PM +0200, Frantisek Hrbata wrote:
> I'm not sure if coexistence of .ctors and .init_array sections should result in
> denial of module, but I for sure know nothing about this :). Could you maybe
> privide one example of the "weird thing"?
> 

They shouldn't exist unless placed there intentionally... I suspect a
call_if_changed Makefile target to regenerated a header would solve this
problem sufficiently for a given toolchain version.

A little exposition:
.init_array and .ctors are laid out on top of each other, with an
ordering that's a bit complicated... the sort of the ctor functions ends
up being .ctor.x upwards towards 65535, and .init_array.x downwards
from 65535 towards 0, with priority 65535-x, so that

.init_array.32768 would be called before .ctor.32768.

It's all a complete mess.

Perhaps if CONFIG_GCOV is on, we should enforce MODVERSIONS and make
sure the GCC version doesn't change for the running kernel?

Maybe it would be sufficient to just detect what the toolchain supports
and do that? I have a patch based on the configure.ac in gcc that
does something like that, which would be trivial to use to generate a
header based on gcc version.

> Anyway many thanks for taking time to look at this. Below is my attepmt to
> implement the check you proposed. 
> 
> untested/uncompiled

regards, Kyle

Re: [PATCH v2 4/4] kernel: add support for init_array constructors

[Rusty Russell]

Frantisek Hrbata <fhrbata@redhat.com> writes:
> On Mon, Sep 09, 2013 at 10:44:03AM +0930, Rusty Russell wrote:
>> Kyle McMartin <kyle@infradead.org> writes:
>> > On Fri, Sep 06, 2013 at 07:51:18PM +0200, Frantisek Hrbata wrote:
>> >> > > v2: - reuse mod->ctors for .init_array section for modules, because gcc uses
>> >> > >       .ctors or .init_array, but not both at the same time
>> >> > >
>> >> > > Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
>> >> > 
>> >> > Might be nice to document which gcc version changed this, so people can
>> >> > choose whether to cherry-pick this change?
>> >> 
>> >> Thank you for pointing this out. As per gcc git this was introduced by commit
>> >> ef1da80 and released in 4.7 version.
>> >> 
>> >> $ git describe --contains ef1da80
>> >> gcc-4_7_0-release~4358
>> >> 
>> >> Do you want me to post v3 with this info included in the descrition?
>> >> 
>> >
>> > It actually depends on the combination of binutils/ld and gcc you use, not
>> > simply which gcc version you use. :/
>> 
>> Indeed, and seems it was binutils 20110507 which actually handled it
>> properly.
>> 
>> AFAICT it's theoretically possible to have .ctors and .init_array in a
>> module.  Unlikely, but the patch should check for both and refuse to
>> load the module in that case.  Otherwise weird things would happen.
>
> I'm not sure if coexistence of .ctors and .init_array sections should result in
> denial of module, but I for sure know nothing about this :). Could you maybe
> privide one example of the "weird thing"?

Well, if we have both ctors and init_array, and we only call the ctors,
part of the module will be uninitialized.

I was thinking about something like the following (based on your
previous patch).

Thoughts?
Rusty.

From: Frantisek Hrbata <fhrbata@redhat.com>
Subject: kernel: add support for init_array constructors

This adds the .init_array section as yet another section with constructors. This
is needed because gcc could add __gcov_init calls to .init_array or .ctors
section, depending on gcc (and binutils) version .

v2: - reuse mod->ctors for .init_array section for modules, because gcc uses
      .ctors or .init_array, but not both at the same time
v3: - fail to load if that does happen somehow.

Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 83e2c31..bc2121f 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -473,6 +473,7 @@
 #define KERNEL_CTORS()	. = ALIGN(8);			   \
 			VMLINUX_SYMBOL(__ctors_start) = .; \
 			*(.ctors)			   \
+			*(.init_array)			   \
 			VMLINUX_SYMBOL(__ctors_end) = .;
 #else
 #define KERNEL_CTORS()
diff --git a/kernel/module.c b/kernel/module.c
index dc58274..d3f5a58 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2738,7 +2738,7 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
 	return 0;
 }
 
-static void find_module_sections(struct module *mod, struct load_info *info)
+static int find_module_sections(struct module *mod, struct load_info *info)
 {
 	mod->kp = section_objs(info, "__param",
 			       sizeof(*mod->kp), &mod->num_kp);
@@ -2768,6 +2768,18 @@ static void find_module_sections(struct module *mod, struct load_info *info)
 #ifdef CONFIG_CONSTRUCTORS
 	mod->ctors = section_objs(info, ".ctors",
 				  sizeof(*mod->ctors), &mod->num_ctors);
+	if (!mod->ctors)
+		mod->ctors = section_objs(info, ".init_array",
+				sizeof(*mod->ctors), &mod->num_ctors);
+	else if (find_sec(info, ".init_array")) {
+		/*
+		 * This shouldn't happen with same compiler and binutils
+		 * building all parts of the module.
+		 */
+		printk(KERN_WARNING "%s: has both .ctors and .init_array.\n",
+		       mod->name);
+		return -EINVAL;
+	}
 #endif
 
 #ifdef CONFIG_TRACEPOINTS
@@ -2806,6 +2818,8 @@ static void find_module_sections(struct module *mod, struct load_info *info)
 
 	info->debug = section_objs(info, "__verbose",
 				   sizeof(*info->debug), &info->num_debug);
+
+	return 0;
 }
 
 static int move_module(struct module *mod, struct load_info *info)
@@ -3263,7 +3277,9 @@ static int load_module(struct load_info *info, const char __user *uargs,
 
 	/* Now we've got everything in the final locations, we can
 	 * find optional sections. */
-	find_module_sections(mod, info);
+	err = find_module_sections(mod, info);
+	if (err)
+		goto free_unload;
 
 	err = check_module_license_and_versions(mod);
 	if (err)

Re: [PATCH v2 4/4] kernel: add support for init_array constructors

[Frantisek Hrbata]

On Tue, Sep 10, 2013 at 03:05:57PM +0930, Rusty Russell wrote:
> Frantisek Hrbata <fhrbata@redhat.com> writes:
> > On Mon, Sep 09, 2013 at 10:44:03AM +0930, Rusty Russell wrote:
> >> Kyle McMartin <kyle@infradead.org> writes:
> >> > On Fri, Sep 06, 2013 at 07:51:18PM +0200, Frantisek Hrbata wrote:
> >> >> > > v2: - reuse mod->ctors for .init_array section for modules, because gcc uses
> >> >> > >       .ctors or .init_array, but not both at the same time
> >> >> > >
> >> >> > > Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
> >> >> > 
> >> >> > Might be nice to document which gcc version changed this, so people can
> >> >> > choose whether to cherry-pick this change?
> >> >> 
> >> >> Thank you for pointing this out. As per gcc git this was introduced by commit
> >> >> ef1da80 and released in 4.7 version.
> >> >> 
> >> >> $ git describe --contains ef1da80
> >> >> gcc-4_7_0-release~4358
> >> >> 
> >> >> Do you want me to post v3 with this info included in the descrition?
> >> >> 
> >> >
> >> > It actually depends on the combination of binutils/ld and gcc you use, not
> >> > simply which gcc version you use. :/
> >> 
> >> Indeed, and seems it was binutils 20110507 which actually handled it
> >> properly.
> >> 
> >> AFAICT it's theoretically possible to have .ctors and .init_array in a
> >> module.  Unlikely, but the patch should check for both and refuse to
> >> load the module in that case.  Otherwise weird things would happen.
> >
> > I'm not sure if coexistence of .ctors and .init_array sections should result in
> > denial of module, but I for sure know nothing about this :). Could you maybe
> > privide one example of the "weird thing"?
> 
> Well, if we have both ctors and init_array, and we only call the ctors,
> part of the module will be uninitialized.
> 
> I was thinking about something like the following (based on your
> previous patch).
> 
> Thoughts?
> Rusty.

Thank you Rusty, from what I can say it looks ok to me. So I would go with this
version. Is there anything that needs to be done to consider this as the
correct version of the 4/4 patch? Meaning should we repost this as v3 or could
your version of the patch be picked as you posted it?

> 
> From: Frantisek Hrbata <fhrbata@redhat.com>
> Subject: kernel: add support for init_array constructors
> 
> This adds the .init_array section as yet another section with constructors. This
> is needed because gcc could add __gcov_init calls to .init_array or .ctors
> section, depending on gcc (and binutils) version .
> 
> v2: - reuse mod->ctors for .init_array section for modules, because gcc uses
>       .ctors or .init_array, but not both at the same time
> v3: - fail to load if that does happen somehow.
> 
> Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> 
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 83e2c31..bc2121f 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -473,6 +473,7 @@
>  #define KERNEL_CTORS()	. = ALIGN(8);			   \
>  			VMLINUX_SYMBOL(__ctors_start) = .; \
>  			*(.ctors)			   \
> +			*(.init_array)			   \
>  			VMLINUX_SYMBOL(__ctors_end) = .;
>  #else
>  #define KERNEL_CTORS()
> diff --git a/kernel/module.c b/kernel/module.c
> index dc58274..d3f5a58 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2738,7 +2738,7 @@ static int check_modinfo(struct module *mod, struct load_info *info, int flags)
>  	return 0;
>  }
>  
> -static void find_module_sections(struct module *mod, struct load_info *info)
> +static int find_module_sections(struct module *mod, struct load_info *info)
>  {
>  	mod->kp = section_objs(info, "__param",
>  			       sizeof(*mod->kp), &mod->num_kp);
> @@ -2768,6 +2768,18 @@ static void find_module_sections(struct module *mod, struct load_info *info)
>  #ifdef CONFIG_CONSTRUCTORS
>  	mod->ctors = section_objs(info, ".ctors",
>  				  sizeof(*mod->ctors), &mod->num_ctors);
> +	if (!mod->ctors)
> +		mod->ctors = section_objs(info, ".init_array",
> +				sizeof(*mod->ctors), &mod->num_ctors);
> +	else if (find_sec(info, ".init_array")) {
> +		/*
> +		 * This shouldn't happen with same compiler and binutils
> +		 * building all parts of the module.
> +		 */
> +		printk(KERN_WARNING "%s: has both .ctors and .init_array.\n",
> +		       mod->name);
> +		return -EINVAL;
> +	}
>  #endif
>  
>  #ifdef CONFIG_TRACEPOINTS
> @@ -2806,6 +2818,8 @@ static void find_module_sections(struct module *mod, struct load_info *info)
>  
>  	info->debug = section_objs(info, "__verbose",
>  				   sizeof(*info->debug), &info->num_debug);
> +
> +	return 0;
>  }
>  
>  static int move_module(struct module *mod, struct load_info *info)
> @@ -3263,7 +3277,9 @@ static int load_module(struct load_info *info, const char __user *uargs,
>  
>  	/* Now we've got everything in the final locations, we can
>  	 * find optional sections. */
> -	find_module_sections(mod, info);
> +	err = find_module_sections(mod, info);
> +	if (err)
> +		goto free_unload;
>  
>  	err = check_module_license_and_versions(mod);
>  	if (err)

-- 
Frantisek Hrbata

Re: [PATCH v2 4/4] kernel: add support for init_array constructors

[Rusty Russell]

Frantisek Hrbata <fhrbata@redhat.com> writes:
> On Tue, Sep 10, 2013 at 03:05:57PM +0930, Rusty Russell wrote:
>> Frantisek Hrbata <fhrbata@redhat.com> writes:
>> > On Mon, Sep 09, 2013 at 10:44:03AM +0930, Rusty Russell wrote:
>> >> Kyle McMartin <kyle@infradead.org> writes:
>> >> > On Fri, Sep 06, 2013 at 07:51:18PM +0200, Frantisek Hrbata wrote:
>> >> >> > > v2: - reuse mod->ctors for .init_array section for modules, because gcc uses
>> >> >> > >       .ctors or .init_array, but not both at the same time
>> >> >> > >
>> >> >> > > Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
>> >> >> > 
>> >> >> > Might be nice to document which gcc version changed this, so people can
>> >> >> > choose whether to cherry-pick this change?
>> >> >> 
>> >> >> Thank you for pointing this out. As per gcc git this was introduced by commit
>> >> >> ef1da80 and released in 4.7 version.
>> >> >> 
>> >> >> $ git describe --contains ef1da80
>> >> >> gcc-4_7_0-release~4358
>> >> >> 
>> >> >> Do you want me to post v3 with this info included in the descrition?
>> >> >> 
>> >> >
>> >> > It actually depends on the combination of binutils/ld and gcc you use, not
>> >> > simply which gcc version you use. :/
>> >> 
>> >> Indeed, and seems it was binutils 20110507 which actually handled it
>> >> properly.
>> >> 
>> >> AFAICT it's theoretically possible to have .ctors and .init_array in a
>> >> module.  Unlikely, but the patch should check for both and refuse to
>> >> load the module in that case.  Otherwise weird things would happen.
>> >
>> > I'm not sure if coexistence of .ctors and .init_array sections should result in
>> > denial of module, but I for sure know nothing about this :). Could you maybe
>> > privide one example of the "weird thing"?
>> 
>> Well, if we have both ctors and init_array, and we only call the ctors,
>> part of the module will be uninitialized.
>> 
>> I was thinking about something like the following (based on your
>> previous patch).
>> 
>> Thoughts?
>> Rusty.
>
> Thank you Rusty, from what I can say it looks ok to me. So I would go with this
> version. Is there anything that needs to be done to consider this as the
> correct version of the 4/4 patch? Meaning should we repost this as v3 or could
> your version of the patch be picked as you posted it?

Take that as posted.  I could push it through my tree, but I think
you'll want to keep them all together.

Cheers,
Rusty.

Re: [PATCH v2 2/4] gcov: add support for gcc 4.7 gcov format

[Andrew Morton]

On Wed,  4 Sep 2013 16:42:54 +0200 Frantisek Hrbata <fhrbata@redhat.com> wrote:

> The gcov in-memory format changed in gcc 4.7. The biggest change, which
> requires this special implementation, is that gcov_info no longer contains
> array of counters for each counter type for all functions and gcov_fn_info is
> not used for mapping of function's counters to these arrays(offset). Now each
> gcov_fn_info contans it's counters, which makes things a little bit easier.
> 
> This is heavily based on the previous gcc_3_4.c implementation and patches
> provided by Peter Oberparleiter. Specially the buffer gcda implementation for
> iterator.

A couple of little tweaks:

--- a/kernel/gcov/gcc_4_7.c~gcov-add-support-for-gcc-47-gcov-format-fix
+++ a/kernel/gcov/gcc_4_7.c
@@ -254,11 +254,10 @@ struct gcov_info *gcov_info_dup(struct g
 	size_t fi_size; /* function info size */
 	size_t cv_size; /* counter values size */
 
-	dup = kmalloc(sizeof(struct gcov_info), GFP_KERNEL);
+	dup = kmemdup(info, sizeof(*dup), GFP_KERNEL);
 	if (!dup)
 		return NULL;
 
-	*dup = *info;
 	dup->next = NULL;
 	dup->filename = NULL;
 	dup->functions = NULL;
@@ -267,8 +266,8 @@ struct gcov_info *gcov_info_dup(struct g
 	if (!dup->filename)
 		goto err_free;
 
-	dup->functions = kzalloc(sizeof(struct gcov_fn_info *) *
-			info->n_functions, GFP_KERNEL);
+	dup->functions = kcalloc(sizeof(struct gcov_fn_info *),
+				 info->n_functions, GFP_KERNEL);
 	if (!dup->functions)
 		goto err_free;
 
_

Re: [PATCH v2 2/4] gcov: add support for gcc 4.7 gcov format

[Joe Perches]

On Wed, 2013-09-18 at 14:22 -0700, Andrew Morton wrote:
> On Wed,  4 Sep 2013 16:42:54 +0200 Frantisek Hrbata <fhrbata@redhat.com> wrote:
> > The gcov in-memory format changed in gcc 4.7. The biggest change, which
> > requires this special implementation, is that gcov_info no longer contains
> > array of counters for each counter type for all functions and gcov_fn_info is
> > not used for mapping of function's counters to these arrays(offset). Now each
> > gcov_fn_info contans it's counters, which makes things a little bit easier.
> > 
> > This is heavily based on the previous gcc_3_4.c implementation and patches
> > provided by Peter Oberparleiter. Specially the buffer gcda implementation for
> > iterator.
> 
> A couple of little tweaks:
[]
> +++ a/kernel/gcov/gcc_4_7.c
[]
> @@ -267,8 +266,8 @@ struct gcov_info *gcov_info_dup(struct g
>  	if (!dup->filename)
>  		goto err_free;
>  
> -	dup->functions = kzalloc(sizeof(struct gcov_fn_info *) *
> -			info->n_functions, GFP_KERNEL);
> +	dup->functions = kcalloc(sizeof(struct gcov_fn_info *),
> +				 info->n_functions, GFP_KERNEL);

kcalloc(n, size_t, flags)

	dup->functions = kcalloc(info->n_functions,
				 sizeof(struct gcov_fn_info *), GFP_KERNEL);

Re: [PATCH v2 2/4] gcov: add support for gcc 4.7 gcov format

[Andrew Morton]

On Wed, 18 Sep 2013 14:27:05 -0700 Joe Perches <joe@perches.com> wrote:

> On Wed, 2013-09-18 at 14:22 -0700, Andrew Morton wrote:
> > On Wed,  4 Sep 2013 16:42:54 +0200 Frantisek Hrbata <fhrbata@redhat.com> wrote:
> > > The gcov in-memory format changed in gcc 4.7. The biggest change, which
> > > requires this special implementation, is that gcov_info no longer contains
> > > array of counters for each counter type for all functions and gcov_fn_info is
> > > not used for mapping of function's counters to these arrays(offset). Now each
> > > gcov_fn_info contans it's counters, which makes things a little bit easier.
> > > 
> > > This is heavily based on the previous gcc_3_4.c implementation and patches
> > > provided by Peter Oberparleiter. Specially the buffer gcda implementation for
> > > iterator.
> > 
> > A couple of little tweaks:
> []
> > +++ a/kernel/gcov/gcc_4_7.c
> []
> > @@ -267,8 +266,8 @@ struct gcov_info *gcov_info_dup(struct g
> >  	if (!dup->filename)
> >  		goto err_free;
> >  
> > -	dup->functions = kzalloc(sizeof(struct gcov_fn_info *) *
> > -			info->n_functions, GFP_KERNEL);
> > +	dup->functions = kcalloc(sizeof(struct gcov_fn_info *),
> > +				 info->n_functions, GFP_KERNEL);
> 
> kcalloc(n, size_t, flags)
> 
> 	dup->functions = kcalloc(info->n_functions,
> 				 sizeof(struct gcov_fn_info *), GFP_KERNEL);
> 

Already did that after checkpatch whined at me.  Stupid thing.

Re: [PATCH v2 2/4] gcov: add support for gcc 4.7 gcov format

[Peter Oberparleiter]

On 04.09.2013 16:42, Frantisek Hrbata wrote:
> The gcov in-memory format changed in gcc 4.7. The biggest change, which
> requires this special implementation, is that gcov_info no longer contains
> array of counters for each counter type for all functions and gcov_fn_info is
> not used for mapping of function's counters to these arrays(offset). Now each
> gcov_fn_info contans it's counters, which makes things a little bit easier.
> 
> This is heavily based on the previous gcc_3_4.c implementation and patches
> provided by Peter Oberparleiter. Specially the buffer gcda implementation for
> iterator.
> 
> v2: - removed const definition for gcov_fn_info in gcov_info
>     - use vmalloc for counter values in gcov_info_dup
>     - use iter buffer for gcda
> 
> Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>

The patch is missing an include statement:

  CC      kernel/gcov/gcc_4_7.o
kernel/gcov/gcc_4_7.c: In function ‘gcov_info_dup’:
kernel/gcov/gcc_4_7.c:293:4: error: implicit declaration of function ‘vmalloc’ [-Werror=implicit-function-declaration]
kernel/gcov/gcc_4_7.c:293:20: warning: assignment makes pointer from integer without a cast [enabled by default]

With that added, it compiles and works with gcc 4.3.4 and 4.7.2 on
kernel 3.21-rc1 on s390x.

--- a/kernel/gcov/gcc_4_7.c
+++ b/kernel/gcov/gcc_4_7.c
@@ -15,6 +15,7 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 #include <linux/seq_file.h>
+#include <linux/vmalloc.h>
 #include "gcov.h"

 #define GCOV_COUNTERS			8

Re: [PATCH v2 2/4] gcov: add support for gcc 4.7 gcov format

[Frantisek Hrbata]

On Wed, Sep 18, 2013 at 02:31:27PM -0700, Andrew Morton wrote:
> On Wed, 18 Sep 2013 14:27:05 -0700 Joe Perches <joe@perches.com> wrote:
> 
> > On Wed, 2013-09-18 at 14:22 -0700, Andrew Morton wrote:
> > > On Wed,  4 Sep 2013 16:42:54 +0200 Frantisek Hrbata <fhrbata@redhat.com> wrote:
> > > > The gcov in-memory format changed in gcc 4.7. The biggest change, which
> > > > requires this special implementation, is that gcov_info no longer contains
> > > > array of counters for each counter type for all functions and gcov_fn_info is
> > > > not used for mapping of function's counters to these arrays(offset). Now each
> > > > gcov_fn_info contans it's counters, which makes things a little bit easier.
> > > > 
> > > > This is heavily based on the previous gcc_3_4.c implementation and patches
> > > > provided by Peter Oberparleiter. Specially the buffer gcda implementation for
> > > > iterator.
> > > 
> > > A couple of little tweaks:
> > []
> > > +++ a/kernel/gcov/gcc_4_7.c
> > []
> > > @@ -267,8 +266,8 @@ struct gcov_info *gcov_info_dup(struct g
> > >  	if (!dup->filename)
> > >  		goto err_free;
> > >  
> > > -	dup->functions = kzalloc(sizeof(struct gcov_fn_info *) *
> > > -			info->n_functions, GFP_KERNEL);
> > > +	dup->functions = kcalloc(sizeof(struct gcov_fn_info *),
> > > +				 info->n_functions, GFP_KERNEL);
> > 
> > kcalloc(n, size_t, flags)
> > 
> > 	dup->functions = kcalloc(info->n_functions,
> > 				 sizeof(struct gcov_fn_info *), GFP_KERNEL);
> > 
> 
> Already did that after checkpatch whined at me.  Stupid thing.

Many thanks Andrew and I'm really sorry about the "style problems". Lesson well
learned: use checkpatch before posting.

-- 
Frantisek Hrbata

Re: [PATCH v2 2/4] gcov: add support for gcc 4.7 gcov format

[Frantisek Hrbata]

On Thu, Sep 19, 2013 at 11:04:16AM +0200, Peter Oberparleiter wrote:
> On 04.09.2013 16:42, Frantisek Hrbata wrote:
> > The gcov in-memory format changed in gcc 4.7. The biggest change, which
> > requires this special implementation, is that gcov_info no longer contains
> > array of counters for each counter type for all functions and gcov_fn_info is
> > not used for mapping of function's counters to these arrays(offset). Now each
> > gcov_fn_info contans it's counters, which makes things a little bit easier.
> > 
> > This is heavily based on the previous gcc_3_4.c implementation and patches
> > provided by Peter Oberparleiter. Specially the buffer gcda implementation for
> > iterator.
> > 
> > v2: - removed const definition for gcov_fn_info in gcov_info
> >     - use vmalloc for counter values in gcov_info_dup
> >     - use iter buffer for gcda
> > 
> > Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
> 
> The patch is missing an include statement:
> 
>   CC      kernel/gcov/gcc_4_7.o
> kernel/gcov/gcc_4_7.c: In function ‘gcov_info_dup’:
> kernel/gcov/gcc_4_7.c:293:4: error: implicit declaration of function ‘vmalloc’ [-Werror=implicit-function-declaration]
> kernel/gcov/gcc_4_7.c:293:20: warning: assignment makes pointer from integer without a cast [enabled by default]
> 
> With that added, it compiles and works with gcc 4.3.4 and 4.7.2 on
> kernel 3.21-rc1 on s390x.
> 
> --- a/kernel/gcov/gcc_4_7.c
> +++ b/kernel/gcov/gcc_4_7.c
> @@ -15,6 +15,7 @@
>  #include <linux/slab.h>
>  #include <linux/string.h>
>  #include <linux/seq_file.h>
> +#include <linux/vmalloc.h>
>  #include "gcov.h"
> 
>  #define GCOV_COUNTERS			8
> 

Hi Peter,

I'm sorry I missed that. Many thanks. In addition, do you have any other
objedtions to the code? Andrew already added the patches to the -mm tree.
Andrew, should I post a follow up patch adding the vmalloc.h include or is it
possible to fix it directly as you did for the previous patches? 
Again I'm sorry for the inconvenience.

Many thanks

-- 
Frantisek Hrbata

Re: [PATCH v2 2/4] gcov: add support for gcc 4.7 gcov format

[Peter Oberparleiter]

On 19.09.2013 12:21, Frantisek Hrbata wrote:
> On Thu, Sep 19, 2013 at 11:04:16AM +0200, Peter Oberparleiter wrote:
>> On 04.09.2013 16:42, Frantisek Hrbata wrote:
>>> The gcov in-memory format changed in gcc 4.7. The biggest change, which
>>> requires this special implementation, is that gcov_info no longer contains
>>> array of counters for each counter type for all functions and gcov_fn_info is
>>> not used for mapping of function's counters to these arrays(offset). Now each
>>> gcov_fn_info contans it's counters, which makes things a little bit easier.
>>>
>>> This is heavily based on the previous gcc_3_4.c implementation and patches
>>> provided by Peter Oberparleiter. Specially the buffer gcda implementation for
>>> iterator.
>>>
>>> v2: - removed const definition for gcov_fn_info in gcov_info
>>>     - use vmalloc for counter values in gcov_info_dup
>>>     - use iter buffer for gcda
>>>
>>> Signed-off-by: Frantisek Hrbata <fhrbata@redhat.com>
>>
>> The patch is missing an include statement:
>>
>>   CC      kernel/gcov/gcc_4_7.o
>> kernel/gcov/gcc_4_7.c: In function ‘gcov_info_dup’:
>> kernel/gcov/gcc_4_7.c:293:4: error: implicit declaration of function ‘vmalloc’ [-Werror=implicit-function-declaration]
>> kernel/gcov/gcc_4_7.c:293:20: warning: assignment makes pointer from integer without a cast [enabled by default]
>>
>> With that added, it compiles and works with gcc 4.3.4 and 4.7.2 on
>> kernel 3.21-rc1 on s390x.
>>
>> --- a/kernel/gcov/gcc_4_7.c
>> +++ b/kernel/gcov/gcc_4_7.c
>> @@ -15,6 +15,7 @@
>>  #include <linux/slab.h>
>>  #include <linux/string.h>
>>  #include <linux/seq_file.h>
>> +#include <linux/vmalloc.h>
>>  #include "gcov.h"
>>
>>  #define GCOV_COUNTERS			8
>>
> 
> Hi Peter,
> 
> I'm sorry I missed that. Many thanks. In addition, do you have any other
> objedtions to the code? Andrew already added the patches to the -mm tree.
> Andrew, should I post a follow up patch adding the vmalloc.h include or is it
> possible to fix it directly as you did for the previous patches? 
> Again I'm sorry for the inconvenience.

With the fix applied I have no further objections.

Reviewed-by: Peter Oberparleiter <oberpar@linux.vnet.ibm.com>

Thanks!

-- 
Peter Oberparleiter
Linux on System z Development - IBM Germany