From: Bernd Edlinger <bernd.edlinger@hotmail.de>
To: Nathan Sidwell <nathan@acm.org>, Joseph Myers <joseph@codesourcery.com>
Cc: "gcc-patches@gcc.gnu.org" <gcc-patches@gcc.gnu.org>,
Jan Hubicka <hubicka@ucw.cz>
Subject: Re: [PING**3] [PATCH] Force use of absolute path names for gcov
Date: Mon, 05 Jun 2017 16:31:00 -0000 [thread overview]
Message-ID: <AM4PR0701MB2162C57E13155CDC39E5D841E4CA0@AM4PR0701MB2162.eurprd07.prod.outlook.com> (raw)
In-Reply-To: <8480b83a-221a-9dbc-691f-cbecbb0d08d4@acm.org>
[-- Attachment #1: Type: text/plain, Size: 2249 bytes --]
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 #2: changelog-gcov-abs-path.txt --]
[-- Type: text/plain, Size: 498 bytes --]
gcc:
2017-04-21 Bernd Edlinger <bernd.edlinger@hotmail.de>
* doc/invoke.texi: Document the -fprofile-abs-path option.
* common.opt (fprofile-abs-path): New option.
* gcov-io.h (gcov_write_filename): Declare.
* gcov-io.c (gcov_write_filename): New function.
* coverage.c (coverage_begin_function): Use gcov_write_filename.
* profile.c (output_location): Likewise.
gcc/testsuite:
2017-04-21 Bernd Edlinger <bernd.edlinger@hotmail.de>
* gcc.misc-tests/gcov-1a.c: New test.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch-gcov-abs-path.diff --]
[-- Type: text/x-patch; name="patch-gcov-abs-path.diff", Size: 5549 bytes --]
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 } } */
next prev parent reply other threads:[~2017-06-05 16:31 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-21 18:51 Bernd Edlinger
2017-04-21 20:27 ` Joseph Myers
2017-04-21 20:57 ` Bernd Edlinger
2017-04-28 18:14 ` [PING] " Bernd Edlinger
2017-05-12 16:48 ` [PING**2] " Bernd Edlinger
2017-06-01 15:59 ` [PING**3] " Bernd Edlinger
2017-06-01 17:52 ` Nathan Sidwell
2017-06-01 19:24 ` Bernd Edlinger
2017-06-02 11:35 ` Nathan Sidwell
2017-06-02 14:43 ` Bernd Edlinger
2017-06-05 11:11 ` Nathan Sidwell
2017-06-05 16:31 ` Bernd Edlinger [this message]
2017-06-05 19:00 ` Nathan Sidwell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=AM4PR0701MB2162C57E13155CDC39E5D841E4CA0@AM4PR0701MB2162.eurprd07.prod.outlook.com \
--to=bernd.edlinger@hotmail.de \
--cc=gcc-patches@gcc.gnu.org \
--cc=hubicka@ucw.cz \
--cc=joseph@codesourcery.com \
--cc=nathan@acm.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).