From patchwork Thu Jan 16 22:34:57 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Teresa Johnson X-Patchwork-Id: 311875 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 16AF42C0097 for ; Fri, 17 Jan 2014 09:35:09 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=A6vJeTpah2nTYs7EgJ XtXZsmJWn4cVmxNWTCkshSm0TMTOPsoe9Fefgg4UcskHWUnE9wayynE6dZER2Nd+ QhJyZfWKjuIMszHScDbyCBTS/NwtGZZwCuGe8oPon8P5Amx+MCHjqVQiVVaysWc6 zxzXerdjmmc36+WfNUGfuVI+o= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=kM68Fihmxe+r07INbXg1JQCC OM4=; b=M6W22JRkuSkUgZRIVnMuwCLRMeWMbOnpxy9ehOoG00sXogPZL8DQqB5e jTzgH2ut2afCdT50IBDj9H6g+ep3WSpbQCYnvguS66bPekfltQ78qbHIDMT+d4Na +GHvQziU8vzFETTQiQfKK4qRnlo8mUikt0okAVpkpOVX4YwsQDE= Received: (qmail 5623 invoked by alias); 16 Jan 2014 22:35:02 -0000 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 Received: (qmail 5606 invoked by uid 89); 16 Jan 2014 22:35:01 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.1 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_LOW, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-qc0-f172.google.com Received: from mail-qc0-f172.google.com (HELO mail-qc0-f172.google.com) (209.85.216.172) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Thu, 16 Jan 2014 22:34:59 +0000 Received: by mail-qc0-f172.google.com with SMTP id c9so2955723qcz.3 for ; Thu, 16 Jan 2014 14:34:57 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=eZmUuzCGo7dxMpG4mGmejZlc4UFrfkH27WCvfSzcabs=; b=RSGwIp3yIzg8TQ/J/eXDR0hhoXsmLlUYoliUQyiqquN2itgNBYwgS7AKCBLAEHfMhX nZcSxyx0+WfDOaa7hbriLAr0SkGUq4zOWKh/wp3yESd3GYphnrDf4QBdDHhambzQi0fY AUUu0QIWy34QzQAj+bI8UpcNJAr3PrqDnGLrEROnNsGUOj4B+hZWvRAGrHC8S4x+ibEc lxT9vYrlExMh7eUowrainaCSt0u9yEw8VSqdYpSt948swZEElSutLt77b8XW0f/KVZBe kqfUgOZnPCnJG5FAZy1Qcc7ZeQexTkKiyRfuBw398Rdw+IvHNreWsPcP6hVMNSw8Vuem pVMA== X-Gm-Message-State: ALoCoQkp/NDDDqdJmNJZEpREP/zRYLKiXMaNF4h3YPkw6vo+wh+BpbgVgqF3YI3tSoGShXxGIit8kAloepJNo3XAu62kEk92MSNMD0XmG8YH2AnLIfszuM5vjLdzj+tnSjLQy8S2LIQJ/vqWEFPujLpVG7RKqFAyAzQqZDTqQY4hvET4FF4fYfk5ZeUh6EENV5AQdCK2gixXkl+DuNs3mTt9IwK/rD8QQw== MIME-Version: 1.0 X-Received: by 10.140.49.130 with SMTP id q2mr12422875qga.83.1389911697402; Thu, 16 Jan 2014 14:34:57 -0800 (PST) Received: by 10.229.117.65 with HTTP; Thu, 16 Jan 2014 14:34:57 -0800 (PST) In-Reply-To: <20140116043923.GA22909@kam.mff.cuni.cz> References: <20140109145623.GB19409@kam.mff.cuni.cz> <20140116043923.GA22909@kam.mff.cuni.cz> Date: Thu, 16 Jan 2014 14:34:57 -0800 Message-ID: Subject: Re: [Patch] Avoid gcc_assert in libgcov From: Teresa Johnson To: Jan Hubicka Cc: "gcc-patches@gcc.gnu.org" , David Li X-IsSubscribed: yes On Wed, Jan 15, 2014 at 8:39 PM, Jan Hubicka wrote: >> >> In that case should we call gcov_error when IN_LIBGCOV? One >> possibility would be to simply make gcov_nonruntime_assert be defined >> as if (!EXPR) gcov_error in the IN_LIBGCOV case. But I think what you >> wanted here was to reduce libgcov bloat by removing calls altogether, >> which this wouldn't solve. But if we want to call gcov_error in some >> cases, I think I need to add another macro that will either do >> gcc_assert when !IN_LIBGCOV and "if (!EXPR) gcov_error" when >> IN_LIBGCOV. Is that what you had in mind? > > I think for errors that can be triggered by data corruption, we ought to > produce resonable error messages in both IN_LIBGCOV or for offline tools. > Just unwound sequence if(...) gcov_error seems fine to me in this case, > but we may also have assert like wrapper. > I see we do not provide gcov_error outside libgcov, we probably ought to map > it to fatal_error in GCC binary. > > thanks, > Honza Ok, here is the new patch. Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk? Thanks, Teresa 2014-01-16 Teresa Johnson * gcov-io.c (gcov_position): Use gcov_nonruntime_assert. (gcov_is_error): Remove gcc_assert from IN_LIBGCOV code. (gcov_rewrite): Use gcov_nonruntime_assert. (gcov_open): Ditto. (gcov_write_words): Ditto. (gcov_write_length): Ditto. (gcov_read_words): Use gcov_nonruntime_assert, and remove gcc_assert from IN_LIBGCOV code. (gcov_read_summary): Use gcov_error to flag profile corruption. (gcov_sync): Use gcov_nonruntime_assert. (gcov_seek): Remove gcc_assert from IN_LIBGCOV code. (gcov_histo_index): Use gcov_nonruntime_assert. (static void gcov_histogram_merge): Ditto. (compute_working_sets): Ditto. * gcov-io.h (gcov_nonruntime_assert): Define. (gcov_error): Define for !IN_LIBGCOV Index: gcov-io.c =================================================================== --- gcov-io.c (revision 206435) +++ gcov-io.c (working copy) @@ -67,7 +67,7 @@ GCOV_LINKAGE struct gcov_var static inline gcov_position_t gcov_position (void) { - gcc_assert (gcov_var.mode > 0); + gcov_nonruntime_assert (gcov_var.mode > 0); return gcov_var.start + gcov_var.offset; } @@ -83,7 +83,6 @@ gcov_is_error (void) GCOV_LINKAGE inline void gcov_rewrite (void) { - gcc_assert (gcov_var.mode > 0); gcov_var.mode = -1; gcov_var.start = 0; gcov_var.offset = 0; @@ -133,7 +132,7 @@ gcov_open (const char *name, int mode) s_flock.l_pid = getpid (); #endif - gcc_assert (!gcov_var.file); + gcov_nonruntime_assert (!gcov_var.file); gcov_var.start = 0; gcov_var.offset = gcov_var.length = 0; gcov_var.overread = -1u; @@ -291,14 +290,13 @@ gcov_write_words (unsigned words) { gcov_unsigned_t *result; - gcc_assert (gcov_var.mode < 0); + gcov_nonruntime_assert (gcov_var.mode < 0); #if IN_LIBGCOV if (gcov_var.offset >= GCOV_BLOCK_SIZE) { gcov_write_block (GCOV_BLOCK_SIZE); if (gcov_var.offset) { - gcc_assert (gcov_var.offset == 1); memcpy (gcov_var.buffer, gcov_var.buffer + GCOV_BLOCK_SIZE, 4); } } @@ -393,9 +391,9 @@ gcov_write_length (gcov_position_t position) gcov_unsigned_t length; gcov_unsigned_t *buffer; - gcc_assert (gcov_var.mode < 0); - gcc_assert (position + 2 <= gcov_var.start + gcov_var.offset); - gcc_assert (position >= gcov_var.start); + gcov_nonruntime_assert (gcov_var.mode < 0); + gcov_nonruntime_assert (position + 2 <= gcov_var.start + gcov_var.offset); + gcov_nonruntime_assert (position >= gcov_var.start); offset = position - gcov_var.start; length = gcov_var.offset - offset - 2; buffer = (gcov_unsigned_t *) &gcov_var.buffer[offset]; @@ -481,14 +479,13 @@ gcov_read_words (unsigned words) const gcov_unsigned_t *result; unsigned excess = gcov_var.length - gcov_var.offset; - gcc_assert (gcov_var.mode > 0); + gcov_nonruntime_assert (gcov_var.mode > 0); if (excess < words) { gcov_var.start += gcov_var.offset; #if IN_LIBGCOV if (excess) { - gcc_assert (excess == 1); memcpy (gcov_var.buffer, gcov_var.buffer + gcov_var.offset, 4); } #else @@ -497,7 +494,6 @@ gcov_read_words (unsigned words) gcov_var.offset = 0; gcov_var.length = excess; #if IN_LIBGCOV - gcc_assert (!gcov_var.length || gcov_var.length == 1); excess = GCOV_BLOCK_SIZE; #else if (gcov_var.length + words > gcov_var.alloc) @@ -614,7 +610,9 @@ gcov_read_summary (struct gcov_summary *summary) while (!cur_bitvector) { h_ix = bv_ix * 32; - gcc_assert (bv_ix < GCOV_HISTOGRAM_BITVECTOR_SIZE); + if (bv_ix >= GCOV_HISTOGRAM_BITVECTOR_SIZE) + gcov_error ("corrupted profile info: summary histogram " + "bitvector is corrupt"); cur_bitvector = histo_bitvector[bv_ix++]; } while (!(cur_bitvector & 0x1)) @@ -622,7 +620,9 @@ gcov_read_summary (struct gcov_summary *summary) h_ix++; cur_bitvector >>= 1; } - gcc_assert (h_ix < GCOV_HISTOGRAM_SIZE); + if (h_ix >= GCOV_HISTOGRAM_SIZE) + gcov_error ("corrupted profile info: summary histogram " + "index is corrupt"); csum->histogram[h_ix].num_counters = gcov_read_unsigned (); csum->histogram[h_ix].min_value = gcov_read_counter (); @@ -642,7 +642,7 @@ gcov_read_summary (struct gcov_summary *summary) GCOV_LINKAGE void gcov_sync (gcov_position_t base, gcov_unsigned_t length) { - gcc_assert (gcov_var.mode > 0); + gcov_nonruntime_assert (gcov_var.mode > 0); base += length; if (base - gcov_var.start <= gcov_var.length) gcov_var.offset = base - gcov_var.start; @@ -661,7 +661,6 @@ gcov_sync (gcov_position_t base, gcov_unsigned_t l GCOV_LINKAGE void gcov_seek (gcov_position_t base) { - gcc_assert (gcov_var.mode < 0); if (gcov_var.offset) gcov_write_block (gcov_var.offset); fseek (gcov_var.file, base << 2, SEEK_SET); @@ -736,7 +735,7 @@ gcov_histo_index (gcov_type value) if (r < 2) return (unsigned)value; - gcc_assert (r < 64); + gcov_nonruntime_assert (r < 64); /* Find the two next most significant bits to determine which of the four linear sub-buckets to select. */ @@ -853,7 +852,7 @@ static void gcov_histogram_merge (gcov_bucket_type /* The merged counters get placed in the new merged histogram at the entry for the merged min_value. */ tmp_i = gcov_histo_index (merge_min); - gcc_assert (tmp_i < GCOV_HISTOGRAM_SIZE); + gcov_nonruntime_assert (tmp_i < GCOV_HISTOGRAM_SIZE); tmp_histo[tmp_i].num_counters += merge_num; tmp_histo[tmp_i].cum_value += merge_cum; if (!tmp_histo[tmp_i].min_value || @@ -867,7 +866,7 @@ static void gcov_histogram_merge (gcov_bucket_type } } - gcc_assert (tgt_i < 0); + gcov_nonruntime_assert (tgt_i < 0); /* In the case where there were more counters in the source histogram, accumulate the remaining unmerged cumulative counter values. Add @@ -884,8 +883,8 @@ static void gcov_histogram_merge (gcov_bucket_type } /* At this point, tmp_i should be the smallest non-zero entry in the tmp_histo. */ - gcc_assert (tmp_i >= 0 && tmp_i < GCOV_HISTOGRAM_SIZE - && tmp_histo[tmp_i].num_counters > 0); + gcov_nonruntime_assert (tmp_i >= 0 && tmp_i < GCOV_HISTOGRAM_SIZE + && tmp_histo[tmp_i].num_counters > 0); tmp_histo[tmp_i].cum_value += src_cum; /* Finally, copy the merged histogram into tgt_histo. */ @@ -997,6 +996,6 @@ compute_working_sets (const struct gcov_ctr_summar using a temporary above. */ cum += histo_bucket->cum_value; } - gcc_assert (ws_ix == NUM_GCOV_WORKING_SETS); + gcov_nonruntime_assert (ws_ix == NUM_GCOV_WORKING_SETS); } #endif /* IN_GCOV <= 0 && !IN_LIBGCOV */ Index: gcov-io.h =================================================================== --- gcov-io.h (revision 206435) +++ gcov-io.h (working copy) @@ -195,6 +195,13 @@ typedef unsigned HOST_WIDEST_INT gcov_type_unsigne #define GCOV_LINKAGE extern #endif +#if IN_LIBGCOV +#define gcov_nonruntime_assert(EXPR) ((void)(0 && (EXPR))) +#else +#define gcov_nonruntime_assert(EXPR) gcc_assert (EXPR) +#define gcov_error(...) fatal_error (__VA_ARGS__) +#endif + /* File suffixes. */ #define GCOV_DATA_SUFFIX ".gcda" #define GCOV_NOTE_SUFFIX ".gcno"