public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "marxin at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug gcov-profile/94570] -fprofile-dir is broken on Cygwin
Date: Wed, 15 Apr 2020 08:55:14 +0000	[thread overview]
Message-ID: <bug-94570-4-gGa3yQVkFO@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-94570-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94570

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |10walls at gmail dot com

--- Comment #4 from Martin Liška <marxin at gcc dot gnu.org> ---
(In reply to John Selbie from comment #3)
> I am not sure I agree.  But I do defer to your expertise.
> 
> The problem is that Cygwin is not really a DOS_BASED_FILE_SYSTEM.  It's
> emulating Unix - including using forward-slashes everywhere.  So I don't
> know why it's trying to embed backslashes in the code.

All right, so it's normal Unix file system. Can you please show me output
of the 'pwd' command?

> 
> Wouldn't the fix be this:
> 
> @@ -1215,7 +1215,7 @@ coverage_init (const char *filename)
>          of filename in order to prevent file path clashing.  */
>        if (profile_data_prefix)
>         {
> -#if HAVE_DOS_BASED_FILE_SYSTEM
> +#if HAVE_DOS_BASED_FILE_SYSTEM && !CYGWIN
>           const char *separator = "\\";
>  #else
>           const char *separator = "/";
> 
> 
> There's so reason to use backslashes at all, even for native Win32. Windows
> and DOS can't handle forward slashes at the command line, but absolutely can
> handle paths with forward slashes in code.  So an even simpler fix:

But I bet they don't use them as directory separator character. And that's what
we want for concatenation of -profile-dir and an object name.

> 
> @@ -1215,12 +1215,7 @@ coverage_init (const char *filename)
>          of filename in order to prevent file path clashing.  */
>        if (profile_data_prefix)
>         {
> -#if HAVE_DOS_BASED_FILE_SYSTEM
> -         const char *separator = "\\";
> -#else
> -         const char *separator = "/";
> -#endif
> -         filename = concat (getpwd (), separator, filename, NULL);
> +         filename = concat (getpwd (), "/", filename, NULL);
>           filename = mangle_path (filename);
>           len = strlen (filename);
>         }
> 
> 
> But even then I'm skeptical. I had "hacked the binary" with a hex editor to
> change the backslash to a forward slash.  Same error, even though the
> modified path appears on the screen:
> 
>    profiling:profile/#home#jselbie#bench/anyprogram.gcda:Skip

It goes through:

libgcc/libgcov-driver-system.c:
create_file_directory:

static int
create_file_directory (char *filename)
{
#if !defined(TARGET_POSIX_IO) && !defined(_WIN32)
  (void) filename;
  return -1;
#else
  char *s;

  s = filename;

  if (HAS_DRIVE_SPEC(s))
    s += 2;
  if (IS_DIR_SEPARATOR(*s))
    ++s;
  for (; *s != '\0'; s++)
    if (IS_DIR_SEPARATOR(*s))
      {
        char sep = *s;
        *s  = '\0';
...

so again, it uses IS_DIR_SEPARATOR which is:

#if defined(__MSDOS__) || defined(_WIN32) || defined(__OS2__) || defined
(__CYGWIN__)
...
#  define HAS_DRIVE_SPEC(f) HAS_DOS_DRIVE_SPEC (f)
#  define IS_DIR_SEPARATOR(c) IS_DOS_DIR_SEPARATOR (c)
#  define IS_ABSOLUTE_PATH(f) IS_DOS_ABSOLUTE_PATH (f)

That's why you can't only replace strings in the instrumented binary.

You can test the following patch:

diff --git a/include/filenames.h b/include/filenames.h
index 1ed441221ac..773ba4865b3 100644
--- a/include/filenames.h
+++ b/include/filenames.h
@@ -32,7 +32,7 @@ Foundation, Inc., 51 Franklin Street - Fifth Floor, Boston,
MA 02110-1301, USA.
 extern "C" {
 #endif

-#if defined(__MSDOS__) || defined(_WIN32) || defined(__OS2__) || defined
(__CYGWIN__)
+#if defined(__MSDOS__) || defined(_WIN32) || defined(__OS2__)
 #  ifndef HAVE_DOS_BASED_FILE_SYSTEM
 #    define HAVE_DOS_BASED_FILE_SYSTEM 1
 #  endif

I'm adding Cygwin port maintainer, he can help us with the path separators.

> 
> I even hacked it the other way to use forward slashes more consistently. 
> Same thing:
> 
>     profiling:profile\#home#jselbie#bench\anyprogram.gcda:Skip
> 
> So while I think forward-slash consistency is a key.  It's not the only bug
> preventing the -fprofile-dir flag to work.

  parent reply	other threads:[~2020-04-15  8:55 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-12 21:02 [Bug gcov-profile/94570] New: " john at selbie dot com
2020-04-14  6:22 ` [Bug gcov-profile/94570] " marxin at gcc dot gnu.org
2020-04-15  7:47 ` marxin at gcc dot gnu.org
2020-04-15  8:32 ` john at selbie dot com
2020-04-15  8:55 ` marxin at gcc dot gnu.org [this message]
2020-04-15  9:07 ` john at selbie dot com
2020-04-15 10:40 ` marxin at gcc dot gnu.org
2020-04-15 12:24 ` 10walls at gmail dot com
2020-04-15 12:27 ` 10walls at gmail dot com
2020-04-15 12:46 ` 10walls at gmail dot com
2020-04-15 16:11 ` 10walls at gmail dot com
2020-04-16  7:57 ` marxin at gcc dot gnu.org
2020-04-17  7:23 ` cvs-commit at gcc dot gnu.org
2020-04-17  7:26 ` marxin at gcc dot gnu.org
2020-04-20  9:27 ` cvs-commit at gcc dot gnu.org
2020-04-20  9:28 ` marxin at gcc dot gnu.org

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=bug-94570-4-gGa3yQVkFO@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.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).