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-boun...@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

Reply via email to