gcc 4.7 changed format used internally for coverage data.
This patch address these changes.
The information that gcc generate are mostly the same but to support common
sections.
The only difference in the blob exported by Xen is that functions have 2
different checksums instead of one.
Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
---
tools/misc/xencov_split | 26 ++++++--
xen/arch/x86/xen.lds.S | 1 +
xen/common/gcov/gcov.c | 161 +++++++++++++++++++++++++++++++++++++--------
xen/include/public/gcov.h | 16 ++++-
xen/include/xen/gcov.h | 68 ++++++++++++++++++-
5 files changed, 235 insertions(+), 37 deletions(-)
Patch tested with gcc 4.7, x64 with x86 dom0 and lcov 1.10.
diff --git a/tools/misc/xencov_split b/tools/misc/xencov_split
index 2e5aa80..af0f580 100755
--- a/tools/misc/xencov_split
+++ b/tools/misc/xencov_split
@@ -27,7 +27,8 @@ my $magic = 0x67636461;
my $ctrBase = 0x01a10000;
my $xenMagic = 0x58544346; # file header
-my $xenTagFunc = 0x58544366; # functions tag
+my $xenTagFunc = 0x58544366; # functions tag
+my $xenTagFunc2 = 0x58544367; # functions tag2
my $xenTagCount0 = 0x58544330; # counter 0 tag
my $xenTagEnd = 0x5854432e; # end file
@@ -86,9 +87,9 @@ sub getS()
return $res;
}
-sub parseFunctions($)
+sub parseFunctions($$)
{
- my $numCounters = shift;
+ my ($numCounters, $ver) = @_;
my $num = get32();
my @funcs;
@@ -96,10 +97,12 @@ sub parseFunctions($)
my @data;
my $ident = get32();
my $checksum = get32();
+ my $checksum2 = 0;
+ $checksum2 = get32() if $ver > 1;
for my $n (1..$numCounters) {
push @data, get32(); # number of counters for a type
}
- push @funcs, [$ident, $checksum, \@data];
+ push @funcs, [$ver, $ident, $checksum, $checksum2, \@data];
}
align();
return @funcs;
@@ -147,7 +150,12 @@ sub parseFile()
last if ($tag == $xenMagic || $tag == $xenTagEnd);
if ($tag == $xenTagFunc) {
die if scalar(@funcs);
- @funcs = parseFunctions(scalar(@ctrs));
+ @funcs = parseFunctions(scalar(@ctrs), 1);
+ next;
+ }
+ if ($tag == $xenTagFunc2) {
+ die if scalar(@funcs);
+ @funcs = parseFunctions(scalar(@ctrs), 2);
next;
}
@@ -159,10 +167,14 @@ sub parseFile()
# print all functions
for my $f (@funcs) {
# tag tag_len ident checksum
- print OUT pack(''VVVV'', 0x01000000, 2, $f->[0],
$f->[1]);
+ if ($f->[0] == 1) {
+ print OUT pack(''VVVV'', 0x01000000, 2,
$f->[1], $f->[2]);
+ } else {
+ print OUT pack(''VVVVV'', 0x01000000, 3,
$f->[1], $f->[2], $f->[3]);
+ }
# all counts
my $n = 0;
- for my $c (@{$f->[2]}) {
+ for my $c (@{$f->[4]}) {
my ($type, $data) = @{$ctrs[$n]};
print OUT pack(''VV'', $ctrBase + 0x20000 * $type,
$c*2);
die "--$c--$type--$data--" if length($data) < $c * 8;
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index d959941..bdc4c91 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -112,6 +112,7 @@ SECTIONS
. = ALIGN(8);
__ctors_start = .;
*(.ctors)
+ *(.init_array)
__ctors_end = .;
} :text
. = ALIGN(32);
diff --git a/xen/common/gcov/gcov.c b/xen/common/gcov/gcov.c
index b5717b9..1d32d8f 100644
--- a/xen/common/gcov/gcov.c
+++ b/xen/common/gcov/gcov.c
@@ -106,17 +106,120 @@ static int write_string(write_iter_t *iter, const char
*s)
static inline int next_type(const struct gcov_info *info, int *type)
{
- while ( ++*type < XENCOV_COUNTERS && !counter_active(info,
*type) )
+ while ( ++*type < XENCOV_COUNTERS_MASK && !counter_active(info,
*type) )
continue;
return *type;
}
+static inline const struct gcov_fn_info_407 *
+next_func(const struct gcov_info_407 *info, int *n_func)
+{
+ while ( ++*n_func < info->n_functions ) {
+ const struct gcov_fn_info_407 *fn = info->functions[*n_func];
+
+ /* the test for info member handle common data redefinitions
+ in object files */
+ if ( fn && fn->info == info)
+ return fn;
+ }
+
+ return NULL;
+}
+
+static inline const struct gcov_ctr_info_407 *
+next_ctr(const struct gcov_fn_info_407 *fn, int *n_ctr)
+{
+ while ( ++*n_ctr < XENCOV_COUNTERS_407 )
+ if ( fn->info->merge[*n_ctr] )
+ return &fn->ctrs[*n_ctr];
+
+ return NULL;
+}
+
static inline void align_iter(write_iter_t *iter)
{
iter->write_offset (iter->write_offset + sizeof(uint64_t) -
1) & -sizeof(uint64_t);
}
+static int write_info(write_iter_t *iter, const struct gcov_info* info)
+{
+ const struct gcov_ctr_info *ctr;
+ int type, ret;
+ size_t size_fn = sizeof(struct gcov_fn_info);
+
+ /* dump counters */
+ ctr = info->counts;
+ for ( type = -1; next_type(info, &type) < XENCOV_COUNTERS_MASK;
++ctr )
+ {
+ align_iter(iter);
+ chk(write32(iter, XENCOV_TAG_COUNTER(type)));
+ chk(write32(iter, ctr->num));
+ chk(write_raw(iter, ctr->values,
+ ctr->num * sizeof(ctr->values[0])));
+
+ size_fn += sizeof(unsigned);
+ }
+
+ /* dump all functions together */
+ align_iter(iter);
+ chk(write32(iter, XENCOV_TAG_FUNC));
+ chk(write32(iter, info->n_functions));
+ chk(write_raw(iter, info->functions, info->n_functions * size_fn));
+
+ return 0;
+}
+
+static int write_info_407(write_iter_t *iter, const struct gcov_info_407* info)
+{
+ int ret;
+ const struct gcov_fn_info_407 *fn;
+ const struct gcov_ctr_info_407 *ctr;
+ int n_func, n_ctr;
+ unsigned int num_func = 0;
+ unsigned int ctrs[XENCOV_COUNTERS_407];
+
+ for ( n_ctr = 0; n_ctr < XENCOV_COUNTERS_407; ++n_ctr )
+ ctrs[n_ctr] = 0;
+
+ /* scan to total counters */
+ for ( n_func = -1; (fn = next_func(info, &n_func)) != NULL; )
+ {
+ ++num_func;
+ for ( n_ctr = -1; (ctr = next_ctr(fn, &n_ctr)) != NULL; )
+ ctrs[n_ctr] += ctr->num;
+ }
+
+ /* output counters */
+ for ( n_ctr = 0; n_ctr < XENCOV_COUNTERS_407; ++n_ctr )
+ {
+ if ( !ctrs[n_ctr] ) continue;
+ align_iter(iter);
+ chk(write32(iter, XENCOV_TAG_COUNTER(n_ctr)));
+ chk(write32(iter, ctrs[n_ctr]));
+ for ( n_func = -1; (fn = next_func(info, &n_func)) != NULL; )
+ {
+ ctr = &fn->ctrs[n_ctr];
+ chk(write_raw(iter, ctr->values,
+ ctr->num * sizeof(ctr->values[0])));
+ }
+ }
+
+ /* dump all functions together */
+ align_iter(iter);
+ chk(write32(iter, XENCOV_TAG_FUNC2));
+ chk(write32(iter, num_func));
+ for ( n_func = -1; (fn = next_func(info, &n_func)) != NULL; )
+ {
+ chk(write32(iter, fn->ident));
+ chk(write32(iter, fn->lineno_checksum));
+ chk(write32(iter, fn->cfg_checksum));
+ for ( n_ctr = -1; (ctr = next_ctr(fn, &n_ctr)) != NULL; )
+ chk(write32(iter, ctr->num));
+ }
+ return 0;
+}
+
static int write_gcov(write_iter_t *iter)
{
struct gcov_info *info;
@@ -128,10 +231,6 @@ static int write_gcov(write_iter_t *iter)
/* dump all files */
for ( info = info_list ; info; info = info->next )
{
- const struct gcov_ctr_info *ctr;
- int type;
- size_t size_fn = sizeof(struct gcov_fn_info);
-
align_iter(iter);
chk(write32(iter, XENCOV_TAG_FILE));
chk(write32(iter, info->version));
@@ -139,23 +238,10 @@ static int write_gcov(write_iter_t *iter)
chk(write_string(iter, info->filename));
/* dump counters */
- ctr = info->counts;
- for ( type = -1; next_type(info, &type) < XENCOV_COUNTERS; ++ctr
)
- {
- align_iter(iter);
- chk(write32(iter, XENCOV_TAG_COUNTER(type)));
- chk(write32(iter, ctr->num));
- chk(write_raw(iter, ctr->values,
- ctr->num * sizeof(ctr->values[0])));
-
- size_fn += sizeof(unsigned);
- }
-
- /* dump all functions together */
- align_iter(iter);
- chk(write32(iter, XENCOV_TAG_FUNC));
- chk(write32(iter, info->n_functions));
- chk(write_raw(iter, info->functions, info->n_functions *
size_fn));
+ if (info->version < XENCOV_VERSION_407)
+ chk(write_info(iter, info));
+ else
+ chk(write_info_407(iter, (struct gcov_info_407 *) info));
}
/* stop tag */
@@ -164,19 +250,38 @@ static int write_gcov(write_iter_t *iter)
return 0;
}
+static void reset_info(struct gcov_info *info)
+{
+ const struct gcov_ctr_info *ctr;
+ int type;
+
+ ctr = info->counts;
+ for ( type = -1; next_type(info, &type) < XENCOV_COUNTERS_MASK;
++ctr )
+ memset(ctr->values, 0, ctr->num * sizeof(ctr->values[0]));
+}
+
+static void reset_info_407(struct gcov_info_407 *info)
+{
+ const struct gcov_fn_info_407 *fn;
+ const struct gcov_ctr_info_407 *ctr;
+ int n_func, n_ctr;
+
+ for ( n_func = -1; (fn = next_func(info, &n_func)) != NULL; )
+ for ( n_ctr = -1; (ctr = next_ctr(fn, &n_ctr)) != NULL; )
+ memset(ctr->values, 0, ctr->num * sizeof(ctr->values[0]));
+}
+
static int reset_counters(void)
{
struct gcov_info *info;
for ( info = info_list ; info; info = info->next )
{
- const struct gcov_ctr_info *ctr;
- int type;
-
/* reset counters */
- ctr = info->counts;
- for ( type = -1; next_type(info, &type) < XENCOV_COUNTERS; ++ctr
)
- memset(ctr->values, 0, ctr->num * sizeof(ctr->values[0]));
+ if ( info->version < XENCOV_VERSION_407 )
+ reset_info(info);
+ else
+ reset_info_407((struct gcov_info_407 *) info);
}
return 0;
diff --git a/xen/include/public/gcov.h b/xen/include/public/gcov.h
index 1b29b48..e7573fb 100644
--- a/xen/include/public/gcov.h
+++ b/xen/include/public/gcov.h
@@ -28,10 +28,12 @@
#ifndef __XEN_PUBLIC_GCOV_H__
#define __XEN_PUBLIC_GCOV_H__ __XEN_PUBLIC_GCOV_H__
-#define XENCOV_COUNTERS 5
+#define XENCOV_COUNTERS_MASK 5
+#define XENCOV_COUNTERS 8
#define XENCOV_TAG_BASE 0x58544300u
#define XENCOV_TAG_FILE (XENCOV_TAG_BASE+0x46u)
#define XENCOV_TAG_FUNC (XENCOV_TAG_BASE+0x66u)
+#define XENCOV_TAG_FUNC2 (XENCOV_TAG_BASE+0x67u)
#define XENCOV_TAG_COUNTER(n) (XENCOV_TAG_BASE+0x30u+((n)&0xfu))
#define XENCOV_TAG_END (XENCOV_TAG_BASE+0x2eu)
#define XENCOV_IS_TAG_COUNTER(n) \
@@ -93,6 +95,18 @@ struct xencov_function
};
/**
+ * Information for each function
+ * Number of counter is equal to the number of counter structures got before
+ */
+struct xencov_function2
+{
+ uint32_t ident;
+ uint32_t lineno_checksum;
+ uint32_t cfg_checksum;
+ uint32_t num_counters[1];
+};
+
+/**
* Information for all functions
* Aligned to 8 bytes
*/
diff --git a/xen/include/xen/gcov.h b/xen/include/xen/gcov.h
index 27c5c37..f36388a 100644
--- a/xen/include/xen/gcov.h
+++ b/xen/include/xen/gcov.h
@@ -40,6 +40,8 @@ struct gcov_fn_info
unsigned int n_ctrs[0];
};
+typedef void (*gcov_merge_func)(gcov_type *, unsigned int);
+
/**
* struct gcov_ctr_info - profiling data per counter type
* @num: number of counter values for this type
@@ -53,7 +55,7 @@ struct gcov_ctr_info
{
unsigned int num;
gcov_type *values;
- void (*merge)(gcov_type *, unsigned int);
+ gcov_merge_func merge;
};
/**
@@ -82,6 +84,70 @@ struct gcov_info
struct gcov_ctr_info counts[0];
};
+struct gcov_info_407;
+
+/**
+ * struct gcov_ctr_info_407 - 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_407
+{
+ unsigned int num;
+ gcov_type *values;
+};
+
+
+/**
+ * struct gcov_fn_info_407 - profiling meta data per function
+ * @ident: object file-unique function identifier
+ * @lineno_checksum: function lineno checksum
+ * @cfg_checksum: function cfg checksum
+ * @ctrs: counters for this function
+ *
+ * This data is generated by gcc during compilation and doesn''t change
+ * at run-time.
+ */
+struct gcov_fn_info_407
+{
+ const struct gcov_info_407 *info;
+ unsigned int ident;
+ unsigned int lineno_checksum;
+ unsigned int cfg_checksum;
+ struct gcov_ctr_info_407 ctrs[0];
+};
+
+#define XENCOV_COUNTERS_407 8
+#define XENCOV_VERSION_407 0x34303700
+
+/**
+ * struct gcov_info_407 - 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
+ * @merge: merge functions
+ * @n_functions: number of instrumented functions
+ * @functions: function data
+ *
+ * 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_407
+{
+ unsigned int version;
+ struct gcov_info *next;
+ unsigned int stamp;
+ const char *filename;
+ gcov_merge_func merge[XENCOV_COUNTERS_407];
+ unsigned int n_functions;
+ const struct gcov_fn_info_407 * const *functions;
+};
+
/**
* Sysctl operations for coverage
--
1.7.10.4
>>> On 17.06.13 at 10:29, Frediano Ziglio <frediano.ziglio@citrix.com> wrote: > --- a/xen/arch/x86/xen.lds.S > +++ b/xen/arch/x86/xen.lds.S > @@ -112,6 +112,7 @@ SECTIONS > . = ALIGN(8); > __ctors_start = .; > *(.ctors) > + *(.init_array) > __ctors_end = .; > } :text > . = ALIGN(32);First of all - again an x86 change without a matching ARM one? And then this is different from how binutils deals with these in a number of aspects: - .init_array.* not handled (raises the question why .ctors.* didn''t get handled already) - .init_array following .ctors here, while binutils has it the other way around - final section being .init_array instead of .ctors - enclosing symbols prefixed with __init_array_ instead of __ctors_ Some or all of these may not matter, but I think any difference to how binutils deals with these sections needs to be explained in the patch description.> +static inline const struct gcov_fn_info_407 * > +next_func(const struct gcov_info_407 *info, int *n_func)What are these "_407" suffixes intended to mean? They look rather arbitrary, and to me aren''t connected to the gcc version talked about here.> +{ > + while ( ++*n_func < info->n_functions ) {Coding style.> + const struct gcov_fn_info_407 *fn = info->functions[*n_func]; > + > + /* the test for info member handle common data redefinitions > + in object files */Again. Stopping here; there are more.> --- a/xen/include/public/gcov.h > +++ b/xen/include/public/gcov.h > @@ -28,10 +28,12 @@ > #ifndef __XEN_PUBLIC_GCOV_H__ > #define __XEN_PUBLIC_GCOV_H__ __XEN_PUBLIC_GCOV_H__ > > -#define XENCOV_COUNTERS 5 > +#define XENCOV_COUNTERS_MASK 5Misnamed manifest constant? It''s never being used as a mask and also doesn''t look like one.> +#define XENCOV_COUNTERS 8And this one doesn''t appear to get used anywhere, so why are these changes being done in the first place?> #define XENCOV_TAG_BASE 0x58544300u > #define XENCOV_TAG_FILE (XENCOV_TAG_BASE+0x46u) > #define XENCOV_TAG_FUNC (XENCOV_TAG_BASE+0x66u) > +#define XENCOV_TAG_FUNC2 (XENCOV_TAG_BASE+0x67u) > #define XENCOV_TAG_COUNTER(n) (XENCOV_TAG_BASE+0x30u+((n)&0xfu)) > #define XENCOV_TAG_END (XENCOV_TAG_BASE+0x2eu) > #define XENCOV_IS_TAG_COUNTER(n) \ > @@ -93,6 +95,18 @@ struct xencov_function > }; > > /** > + * Information for each function > + * Number of counter is equal to the number of counter structures got before > + */ > +struct xencov_function2 > +{ > + uint32_t ident; > + uint32_t lineno_checksum; > + uint32_t cfg_checksum; > + uint32_t num_counters[1]; > +};Nor can I seem to be able to spot a use of this structure. Jan
On 17/06/13 09:29, Frediano Ziglio wrote:> gcc 4.7 changed format used internally for coverage data. > This patch address these changes. > The information that gcc generate are mostly the same but to support common > sections. > The only difference in the blob exported by Xen is that functions have 2 > different checksums instead of one. > > Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>At this point we''d like to completely freeze the tree except for critical bugfixes. I think this will have to wait for 4.4. -George> --- > tools/misc/xencov_split | 26 ++++++-- > xen/arch/x86/xen.lds.S | 1 + > xen/common/gcov/gcov.c | 161 +++++++++++++++++++++++++++++++++++++-------- > xen/include/public/gcov.h | 16 ++++- > xen/include/xen/gcov.h | 68 ++++++++++++++++++- > 5 files changed, 235 insertions(+), 37 deletions(-) > > Patch tested with gcc 4.7, x64 with x86 dom0 and lcov 1.10. > > diff --git a/tools/misc/xencov_split b/tools/misc/xencov_split > index 2e5aa80..af0f580 100755 > --- a/tools/misc/xencov_split > +++ b/tools/misc/xencov_split > @@ -27,7 +27,8 @@ my $magic = 0x67636461; > my $ctrBase = 0x01a10000; > > my $xenMagic = 0x58544346; # file header > -my $xenTagFunc = 0x58544366; # functions tag > +my $xenTagFunc = 0x58544366; # functions tag > +my $xenTagFunc2 = 0x58544367; # functions tag2 > my $xenTagCount0 = 0x58544330; # counter 0 tag > my $xenTagEnd = 0x5854432e; # end file > > @@ -86,9 +87,9 @@ sub getS() > return $res; > } > > -sub parseFunctions($) > +sub parseFunctions($$) > { > - my $numCounters = shift; > + my ($numCounters, $ver) = @_; > my $num = get32(); > > my @funcs; > @@ -96,10 +97,12 @@ sub parseFunctions($) > my @data; > my $ident = get32(); > my $checksum = get32(); > + my $checksum2 = 0; > + $checksum2 = get32() if $ver > 1; > for my $n (1..$numCounters) { > push @data, get32(); # number of counters for a type > } > - push @funcs, [$ident, $checksum, \@data]; > + push @funcs, [$ver, $ident, $checksum, $checksum2, \@data]; > } > align(); > return @funcs; > @@ -147,7 +150,12 @@ sub parseFile() > last if ($tag == $xenMagic || $tag == $xenTagEnd); > if ($tag == $xenTagFunc) { > die if scalar(@funcs); > - @funcs = parseFunctions(scalar(@ctrs)); > + @funcs = parseFunctions(scalar(@ctrs), 1); > + next; > + } > + if ($tag == $xenTagFunc2) { > + die if scalar(@funcs); > + @funcs = parseFunctions(scalar(@ctrs), 2); > next; > } > > @@ -159,10 +167,14 @@ sub parseFile() > # print all functions > for my $f (@funcs) { > # tag tag_len ident checksum > - print OUT pack(''VVVV'', 0x01000000, 2, $f->[0], $f->[1]); > + if ($f->[0] == 1) { > + print OUT pack(''VVVV'', 0x01000000, 2, $f->[1], $f->[2]); > + } else { > + print OUT pack(''VVVVV'', 0x01000000, 3, $f->[1], $f->[2], $f->[3]); > + } > # all counts > my $n = 0; > - for my $c (@{$f->[2]}) { > + for my $c (@{$f->[4]}) { > my ($type, $data) = @{$ctrs[$n]}; > print OUT pack(''VV'', $ctrBase + 0x20000 * $type, $c*2); > die "--$c--$type--$data--" if length($data) < $c * 8; > diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S > index d959941..bdc4c91 100644 > --- a/xen/arch/x86/xen.lds.S > +++ b/xen/arch/x86/xen.lds.S > @@ -112,6 +112,7 @@ SECTIONS > . = ALIGN(8); > __ctors_start = .; > *(.ctors) > + *(.init_array) > __ctors_end = .; > } :text > . = ALIGN(32); > diff --git a/xen/common/gcov/gcov.c b/xen/common/gcov/gcov.c > index b5717b9..1d32d8f 100644 > --- a/xen/common/gcov/gcov.c > +++ b/xen/common/gcov/gcov.c > @@ -106,17 +106,120 @@ static int write_string(write_iter_t *iter, const char *s) > > static inline int next_type(const struct gcov_info *info, int *type) > { > - while ( ++*type < XENCOV_COUNTERS && !counter_active(info, *type) ) > + while ( ++*type < XENCOV_COUNTERS_MASK && !counter_active(info, *type) ) > continue; > return *type; > } > > +static inline const struct gcov_fn_info_407 * > +next_func(const struct gcov_info_407 *info, int *n_func) > +{ > + while ( ++*n_func < info->n_functions ) { > + const struct gcov_fn_info_407 *fn = info->functions[*n_func]; > + > + /* the test for info member handle common data redefinitions > + in object files */ > + if ( fn && fn->info == info) > + return fn; > + } > + > + return NULL; > +} > + > +static inline const struct gcov_ctr_info_407 * > +next_ctr(const struct gcov_fn_info_407 *fn, int *n_ctr) > +{ > + while ( ++*n_ctr < XENCOV_COUNTERS_407 ) > + if ( fn->info->merge[*n_ctr] ) > + return &fn->ctrs[*n_ctr]; > + > + return NULL; > +} > + > static inline void align_iter(write_iter_t *iter) > { > iter->write_offset > (iter->write_offset + sizeof(uint64_t) - 1) & -sizeof(uint64_t); > } > > +static int write_info(write_iter_t *iter, const struct gcov_info* info) > +{ > + const struct gcov_ctr_info *ctr; > + int type, ret; > + size_t size_fn = sizeof(struct gcov_fn_info); > + > + /* dump counters */ > + ctr = info->counts; > + for ( type = -1; next_type(info, &type) < XENCOV_COUNTERS_MASK; ++ctr ) > + { > + align_iter(iter); > + chk(write32(iter, XENCOV_TAG_COUNTER(type))); > + chk(write32(iter, ctr->num)); > + chk(write_raw(iter, ctr->values, > + ctr->num * sizeof(ctr->values[0]))); > + > + size_fn += sizeof(unsigned); > + } > + > + /* dump all functions together */ > + align_iter(iter); > + chk(write32(iter, XENCOV_TAG_FUNC)); > + chk(write32(iter, info->n_functions)); > + chk(write_raw(iter, info->functions, info->n_functions * size_fn)); > + > + return 0; > +} > + > +static int write_info_407(write_iter_t *iter, const struct gcov_info_407* info) > +{ > + int ret; > + const struct gcov_fn_info_407 *fn; > + const struct gcov_ctr_info_407 *ctr; > + int n_func, n_ctr; > + unsigned int num_func = 0; > + unsigned int ctrs[XENCOV_COUNTERS_407]; > + > + for ( n_ctr = 0; n_ctr < XENCOV_COUNTERS_407; ++n_ctr ) > + ctrs[n_ctr] = 0; > + > + /* scan to total counters */ > + for ( n_func = -1; (fn = next_func(info, &n_func)) != NULL; ) > + { > + ++num_func; > + for ( n_ctr = -1; (ctr = next_ctr(fn, &n_ctr)) != NULL; ) > + ctrs[n_ctr] += ctr->num; > + } > + > + /* output counters */ > + for ( n_ctr = 0; n_ctr < XENCOV_COUNTERS_407; ++n_ctr ) > + { > + if ( !ctrs[n_ctr] ) continue; > + align_iter(iter); > + chk(write32(iter, XENCOV_TAG_COUNTER(n_ctr))); > + chk(write32(iter, ctrs[n_ctr])); > + for ( n_func = -1; (fn = next_func(info, &n_func)) != NULL; ) > + { > + ctr = &fn->ctrs[n_ctr]; > + chk(write_raw(iter, ctr->values, > + ctr->num * sizeof(ctr->values[0]))); > + } > + } > + > + /* dump all functions together */ > + align_iter(iter); > + chk(write32(iter, XENCOV_TAG_FUNC2)); > + chk(write32(iter, num_func)); > + for ( n_func = -1; (fn = next_func(info, &n_func)) != NULL; ) > + { > + chk(write32(iter, fn->ident)); > + chk(write32(iter, fn->lineno_checksum)); > + chk(write32(iter, fn->cfg_checksum)); > + for ( n_ctr = -1; (ctr = next_ctr(fn, &n_ctr)) != NULL; ) > + chk(write32(iter, ctr->num)); > + } > + return 0; > +} > + > static int write_gcov(write_iter_t *iter) > { > struct gcov_info *info; > @@ -128,10 +231,6 @@ static int write_gcov(write_iter_t *iter) > /* dump all files */ > for ( info = info_list ; info; info = info->next ) > { > - const struct gcov_ctr_info *ctr; > - int type; > - size_t size_fn = sizeof(struct gcov_fn_info); > - > align_iter(iter); > chk(write32(iter, XENCOV_TAG_FILE)); > chk(write32(iter, info->version)); > @@ -139,23 +238,10 @@ static int write_gcov(write_iter_t *iter) > chk(write_string(iter, info->filename)); > > /* dump counters */ > - ctr = info->counts; > - for ( type = -1; next_type(info, &type) < XENCOV_COUNTERS; ++ctr ) > - { > - align_iter(iter); > - chk(write32(iter, XENCOV_TAG_COUNTER(type))); > - chk(write32(iter, ctr->num)); > - chk(write_raw(iter, ctr->values, > - ctr->num * sizeof(ctr->values[0]))); > - > - size_fn += sizeof(unsigned); > - } > - > - /* dump all functions together */ > - align_iter(iter); > - chk(write32(iter, XENCOV_TAG_FUNC)); > - chk(write32(iter, info->n_functions)); > - chk(write_raw(iter, info->functions, info->n_functions * size_fn)); > + if (info->version < XENCOV_VERSION_407) > + chk(write_info(iter, info)); > + else > + chk(write_info_407(iter, (struct gcov_info_407 *) info)); > } > > /* stop tag */ > @@ -164,19 +250,38 @@ static int write_gcov(write_iter_t *iter) > return 0; > } > > +static void reset_info(struct gcov_info *info) > +{ > + const struct gcov_ctr_info *ctr; > + int type; > + > + ctr = info->counts; > + for ( type = -1; next_type(info, &type) < XENCOV_COUNTERS_MASK; ++ctr ) > + memset(ctr->values, 0, ctr->num * sizeof(ctr->values[0])); > +} > + > +static void reset_info_407(struct gcov_info_407 *info) > +{ > + const struct gcov_fn_info_407 *fn; > + const struct gcov_ctr_info_407 *ctr; > + int n_func, n_ctr; > + > + for ( n_func = -1; (fn = next_func(info, &n_func)) != NULL; ) > + for ( n_ctr = -1; (ctr = next_ctr(fn, &n_ctr)) != NULL; ) > + memset(ctr->values, 0, ctr->num * sizeof(ctr->values[0])); > +} > + > static int reset_counters(void) > { > struct gcov_info *info; > > for ( info = info_list ; info; info = info->next ) > { > - const struct gcov_ctr_info *ctr; > - int type; > - > /* reset counters */ > - ctr = info->counts; > - for ( type = -1; next_type(info, &type) < XENCOV_COUNTERS; ++ctr ) > - memset(ctr->values, 0, ctr->num * sizeof(ctr->values[0])); > + if ( info->version < XENCOV_VERSION_407 ) > + reset_info(info); > + else > + reset_info_407((struct gcov_info_407 *) info); > } > > return 0; > diff --git a/xen/include/public/gcov.h b/xen/include/public/gcov.h > index 1b29b48..e7573fb 100644 > --- a/xen/include/public/gcov.h > +++ b/xen/include/public/gcov.h > @@ -28,10 +28,12 @@ > #ifndef __XEN_PUBLIC_GCOV_H__ > #define __XEN_PUBLIC_GCOV_H__ __XEN_PUBLIC_GCOV_H__ > > -#define XENCOV_COUNTERS 5 > +#define XENCOV_COUNTERS_MASK 5 > +#define XENCOV_COUNTERS 8 > #define XENCOV_TAG_BASE 0x58544300u > #define XENCOV_TAG_FILE (XENCOV_TAG_BASE+0x46u) > #define XENCOV_TAG_FUNC (XENCOV_TAG_BASE+0x66u) > +#define XENCOV_TAG_FUNC2 (XENCOV_TAG_BASE+0x67u) > #define XENCOV_TAG_COUNTER(n) (XENCOV_TAG_BASE+0x30u+((n)&0xfu)) > #define XENCOV_TAG_END (XENCOV_TAG_BASE+0x2eu) > #define XENCOV_IS_TAG_COUNTER(n) \ > @@ -93,6 +95,18 @@ struct xencov_function > }; > > /** > + * Information for each function > + * Number of counter is equal to the number of counter structures got before > + */ > +struct xencov_function2 > +{ > + uint32_t ident; > + uint32_t lineno_checksum; > + uint32_t cfg_checksum; > + uint32_t num_counters[1]; > +}; > + > +/** > * Information for all functions > * Aligned to 8 bytes > */ > diff --git a/xen/include/xen/gcov.h b/xen/include/xen/gcov.h > index 27c5c37..f36388a 100644 > --- a/xen/include/xen/gcov.h > +++ b/xen/include/xen/gcov.h > @@ -40,6 +40,8 @@ struct gcov_fn_info > unsigned int n_ctrs[0]; > }; > > +typedef void (*gcov_merge_func)(gcov_type *, unsigned int); > + > /** > * struct gcov_ctr_info - profiling data per counter type > * @num: number of counter values for this type > @@ -53,7 +55,7 @@ struct gcov_ctr_info > { > unsigned int num; > gcov_type *values; > - void (*merge)(gcov_type *, unsigned int); > + gcov_merge_func merge; > }; > > /** > @@ -82,6 +84,70 @@ struct gcov_info > struct gcov_ctr_info counts[0]; > }; > > +struct gcov_info_407; > + > +/** > + * struct gcov_ctr_info_407 - 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_407 > +{ > + unsigned int num; > + gcov_type *values; > +}; > + > + > +/** > + * struct gcov_fn_info_407 - profiling meta data per function > + * @ident: object file-unique function identifier > + * @lineno_checksum: function lineno checksum > + * @cfg_checksum: function cfg checksum > + * @ctrs: counters for this function > + * > + * This data is generated by gcc during compilation and doesn''t change > + * at run-time. > + */ > +struct gcov_fn_info_407 > +{ > + const struct gcov_info_407 *info; > + unsigned int ident; > + unsigned int lineno_checksum; > + unsigned int cfg_checksum; > + struct gcov_ctr_info_407 ctrs[0]; > +}; > + > +#define XENCOV_COUNTERS_407 8 > +#define XENCOV_VERSION_407 0x34303700 > + > +/** > + * struct gcov_info_407 - 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 > + * @merge: merge functions > + * @n_functions: number of instrumented functions > + * @functions: function data > + * > + * 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_407 > +{ > + unsigned int version; > + struct gcov_info *next; > + unsigned int stamp; > + const char *filename; > + gcov_merge_func merge[XENCOV_COUNTERS_407]; > + unsigned int n_functions; > + const struct gcov_fn_info_407 * const *functions; > +}; > + > > /** > * Sysctl operations for coverage
On Mon, 2013-06-17 at 09:29 +0100, Frediano Ziglio wrote:> gcc 4.7 changed format used internally for coverage data. > This patch address these changes. > The information that gcc generate are mostly the same but to support common > sections. > The only difference in the blob exported by Xen is that functions have 2 > different checksums instead of one.Do we really need to reflect that in the Xen public API though? Could we condense it into the things we want/need and try and keep our API relatively stable? I''m a bit concerned that we are going to end up with dozens of variants of the API (and the internal code) supporting each new variant of this internal-to-gcc ABI. I think you explained before why this couldn''t be exported from the hypervisor as an opaque set of bytes to be interpreted by a userspace tool, which is a shame. I wonder if it is worth asking upstream if they have any advice. Ian.
On Mon, 2013-06-17 at 11:42 +0100, Ian Campbell wrote:> On Mon, 2013-06-17 at 09:29 +0100, Frediano Ziglio wrote: > > gcc 4.7 changed format used internally for coverage data. > > This patch address these changes. > > The information that gcc generate are mostly the same but to support common > > sections. > > The only difference in the blob exported by Xen is that functions have 2 > > different checksums instead of one. > > Do we really need to reflect that in the Xen public API though? Could we > condense it into the things we want/need and try and keep our API > relatively stable? > > I''m a bit concerned that we are going to end up with dozens of variants > of the API (and the internal code) supporting each new variant of this > internal-to-gcc ABI. > > I think you explained before why this couldn''t be exported from the > hypervisor as an opaque set of bytes to be interpreted by a userspace > tool, which is a shame. I wonder if it is worth asking upstream if they > have any advice. > > Ian. > >Perhaps a simple easy solution would be to allow privileged domains to read/write arbitrary memory from Xen (write to reset counters). Frediano
On 17.06.13 12:42, Ian Campbell wrote:> On Mon, 2013-06-17 at 09:29 +0100, Frediano Ziglio wrote: >> gcc 4.7 changed format used internally for coverage data. >> This patch address these changes. >> The information that gcc generate are mostly the same but to support common >> sections. >> The only difference in the blob exported by Xen is that functions have 2 >> different checksums instead of one. > > Do we really need to reflect that in the Xen public API though? Could we > condense it into the things we want/need and try and keep our API > relatively stable? > > I''m a bit concerned that we are going to end up with dozens of variants > of the API (and the internal code) supporting each new variant of this > internal-to-gcc ABI. > > I think you explained before why this couldn''t be exported from the > hypervisor as an opaque set of bytes to be interpreted by a userspace > tool, which is a shame.I think this is possible the same way the hypervisor exports machine check error telemetry. Christoph
On Mon, 2013-06-17 at 11:46 +0100, Frediano Ziglio wrote:> Perhaps a simple easy solution would be to allow privileged domains to > read/write arbitrary memory from Xen (write to reset counters)."Arbitrary memory" would be a very hard sell. If all these counters could be constrained to pages which contains these counters and nothing else then that might be a possibility to consider. Ian.
On Mon, 2013-06-17 at 11:49 +0100, Ian Campbell wrote:> On Mon, 2013-06-17 at 11:46 +0100, Frediano Ziglio wrote: > > Perhaps a simple easy solution would be to allow privileged domains to > > read/write arbitrary memory from Xen (write to reset counters). > > "Arbitrary memory" would be a very hard sell. > > If all these counters could be constrained to pages which contains these > counters and nothing else then that might be a possibility to consider. > > Ian. >It would be very good if gcc could provide some options to move all counter in a given separate section and another for all coverage data (structures and strings). Actually there are no such options. I tried to change generated .s file but with no success (I don''t know how to generate a common section in a new section, .comm/.lcomm generate always in .bss section). I''ll write to gcc ML. Frediano
On Mon, 2013-06-17 at 11:49 +0100, Ian Campbell wrote:> On Mon, 2013-06-17 at 11:46 +0100, Frediano Ziglio wrote: > > Perhaps a simple easy solution would be to allow privileged domains to > > read/write arbitrary memory from Xen (write to reset counters). > > "Arbitrary memory" would be a very hard sell. > > If all these counters could be constrained to pages which contains these > counters and nothing else then that might be a possibility to consider. > > Ian. >It depends on how this would be implemented. A future option of gcc could help at least having everything in contiguous memory regions. The fact that these information are generated by the compiler does not help that much. Every version could change the format "slightly" but enough to make Xen core when such information are requested! Actually they changed format for these version of gcc: - 3.3 - 3.4 - 4.7 And honestly I still don''t understand clearly why they changed for gcc 4.7! Probably not for a C feature but to support C++ templates and comdat. A "smart" preprocessor sit in the middle of gcc and gas that process assembly generated, check for structures and put everything in different sections could really help... but is not that easy project! Another similar way would be to parse the xen-syms output file and generate needed blob/code. Quite interesting challenge (I did more terrible things then this) but I don''t thing that Citrix would agree. Possibly a safe thing to do would be in the code to check that compiler is really gcc and that we can support the version that is compiling, just to make sure. But is also true that this feature is a debug one disabled on production so I don''t know how worth would be. Not speaking of clang support, currently quite hard (the structures are less documented, the code hooks destructors path which is not safe to call from Xen code which is not expected to terminate). Frediano
On 06/18/2013 03:32 PM, Frediano Ziglio wrote:> On Mon, 2013-06-17 at 11:49 +0100, Ian Campbell wrote: >> On Mon, 2013-06-17 at 11:46 +0100, Frediano Ziglio wrote: >>> Perhaps a simple easy solution would be to allow privileged domains to >>> read/write arbitrary memory from Xen (write to reset counters). >> >> "Arbitrary memory" would be a very hard sell. >> >> If all these counters could be constrained to pages which contains these >> counters and nothing else then that might be a possibility to consider. >> >> Ian. >> > > It depends on how this would be implemented. A future option of gcc > could help at least having everything in contiguous memory regions. > > The fact that these information are generated by the compiler does not > help that much. Every version could change the format "slightly" but > enough to make Xen core when such information are requested! > > Actually they changed format for these version of gcc: > - 3.3 > - 3.4 > - 4.7 > > And honestly I still don''t understand clearly why they changed for gcc > 4.7! Probably not for a C feature but to support C++ templates and > comdat. A "smart" preprocessor sit in the middle of gcc and gas that > process assembly generated, check for structures and put everything in > different sections could really help... but is not that easy project! > Another similar way would be to parse the xen-syms output file and > generate needed blob/code. Quite interesting challenge (I did more > terrible things then this) but I don''t thing that Citrix would agree. > Possibly a safe thing to do would be in the code to check that compiler > is really gcc and that we can support the version that is compiling, > just to make sure. But is also true that this feature is a debug one > disabled on production so I don''t know how worth would be. > > Not speaking of clang support, currently quite hard (the structures are > less documented, the code hooks destructors path which is not safe to > call from Xen code which is not expected to terminate).Doesn''t Linux have gcov support via lcov? Do they have to deal with this stuff as well, or is there a generic way to deal with it? -George
On Tue, 2013-06-18 at 15:53 +0100, George Dunlap wrote:> On 06/18/2013 03:32 PM, Frediano Ziglio wrote: > > On Mon, 2013-06-17 at 11:49 +0100, Ian Campbell wrote: > >> On Mon, 2013-06-17 at 11:46 +0100, Frediano Ziglio wrote: > >>> Perhaps a simple easy solution would be to allow privileged domains to > >>> read/write arbitrary memory from Xen (write to reset counters). > >> > >> "Arbitrary memory" would be a very hard sell. > >> > >> If all these counters could be constrained to pages which contains these > >> counters and nothing else then that might be a possibility to consider. > >> > >> Ian. > >> > > > > It depends on how this would be implemented. A future option of gcc > > could help at least having everything in contiguous memory regions. > > > > The fact that these information are generated by the compiler does not > > help that much. Every version could change the format "slightly" but > > enough to make Xen core when such information are requested! > > > > Actually they changed format for these version of gcc: > > - 3.3 > > - 3.4 > > - 4.7 > > > > And honestly I still don''t understand clearly why they changed for gcc > > 4.7! Probably not for a C feature but to support C++ templates and > > comdat. A "smart" preprocessor sit in the middle of gcc and gas that > > process assembly generated, check for structures and put everything in > > different sections could really help... but is not that easy project! > > Another similar way would be to parse the xen-syms output file and > > generate needed blob/code. Quite interesting challenge (I did more > > terrible things then this) but I don''t thing that Citrix would agree. > > Possibly a safe thing to do would be in the code to check that compiler > > is really gcc and that we can support the version that is compiling, > > just to make sure. But is also true that this feature is a debug one > > disabled on production so I don''t know how worth would be. > > > > Not speaking of clang support, currently quite hard (the structures are > > less documented, the code hooks destructors path which is not safe to > > call from Xen code which is not expected to terminate). > > Doesn''t Linux have gcov support via lcov? Do they have to deal with > this stuff as well, or is there a generic way to deal with it? > > -GeorgeThey only support gcc 3.4 format (the one we currently support without my new patch which add support for 4.7 format) Frediano
On Mon, 2013-06-17 at 09:58 +0100, Jan Beulich wrote:> >>> On 17.06.13 at 10:29, Frediano Ziglio <frediano.ziglio@citrix.com> wrote: > > --- a/xen/arch/x86/xen.lds.S > > +++ b/xen/arch/x86/xen.lds.S > > @@ -112,6 +112,7 @@ SECTIONS > > . = ALIGN(8); > > __ctors_start = .; > > *(.ctors) > > + *(.init_array) > > __ctors_end = .; > > } :text > > . = ALIGN(32); > > First of all - again an x86 change without a matching ARM one? > > And then this is different from how binutils deals with these in a > number of aspects: > - .init_array.* not handled (raises the question why .ctors.* didn''t > get handled already) > - .init_array following .ctors here, while binutils has it the other > way around > - final section being .init_array instead of .ctors > - enclosing symbols prefixed with __init_array_ instead of __ctors_ > > Some or all of these may not matter, but I think any difference to > how binutils deals with these sections needs to be explained in the > patch description. >I don''t know why gcc changed from .ctors to .init_array, I found some reference for some architectural coherency (it seems other architectures use init_array, for instance arm already used init_array, the structure is the same). I''m looking at binutils scripts: ... .init_array : { PROVIDE_HIDDEN (__init_array_start = .); KEEP (*(SORT_BY_INIT_PRIORITY(.init_array.*) SORT_BY_INIT_PRIORITY(.ctors.*))) KEEP (*(.init_array)) KEEP (*(EXCLUDE_FILE (*crtbegin.o *crtbegin?.o *crtend.o *crtend?.o ) .ctors)) PROVIDE_HIDDEN (__init_array_end = .); } ... .ctors : { /* gcc uses crtbegin.o to find the start of the constructors, so we make sure it is first. Because this is a wildcard, it doesn''t matter if the user does not actually link against crtbegin.o; the linker won''t look for a file to match a wildcard. The wildcard also means that it doesn''t matter which directory crtbegin.o is in. */ KEEP (*crtbegin.o(.ctors)) KEEP (*crtbegin?.o(.ctors)) /* We don''t want to include the .ctor section from the crtend.o file until after the sorted ctors. The .ctor section from the crtend file contains the end of ctors marker and it must be last */ KEEP (*(EXCLUDE_FILE (*crtend.o *crtend?.o ) .ctors)) KEEP (*(SORT(.ctors.*))) KEEP (*(.ctors)) } .. quite complicated. Can you read it ?> > +static inline const struct gcov_fn_info_407 * > > +next_func(const struct gcov_info_407 *info, int *n_func) > > What are these "_407" suffixes intended to mean? They look rather > arbitrary, and to me aren''t connected to the gcc version talked > about here. >Do you prefer 4_7, perhaps is more readable.> > +{ > > + while ( ++*n_func < info->n_functions ) { > > Coding style. > > > + const struct gcov_fn_info_407 *fn = info->functions[*n_func]; > > + > > + /* the test for info member handle common data redefinitions > > + in object files */ > > Again. Stopping here; there are more. >Fixed, found some others.> > --- a/xen/include/public/gcov.h > > +++ b/xen/include/public/gcov.h > > @@ -28,10 +28,12 @@ > > #ifndef __XEN_PUBLIC_GCOV_H__ > > #define __XEN_PUBLIC_GCOV_H__ __XEN_PUBLIC_GCOV_H__ > > > > -#define XENCOV_COUNTERS 5 > > +#define XENCOV_COUNTERS_MASK 5 > > Misnamed manifest constant? It''s never being used as a mask and > also doesn''t look like one. >Possibly is better to use XENCOV_COUNTERS_3_4 then. I used _MASK cause 3.4 used a strange mask field.> > +#define XENCOV_COUNTERS 8 > > And this one doesn''t appear to get used anywhere, so why are > these changes being done in the first place? >It''s used in the macro some lines below.> > #define XENCOV_TAG_BASE 0x58544300u > > #define XENCOV_TAG_FILE (XENCOV_TAG_BASE+0x46u) > > #define XENCOV_TAG_FUNC (XENCOV_TAG_BASE+0x66u) > > +#define XENCOV_TAG_FUNC2 (XENCOV_TAG_BASE+0x67u) > > #define XENCOV_TAG_COUNTER(n) (XENCOV_TAG_BASE+0x30u+((n)&0xfu)) > > #define XENCOV_TAG_END (XENCOV_TAG_BASE+0x2eu) > > #define XENCOV_IS_TAG_COUNTER(n) \ > > @@ -93,6 +95,18 @@ struct xencov_function > > }; > > > > /** > > + * Information for each function > > + * Number of counter is equal to the number of counter structures got before > > + */ > > +struct xencov_function2 > > +{ > > + uint32_t ident; > > + uint32_t lineno_checksum; > > + uint32_t cfg_checksum; > > + uint32_t num_counters[1]; > > +}; > > Nor can I seem to be able to spot a use of this structure. > > Jan >Yes, cause actually there is no C program that use these information (perl is used) Frediano
>>> On 28.06.13 at 12:18, Frediano Ziglio <frediano.ziglio@citrix.com> wrote: > On Mon, 2013-06-17 at 09:58 +0100, Jan Beulich wrote: >> >>> On 17.06.13 at 10:29, Frediano Ziglio <frediano.ziglio@citrix.com> wrote: >> > --- a/xen/arch/x86/xen.lds.S >> > +++ b/xen/arch/x86/xen.lds.S >> > @@ -112,6 +112,7 @@ SECTIONS >> > . = ALIGN(8); >> > __ctors_start = .; >> > *(.ctors) >> > + *(.init_array) >> > __ctors_end = .; >> > } :text >> > . = ALIGN(32); >> >> First of all - again an x86 change without a matching ARM one? >> >> And then this is different from how binutils deals with these in a >> number of aspects: >> - .init_array.* not handled (raises the question why .ctors.* didn''t >> get handled already) >> - .init_array following .ctors here, while binutils has it the other >> way around >> - final section being .init_array instead of .ctors >> - enclosing symbols prefixed with __init_array_ instead of __ctors_ >> >> Some or all of these may not matter, but I think any difference to >> how binutils deals with these sections needs to be explained in the >> patch description. >> > > I don''t know why gcc changed from .ctors to .init_array, I found some > reference for some architectural coherency (it seems other architectures > use init_array, for instance arm already used init_array, the structure > is the same). > > I''m looking at binutils scripts: > > ... > .init_array : > { > PROVIDE_HIDDEN (__init_array_start = .); > KEEP (*(SORT_BY_INIT_PRIORITY(.init_array.*) SORT_BY_INIT_PRIORITY(.ctors.*))) > KEEP (*(.init_array)) > KEEP (*(EXCLUDE_FILE (*crtbegin.o *crtbegin?.o *crtend.o *crtend?.o ) .ctors)) > PROVIDE_HIDDEN (__init_array_end = .); > } > ... > .ctors : > { > /* gcc uses crtbegin.o to find the start of > the constructors, so we make sure it is > first. Because this is a wildcard, it > doesn''t matter if the user does not > actually link against crtbegin.o; the > linker won''t look for a file to match a > wildcard. The wildcard also means that it > doesn''t matter which directory crtbegin.o > is in. */ > KEEP (*crtbegin.o(.ctors)) > KEEP (*crtbegin?.o(.ctors)) > /* We don''t want to include the .ctor section from > the crtend.o file until after the sorted ctors. > The .ctor section from the crtend file contains the > end of ctors marker and it must be last */ > KEEP (*(EXCLUDE_FILE (*crtend.o *crtend?.o ) .ctors)) > KEEP (*(SORT(.ctors.*))) > KEEP (*(.ctors)) > } > .. > > quite complicated. Can you read it ?It''s not _that_ difficult, but yes, this is what I looked at when putting together the earlier response.>> > +static inline const struct gcov_fn_info_407 * >> > +next_func(const struct gcov_info_407 *info, int *n_func) >> >> What are these "_407" suffixes intended to mean? They look rather >> arbitrary, and to me aren''t connected to the gcc version talked >> about here. >> > > Do you prefer 4_7, perhaps is more readable.Unless 4.7 is the gcov version number, please call it gcc_4_7 or gcc4_7, so that it''s clear the version number of what you''re referring to.>> > --- a/xen/include/public/gcov.h >> > +++ b/xen/include/public/gcov.h >> > @@ -28,10 +28,12 @@ >> > #ifndef __XEN_PUBLIC_GCOV_H__ >> > #define __XEN_PUBLIC_GCOV_H__ __XEN_PUBLIC_GCOV_H__ >> > >> > -#define XENCOV_COUNTERS 5 >> > +#define XENCOV_COUNTERS_MASK 5 >> >> Misnamed manifest constant? It''s never being used as a mask and >> also doesn''t look like one. >> > > Possibly is better to use XENCOV_COUNTERS_3_4 then. I used _MASK cause > 3.4 used a strange mask field.Same here - if this is a gcc version, include gcc in the name.>> > + * Information for each function >> > + * Number of counter is equal to the number of counter structures got before >> > + */ >> > +struct xencov_function2 >> > +{ >> > + uint32_t ident; >> > + uint32_t lineno_checksum; >> > + uint32_t cfg_checksum; >> > + uint32_t num_counters[1]; >> > +}; >> >> Nor can I seem to be able to spot a use of this structure. > > Yes, cause actually there is no C program that use these information > (perl is used)Okay, so it''s here then just for documentation purposes. That should be said in the preceding comment, or else you risk it getting removed by some cleanup patch. Even better would be to end the comment only after the structure declaration, to avoid cluttering the name space. Jan