From patchwork Fri Dec 30 18:16:56 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nathan Sidwell X-Patchwork-Id: 133691 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 11A5AB6F9C for ; Sat, 31 Dec 2011 05:17:24 +1100 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1325873845; h=Comment: DomainKey-Signature:Received:Received:Received:Received:Received: Received:Received:Message-ID:Date:From:User-Agent:MIME-Version: To:Subject:Content-Type:Mailing-List:Precedence:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=Oh1XEDM7DslOIhB2lFR6n0OwxZY=; b=bFpVqhhQXIjuhKp BrCoU5K+sEVzazMqXFj+ajSU6AuXgP7HXbtmuaNlB90nx+Ygh3S+Urcf9K2Y0Aqr Kijci9roQH6oFnbcuvqql9La9iKLBLsWg/kJD9eFmNo2SN2OcUAfv851gwvKVtKO PPUvpHS79ldhqxfvyCy4WUVOYuxQ= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:Received:Received:Received:Message-ID:Date:From:User-Agent:MIME-Version:To:Subject:Content-Type:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=V/u4JhCllamWFsfozTypnW3JfXEB+gCRyFQdiKeRalrMqRQw+qE9NJsJWq+vS8 osTDClTG7605fwwQAUygJGZqS1exVM37Bh0NNBVd7w/kRXO72ODZ8i2a9mSnD2ya JRxVQ9UXVNSFE0stTpwLR+Ir79Q0ibRkBaV/iav+7lq20=; Received: (qmail 13282 invoked by alias); 30 Dec 2011 18:17:19 -0000 Received: (qmail 13273 invoked by uid 22791); 30 Dec 2011 18:17:16 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from mail-ww0-f51.google.com (HELO mail-ww0-f51.google.com) (74.125.82.51) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 30 Dec 2011 18:17:01 +0000 Received: by wgbdr1 with SMTP id dr1so21504498wgb.8 for ; Fri, 30 Dec 2011 10:17:00 -0800 (PST) Received: by 10.227.206.4 with SMTP id fs4mr39539385wbb.21.1325269020211; Fri, 30 Dec 2011 10:17:00 -0800 (PST) Received: by 10.227.206.4 with SMTP id fs4mr39539375wbb.21.1325269020091; Fri, 30 Dec 2011 10:17:00 -0800 (PST) Received: from [192.168.44.102] (5ac3c85d.bb.sky.com. [90.195.200.93]) by mx.google.com with ESMTPS id k5sm94203607wiz.9.2011.12.30.10.16.59 (version=SSLv3 cipher=OTHER); Fri, 30 Dec 2011 10:16:59 -0800 (PST) Message-ID: <4EFE0018.8000106@acm.org> Date: Fri, 30 Dec 2011 18:16:56 +0000 From: Nathan Sidwell User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.24) Gecko/20111108 Lightning/1.0b2 Thunderbird/3.1.16 MIME-Version: 1.0 To: GCC Patches Subject: [gcov] fix program detection X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org I've committed this patch which does 2 things in libgcov: 1) Robustifies the error recovery if malloc fails during buffering coverage data. It also generates more informative error messages. 2) Fixes a long standing bug regarding detecting when an object file is used in multiple programs. Consider a program made of a.o and b.o. If b.c is edited and a new b.o generated, the resulting program needs to be considered as a different program to the original program. Not doing so can result in an erroneous merge error on a.o's coverage data as the program's counts can have changed. Previously we just used the set of object file names to detect a different program, but this example shows that filenames are insufficient (and essentially irrelevant) for this purpose. This patch uses the actual coverage data of the entire program to generate a crc, and verifies the expected coverage counts up front. Notice this example is not contrived -- it's what the usually happens with a make-based build system. built and tested on i686-pc-linux-gnu. nathan 2011-12-30 Nathan Sidwell * libgcov.c (gcov_crc32): Remove global var. (free_fn_data): New function. (buffer_fn_data): Pass in filename, more robust error recovery. (crc32_unsigned): New function. (gcov_exit): More robust detection of new program. More robust error recovery. (__gcov_init): Do not update program's crc here. Index: libgcc/libgcov.c =================================================================== --- libgcc/libgcov.c (revision 182730) +++ libgcc/libgcov.c (working copy) @@ -89,10 +89,6 @@ struct gcov_fn_buffer /* Chain of per-object gcov structures. */ static struct gcov_info *gcov_list; -/* A program checksum allows us to distinguish program data for an - object file included in multiple programs. */ -static gcov_unsigned_t gcov_crc32; - /* Size of the longest file name. */ static size_t gcov_max_filename = 0; @@ -143,22 +139,41 @@ create_file_directory (char *filename) #endif } +static struct gcov_fn_buffer * +free_fn_data (const struct gcov_info *gi_ptr, struct gcov_fn_buffer *buffer, + unsigned limit) +{ + struct gcov_fn_buffer *next; + unsigned ix, n_ctr = 0; + + if (!buffer) + return 0; + next = buffer->next; + + for (ix = 0; ix != limit; ix++) + if (gi_ptr->merge[ix]) + free (buffer->info.ctrs[n_ctr++].values); + free (buffer); + return next; +} + static struct gcov_fn_buffer ** -buffer_fn_data (struct gcov_info *gi_ptr, struct gcov_fn_buffer **end_ptr, - unsigned fn_ix) +buffer_fn_data (const char *filename, const struct gcov_info *gi_ptr, + struct gcov_fn_buffer **end_ptr, unsigned fn_ix) { - unsigned n_ctrs = 0, ix; + unsigned n_ctrs = 0, ix = 0; struct gcov_fn_buffer *fn_buffer; + unsigned len; for (ix = GCOV_COUNTERS; ix--;) if (gi_ptr->merge[ix]) n_ctrs++; - fn_buffer = (struct gcov_fn_buffer *)malloc - (sizeof (*fn_buffer) + sizeof (fn_buffer->info.ctrs[0]) * n_ctrs); + len = sizeof (*fn_buffer) + sizeof (fn_buffer->info.ctrs[0]) * n_ctrs; + fn_buffer = (struct gcov_fn_buffer *)malloc (len); if (!fn_buffer) - return 0; /* We'll horribly fail. */ + goto fail; fn_buffer->next = 0; fn_buffer->fn_ix = fn_ix; @@ -175,16 +190,17 @@ buffer_fn_data (struct gcov_info *gi_ptr continue; if (gcov_read_unsigned () != GCOV_TAG_FOR_COUNTER (ix)) - goto fail; - - length = GCOV_TAG_COUNTER_NUM (gcov_read_unsigned ()); - values = (gcov_type *)malloc (length * sizeof (gcov_type)); - if (!values) { - while (n_ctrs--) - free (fn_buffer->info.ctrs[n_ctrs].values); + len = 0; goto fail; } + + length = GCOV_TAG_COUNTER_NUM (gcov_read_unsigned ()); + len = length * sizeof (gcov_type); + values = (gcov_type *)malloc (len); + if (!values) + goto fail; + fn_buffer->info.ctrs[n_ctrs].num = length; fn_buffer->info.ctrs[n_ctrs].values = values; @@ -197,8 +213,29 @@ buffer_fn_data (struct gcov_info *gi_ptr return &fn_buffer->next; fail: - free (fn_buffer); - return 0; + fprintf (stderr, "profiling:%s:Function %u %s %u \n", filename, fn_ix, + len ? "cannot allocate" : "counter mismatch", len ? len : ix); + + return (struct gcov_fn_buffer **)free_fn_data (gi_ptr, fn_buffer, ix); +} + +/* Add an unsigned value to the current crc */ + +static gcov_unsigned_t +crc32_unsigned (gcov_unsigned_t crc32, gcov_unsigned_t value) +{ + unsigned ix; + + for (ix = 32; ix--; value <<= 1) + { + unsigned feedback; + + feedback = (value ^ crc32) & 0x80000000 ? 0x04c11db7 : 0; + crc32 <<= 1; + crc32 ^= feedback; + } + + return crc32; } /* Check if VERSION of the info block PTR matches libgcov one. @@ -241,41 +278,56 @@ gcov_exit (void) struct gcov_summary all_prg; /* summary for all instances of program. */ struct gcov_ctr_summary *cs_ptr; const struct gcov_ctr_info *ci_ptr; - unsigned t_ix, f_ix; + unsigned t_ix; + int f_ix; gcov_unsigned_t c_num; const char *gcov_prefix; int gcov_prefix_strip = 0; size_t prefix_length; char *gi_filename, *gi_filename_up; + gcov_unsigned_t crc32 = 0; memset (&all_prg, 0, sizeof (all_prg)); /* Find the totals for this execution. */ memset (&this_prg, 0, sizeof (this_prg)); for (gi_ptr = gcov_list; gi_ptr; gi_ptr = gi_ptr->next) - for (f_ix = 0; f_ix != gi_ptr->n_functions; f_ix++) - { - gfi_ptr = gi_ptr->functions[f_ix]; - - if (!gfi_ptr || gfi_ptr->key != gi_ptr) - continue; - - ci_ptr = gfi_ptr->ctrs; - for (t_ix = 0; t_ix != GCOV_COUNTERS_SUMMABLE; t_ix++) - { - if (!gi_ptr->merge[t_ix]) - continue; + { + crc32 = crc32_unsigned (crc32, gi_ptr->stamp); + crc32 = crc32_unsigned (crc32, gi_ptr->n_functions); + + for (f_ix = 0; (unsigned)f_ix != gi_ptr->n_functions; f_ix++) + { + gfi_ptr = gi_ptr->functions[f_ix]; - cs_ptr = &this_prg.ctrs[t_ix]; - cs_ptr->num += ci_ptr->num; - for (c_num = 0; c_num < ci_ptr->num; c_num++) - { - cs_ptr->sum_all += ci_ptr->values[c_num]; - if (cs_ptr->run_max < ci_ptr->values[c_num]) - cs_ptr->run_max = ci_ptr->values[c_num]; - } - ci_ptr++; - } - } + if (gfi_ptr && gfi_ptr->key != gi_ptr) + gfi_ptr = 0; + + crc32 = crc32_unsigned (crc32, gfi_ptr ? gfi_ptr->cfg_checksum : 0); + crc32 = crc32_unsigned (crc32, + gfi_ptr ? gfi_ptr->lineno_checksum : 0); + if (!gfi_ptr) + continue; + + ci_ptr = gfi_ptr->ctrs; + for (t_ix = 0; t_ix != GCOV_COUNTERS_SUMMABLE; t_ix++) + { + if (!gi_ptr->merge[t_ix]) + continue; + + cs_ptr = &this_prg.ctrs[t_ix]; + cs_ptr->num += ci_ptr->num; + crc32 = crc32_unsigned (crc32, ci_ptr->num); + + for (c_num = 0; c_num < ci_ptr->num; c_num++) + { + cs_ptr->sum_all += ci_ptr->values[c_num]; + if (cs_ptr->run_max < ci_ptr->values[c_num]) + cs_ptr->run_max = ci_ptr->values[c_num]; + } + ci_ptr++; + } + } + } { /* Check if the level of dirs to strip off specified. */ @@ -400,7 +452,7 @@ gcov_exit (void) goto rewrite; /* Look for program summary. */ - for (f_ix = ~0u;;) + for (f_ix = 0;;) { struct gcov_summary tmp; @@ -409,29 +461,35 @@ gcov_exit (void) if (tag != GCOV_TAG_PROGRAM_SUMMARY) break; + f_ix--; length = gcov_read_unsigned (); if (length != GCOV_TAG_SUMMARY_LENGTH) goto read_mismatch; gcov_read_summary (&tmp); if ((error = gcov_is_error ())) goto read_error; - if (!summary_pos && tmp.checksum == gcov_crc32) - { - prg = tmp; - summary_pos = eof_pos; - } + if (summary_pos || tmp.checksum != crc32) + goto next_summary; + + for (t_ix = 0; t_ix != GCOV_COUNTERS_SUMMABLE; t_ix++) + if (tmp.ctrs[t_ix].num != this_prg.ctrs[t_ix].num) + goto next_summary; + prg = tmp; + summary_pos = eof_pos; + + next_summary:; } /* Merge execution counts for each function. */ - for (f_ix = 0; f_ix != gi_ptr->n_functions; + for (f_ix = 0; (unsigned)f_ix != gi_ptr->n_functions; f_ix++, tag = gcov_read_unsigned ()) { gfi_ptr = gi_ptr->functions[f_ix]; if (tag != GCOV_TAG_FUNCTION) goto read_mismatch; - length = gcov_read_unsigned (); + length = gcov_read_unsigned (); if (!length) /* This function did not appear in the other program. We have nothing to merge. */ @@ -447,15 +505,23 @@ gcov_exit (void) it back out -- we'll be inserting data before this point, so cannot simply keep the data in the file. */ - fn_tail = buffer_fn_data (gi_ptr, fn_tail, f_ix); + fn_tail = buffer_fn_data (gi_filename, + gi_ptr, fn_tail, f_ix); if (!fn_tail) goto read_mismatch; continue; } - if (gcov_read_unsigned () != gfi_ptr->ident - || gcov_read_unsigned () != gfi_ptr->lineno_checksum - || gcov_read_unsigned () != gfi_ptr->cfg_checksum) + length = gcov_read_unsigned (); + if (length != gfi_ptr->ident) + goto read_mismatch; + + length = gcov_read_unsigned (); + if (length != gfi_ptr->lineno_checksum) + goto read_mismatch; + + length = gcov_read_unsigned (); + if (length != gfi_ptr->cfg_checksum) goto read_mismatch; ci_ptr = gfi_ptr->ctrs; @@ -481,8 +547,9 @@ gcov_exit (void) if (tag) { read_mismatch:; - fprintf (stderr, "profiling:%s:Merge mismatch for %s\n", - gi_filename, f_ix + 1 ? "function" : "summaries"); + fprintf (stderr, "profiling:%s:Merge mismatch for %s %u\n", + gi_filename, f_ix >= 0 ? "function" : "summary", + f_ix < 0 ? -1 - f_ix : f_ix); goto read_fatal; } } @@ -492,9 +559,7 @@ gcov_exit (void) fprintf (stderr, "profiling:%s:%s merging\n", gi_filename, error < 0 ? "Overflow": "Error"); - read_fatal:; - gcov_close (); - continue; + goto read_fatal; rewrite:; gcov_rewrite (); @@ -515,8 +580,6 @@ gcov_exit (void) { if (!cs_prg->runs++) cs_prg->num = cs_tprg->num; - else if (cs_prg->num != cs_tprg->num) - goto read_mismatch; cs_prg->sum_all += cs_tprg->sum_all; if (cs_prg->run_max < cs_tprg->run_max) cs_prg->run_max = cs_tprg->run_max; @@ -538,7 +601,7 @@ gcov_exit (void) } } - prg.checksum = gcov_crc32; + prg.checksum = crc32; /* Write out the data. */ if (!eof_pos) @@ -557,11 +620,11 @@ gcov_exit (void) gcov_seek (eof_pos); /* Write execution counts for each function. */ - for (f_ix = 0; f_ix < gi_ptr->n_functions; f_ix++) + for (f_ix = 0; (unsigned)f_ix != gi_ptr->n_functions; f_ix++) { unsigned buffered = 0; - if (fn_buffer && fn_buffer->fn_ix == f_ix) + if (fn_buffer && fn_buffer->fn_ix == (unsigned)f_ix) { /* Buffered data from another program. */ buffered = 1; @@ -597,19 +660,18 @@ gcov_exit (void) gcov_type *c_ptr = ci_ptr->values; while (n_counts--) gcov_write_counter (*c_ptr++); - if (buffered) - free (ci_ptr->values); ci_ptr++; } if (buffered) - { - struct gcov_fn_buffer *tmp = fn_buffer; - fn_buffer = fn_buffer->next; - free (tmp); - } + fn_buffer = free_fn_data (gi_ptr, fn_buffer, GCOV_COUNTERS); } gcov_write_unsigned (0); + + read_fatal:; + while (fn_buffer) + fn_buffer = free_fn_data (gi_ptr, fn_buffer, GCOV_COUNTERS); + if ((error = gcov_close ())) fprintf (stderr, error < 0 ? "profiling:%s:Overflow writing\n" : @@ -628,32 +690,12 @@ __gcov_init (struct gcov_info *info) return; if (gcov_version (info, info->version, 0)) { - const char *ptr = info->filename; - gcov_unsigned_t crc32 = gcov_crc32; - size_t filename_length = strlen(info->filename); + size_t filename_length = strlen(info->filename); /* Refresh the longest file name information */ if (filename_length > gcov_max_filename) gcov_max_filename = filename_length; - do - { - unsigned ix; - gcov_unsigned_t value = *ptr << 24; - - for (ix = 8; ix--; value <<= 1) - { - gcov_unsigned_t feedback; - - feedback = (value ^ crc32) & 0x80000000 ? 0x04c11db7 : 0; - crc32 <<= 1; - crc32 ^= feedback; - } - } - while (*ptr++); - - gcov_crc32 = crc32; - if (!gcov_list) atexit (gcov_exit);