Re: [PING**3] [PATCH] Force use of absolute path names for gcov

[Bernd Edlinger <bernd dot edlinger at hotmail dot de>]

On 06/05/17 13:11, Nathan Sidwell wrote:
> +Compile the source files additionally with @option{-fprofile-abs-path}
> +to create absolute path names in the @file{.gcno} files.  This allows
> +@command{gcov} to find the correct sources in projects with multiple
> +directories.
> 
> I think the second sentence could be better.  It's not that the sources 
> are in different directories, it's that the compiler is invoked with 
> different working directories.  How about
>    'This allows @command{gcov} to find the correct sources in projects
>     where compilations occur with different working directories.'
> 
> modify as you see fit.
> 

done.

> gcov_write_filename (const char *filename)
> +{
> +  char buf[1024];
> +  size_t len;
> 
> Ew.  (a) bad fixed length (b) bad stack frame bloat.  Please use malloc 
> & MAX_PATH_LEN (or whatever the right #define is).
> 

MAX_PATH is just a portability nightmare and may or may not be
defined or sometimes is called PATH_MAX etc.

But I see getcwd (NULL, 0) is used in gcov-tools already,
and although that is a posix extension, it seems to be safe to use,
because libiberty provides a fall-back.

So I would go for getcwd(NULL, 0) and xrealloc the result buffer
as needed.

> +  if (profile_abs_path_flag && filename && filename[0]
> 
> Can filename ever be null here?  Can it ever be the empty string?
> 

I am unable to prove that, if the call from coverage.c where
filename comes directly from expand_location and there are
numerous places like:

  if (xloc.file)
    chksum = coverage_checksum_string (chksum, xloc.file);

which tells me that xloc.file might be NULL, typically in
case of UNKNOWN_LOCATION.

I am anxious about empty string here because of the code in
#ifdef HAVE_DOS_BASED_FILE_SYSTEM does not work with empty string,
while the original gcov_write_string works fine with NULL string
and empty string.


> +      if (getcwd (buf, sizeof (buf) - len - 1) != NULL)
> 
> This is going to getcwd on every file (most likely).  Isn't this already 
> available somewhere?
> 
> 

No, at least grep does not see any other place, except in gcov-tool.c :(


So how about this new patch?
Is it OK for trunk?


Bernd.

Attachment: changelog-gcov-abs-path.txt
Description: changelog-gcov-abs-path.txt

Index: gcc/common.opt
===================================================================
--- gcc/common.opt	(revision 248852)
+++ gcc/common.opt	(working copy)
@@ -1969,6 +1969,10 @@ fprofile
 Common Report Var(profile_flag)
 Enable basic program profiling code.
 
+fprofile-abs-path
+Common Report Var(profile_abs_path_flag)
+Generate absolute source path names for gcov.
+
 fprofile-arcs
 Common Report Var(profile_arc_flag)
 Insert arc-based program profiling code.
Index: gcc/coverage.c
===================================================================
--- gcc/coverage.c	(revision 248852)
+++ gcc/coverage.c	(working copy)
@@ -663,7 +663,7 @@ coverage_begin_function (unsigned lineno_checksum,
   gcov_write_unsigned (cfg_checksum);
   gcov_write_string (IDENTIFIER_POINTER
 		     (DECL_ASSEMBLER_NAME (current_function_decl)));
-  gcov_write_string (xloc.file);
+  gcov_write_filename (xloc.file);
   gcov_write_unsigned (xloc.line);
   gcov_write_length (offset);
 
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 248852)
+++ gcc/doc/invoke.texi	(working copy)
@@ -443,6 +443,7 @@ Objective-C and Objective-C++ Dialects}.
 @item Program Instrumentation Options
 @xref{Instrumentation Options,,Program Instrumentation Options}.
 @gccoptlist{-p  -pg  -fprofile-arcs  --coverage  -ftest-coverage @gol
+-fprofile-abs-path @gol
 -fprofile-dir=@var{path}  -fprofile-generate  -fprofile-generate=@var{path} @gol
 -fsanitize=@var{style}  -fsanitize-recover  -fsanitize-recover=@var{style} @gol
 -fasan-shadow-offset=@var{number}  -fsanitize-sections=@var{s1},@var{s2},... @gol
@@ -10694,6 +10695,12 @@ additional @option{-ftest-coverage} option.  You d
 every source file in a program.
 
 @item
+Compile the source files additionally with @option{-fprofile-abs-path}
+to create absolute path names in the @file{.gcno} files.  This allows
+@command{gcov} to find the correct sources in projects where compilations
+occur with different working directories.
+
+@item
 Link your object files with @option{-lgcov} or @option{-fprofile-arcs}
 (the latter implies the former).
 
@@ -10737,6 +10744,13 @@ above for a description of @var{auxname} and instr
 generate test coverage data.  Coverage data matches the source files
 more closely if you do not optimize.
 
+@item -fprofile-abs-path
+@opindex fprofile-abs-path
+Automatically convert relative source file names to absolute path names
+in the @file{.gcno} files.  This allows @command{gcov} to find the correct
+sources in projects where compilations occur with different working
+directories.
+
 @item -fprofile-dir=@var{path}
 @opindex fprofile-dir
 
Index: gcc/gcov-io.c
===================================================================
--- gcc/gcov-io.c	(revision 248852)
+++ gcc/gcov-io.c	(working copy)
@@ -357,6 +357,38 @@ gcov_write_string (const char *string)
 #endif
 
 #if !IN_LIBGCOV
+/* Write FILENAME to coverage file.  Sets error flag on file
+   error, overflow flag on overflow */
+
+GCOV_LINKAGE void
+gcov_write_filename (const char *filename)
+{
+  if (profile_abs_path_flag && filename && filename[0]
+      && !(IS_DIR_SEPARATOR (filename[0])
+#if HAVE_DOS_BASED_FILE_SYSTEM
+	   || filename[1] == ':'
+#endif
+	  ))
+    {
+      char *buf = getcwd (NULL, 0);
+      if (buf != NULL && buf[0])
+	{
+	  size_t len = strlen (buf);
+	  buf = (char*)xrealloc (buf, len + strlen (filename) + 2);
+	  if (!IS_DIR_SEPARATOR (buf[len - 1]))
+	    strcat (buf, "/");
+	  strcat (buf, filename);
+	  gcov_write_string (buf);
+	  free (buf);
+	  return;
+	}
+    }
+
+  gcov_write_string (filename);
+}
+#endif
+
+#if !IN_LIBGCOV
 /* Write a tag TAG and reserve space for the record length. Return a
    value to be used for gcov_write_length.  */
 
Index: gcc/gcov-io.h
===================================================================
--- gcc/gcov-io.h	(revision 248852)
+++ gcc/gcov-io.h	(working copy)
@@ -387,6 +387,7 @@ GCOV_LINKAGE void gcov_write_unsigned (gcov_unsign
 /* Available only in compiler */
 GCOV_LINKAGE unsigned gcov_histo_index (gcov_type value);
 GCOV_LINKAGE void gcov_write_string (const char *);
+GCOV_LINKAGE void gcov_write_filename (const char *);
 GCOV_LINKAGE gcov_position_t gcov_write_tag (gcov_unsigned_t);
 GCOV_LINKAGE void gcov_write_length (gcov_position_t /*position*/);
 #endif
Index: gcc/profile.c
===================================================================
--- gcc/profile.c	(revision 248852)
+++ gcc/profile.c	(working copy)
@@ -954,7 +954,7 @@ output_location (char const *file_name, int line,
     {
       prev_file_name = file_name;
       gcov_write_unsigned (0);
-      gcov_write_string (prev_file_name);
+      gcov_write_filename (prev_file_name);
     }
   if (line_differs)
     {
Index: gcc/testsuite/gcc.misc-tests/gcov-1a.c
===================================================================
--- gcc/testsuite/gcc.misc-tests/gcov-1a.c	(revision 0)
+++ gcc/testsuite/gcc.misc-tests/gcov-1a.c	(working copy)
@@ -0,0 +1,20 @@
+/* Test Gcov basics.  */
+
+/* { dg-options "-fprofile-arcs -ftest-coverage -fprofile-abs-path" } */
+/* { dg-do run { target native } } */
+
+void noop ()
+{
+}
+
+int main ()
+{
+  int i;
+
+  for (i = 0; i < 10; i++)	/* count(11) */
+    noop ();			/* count(10) */
+
+  return 0;			/* count(1) */
+}
+
+/* { dg-final { run-gcov gcov-1a.c } } */