[Patch] Avoid gcc_assert in libgcov

[Patch] Avoid gcc_assert in libgcov

[Teresa Johnson]

As suggested by Honza, avoid bloating libgcov from gcc_assert by using
a new macro gcov_nonruntime_assert in gcov-io.c that is only mapped to
gcc_assert when not in libgcov.

Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk?

Thanks,
Teresa

2014-01-09  Teresa Johnson  <tejohnson@google.com>

        * gcov-io.c (gcov_position): Use gcov_nonruntime_assert.
        (gcov_is_error): Ditto.
        (gcov_rewrite): Ditto.
        (gcov_open): Ditto.
        (gcov_write_words): Ditto.
        (gcov_write_length): Ditto.
        (gcov_read_words): Ditto.
        (gcov_read_summary): Ditto.
        (gcov_sync): Ditto.
        (gcov_seek): Ditto.
        (gcov_histo_index): Ditto.
        (static void gcov_histogram_merge): Ditto.
        (compute_working_sets): Ditto.
        * gcov-io.h (gcov_nonruntime_assert): Define.

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,7 @@ gcov_is_error (void)
 GCOV_LINKAGE inline void
 gcov_rewrite (void)
 {
-  gcc_assert (gcov_var.mode > 0);
+  gcov_nonruntime_assert (gcov_var.mode > 0);
   gcov_var.mode = -1;
   gcov_var.start = 0;
   gcov_var.offset = 0;
@@ -133,7 +133,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 +291,14 @@ 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);
+         gcov_nonruntime_assert (gcov_var.offset == 1);
          memcpy (gcov_var.buffer, gcov_var.buffer + GCOV_BLOCK_SIZE, 4);
        }
     }
@@ -393,9 +393,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 +481,14 @@ 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);
+         gcov_nonruntime_assert (excess == 1);
          memcpy (gcov_var.buffer, gcov_var.buffer + gcov_var.offset, 4);
        }
 #else
@@ -497,7 +497,7 @@ 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);
+      gcov_nonruntime_assert (!gcov_var.length || gcov_var.length == 1);
       excess = GCOV_BLOCK_SIZE;
 #else
       if (gcov_var.length + words > gcov_var.alloc)
@@ -614,7 +614,7 @@ gcov_read_summary (struct gcov_summary *summary)
           while (!cur_bitvector)
             {
               h_ix = bv_ix * 32;
-              gcc_assert (bv_ix < GCOV_HISTOGRAM_BITVECTOR_SIZE);
+              gcov_nonruntime_assert (bv_ix < GCOV_HISTOGRAM_BITVECTOR_SIZE);
               cur_bitvector = histo_bitvector[bv_ix++];
             }
           while (!(cur_bitvector & 0x1))
@@ -622,7 +622,7 @@ gcov_read_summary (struct gcov_summary *summary)
               h_ix++;
               cur_bitvector >>= 1;
             }
-          gcc_assert (h_ix < GCOV_HISTOGRAM_SIZE);
+          gcov_nonruntime_assert (h_ix < GCOV_HISTOGRAM_SIZE);

           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,7 @@ 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);
+  gcov_nonruntime_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 +736,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 +853,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 +867,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 +884,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 +997,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,12 @@ 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)
+#endif
+
 /* File suffixes.  */
 #define GCOV_DATA_SUFFIX ".gcda"
 #define GCOV_NOTE_SUFFIX ".gcno"


-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

Re: [Patch] Avoid gcc_assert in libgcov

[Jan Hubicka]

> As suggested by Honza, avoid bloating libgcov from gcc_assert by using
> a new macro gcov_nonruntime_assert in gcov-io.c that is only mapped to
> gcc_assert when not in libgcov.
> 
> Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk?
> 
> Thanks,
> Teresa
> 
> 2014-01-09  Teresa Johnson  <tejohnson@google.com>
> 
>         * gcov-io.c (gcov_position): Use gcov_nonruntime_assert.
>         (gcov_is_error): Ditto.
>         (gcov_rewrite): Ditto.
>         (gcov_open): Ditto.
>         (gcov_write_words): Ditto.
>         (gcov_write_length): Ditto.
>         (gcov_read_words): Ditto.
>         (gcov_read_summary): Ditto.
>         (gcov_sync): Ditto.
>         (gcov_seek): Ditto.
>         (gcov_histo_index): Ditto.
>         (static void gcov_histogram_merge): Ditto.
>         (compute_working_sets): Ditto.
>         * gcov-io.h (gcov_nonruntime_assert): Define.
> 

> @@ -481,14 +481,14 @@ 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);
> +         gcov_nonruntime_assert (excess == 1);

It probably makes no sense to put nonruntime access into IN_LIBGCOV defines.

>           memcpy (gcov_var.buffer, gcov_var.buffer + gcov_var.offset, 4);
>         }
>  #else
> @@ -497,7 +497,7 @@ 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);
> +      gcov_nonruntime_assert (!gcov_var.length || gcov_var.length == 1);
>        excess = GCOV_BLOCK_SIZE;
>  #else
>        if (gcov_var.length + words > gcov_var.alloc)
> @@ -614,7 +614,7 @@ gcov_read_summary (struct gcov_summary *summary)
>            while (!cur_bitvector)
>              {
>                h_ix = bv_ix * 32;
> -              gcc_assert (bv_ix < GCOV_HISTOGRAM_BITVECTOR_SIZE);
> +              gcov_nonruntime_assert (bv_ix < GCOV_HISTOGRAM_BITVECTOR_SIZE);
>                cur_bitvector = histo_bitvector[bv_ix++];
>              }
>            while (!(cur_bitvector & 0x1))
> @@ -622,7 +622,7 @@ gcov_read_summary (struct gcov_summary *summary)
>                h_ix++;
>                cur_bitvector >>= 1;
>              }
> -          gcc_assert (h_ix < GCOV_HISTOGRAM_SIZE);
> +          gcov_nonruntime_assert (h_ix < GCOV_HISTOGRAM_SIZE);

How many of those asserts can be triggered by a corrupted gcda file?
I would like to make libgcov more safe WRT file corruptions, too, so in that
case we should produce an error message.

The rest of changes seems OK.

Honza

Re: [Patch] Avoid gcc_assert in libgcov

[Teresa Johnson]

On Thu, Jan 9, 2014 at 6:56 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> As suggested by Honza, avoid bloating libgcov from gcc_assert by using
>> a new macro gcov_nonruntime_assert in gcov-io.c that is only mapped to
>> gcc_assert when not in libgcov.
>>
>> Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk?
>>
>> Thanks,
>> Teresa
>>
>> 2014-01-09  Teresa Johnson  <tejohnson@google.com>
>>
>>         * gcov-io.c (gcov_position): Use gcov_nonruntime_assert.
>>         (gcov_is_error): Ditto.
>>         (gcov_rewrite): Ditto.
>>         (gcov_open): Ditto.
>>         (gcov_write_words): Ditto.
>>         (gcov_write_length): Ditto.
>>         (gcov_read_words): Ditto.
>>         (gcov_read_summary): Ditto.
>>         (gcov_sync): Ditto.
>>         (gcov_seek): Ditto.
>>         (gcov_histo_index): Ditto.
>>         (static void gcov_histogram_merge): Ditto.
>>         (compute_working_sets): Ditto.
>>         * gcov-io.h (gcov_nonruntime_assert): Define.
>>
>
>> @@ -481,14 +481,14 @@ 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);
>> +         gcov_nonruntime_assert (excess == 1);
>
> It probably makes no sense to put nonruntime access into IN_LIBGCOV defines.

You are right - there were several that were in IN_LIBGCOV defines
that I can just remove.

>
>>           memcpy (gcov_var.buffer, gcov_var.buffer + gcov_var.offset, 4);
>>         }
>>  #else
>> @@ -497,7 +497,7 @@ 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);
>> +      gcov_nonruntime_assert (!gcov_var.length || gcov_var.length == 1);
>>        excess = GCOV_BLOCK_SIZE;
>>  #else
>>        if (gcov_var.length + words > gcov_var.alloc)
>> @@ -614,7 +614,7 @@ gcov_read_summary (struct gcov_summary *summary)
>>            while (!cur_bitvector)
>>              {
>>                h_ix = bv_ix * 32;
>> -              gcc_assert (bv_ix < GCOV_HISTOGRAM_BITVECTOR_SIZE);
>> +              gcov_nonruntime_assert (bv_ix < GCOV_HISTOGRAM_BITVECTOR_SIZE);
>>                cur_bitvector = histo_bitvector[bv_ix++];
>>              }
>>            while (!(cur_bitvector & 0x1))
>> @@ -622,7 +622,7 @@ gcov_read_summary (struct gcov_summary *summary)
>>                h_ix++;
>>                cur_bitvector >>= 1;
>>              }
>> -          gcc_assert (h_ix < GCOV_HISTOGRAM_SIZE);
>> +          gcov_nonruntime_assert (h_ix < GCOV_HISTOGRAM_SIZE);
>
> How many of those asserts can be triggered by a corrupted gcda file?
> I would like to make libgcov more safe WRT file corruptions, too, so in that
> case we should produce an error message.

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?

Thanks,
Teresa

>
> The rest of changes seems OK.
>
> Honza



-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

Re: [Patch] Avoid gcc_assert in libgcov

[Jan Hubicka]

> 
> 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

Re: [Patch] Avoid gcc_assert in libgcov

[Teresa Johnson]

On Wed, Jan 15, 2014 at 8:39 PM, Jan Hubicka <hubicka@ucw.cz> 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  <tejohnson@google.com>

        * 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"


-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

Re: [Patch] Avoid gcc_assert in libgcov

[Jan Hubicka]

> On Wed, Jan 15, 2014 at 8:39 PM, Jan Hubicka <hubicka@ucw.cz> 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  <tejohnson@google.com>
> 
>         * 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!
Honza

Re: [Patch] Avoid gcc_assert in libgcov

[Teresa Johnson]

On Thu, Jan 16, 2014 at 2:41 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Wed, Jan 15, 2014 at 8:39 PM, Jan Hubicka <hubicka@ucw.cz> 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  <tejohnson@google.com>
>>
>>         * 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!
> Honza

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



-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413

Re: [Patch] Avoid gcc_assert in libgcov

[Rong Xu]

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).

Thanks,

-Rong

On Thu, May 22, 2014 at 7:09 AM, Teresa Johnson <tejohnson@google.com> wrote:
> On Thu, Jan 16, 2014 at 2:41 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>> On Wed, Jan 15, 2014 at 8:39 PM, Jan Hubicka <hubicka@ucw.cz> 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  <tejohnson@google.com>
>>>
>>>         * 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!
>> Honza
>
> 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
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413