diff mbox series

gcov: Use system IO buffering

Message ID e5a625db-dd5e-aee7-8a07-85d1700ec88f@suse.cz
State New
Headers show
Series gcov: Use system IO buffering | expand

Commit Message

Martin Liška April 21, 2021, 7:52 a.m. UTC
Hey.

I/O buffering in gcov seems duplicite to what modern C library can provide.
The patch is a simplification and can provide easier interface for system
that don't have a filesystem and would like using GCOV.

I'm going to install the patch after 11.1 if there are no objections.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Thanks,
Martin

gcc/ChangeLog:

	* gcov-io.c (gcov_write_block): Remove.
	(gcov_write_words): Likewise.
	(gcov_read_words): Re-implement using gcov_read_bytes.
	(gcov_allocate): Remove.
	(GCOV_BLOCK_SIZE): Likewise.
	(struct gcov_var): Remove most of the fields.
	(gcov_position): Implement with ftell.
	(gcov_rewrite): Remove setting of start and offset fields.
	(from_file): Re-format.
	(gcov_open): Remove setbuf call. It should not be needed.
	(gcov_close): Remove internal buffer handling.
	(gcov_magic): Use __builtin_bswap32.
	(gcov_write_counter): Use directly gcov_write_unsigned.
	(gcov_write_string): Use direct fwrite and do not round
	to 4 bytes.
	(gcov_seek): Use directly fseek.
	(gcov_write_tag): Use gcov_write_unsigned directly.
	(gcov_write_length): Likewise.
	(gcov_write_tag_length): Likewise.
	(gcov_read_bytes): Use directly fread.
	(gcov_read_unsigned): Use gcov_read_words.
	(gcov_read_counter): Likewise.
	(gcov_read_string): Use gcov_read_bytes.
	* gcov-io.h (GCOV_WORD_SIZE): Adjust to reflect
	that size is not in bytes, but words (4B).
	(GCOV_TAG_FUNCTION_LENGTH): Likewise.
	(GCOV_TAG_ARCS_LENGTH): Likewise.
	(GCOV_TAG_ARCS_NUM): Likewise.
	(GCOV_TAG_COUNTER_LENGTH): Likewise.
	(GCOV_TAG_COUNTER_NUM): Likewise.
	(GCOV_TAG_SUMMARY_LENGTH): Likewise.

libgcc/ChangeLog:

	* libgcov-driver.c: Fix GNU coding style.
---
 gcc/gcov-io.c           | 282 +++++++++-------------------------------
 gcc/gcov-io.h           |  17 ++-
 libgcc/libgcov-driver.c |   6 +-
 3 files changed, 76 insertions(+), 229 deletions(-)

Comments

Andi Kleen April 22, 2021, 4:57 p.m. UTC | #1
Martin Liška <mliska@suse.cz> writes:

> Hey.
>
> I/O buffering in gcov seems duplicite to what modern C library can provide.
> The patch is a simplification and can provide easier interface for system
> that don't have a filesystem and would like using GCOV.
>
> I'm going to install the patch after 11.1 if there are no objections.
>
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

What happens if someone compiles the C library with gcov?

Being as self contained as possible (only system calls) would seem
safer.

-Andi
Richard Biener April 23, 2021, 6:49 a.m. UTC | #2
On Thu, Apr 22, 2021 at 9:47 PM Andi Kleen via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Martin Liška <mliska@suse.cz> writes:
>
> > Hey.
> >
> > I/O buffering in gcov seems duplicite to what modern C library can provide.
> > The patch is a simplification and can provide easier interface for system
> > that don't have a filesystem and would like using GCOV.
> >
> > I'm going to install the patch after 11.1 if there are no objections.
> >
> > Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>
> What happens if someone compiles the C library with gcov?

Yeah, I think this is the wrong direction - we're already having
issues with using
malloc, this makes it much worse.

Richard.

> Being as self contained as possible (only system calls) would seem
> safer.
>
> -Andi
Martin Liška April 23, 2021, 9:24 a.m. UTC | #3
On 4/23/21 8:49 AM, Richard Biener wrote:
> On Thu, Apr 22, 2021 at 9:47 PM Andi Kleen via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> Martin Liška <mliska@suse.cz> writes:
>>
>>> Hey.
>>>
>>> I/O buffering in gcov seems duplicite to what modern C library can provide.
>>> The patch is a simplification and can provide easier interface for system
>>> that don't have a filesystem and would like using GCOV.
>>>
>>> I'm going to install the patch after 11.1 if there are no objections.
>>>
>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> What happens if someone compiles the C library with gcov?

Haven't tried that..

> 
> Yeah, I think this is the wrong direction - we're already having
> issues with using
> malloc, this makes it much worse.

I don't see where problem with my patch? It's not adding usage of any additional
system routines.

Martin

> 
> Richard.
> 
>> Being as self contained as possible (only system calls) would seem
>> safer.
>>
>> -Andi
Richard Biener April 23, 2021, 9:44 a.m. UTC | #4
On Fri, Apr 23, 2021 at 11:24 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 4/23/21 8:49 AM, Richard Biener wrote:
> > On Thu, Apr 22, 2021 at 9:47 PM Andi Kleen via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >> Martin Liška <mliska@suse.cz> writes:
> >>
> >>> Hey.
> >>>
> >>> I/O buffering in gcov seems duplicite to what modern C library can provide.
> >>> The patch is a simplification and can provide easier interface for system
> >>> that don't have a filesystem and would like using GCOV.
> >>>
> >>> I'm going to install the patch after 11.1 if there are no objections.
> >>>
> >>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> >>
> >> What happens if someone compiles the C library with gcov?
>
> Haven't tried that..
>
> >
> > Yeah, I think this is the wrong direction - we're already having
> > issues with using
> > malloc, this makes it much worse.
>
> I don't see where problem with my patch? It's not adding usage of any additional
> system routines.

True.  So the only impact should be more calls to libc (not necessarily more
I/O since libc should do buffering).

"The patch is a simplification and can provide easier interface for system
that don't have a filesystem and would like using GCOV."

Can you explain?

> Martin
>
> >
> > Richard.
> >
> >> Being as self contained as possible (only system calls) would seem
> >> safer.
> >>
> >> -Andi
>
Martin Liška April 23, 2021, 11:54 a.m. UTC | #5
On 4/23/21 11:44 AM, Richard Biener wrote:
> On Fri, Apr 23, 2021 at 11:24 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> On 4/23/21 8:49 AM, Richard Biener wrote:
>>> On Thu, Apr 22, 2021 at 9:47 PM Andi Kleen via Gcc-patches
>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>
>>>> Martin Liška <mliska@suse.cz> writes:
>>>>
>>>>> Hey.
>>>>>
>>>>> I/O buffering in gcov seems duplicite to what modern C library can provide.
>>>>> The patch is a simplification and can provide easier interface for system
>>>>> that don't have a filesystem and would like using GCOV.
>>>>>
>>>>> I'm going to install the patch after 11.1 if there are no objections.
>>>>>
>>>>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>>>
>>>> What happens if someone compiles the C library with gcov?
>>
>> Haven't tried that..
>>
>>>
>>> Yeah, I think this is the wrong direction - we're already having
>>> issues with using
>>> malloc, this makes it much worse.
>>
>> I don't see where problem with my patch? It's not adding usage of any additional
>> system routines.
> 
> True.  So the only impact should be more calls to libc (not necessarily more
> I/O since libc should do buffering).

Exactly.

> 
> "The patch is a simplification and can provide easier interface for system
> that don't have a filesystem and would like using GCOV."
> 
> Can you explain?

It's described in the following thread:
https://gcc.gnu.org/pipermail/gcc-patches/2020-November/559342.html

Martin

> 
>> Martin
>>
>>>
>>> Richard.
>>>
>>>> Being as self contained as possible (only system calls) would seem
>>>> safer.
>>>>
>>>> -Andi
>>
Eugene Rozenfeld June 17, 2021, 1:59 a.m. UTC | #6
The commit from this patch (https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=23eb66d1d46a34cb28c4acbdf8a1deb80a7c5a05) changed the semantics of gcov_read_string and gcov_write_string. Before this change the string storage was as described in gcov-io.h:

"Strings are
   padded with 1 to 4 NUL bytes, to bring the length up to a multiple
   of 4. The number of 4 bytes is stored, followed by the padded
   string."

After this change the number before the string indicates the number of bytes (not words) and there is no padding.

Was this file format change intentional? It breaks AutoFDO because create_gcov produces strings in the format specified in gcov-io.h.

Thanks,

Eugene

-----Original Message-----
From: Gcc-patches <gcc-patches-bounces@gcc.gnu.org> On Behalf Of Martin Liška
Sent: Wednesday, April 21, 2021 12:52 AM
To: gcc-patches@gcc.gnu.org
Subject: [EXTERNAL] [PATCH] gcov: Use system IO buffering

Hey.

I/O buffering in gcov seems duplicite to what modern C library can provide.
The patch is a simplification and can provide easier interface for system that don't have a filesystem and would like using GCOV.

I'm going to install the patch after 11.1 if there are no objections.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Thanks,
Martin

gcc/ChangeLog:

	* gcov-io.c (gcov_write_block): Remove.
	(gcov_write_words): Likewise.
	(gcov_read_words): Re-implement using gcov_read_bytes.
	(gcov_allocate): Remove.
	(GCOV_BLOCK_SIZE): Likewise.
	(struct gcov_var): Remove most of the fields.
	(gcov_position): Implement with ftell.
	(gcov_rewrite): Remove setting of start and offset fields.
	(from_file): Re-format.
	(gcov_open): Remove setbuf call. It should not be needed.
	(gcov_close): Remove internal buffer handling.
	(gcov_magic): Use __builtin_bswap32.
	(gcov_write_counter): Use directly gcov_write_unsigned.
	(gcov_write_string): Use direct fwrite and do not round
	to 4 bytes.
	(gcov_seek): Use directly fseek.
	(gcov_write_tag): Use gcov_write_unsigned directly.
	(gcov_write_length): Likewise.
	(gcov_write_tag_length): Likewise.
	(gcov_read_bytes): Use directly fread.
	(gcov_read_unsigned): Use gcov_read_words.
	(gcov_read_counter): Likewise.
	(gcov_read_string): Use gcov_read_bytes.
	* gcov-io.h (GCOV_WORD_SIZE): Adjust to reflect
	that size is not in bytes, but words (4B).
	(GCOV_TAG_FUNCTION_LENGTH): Likewise.
	(GCOV_TAG_ARCS_LENGTH): Likewise.
	(GCOV_TAG_ARCS_NUM): Likewise.
	(GCOV_TAG_COUNTER_LENGTH): Likewise.
	(GCOV_TAG_COUNTER_NUM): Likewise.
	(GCOV_TAG_SUMMARY_LENGTH): Likewise.

libgcc/ChangeLog:

	* libgcov-driver.c: Fix GNU coding style.
---
 gcc/gcov-io.c           | 282 +++++++++-------------------------------
 gcc/gcov-io.h           |  17 ++-
 libgcc/libgcov-driver.c |   6 +-
 3 files changed, 76 insertions(+), 229 deletions(-)

diff --git a/gcc/gcov-io.c b/gcc/gcov-io.c index 80c9082a649..bd2316dedab 100644
--- a/gcc/gcov-io.c
+++ b/gcc/gcov-io.c
@@ -27,40 +27,14 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 /* Routines declared in gcov-io.h.  This file should be #included by
    another source file, after having #included gcov-io.h.  */
 
-#if !IN_GCOV
-static void gcov_write_block (unsigned); -static gcov_unsigned_t *gcov_write_words (unsigned); -#endif -static const gcov_unsigned_t *gcov_read_words (unsigned); -#if !IN_LIBGCOV -static void gcov_allocate (unsigned); -#endif
-
-/* Optimum number of gcov_unsigned_t's read from or written to disk.  */ -#define GCOV_BLOCK_SIZE (1 << 10)
+static gcov_unsigned_t *gcov_read_words (void *buffer, unsigned);
 
 struct gcov_var
 {
   FILE *file;
-  gcov_position_t start;	/* Position of first byte of block */
-  unsigned offset;		/* Read/write position within the block.  */
-  unsigned length;		/* Read limit in the block.  */
-  unsigned overread;		/* Number of words overread.  */
   int error;			/* < 0 overflow, > 0 disk error.  */
-  int mode;	                /* < 0 writing, > 0 reading */
+  int mode;			/* < 0 writing, > 0 reading.  */
   int endian;			/* Swap endianness.  */
-#if IN_LIBGCOV
-  /* Holds one block plus 4 bytes, thus all coverage reads & writes
-     fit within this buffer and we always can transfer GCOV_BLOCK_SIZE
-     to and from the disk. libgcov never backtracks and only writes 4
-     or 8 byte objects.  */
-  gcov_unsigned_t buffer[GCOV_BLOCK_SIZE + 1]; -#else
-  /* Holds a variable length block, as the compiler can write
-     strings and needs to backtrack.  */
-  size_t alloc;
-  gcov_unsigned_t *buffer;
-#endif
 } gcov_var;
 
 /* Save the current position in the gcov file.  */ @@ -71,8 +45,7 @@ static inline  gcov_position_t  gcov_position (void)  {
-  gcov_nonruntime_assert (gcov_var.mode > 0);
-  return gcov_var.start + gcov_var.offset;
+  return ftell (gcov_var.file);
 }
 
 /* Return nonzero if the error flag is set.  */ @@ -92,20 +65,16 @@ GCOV_LINKAGE inline void  gcov_rewrite (void)  {
   gcov_var.mode = -1;
-  gcov_var.start = 0;
-  gcov_var.offset = 0;
   fseek (gcov_var.file, 0L, SEEK_SET);
 }
 #endif
 
-static inline gcov_unsigned_t from_file (gcov_unsigned_t value)
+static inline gcov_unsigned_t
+from_file (gcov_unsigned_t value)
 {
 #if !IN_LIBGCOV || defined (IN_GCOV_TOOL)
   if (gcov_var.endian)
-    {
-      value = (value >> 16) | (value << 16);
-      value = ((value & 0xff00ff) << 8) | ((value >> 8) & 0xff00ff);
-    }
+    return __builtin_bswap32 (value);
 #endif
   return value;
 }
@@ -140,9 +109,6 @@ gcov_open (const char *name, int mode)  #endif
 
   gcov_nonruntime_assert (!gcov_var.file);
-  gcov_var.start = 0;
-  gcov_var.offset = gcov_var.length = 0;
-  gcov_var.overread = -1u;
   gcov_var.error = 0;
 #if !IN_LIBGCOV || defined (IN_GCOV_TOOL)
   gcov_var.endian = 0;
@@ -192,8 +158,6 @@ gcov_open (const char *name, int mode)
 
   gcov_var.mode = mode ? mode : 1;
 
-  setbuf (gcov_var.file, (char *)0);
-
   return 1;
 }
 
@@ -205,19 +169,9 @@ gcov_close (void)
 {
   if (gcov_var.file)
     {
-#if !IN_GCOV
-      if (gcov_var.offset && gcov_var.mode < 0)
-	gcov_write_block (gcov_var.offset);
-#endif
       fclose (gcov_var.file);
       gcov_var.file = 0;
-      gcov_var.length = 0;
     }
-#if !IN_LIBGCOV
-  free (gcov_var.buffer);
-  gcov_var.alloc = 0;
-  gcov_var.buffer = 0;
-#endif
   gcov_var.mode = 0;
   return gcov_var.error;
 }
@@ -232,9 +186,8 @@ gcov_magic (gcov_unsigned_t magic, gcov_unsigned_t expected)  {
   if (magic == expected)
     return 1;
-  magic = (magic >> 16) | (magic << 16);
-  magic = ((magic & 0xff00ff) << 8) | ((magic >> 8) & 0xff00ff);
-  if (magic == expected)
+
+  if (__builtin_bswap32 (magic) == expected)
     {
       gcov_var.endian = 1;
       return -1;
@@ -243,71 +196,15 @@ gcov_magic (gcov_unsigned_t magic, gcov_unsigned_t expected)  }  #endif
 
-#if !IN_LIBGCOV
-static void
-gcov_allocate (unsigned length)
-{
-  size_t new_size = gcov_var.alloc;
-
-  if (!new_size)
-    new_size = GCOV_BLOCK_SIZE;
-  new_size += length;
-  new_size *= 2;
-
-  gcov_var.alloc = new_size;
-  gcov_var.buffer = XRESIZEVAR (gcov_unsigned_t, gcov_var.buffer, new_size << 2); -} -#endif
-
 #if !IN_GCOV
-/* Write out the current block, if needs be.  */
-
-static void
-gcov_write_block (unsigned size)
-{
-  if (fwrite (gcov_var.buffer, size << 2, 1, gcov_var.file) != 1)
-    gcov_var.error = 1;
-  gcov_var.start += size;
-  gcov_var.offset -= size;
-}
-
-/* Allocate space to write BYTES bytes to the gcov file. Return a
-   pointer to those bytes, or NULL on failure.  */
-
-static gcov_unsigned_t *
-gcov_write_words (unsigned words)
-{
-  gcov_unsigned_t *result;
-
-  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)
-	{
-	  memcpy (gcov_var.buffer, gcov_var.buffer + GCOV_BLOCK_SIZE, 4);
-	}
-    }
-#else
-  if (gcov_var.offset + words > gcov_var.alloc)
-    gcov_allocate (gcov_var.offset + words);
-#endif
-  result = &gcov_var.buffer[gcov_var.offset];
-  gcov_var.offset += words;
-
-  return result;
-}
-
-/* Write unsigned VALUE to coverage file.  Sets error flag
-   appropriately.  */
+/* Write unsigned VALUE to coverage file.  */
 
 GCOV_LINKAGE void
 gcov_write_unsigned (gcov_unsigned_t value)  {
-  gcov_unsigned_t *buffer = gcov_write_words (1);
-
-  buffer[0] = value;
+  gcov_unsigned_t r = fwrite (&value, sizeof (value), 1, 
+ gcov_var.file);  if (r != 1)
+    gcov_var.error = 1;
 }
 
 /* Write counter VALUE to coverage file.  Sets error flag @@ -317,13 +214,11 @@ gcov_write_unsigned (gcov_unsigned_t value)  GCOV_LINKAGE void  gcov_write_counter (gcov_type value)  {
-  gcov_unsigned_t *buffer = gcov_write_words (2);
-
-  buffer[0] = (gcov_unsigned_t) value;
+  gcov_write_unsigned ((gcov_unsigned_t) value);
   if (sizeof (value) > sizeof (gcov_unsigned_t))
-    buffer[1] = (gcov_unsigned_t) (value >> 32);
+    gcov_write_unsigned ((gcov_unsigned_t) (value >> 32));
   else
-    buffer[1] = 0;
+    gcov_write_unsigned (0);
 }
 #endif /* IN_LIBGCOV */
 
@@ -335,23 +230,16 @@ GCOV_LINKAGE void
 gcov_write_string (const char *string)
 {
   unsigned length = 0;
-  unsigned alloc = 0;
-  gcov_unsigned_t *buffer;
 
   if (string)
-    {
-      length = strlen (string);
-      alloc = (length + 4) >> 2;
-    }
-
-  buffer = gcov_write_words (1 + alloc);
+    length = strlen (string) + 1;
 
-  buffer[0] = alloc;
-
-  if (alloc > 0)
+  gcov_write_unsigned (length);
+  if (length > 0)
     {
-      buffer[alloc] = 0; /* place nul terminators.  */
-      memcpy (&buffer[1], string, length);
+      gcov_unsigned_t r = fwrite (string, length, 1, gcov_var.file);
+      if (r != 1)
+	gcov_var.error = 1;
     }
 }
 #endif
@@ -388,6 +276,14 @@ gcov_write_filename (const char *filename)  }  #endif
 
+/* Move to a given position in a gcov file.  */
+
+GCOV_LINKAGE void
+gcov_seek (gcov_position_t base)
+{
+  fseek (gcov_var.file, base, SEEK_SET); }
+
 #if !IN_LIBGCOV
 /* Write a tag TAG and reserve space for the record length. Return a
    value to be used for gcov_write_length.  */ @@ -395,11 +291,9 @@ gcov_write_filename (const char *filename)  GCOV_LINKAGE gcov_position_t  gcov_write_tag (gcov_unsigned_t tag)  {
-  gcov_position_t result = gcov_var.start + gcov_var.offset;
-  gcov_unsigned_t *buffer = gcov_write_words (2);
-
-  buffer[0] = tag;
-  buffer[1] = 0;
+  gcov_position_t result = gcov_position ();  gcov_write_unsigned 
+ (tag);  gcov_write_unsigned (0);
 
   return result;
 }
@@ -412,19 +306,13 @@ gcov_write_tag (gcov_unsigned_t tag)  GCOV_LINKAGE void  gcov_write_length (gcov_position_t position)  {
-  unsigned offset;
-  gcov_unsigned_t length;
-  gcov_unsigned_t *buffer;
-
+  gcov_position_t current_position = gcov_position ();
   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];
-  buffer[1] = length;
-  if (gcov_var.offset >= GCOV_BLOCK_SIZE)
-    gcov_write_block (gcov_var.offset);
+  gcov_nonruntime_assert (current_position >= position + 2 * 
+ GCOV_WORD_SIZE);
+
+  gcov_seek (position + GCOV_WORD_SIZE);  gcov_write_unsigned 
+ (current_position - position - 2 * GCOV_WORD_SIZE);  gcov_seek 
+ (current_position);
 }
 
 #else /* IN_LIBGCOV */
@@ -434,10 +322,8 @@ gcov_write_length (gcov_position_t position)  GCOV_LINKAGE void  gcov_write_tag_length (gcov_unsigned_t tag, gcov_unsigned_t length)  {
-  gcov_unsigned_t *buffer = gcov_write_words (2);
-
-  buffer[0] = tag;
-  buffer[1] = length;
+  gcov_write_unsigned (tag);
+  gcov_write_unsigned (length);
 }
 
 /* Write a summary structure to the gcov file.  Return nonzero on @@ -455,52 +341,28 @@ gcov_write_summary (gcov_unsigned_t tag, const struct gcov_summary *summary)
 
 #endif /*!IN_GCOV */
 
-/* Return a pointer to read BYTES bytes from the gcov file. Returns
+/* Return a pointer to read COUNT bytes from the gcov file.  Returns
    NULL on failure (read past EOF).  */
 
-static const gcov_unsigned_t *
-gcov_read_words (unsigned words)
+static void *
+gcov_read_bytes (void *buffer, unsigned count)
 {
-  const gcov_unsigned_t *result;
-  unsigned excess = gcov_var.length - gcov_var.offset;
-
   if (gcov_var.mode <= 0)
     return NULL;
 
-  if (excess < words)
-    {
-      gcov_var.start += gcov_var.offset;
-      if (excess)
-	{
-#if IN_LIBGCOV
-	  memcpy (gcov_var.buffer, gcov_var.buffer + gcov_var.offset, 4);
-#else
-	  memmove (gcov_var.buffer, gcov_var.buffer + gcov_var.offset,
-		   excess * 4);
-#endif
-	}
-      gcov_var.offset = 0;
-      gcov_var.length = excess;
-#if IN_LIBGCOV
-      excess = GCOV_BLOCK_SIZE;
-#else
-      if (gcov_var.length + words > gcov_var.alloc)
-	gcov_allocate (gcov_var.length + words);
-      excess = gcov_var.alloc - gcov_var.length;
-#endif
-      excess = fread (gcov_var.buffer + gcov_var.length,
-		      1, excess << 2, gcov_var.file) >> 2;
-      gcov_var.length += excess;
-      if (gcov_var.length < words)
-	{
-	  gcov_var.overread += words - gcov_var.length;
-	  gcov_var.length = 0;
-	  return 0;
-	}
-    }
-  result = &gcov_var.buffer[gcov_var.offset];
-  gcov_var.offset += words;
-  return result;
+  unsigned read = fread (buffer, count, 1, gcov_var.file);  if (read != 
+ 1)
+    return NULL;
+
+  return buffer;
+}
+
+/* Read WORDS gcov_unsigned_t values from gcov file.  */
+
+static gcov_unsigned_t *
+gcov_read_words (void *buffer, unsigned words) {
+  return (gcov_unsigned_t *)gcov_read_bytes (buffer, GCOV_WORD_SIZE * 
+words);
 }
 
 /* Read unsigned value from a coverage file. Sets error flag on file @@ -510,10 +372,12 @@ GCOV_LINKAGE gcov_unsigned_t  gcov_read_unsigned (void)  {
   gcov_unsigned_t value;
-  const gcov_unsigned_t *buffer = gcov_read_words (1);
+  gcov_unsigned_t allocated_buffer[1];
+  gcov_unsigned_t *buffer = gcov_read_words (&allocated_buffer, 1);
 
   if (!buffer)
     return 0;
+
   value = from_file (buffer[0]);
   return value;
 }
@@ -525,7 +389,8 @@ GCOV_LINKAGE gcov_type  gcov_read_counter (void)  {
   gcov_type value;
-  const gcov_unsigned_t *buffer = gcov_read_words (2);
+  gcov_unsigned_t allocated_buffer[2];
+  gcov_unsigned_t *buffer = gcov_read_words (&allocated_buffer, 2);
 
   if (!buffer)
     return 0;
@@ -602,7 +467,8 @@ gcov_read_string (void)
   if (!length)
     return 0;
 
-  return (const char *) gcov_read_words (length);
+  void *buffer = XNEWVEC (char *, length);  return (const char *) 
+ gcov_read_bytes (buffer, length);
 }
 #endif
 
@@ -624,27 +490,7 @@ gcov_sync (gcov_position_t base, gcov_unsigned_t length)  {
   gcov_nonruntime_assert (gcov_var.mode > 0);
   base += length;
-  if (base - gcov_var.start <= gcov_var.length)
-    gcov_var.offset = base - gcov_var.start;
-  else
-    {
-      gcov_var.offset = gcov_var.length = 0;
-      fseek (gcov_var.file, base << 2, SEEK_SET);
-      gcov_var.start = ftell (gcov_var.file) >> 2;
-    }
-}
-#endif
-
-#if IN_LIBGCOV
-/* Move to a given position in a gcov file.  */
-
-GCOV_LINKAGE void
-gcov_seek (gcov_position_t base)
-{
-  if (gcov_var.offset)
-    gcov_write_block (gcov_var.offset);
-  fseek (gcov_var.file, base << 2, SEEK_SET);
-  gcov_var.start = ftell (gcov_var.file) >> 2;
+  fseek (gcov_var.file, base, SEEK_SET);
 }
 #endif
 
diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h index 75f16a274c7..c1a0eae4712 100644
--- a/gcc/gcov-io.h
+++ b/gcc/gcov-io.h
@@ -241,22 +241,25 @@ typedef uint64_t gcov_type_unsigned;
 /* The record tags.  Values [1..3f] are for tags which may be in either
    file.  Values [41..9f] for those in the note file and [a1..ff] for
    the data file.  The tag value zero is used as an explicit end of
-   file marker -- it is not required to be present.  */
+   file marker -- it is not required to be present.
+   All length values are in bytes.  */
+
+#define GCOV_WORD_SIZE		4
 
 #define GCOV_TAG_FUNCTION	 ((gcov_unsigned_t)0x01000000)
-#define GCOV_TAG_FUNCTION_LENGTH (3)
+#define GCOV_TAG_FUNCTION_LENGTH (3 * GCOV_WORD_SIZE)
 #define GCOV_TAG_BLOCKS		 ((gcov_unsigned_t)0x01410000)
 #define GCOV_TAG_BLOCKS_LENGTH(NUM) (NUM)
 #define GCOV_TAG_ARCS		 ((gcov_unsigned_t)0x01430000)
-#define GCOV_TAG_ARCS_LENGTH(NUM)  (1 + (NUM) * 2) -#define GCOV_TAG_ARCS_NUM(LENGTH)  (((LENGTH) - 1) / 2)
+#define GCOV_TAG_ARCS_LENGTH(NUM)  (1 + (NUM) * 2 * GCOV_WORD_SIZE) 
+#define GCOV_TAG_ARCS_NUM(LENGTH)  (((LENGTH / GCOV_WORD_SIZE) - 1) / 
+2)
 #define GCOV_TAG_LINES		 ((gcov_unsigned_t)0x01450000)
 #define GCOV_TAG_COUNTER_BASE 	 ((gcov_unsigned_t)0x01a10000)
-#define GCOV_TAG_COUNTER_LENGTH(NUM) ((NUM) * 2) -#define GCOV_TAG_COUNTER_NUM(LENGTH) ((LENGTH) / 2)
+#define GCOV_TAG_COUNTER_LENGTH(NUM) ((NUM) * 2 * GCOV_WORD_SIZE) 
+#define GCOV_TAG_COUNTER_NUM(LENGTH) ((LENGTH / GCOV_WORD_SIZE) / 2)
 #define GCOV_TAG_OBJECT_SUMMARY  ((gcov_unsigned_t)0xa1000000)  #define GCOV_TAG_PROGRAM_SUMMARY ((gcov_unsigned_t)0xa3000000) /* Obsolete */ -#define GCOV_TAG_SUMMARY_LENGTH (2)
+#define GCOV_TAG_SUMMARY_LENGTH (2 * GCOV_WORD_SIZE)
 #define GCOV_TAG_AFDO_FILE_NAMES ((gcov_unsigned_t)0xaa000000)  #define GCOV_TAG_AFDO_FUNCTION ((gcov_unsigned_t)0xac000000)  #define GCOV_TAG_AFDO_WORKING_SET ((gcov_unsigned_t)0xaf000000) diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c index a1338b6e525..02ae265aaa8 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -553,10 +553,8 @@ read_fatal:;
     fn_buffer = free_fn_data (gi_ptr, fn_buffer, GCOV_COUNTERS);
 
   if ((error = gcov_close ()))
-    gcov_error (error  < 0 ?
-		GCOV_PROF_PREFIX "Overflow writing\n" :
-		GCOV_PROF_PREFIX "Error writing\n",
-                gf->filename);
+    gcov_error ((error < 0 ? GCOV_PROF_PREFIX "Overflow writing\n"
+		 : GCOV_PROF_PREFIX "Error writing\n"), gf->filename);
 }
 
 
--
2.31.1
Martin Liška June 17, 2021, 9:37 a.m. UTC | #7
On 6/17/21 3:59 AM, Eugene Rozenfeld wrote:
> |The commit from this patch (https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=23eb66d1d46a34cb28c4acbdf8a1deb80a7c5a05) changed the semantics of gcov_read_string and gcov_write_string. Before this change the string storage was as described in gcov-io.h: "Strings are padded with 1 to 4 NUL bytes, to bring the length up to a multiple of 4. The number of 4 bytes is stored, followed by the padded string." After this change the number before the string indicates the number of bytes (not words) and there is no padding. Was this file format change intentional?

Hello.

Thanks for heads up!

Yes, the change was intentional and I'm going to update documentation entry in gcov-io.h.

> It breaks AutoFDO because create_gcov produces strings in the format specified in gcov-io.h. Thanks, Eugene

Sure, that needs to be adjusted.

Martin
Eugene Rozenfeld June 17, 2021, 4:21 p.m. UTC | #8
Thank you for your reply Martin!

AUTO_PROFILE_VERSION should also be changed. Then create_gcov can be updated to support both the old format and the new format.

Eugene

-----Original Message-----
From: Martin Liška <mliska@suse.cz> 
Sent: Thursday, June 17, 2021 2:38 AM
To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; gcc-patches@gcc.gnu.org
Subject: Re: [EXTERNAL] [PATCH] gcov: Use system IO buffering

On 6/17/21 3:59 AM, Eugene Rozenfeld wrote:
> |The commit from this patch (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fgit%2F%3Fp%3Dgcc.git%3Ba%3Dcommit%3Bh%3D23eb66d1d46a34cb28c4acbdf8a1deb80a7c5a05&amp;data=04%7C01%7CEugene.Rozenfeld%40microsoft.com%7C508d63026ea84be211cc08d9317395bb%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637595194782996821%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=UG%2B41tXMZ94%2Ff80qCnmq%2BtZsFkLXc9NrdWF8KXwPjnk%3D&amp;reserved=0) changed the semantics of gcov_read_string and gcov_write_string. Before this change the string storage was as described in gcov-io.h: "Strings are padded with 1 to 4 NUL bytes, to bring the length up to a multiple of 4. The number of 4 bytes is stored, followed by the padded string." After this change the number before the string indicates the number of bytes (not words) and there is no padding. Was this file format change intentional?

Hello.

Thanks for heads up!

Yes, the change was intentional and I'm going to update documentation entry in gcov-io.h.

> It breaks AutoFDO because create_gcov produces strings in the format specified in gcov-io.h. Thanks, Eugene

Sure, that needs to be adjusted.

Martin
Martin Liška June 22, 2021, 6:56 a.m. UTC | #9
On 6/17/21 6:21 PM, Eugene Rozenfeld wrote:
> Thank you for your reply Martin!
> 
> AUTO_PROFILE_VERSION should also be changed. Then create_gcov can be updated to support both the old format and the new format.

You are right. I've just pushed a commit
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=8819c82ce814a6911e2c1bfebd60b1c2366a3805

Martin

> 
> Eugene
> 
> -----Original Message-----
> From: Martin Liška <mliska@suse.cz>
> Sent: Thursday, June 17, 2021 2:38 AM
> To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; gcc-patches@gcc.gnu.org
> Subject: Re: [EXTERNAL] [PATCH] gcov: Use system IO buffering
> 
> On 6/17/21 3:59 AM, Eugene Rozenfeld wrote:
>> |The commit from this patch (https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fgit%2F%3Fp%3Dgcc.git%3Ba%3Dcommit%3Bh%3D23eb66d1d46a34cb28c4acbdf8a1deb80a7c5a05&amp;data=04%7C01%7CEugene.Rozenfeld%40microsoft.com%7C508d63026ea84be211cc08d9317395bb%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637595194782996821%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=UG%2B41tXMZ94%2Ff80qCnmq%2BtZsFkLXc9NrdWF8KXwPjnk%3D&amp;reserved=0) changed the semantics of gcov_read_string and gcov_write_string. Before this change the string storage was as described in gcov-io.h: "Strings are padded with 1 to 4 NUL bytes, to bring the length up to a multiple of 4. The number of 4 bytes is stored, followed by the padded string." After this change the number before the string indicates the number of bytes (not words) and there is no padding. Was this file format change intentional?
> 
> Hello.
> 
> Thanks for heads up!
> 
> Yes, the change was intentional and I'm going to update documentation entry in gcov-io.h.
> 
>> It breaks AutoFDO because create_gcov produces strings in the format specified in gcov-io.h. Thanks, Eugene
> 
> Sure, that needs to be adjusted.
> 
> Martin
>
diff mbox series

Patch

diff --git a/gcc/gcov-io.c b/gcc/gcov-io.c
index 80c9082a649..bd2316dedab 100644
--- a/gcc/gcov-io.c
+++ b/gcc/gcov-io.c
@@ -27,40 +27,14 @@  see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 /* Routines declared in gcov-io.h.  This file should be #included by
    another source file, after having #included gcov-io.h.  */
 
-#if !IN_GCOV
-static void gcov_write_block (unsigned);
-static gcov_unsigned_t *gcov_write_words (unsigned);
-#endif
-static const gcov_unsigned_t *gcov_read_words (unsigned);
-#if !IN_LIBGCOV
-static void gcov_allocate (unsigned);
-#endif
-
-/* Optimum number of gcov_unsigned_t's read from or written to disk.  */
-#define GCOV_BLOCK_SIZE (1 << 10)
+static gcov_unsigned_t *gcov_read_words (void *buffer, unsigned);
 
 struct gcov_var
 {
   FILE *file;
-  gcov_position_t start;	/* Position of first byte of block */
-  unsigned offset;		/* Read/write position within the block.  */
-  unsigned length;		/* Read limit in the block.  */
-  unsigned overread;		/* Number of words overread.  */
   int error;			/* < 0 overflow, > 0 disk error.  */
-  int mode;	                /* < 0 writing, > 0 reading */
+  int mode;			/* < 0 writing, > 0 reading.  */
   int endian;			/* Swap endianness.  */
-#if IN_LIBGCOV
-  /* Holds one block plus 4 bytes, thus all coverage reads & writes
-     fit within this buffer and we always can transfer GCOV_BLOCK_SIZE
-     to and from the disk. libgcov never backtracks and only writes 4
-     or 8 byte objects.  */
-  gcov_unsigned_t buffer[GCOV_BLOCK_SIZE + 1];
-#else
-  /* Holds a variable length block, as the compiler can write
-     strings and needs to backtrack.  */
-  size_t alloc;
-  gcov_unsigned_t *buffer;
-#endif
 } gcov_var;
 
 /* Save the current position in the gcov file.  */
@@ -71,8 +45,7 @@  static inline
 gcov_position_t
 gcov_position (void)
 {
-  gcov_nonruntime_assert (gcov_var.mode > 0); 
-  return gcov_var.start + gcov_var.offset;
+  return ftell (gcov_var.file);
 }
 
 /* Return nonzero if the error flag is set.  */
@@ -92,20 +65,16 @@  GCOV_LINKAGE inline void
 gcov_rewrite (void)
 {
   gcov_var.mode = -1; 
-  gcov_var.start = 0;
-  gcov_var.offset = 0;
   fseek (gcov_var.file, 0L, SEEK_SET);
 }
 #endif
 
-static inline gcov_unsigned_t from_file (gcov_unsigned_t value)
+static inline gcov_unsigned_t
+from_file (gcov_unsigned_t value)
 {
 #if !IN_LIBGCOV || defined (IN_GCOV_TOOL)
   if (gcov_var.endian)
-    {
-      value = (value >> 16) | (value << 16);
-      value = ((value & 0xff00ff) << 8) | ((value >> 8) & 0xff00ff);
-    }
+    return __builtin_bswap32 (value);
 #endif
   return value;
 }
@@ -140,9 +109,6 @@  gcov_open (const char *name, int mode)
 #endif
 
   gcov_nonruntime_assert (!gcov_var.file);
-  gcov_var.start = 0;
-  gcov_var.offset = gcov_var.length = 0;
-  gcov_var.overread = -1u;
   gcov_var.error = 0;
 #if !IN_LIBGCOV || defined (IN_GCOV_TOOL)
   gcov_var.endian = 0;
@@ -192,8 +158,6 @@  gcov_open (const char *name, int mode)
 
   gcov_var.mode = mode ? mode : 1;
 
-  setbuf (gcov_var.file, (char *)0);
-
   return 1;
 }
 
@@ -205,19 +169,9 @@  gcov_close (void)
 {
   if (gcov_var.file)
     {
-#if !IN_GCOV
-      if (gcov_var.offset && gcov_var.mode < 0)
-	gcov_write_block (gcov_var.offset);
-#endif
       fclose (gcov_var.file);
       gcov_var.file = 0;
-      gcov_var.length = 0;
     }
-#if !IN_LIBGCOV
-  free (gcov_var.buffer);
-  gcov_var.alloc = 0;
-  gcov_var.buffer = 0;
-#endif
   gcov_var.mode = 0;
   return gcov_var.error;
 }
@@ -232,9 +186,8 @@  gcov_magic (gcov_unsigned_t magic, gcov_unsigned_t expected)
 {
   if (magic == expected)
     return 1;
-  magic = (magic >> 16) | (magic << 16);
-  magic = ((magic & 0xff00ff) << 8) | ((magic >> 8) & 0xff00ff);
-  if (magic == expected)
+
+  if (__builtin_bswap32 (magic) == expected)
     {
       gcov_var.endian = 1;
       return -1;
@@ -243,71 +196,15 @@  gcov_magic (gcov_unsigned_t magic, gcov_unsigned_t expected)
 }
 #endif
 
-#if !IN_LIBGCOV
-static void
-gcov_allocate (unsigned length)
-{
-  size_t new_size = gcov_var.alloc;
-
-  if (!new_size)
-    new_size = GCOV_BLOCK_SIZE;
-  new_size += length;
-  new_size *= 2;
-
-  gcov_var.alloc = new_size;
-  gcov_var.buffer = XRESIZEVAR (gcov_unsigned_t, gcov_var.buffer, new_size << 2);
-}
-#endif
-
 #if !IN_GCOV
-/* Write out the current block, if needs be.  */
-
-static void
-gcov_write_block (unsigned size)
-{
-  if (fwrite (gcov_var.buffer, size << 2, 1, gcov_var.file) != 1)
-    gcov_var.error = 1;
-  gcov_var.start += size;
-  gcov_var.offset -= size;
-}
-
-/* Allocate space to write BYTES bytes to the gcov file. Return a
-   pointer to those bytes, or NULL on failure.  */
-
-static gcov_unsigned_t *
-gcov_write_words (unsigned words)
-{
-  gcov_unsigned_t *result;
-
-  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)
-	{
-	  memcpy (gcov_var.buffer, gcov_var.buffer + GCOV_BLOCK_SIZE, 4);
-	}
-    }
-#else
-  if (gcov_var.offset + words > gcov_var.alloc)
-    gcov_allocate (gcov_var.offset + words);
-#endif
-  result = &gcov_var.buffer[gcov_var.offset];
-  gcov_var.offset += words;
-
-  return result;
-}
-
-/* Write unsigned VALUE to coverage file.  Sets error flag
-   appropriately.  */
+/* Write unsigned VALUE to coverage file.  */
 
 GCOV_LINKAGE void
 gcov_write_unsigned (gcov_unsigned_t value)
 {
-  gcov_unsigned_t *buffer = gcov_write_words (1);
-
-  buffer[0] = value;
+  gcov_unsigned_t r = fwrite (&value, sizeof (value), 1, gcov_var.file);
+  if (r != 1)
+    gcov_var.error = 1;
 }
 
 /* Write counter VALUE to coverage file.  Sets error flag
@@ -317,13 +214,11 @@  gcov_write_unsigned (gcov_unsigned_t value)
 GCOV_LINKAGE void
 gcov_write_counter (gcov_type value)
 {
-  gcov_unsigned_t *buffer = gcov_write_words (2);
-
-  buffer[0] = (gcov_unsigned_t) value;
+  gcov_write_unsigned ((gcov_unsigned_t) value);
   if (sizeof (value) > sizeof (gcov_unsigned_t))
-    buffer[1] = (gcov_unsigned_t) (value >> 32);
+    gcov_write_unsigned ((gcov_unsigned_t) (value >> 32));
   else
-    buffer[1] = 0;
+    gcov_write_unsigned (0);
 }
 #endif /* IN_LIBGCOV */
 
@@ -335,23 +230,16 @@  GCOV_LINKAGE void
 gcov_write_string (const char *string)
 {
   unsigned length = 0;
-  unsigned alloc = 0;
-  gcov_unsigned_t *buffer;
 
   if (string)
-    {
-      length = strlen (string);
-      alloc = (length + 4) >> 2;
-    }
-
-  buffer = gcov_write_words (1 + alloc);
+    length = strlen (string) + 1;
 
-  buffer[0] = alloc;
-
-  if (alloc > 0)
+  gcov_write_unsigned (length);
+  if (length > 0)
     {
-      buffer[alloc] = 0; /* place nul terminators.  */
-      memcpy (&buffer[1], string, length);
+      gcov_unsigned_t r = fwrite (string, length, 1, gcov_var.file);
+      if (r != 1)
+	gcov_var.error = 1;
     }
 }
 #endif
@@ -388,6 +276,14 @@  gcov_write_filename (const char *filename)
 }
 #endif
 
+/* Move to a given position in a gcov file.  */
+
+GCOV_LINKAGE void
+gcov_seek (gcov_position_t base)
+{
+  fseek (gcov_var.file, base, SEEK_SET);
+}
+
 #if !IN_LIBGCOV
 /* Write a tag TAG and reserve space for the record length. Return a
    value to be used for gcov_write_length.  */
@@ -395,11 +291,9 @@  gcov_write_filename (const char *filename)
 GCOV_LINKAGE gcov_position_t
 gcov_write_tag (gcov_unsigned_t tag)
 {
-  gcov_position_t result = gcov_var.start + gcov_var.offset;
-  gcov_unsigned_t *buffer = gcov_write_words (2);
-
-  buffer[0] = tag;
-  buffer[1] = 0;
+  gcov_position_t result = gcov_position ();
+  gcov_write_unsigned (tag);
+  gcov_write_unsigned (0);
 
   return result;
 }
@@ -412,19 +306,13 @@  gcov_write_tag (gcov_unsigned_t tag)
 GCOV_LINKAGE void
 gcov_write_length (gcov_position_t position)
 {
-  unsigned offset;
-  gcov_unsigned_t length;
-  gcov_unsigned_t *buffer;
-
+  gcov_position_t current_position = gcov_position ();
   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];
-  buffer[1] = length;
-  if (gcov_var.offset >= GCOV_BLOCK_SIZE)
-    gcov_write_block (gcov_var.offset);
+  gcov_nonruntime_assert (current_position >= position + 2 * GCOV_WORD_SIZE);
+
+  gcov_seek (position + GCOV_WORD_SIZE);
+  gcov_write_unsigned (current_position - position - 2 * GCOV_WORD_SIZE);
+  gcov_seek (current_position);
 }
 
 #else /* IN_LIBGCOV */
@@ -434,10 +322,8 @@  gcov_write_length (gcov_position_t position)
 GCOV_LINKAGE void
 gcov_write_tag_length (gcov_unsigned_t tag, gcov_unsigned_t length)
 {
-  gcov_unsigned_t *buffer = gcov_write_words (2);
-
-  buffer[0] = tag;
-  buffer[1] = length;
+  gcov_write_unsigned (tag);
+  gcov_write_unsigned (length);
 }
 
 /* Write a summary structure to the gcov file.  Return nonzero on
@@ -455,52 +341,28 @@  gcov_write_summary (gcov_unsigned_t tag, const struct gcov_summary *summary)
 
 #endif /*!IN_GCOV */
 
-/* Return a pointer to read BYTES bytes from the gcov file. Returns
+/* Return a pointer to read COUNT bytes from the gcov file.  Returns
    NULL on failure (read past EOF).  */
 
-static const gcov_unsigned_t *
-gcov_read_words (unsigned words)
+static void *
+gcov_read_bytes (void *buffer, unsigned count)
 {
-  const gcov_unsigned_t *result;
-  unsigned excess = gcov_var.length - gcov_var.offset;
-
   if (gcov_var.mode <= 0)
     return NULL;
 
-  if (excess < words)
-    {
-      gcov_var.start += gcov_var.offset;
-      if (excess)
-	{
-#if IN_LIBGCOV
-	  memcpy (gcov_var.buffer, gcov_var.buffer + gcov_var.offset, 4);
-#else
-	  memmove (gcov_var.buffer, gcov_var.buffer + gcov_var.offset,
-		   excess * 4);
-#endif
-	}
-      gcov_var.offset = 0;
-      gcov_var.length = excess;
-#if IN_LIBGCOV
-      excess = GCOV_BLOCK_SIZE;
-#else
-      if (gcov_var.length + words > gcov_var.alloc)
-	gcov_allocate (gcov_var.length + words);
-      excess = gcov_var.alloc - gcov_var.length;
-#endif
-      excess = fread (gcov_var.buffer + gcov_var.length,
-		      1, excess << 2, gcov_var.file) >> 2;
-      gcov_var.length += excess;
-      if (gcov_var.length < words)
-	{
-	  gcov_var.overread += words - gcov_var.length;
-	  gcov_var.length = 0;
-	  return 0;
-	}
-    }
-  result = &gcov_var.buffer[gcov_var.offset];
-  gcov_var.offset += words;
-  return result;
+  unsigned read = fread (buffer, count, 1, gcov_var.file);
+  if (read != 1)
+    return NULL;
+
+  return buffer;
+}
+
+/* Read WORDS gcov_unsigned_t values from gcov file.  */
+
+static gcov_unsigned_t *
+gcov_read_words (void *buffer, unsigned words)
+{
+  return (gcov_unsigned_t *)gcov_read_bytes (buffer, GCOV_WORD_SIZE * words);
 }
 
 /* Read unsigned value from a coverage file. Sets error flag on file
@@ -510,10 +372,12 @@  GCOV_LINKAGE gcov_unsigned_t
 gcov_read_unsigned (void)
 {
   gcov_unsigned_t value;
-  const gcov_unsigned_t *buffer = gcov_read_words (1);
+  gcov_unsigned_t allocated_buffer[1];
+  gcov_unsigned_t *buffer = gcov_read_words (&allocated_buffer, 1);
 
   if (!buffer)
     return 0;
+
   value = from_file (buffer[0]);
   return value;
 }
@@ -525,7 +389,8 @@  GCOV_LINKAGE gcov_type
 gcov_read_counter (void)
 {
   gcov_type value;
-  const gcov_unsigned_t *buffer = gcov_read_words (2);
+  gcov_unsigned_t allocated_buffer[2];
+  gcov_unsigned_t *buffer = gcov_read_words (&allocated_buffer, 2);
 
   if (!buffer)
     return 0;
@@ -602,7 +467,8 @@  gcov_read_string (void)
   if (!length)
     return 0;
 
-  return (const char *) gcov_read_words (length);
+  void *buffer = XNEWVEC (char *, length);
+  return (const char *) gcov_read_bytes (buffer, length);
 }
 #endif
 
@@ -624,27 +490,7 @@  gcov_sync (gcov_position_t base, gcov_unsigned_t length)
 {
   gcov_nonruntime_assert (gcov_var.mode > 0);
   base += length;
-  if (base - gcov_var.start <= gcov_var.length)
-    gcov_var.offset = base - gcov_var.start;
-  else
-    {
-      gcov_var.offset = gcov_var.length = 0;
-      fseek (gcov_var.file, base << 2, SEEK_SET);
-      gcov_var.start = ftell (gcov_var.file) >> 2;
-    }
-}
-#endif
-
-#if IN_LIBGCOV
-/* Move to a given position in a gcov file.  */
-
-GCOV_LINKAGE void
-gcov_seek (gcov_position_t base)
-{
-  if (gcov_var.offset)
-    gcov_write_block (gcov_var.offset);
-  fseek (gcov_var.file, base << 2, SEEK_SET);
-  gcov_var.start = ftell (gcov_var.file) >> 2;
+  fseek (gcov_var.file, base, SEEK_SET);
 }
 #endif
 
diff --git a/gcc/gcov-io.h b/gcc/gcov-io.h
index 75f16a274c7..c1a0eae4712 100644
--- a/gcc/gcov-io.h
+++ b/gcc/gcov-io.h
@@ -241,22 +241,25 @@  typedef uint64_t gcov_type_unsigned;
 /* The record tags.  Values [1..3f] are for tags which may be in either
    file.  Values [41..9f] for those in the note file and [a1..ff] for
    the data file.  The tag value zero is used as an explicit end of
-   file marker -- it is not required to be present.  */
+   file marker -- it is not required to be present.
+   All length values are in bytes.  */
+
+#define GCOV_WORD_SIZE		4
 
 #define GCOV_TAG_FUNCTION	 ((gcov_unsigned_t)0x01000000)
-#define GCOV_TAG_FUNCTION_LENGTH (3)
+#define GCOV_TAG_FUNCTION_LENGTH (3 * GCOV_WORD_SIZE)
 #define GCOV_TAG_BLOCKS		 ((gcov_unsigned_t)0x01410000)
 #define GCOV_TAG_BLOCKS_LENGTH(NUM) (NUM)
 #define GCOV_TAG_ARCS		 ((gcov_unsigned_t)0x01430000)
-#define GCOV_TAG_ARCS_LENGTH(NUM)  (1 + (NUM) * 2)
-#define GCOV_TAG_ARCS_NUM(LENGTH)  (((LENGTH) - 1) / 2)
+#define GCOV_TAG_ARCS_LENGTH(NUM)  (1 + (NUM) * 2 * GCOV_WORD_SIZE)
+#define GCOV_TAG_ARCS_NUM(LENGTH)  (((LENGTH / GCOV_WORD_SIZE) - 1) / 2)
 #define GCOV_TAG_LINES		 ((gcov_unsigned_t)0x01450000)
 #define GCOV_TAG_COUNTER_BASE 	 ((gcov_unsigned_t)0x01a10000)
-#define GCOV_TAG_COUNTER_LENGTH(NUM) ((NUM) * 2)
-#define GCOV_TAG_COUNTER_NUM(LENGTH) ((LENGTH) / 2)
+#define GCOV_TAG_COUNTER_LENGTH(NUM) ((NUM) * 2 * GCOV_WORD_SIZE)
+#define GCOV_TAG_COUNTER_NUM(LENGTH) ((LENGTH / GCOV_WORD_SIZE) / 2)
 #define GCOV_TAG_OBJECT_SUMMARY  ((gcov_unsigned_t)0xa1000000)
 #define GCOV_TAG_PROGRAM_SUMMARY ((gcov_unsigned_t)0xa3000000) /* Obsolete */
-#define GCOV_TAG_SUMMARY_LENGTH (2)
+#define GCOV_TAG_SUMMARY_LENGTH (2 * GCOV_WORD_SIZE)
 #define GCOV_TAG_AFDO_FILE_NAMES ((gcov_unsigned_t)0xaa000000)
 #define GCOV_TAG_AFDO_FUNCTION ((gcov_unsigned_t)0xac000000)
 #define GCOV_TAG_AFDO_WORKING_SET ((gcov_unsigned_t)0xaf000000)
diff --git a/libgcc/libgcov-driver.c b/libgcc/libgcov-driver.c
index a1338b6e525..02ae265aaa8 100644
--- a/libgcc/libgcov-driver.c
+++ b/libgcc/libgcov-driver.c
@@ -553,10 +553,8 @@  read_fatal:;
     fn_buffer = free_fn_data (gi_ptr, fn_buffer, GCOV_COUNTERS);
 
   if ((error = gcov_close ()))
-    gcov_error (error  < 0 ?
-		GCOV_PROF_PREFIX "Overflow writing\n" :
-		GCOV_PROF_PREFIX "Error writing\n",
-                gf->filename);
+    gcov_error ((error < 0 ? GCOV_PROF_PREFIX "Overflow writing\n"
+		 : GCOV_PROF_PREFIX "Error writing\n"), gf->filename);
 }