Re: [Xen-devel] [PATCH 2/4] Adding support for coverage information

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.02.13 at 15:48, Frediano Ziglio <frediano.ziglio@xxxxxxxxxx> wrote:
> --- a/Config.mk
> +++ b/Config.mk
> @@ -228,3 +228,7 @@ QEMU_TAG ?= 2a1354d655d816feaad7dbdb8364f40a208439c1
>  # doing and are prepared for some pain.
>  
>  CONFIG_TESTS       ?= y
> +
> +# Test coverage support
> +coverage ?= n
> +

Alongside debug and debug_symbols please.

> --- a/xen/Rules.mk
> +++ b/xen/Rules.mk
> @@ -103,6 +103,11 @@ subdir-all := $(subdir-y) $(subdir-n)
>  
>  $(filter %.init.o,$(obj-y) $(obj-bin-y)): CFLAGS += -DINIT_SECTIONS_ONLY
>  
> +

Stray blank line.

> +ifeq ($(coverage),y)
> +$(filter-out %.init.o,$(obj-y) $(obj-bin-y)): CFLAGS += -fprofile-arcs 
> -ftest-coverage -DTEST_COVERAGE
> +endif

For one - isn't simply using $(obj-y) here sufficient (i.e. without the
$(filter-out ...))?

And second, I would thing this then ought to become a single line:

$(obj-$(coverage)): CFLAGS += -fprofile-arcs -ftest-coverage -DTEST_COVERAGE

> --- /dev/null
> +++ b/xen/common/gcov/Makefile
> @@ -0,0 +1,5 @@
> +ifneq ($(coverage),y)
> +obj-y += nogcov.o
> +endif
> +obj-$(coverage) += gcov.o
> +

How about

obj-y := nogcov.o
obj-$(coverage) := gcov.o


> +typedef void (*ctor_func_t)(void);
> +extern struct
> +{
> +    unsigned long count;
> +    ctor_func_t funcs[1];
> +} __CTOR_LIST__;

const?

> +
> +void init_coverage(void)

__init

> --- /dev/null
> +++ b/xen/include/public/gcov.h
> @@ -0,0 +1,93 @@
> +/*
> + *  Profiling infrastructure declarations.
> + *
> + *  This file is based on gcc-internal definitions. Data structures are
> + *  defined to be compatible with gcc counterparts. For a better
> + *  understanding, refer to gcc source: gcc/gcov-io.h.
> + *
> + *    Copyright IBM Corp. 2009
> + *    Author(s): Peter Oberparleiter <oberpar@xxxxxxxxxxxxxxxxxx>
> + *
> + *    Uses gcc-internal data definitions.
> + */
> +
> +#ifndef XEN_PUBLIC_GCOV_H
> +#define XEN_PUBLIC_GCOV_H XEN_PUBLIC_GCOV_H
> +
> +/*
> + * Profiling data types used for gcc 3.4 and above - these are defined by
> + * 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)
> +#define GCOV_TAG_FOR_COUNTER(count) \
> +    (GCOV_TAG_COUNTER_BASE + ((unsigned int) (count) << 17))
> +
> +#if BITS_PER_LONG >= 64
> +typedef long gcov_type;
> +#else
> +typedef long long gcov_type;
> +#endif

What's this???

> +/**
> + * 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);
> +};

Pointers, and even more so function ones, can't be part of the
public interface.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel