On Fri, Oct 1, 2021 at 11:55 AM Martin Liška <mli...@suse.cz> wrote:
>
> Support merging of profiles that are built from a different .o files
> but belong to the same source file. Moreover, a checksum is verified
> during profile merging and so we can safely combine such profile.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> I'm going to install the patch if there are no comments about it.
Is the ->stamp field now unused?
I wonder whether crc32 is good enough or whether we need to enable
-fprofile-correction by default for example? But -fprofile-correction
is about -fprofile-use, not profile merging, right? What does the latter
do upon mismatch?
Alternatively would it be possible to keep multiple sets of data in the file,
one for each 'stamp'? How does the profile-use step figure a mismatch
here, or does it not care whether it mixes input with 'different stamp'?
The current behavior is also somewhat odd:
> ./a.out
libgcov profiling error:/tmp/t.gcda:overwriting an existing profile
data with a different timestamp
but it should probably say 'warning' since it indeed simply overwrites data
instead of merging it.
I wonder if we can simply drop the stamp check alltogether and make
the checks that match up the data against the number of counters behave
as to warning and overwriting the data instead of failing fatally? The
actual merging of data would need to be delayed, but at least until
the first actual merge was done we could simply proceed?
Richard.
> Thanks,
> Martin
>
> PR gcov-profile/90364
>
> gcc/ChangeLog:
>
> * coverage.c (build_info): Emit checksum to the global variable.
> (build_info_type): Add new field for checksum.
> (coverage_obj_finish): Pass object_checksum.
> (coverage_init): Use 0 as checksum for .gcno files.
> * gcov-dump.c (dump_gcov_file): Dump also new checksum field.
> * gcov.c (read_graph_file): Read also checksum.
>
> libgcc/ChangeLog:
>
> * libgcov-driver.c (merge_one_data): Skip timestamp and verify
> checksums.
> (write_one_data): Write also checksum.
> * libgcov-util.c (read_gcda_file): Read also checksum field.
> * libgcov.h (struct gcov_info): Add new field.
> ---
> gcc/coverage.c | 50 ++++++++++++++++++++++++++++-------------
> gcc/gcov-dump.c | 9 ++++----
> gcc/gcov.c | 5 +++++
> libgcc/libgcov-driver.c | 8 +++++--
> libgcc/libgcov-util.c | 3 +++
> libgcc/libgcov.h | 1 +
> 6 files changed, 54 insertions(+), 22 deletions(-)
>
> diff --git a/gcc/coverage.c b/gcc/coverage.c
> index 10d7f8366cb..4467f1eaa5c 100644
> --- a/gcc/coverage.c
> +++ b/gcc/coverage.c
> @@ -129,16 +129,7 @@ static const char *const ctr_names[GCOV_COUNTERS] = {
> #undef DEF_GCOV_COUNTER
>
> /* Forward declarations. */
> -static void read_counts_file (void);
> static tree build_var (tree, tree, int);
> -static void build_fn_info_type (tree, unsigned, tree);
> -static void build_info_type (tree, tree);
> -static tree build_fn_info (const struct coverage_data *, tree, tree);
> -static tree build_info (tree, tree);
> -static bool coverage_obj_init (void);
> -static vec<constructor_elt, va_gc> *coverage_obj_fn
> -(vec<constructor_elt, va_gc> *, tree, struct coverage_data const *);
> -static void coverage_obj_finish (vec<constructor_elt, va_gc> *);
>
> /* Return the type node for gcov_type. */
>
> @@ -218,6 +209,9 @@ read_counts_file (void)
> tag = gcov_read_unsigned ();
> bbg_file_stamp = crc32_unsigned (bbg_file_stamp, tag);
>
> + /* Read checksum. */
> + gcov_read_unsigned ();
> +
> counts_hash = new hash_table<counts_entry> (10);
> while ((tag = gcov_read_unsigned ()))
> {
> @@ -935,6 +929,12 @@ build_info_type (tree type, tree fn_info_ptr_type)
> DECL_CHAIN (field) = fields;
> fields = field;
>
> + /* Checksum. */
> + field = build_decl (BUILTINS_LOCATION, FIELD_DECL, NULL_TREE,
> + get_gcov_unsigned_t ());
> + DECL_CHAIN (field) = fields;
> + fields = field;
> +
> /* Filename */
> field = build_decl (BUILTINS_LOCATION, FIELD_DECL, NULL_TREE,
> build_pointer_type (build_qualified_type
> @@ -977,7 +977,7 @@ build_info_type (tree type, tree fn_info_ptr_type)
> function info objects. */
>
> static tree
> -build_info (tree info_type, tree fn_ary)
> +build_info (tree info_type, tree fn_ary, unsigned object_checksum)
> {
> tree info_fields = TYPE_FIELDS (info_type);
> tree merge_fn_type, n_funcs;
> @@ -996,13 +996,19 @@ build_info (tree info_type, tree fn_ary)
> /* next -- NULL */
> CONSTRUCTOR_APPEND_ELT (v1, info_fields, null_pointer_node);
> info_fields = DECL_CHAIN (info_fields);
> -
> +
> /* stamp */
> CONSTRUCTOR_APPEND_ELT (v1, info_fields,
> build_int_cstu (TREE_TYPE (info_fields),
> bbg_file_stamp));
> info_fields = DECL_CHAIN (info_fields);
>
> + /* Checksum. */
> + CONSTRUCTOR_APPEND_ELT (v1, info_fields,
> + build_int_cstu (TREE_TYPE (info_fields),
> + object_checksum));
> + info_fields = DECL_CHAIN (info_fields);
> +
> /* Filename */
> da_file_name_len = strlen (da_file_name);
> filename_string = build_string (da_file_name_len + 1, da_file_name);
> @@ -1214,7 +1220,8 @@ coverage_obj_fn (vec<constructor_elt, va_gc> *ctor,
> tree fn,
> function objects from CTOR. Generate the gcov_info initializer. */
>
> static void
> -coverage_obj_finish (vec<constructor_elt, va_gc> *ctor)
> +coverage_obj_finish (vec<constructor_elt, va_gc> *ctor,
> + unsigned object_checksum)
> {
> unsigned n_functions = vec_safe_length (ctor);
> tree fn_info_ary_type = build_array_type
> @@ -1231,7 +1238,7 @@ coverage_obj_finish (vec<constructor_elt, va_gc> *ctor)
> varpool_node::finalize_decl (fn_info_ary);
>
> DECL_INITIAL (gcov_info_var)
> - = build_info (TREE_TYPE (gcov_info_var), fn_info_ary);
> + = build_info (TREE_TYPE (gcov_info_var), fn_info_ary, object_checksum);
> varpool_node::finalize_decl (gcov_info_var);
> }
>
> @@ -1300,7 +1307,6 @@ coverage_init (const char *filename)
> strcpy (da_file_name + prefix_len + len, GCOV_DATA_SUFFIX);
>
> bbg_file_stamp = local_tick;
> -
> if (flag_auto_profile)
> read_autofdo_file ();
> else if (flag_branch_probabilities)
> @@ -1328,6 +1334,8 @@ coverage_init (const char *filename)
> gcov_write_unsigned (GCOV_NOTE_MAGIC);
> gcov_write_unsigned (GCOV_VERSION);
> gcov_write_unsigned (bbg_file_stamp);
> + /* Use an arbitrary checksum */
> + gcov_write_unsigned (0);
> gcov_write_string (getpwd ());
>
> /* Do not support has_unexecuted_blocks for Ada. */
> @@ -1353,14 +1361,24 @@ coverage_finish (void)
> cannot uniquely stamp it. If we can stamp it, libgcov will DTRT. */
> unlink (da_file_name);
>
> + /* Global GCDA checksum that aggregates all functions. */
> + unsigned object_checksum = 0;
> +
> if (coverage_obj_init ())
> {
> vec<constructor_elt, va_gc> *fn_ctor = NULL;
> struct coverage_data *fn;
>
> for (fn = functions_head; fn; fn = fn->next)
> - fn_ctor = coverage_obj_fn (fn_ctor, fn->fn_decl, fn);
> - coverage_obj_finish (fn_ctor);
> + {
> + fn_ctor = coverage_obj_fn (fn_ctor, fn->fn_decl, fn);
> +
> + object_checksum = crc32_unsigned (object_checksum, fn->ident);
> + object_checksum = crc32_unsigned (object_checksum,
> + fn->lineno_checksum);
> + object_checksum = crc32_unsigned (object_checksum,
> fn->cfg_checksum);
> + }
> + coverage_obj_finish (fn_ctor, object_checksum);
> }
>
> XDELETEVEC (da_file_name);
> diff --git a/gcc/gcov-dump.c b/gcc/gcov-dump.c
> index f1489fe0214..bfaf735d2ff 100644
> --- a/gcc/gcov-dump.c
> +++ b/gcc/gcov-dump.c
> @@ -206,11 +206,12 @@ dump_gcov_file (const char *filename)
> }
>
> /* stamp */
> - {
> - unsigned stamp = gcov_read_unsigned ();
> + unsigned stamp = gcov_read_unsigned ();
> + printf ("%s:stamp %lu\n", filename, (unsigned long)stamp);
>
> - printf ("%s:stamp %lu\n", filename, (unsigned long)stamp);
> - }
> + /* Checksum */
> + unsigned checksum = gcov_read_unsigned ();
> + printf ("%s:checksum %lu\n", filename, (unsigned long)checksum);
>
> if (!is_data_type)
> {
> diff --git a/gcc/gcov.c b/gcc/gcov.c
> index cf0a49d8c30..829e955a63b 100644
> --- a/gcc/gcov.c
> +++ b/gcc/gcov.c
> @@ -1814,6 +1814,8 @@ read_graph_file (void)
> bbg_file_name, v, e);
> }
> bbg_stamp = gcov_read_unsigned ();
> + /* Read checksum. */
> + gcov_read_unsigned ();
> bbg_cwd = xstrdup (gcov_read_string ());
> bbg_supports_has_unexecuted_blocks = gcov_read_unsigned ();
>
> @@ -2031,6 +2033,9 @@ read_count_file (void)
> goto cleanup;
> }
>
> + /* Read checksum. */
> + gcov_read_unsigned ();
> +
> while ((tag = gcov_read_unsigned ()))
> {
> unsigned length = gcov_read_unsigned ();
> diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
> index 087f71e0107..7aa97bbb06a 100644
> --- a/libgcc/libgcov-driver.c
> +++ b/libgcc/libgcov-driver.c
> @@ -260,12 +260,15 @@ merge_one_data (const char *filename,
> if (!gcov_version (gi_ptr, length, filename))
> return -1;
>
> + /* Skip timestamp. */
> + gcov_read_unsigned ();
> +
> length = gcov_read_unsigned ();
> - if (length != gi_ptr->stamp)
> + if (length != gi_ptr->checksum)
> {
> /* Read from a different compilation. Overwrite the file. */
> gcov_error (GCOV_PROF_PREFIX "overwriting an existing profile data "
> - "with a different timestamp\n", filename);
> + "with a different checksum\n", filename);
> return 0;
> }
>
> @@ -495,6 +498,7 @@ write_one_data (const struct gcov_info *gi_ptr,
> dump_unsigned (GCOV_DATA_MAGIC, dump_fn, arg);
> dump_unsigned (GCOV_VERSION, dump_fn, arg);
> dump_unsigned (gi_ptr->stamp, dump_fn, arg);
> + dump_unsigned (gi_ptr->checksum, dump_fn, arg);
>
> #ifdef NEED_L_GCOV
> /* Generate whole program statistics. */
> diff --git a/libgcc/libgcov-util.c b/libgcc/libgcov-util.c
> index bc7416d99bf..766ca3559c4 100644
> --- a/libgcc/libgcov-util.c
> +++ b/libgcc/libgcov-util.c
> @@ -310,6 +310,9 @@ read_gcda_file (const char *filename)
> /* Read stamp. */
> obj_info->stamp = gcov_read_unsigned ();
>
> + /* Read checksum. */
> + obj_info->checksum = gcov_read_unsigned ();
> +
> while (1)
> {
> gcov_position_t base;
> diff --git a/libgcc/libgcov.h b/libgcc/libgcov.h
> index f6354a7a070..2a365c95759 100644
> --- a/libgcc/libgcov.h
> +++ b/libgcc/libgcov.h
> @@ -230,6 +230,7 @@ struct gcov_info
> struct gcov_info *next; /* link to next, used by libgcov */
>
> gcov_unsigned_t stamp; /* uniquifying time stamp */
> + gcov_unsigned_t checksum; /* unique object checksum */
> const char *filename; /* output file name */
>
> gcov_merge_fn merge[GCOV_COUNTERS]; /* merge functions (null for
> --
> 2.33.0
>
