This is the mail archive of the
mailing list for the GCC project.
Re: [Patch] Avoid gcc_assert in libgcov
- From: Rong Xu <xur at google dot com>
- To: Teresa Johnson <tejohnson at google dot com>
- Cc: Jan Hubicka <hubicka at ucw dot cz>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, David Li <davidxl at google dot com>
- Date: Thu, 22 May 2014 14:30:46 -0700
- Subject: Re: [Patch] Avoid gcc_assert in libgcov
- Authentication-results: sourceware.org; auth=none
- References: <CAAe5K+W7mdn1NtHEDFcAbpJ_R1krsNoOVNBAh2HuGdc97A4Wrw at mail dot gmail dot com> <20140109145623 dot GB19409 at kam dot mff dot cuni dot cz> <CAAe5K+V2zQ=mcZKvYPj+Q2BEMhvYQyibtu1-KfkzdnQf_ZZvZg at mail dot gmail dot com> <20140116043923 dot GA22909 at kam dot mff dot cuni dot cz> <CAAe5K+VL3cxy12RGi-GePenz+p91_DxSxWcFy-GcaPwT9OAdbw at mail dot gmail dot com> <20140116224148 dot GG22909 at kam dot mff dot cuni dot cz> <CAAe5K+XsGbH15zMQ7XBufv-X9R=4ecdkuj_sL5FMZ7NukVMJEQ at mail dot gmail dot com>
I think these asserts will be used by gcov-tool. So I prefer to change
them to gcov_nonruntime_assert(). I'll merge them in my new gcov-tool
patch before submitting (waiting for honaz's ok).
On Thu, May 22, 2014 at 7:09 AM, Teresa Johnson <email@example.com> wrote:
> On Thu, Jan 16, 2014 at 2:41 PM, Jan Hubicka <firstname.lastname@example.org> wrote:
>>> On Wed, Jan 15, 2014 at 8:39 PM, Jan Hubicka <email@example.com> 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 <firstname.lastname@example.org>
>>> * 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
>> OK, thanks!
> I just found this old patch sitting in a client! Committed as r210805.
> I also discovered that a couple uses of gcc_assert have snuck into
> libgcc/libgcov* files in the meantime. Looks like this got added
> during some of Rong's refactoring, cc'ing Rong. They are in
> libgcc/libgcov-driver-system.c (allocate_filename_struct) and
> libgcov-merge.c (__gcov_merge_single and __gcov_merge_delta). Should I
> remove those or change to gcov_error?
> Teresa Johnson | Software Engineer | email@example.com | 408-460-2413