[Bug gdb/26880] New: hex instead of filename printed in error message

[Bug gdb/26880] New: hex instead of filename printed in error message

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

            Bug ID: 26880
           Summary: hex instead of filename printed in error message
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: gdb
          Assignee: unassigned at sourceware dot org
          Reporter: vries at gcc dot gnu.org
  Target Milestone: ---

With system gdb 8.3.1:
...
$ gdb -batch test.c
"/home/vries/test.c": not in executable format: file format not recognized
...

With trunk gdb:
...
$ gdb -batch test.c
"0x7ffc87bfc8d0s": not in executable format: file format not recognized
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/26880] hex instead of filename printed in error message

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

--- Comment #1 from Tom de Vries <vries at gcc dot gnu.org> ---
Note: while trying to debug this, I ran into PR26881.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/26880] hex instead of filename printed in error message

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |philippe.waroquiers@skynet.
                   |                            |be, tromey at sourceware dot org

--- Comment #2 from Tom de Vries <vries at gcc dot gnu.org> ---
The root cause for this seems to be that error is used with a styled string
(%ps):
...
          error (_("\"%ps\": not in executable format: %s"),
                 styled_string (file_name_style.style (), scratch_pathname),
                 gdb_bfd_errmsg (bfd_get_error (), matching).c_str ());
...
while error does not support that.

There seem to be two occurrences for that:
...
$ find gdb -type f | xargs grep "error .*%ps"
gdb/exec.c:       error (_("\"%ps\": could not open as an executable file:
%s."),
gdb/exec.c:       error (_("\"%ps\": not in executable format: %s"),
...
both coming from:
...
commit a2fedca99c622e1b523046d09f573b06de0207a6
Author: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Date:   Sat Dec 21 10:55:11 2019 +0100

    Implement 'set/show exec-file-mismatch'.
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/26880] hex instead of filename printed in error message

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

--- Comment #3 from Tom de Vries <vries at gcc dot gnu.org> ---
Hmm, how about warning, do we have the same problem there:
...
$ find gdb -type f | xargs grep "warning .*%ps"
gdb/auto-load.c:  warning (_("File \"%ps\" auto-loading has been declined by
your "
gdb/auto-load.c:    warning (_("Couldn't read %s section of %ps"),
gdb/exec.c:           warning (_("loading %ps %s"),
gdb/exec.c:     warning (_("Cannot find section for the entry point of %ps."),
gdb/main.c:    warning (_("%ps is not a directory."),
gdb/top.c:      warning (_("Could not rename %ps to %ps: %s"),
...

Yep, that works, triggering the one in gdb/main.c:
...
$ ./gdb -q -batch -data-directory bla
./gdb: warning: bla is not a directory.
...

I guess it's bound to be confusing when warning supports %ps, and error does
not :)

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/26880] hex instead of filename printed in error message

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

--- Comment #4 from Tom de Vries <vries at gcc dot gnu.org> ---
Tentative patch:
...
diff --git a/gdb/exec.c b/gdb/exec.c
index 26674a1ff95..8dadd69685b 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -454,9 +454,8 @@ exec_file_attach (const char *filename, int from_tty)

       if (!current_program_space->exec_bfd ())
        {
-         error (_("\"%ps\": could not open as an executable file: %s."),
-                styled_string (file_name_style.style (), scratch_pathname),
-                bfd_errmsg (bfd_get_error ()));
+         error (_("\"%s\": could not open as an executable file: %s."),
+                scratch_pathname, bfd_errmsg (bfd_get_error ()));
        }

       /* gdb_realpath_keepfile resolves symlinks on the local
@@ -476,8 +475,8 @@ exec_file_attach (const char *filename, int from_tty)
          /* Make sure to close exec_bfd, or else "run" might try to use
             it.  */
          current_program_space->exec_close ();
-         error (_("\"%ps\": not in executable format: %s"),
-                styled_string (file_name_style.style (), scratch_pathname),
+         error (_("\"%s\": not in executable format: %s"),
+                scratch_pathname,
                 gdb_bfd_errmsg (bfd_get_error (), matching).c_str ());
        }

...

With the patch, we get:
...
$ gdb -batch test.c
"/home/vries/test.c": not in executable format: file format not recognized
...

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/26880] hex instead of filename printed in error message

[philippe.waroquiers at skynet dot be]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

--- Comment #5 from philippe.waroquiers at skynet dot be ---
Patch looks good to me.

Wondering what it would mean to support styled strings in error.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/26880] hex instead of filename printed in error message

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

--- Comment #6 from Tom Tromey <tromey at sourceware dot org> ---
(In reply to philippe.waroquiers from comment #5)

> Wondering what it would mean to support styled strings in error.

The main difficulty is that when the message is constructed,
we don't know if it will be printed to a stream that allows
styling.  However, we do have the machinery needed to strip
the styling when printing, so maybe that would be an option.

IMO the current situation, with different printf styles in
a single code base, is bad and is going to cause problems
again unless we change it at the root.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/26880] hex instead of filename printed in error message

[kuba at kadziolka dot net]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

Jakub Kądziołka <kuba at kadziolka dot net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |kuba at kadziolka dot net

--- Comment #7 from Jakub Kądziołka <kuba at kadziolka dot net> ---
Hello. This is still happening; the bug has since made it into a release.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/26880] hex instead of filename printed in error message

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

--- Comment #8 from Tom de Vries <vries at gcc dot gnu.org> ---
https://sourceware.org/pipermail/gdb-patches/2021-August/181527.html

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/26880] hex instead of filename printed in error message

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

--- Comment #9 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom de Vries <vries@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=a9680e0e54cfd8a12ddb1a583b4acb96dc2bbdbd

commit a9680e0e54cfd8a12ddb1a583b4acb96dc2bbdbd
Author: Tom de Vries <tdevries@suse.de>
Date:   Mon Aug 23 12:08:25 2021 +0200

    [gdb] Fix 'not in executable format' error message

    With trying to load a non-executable file into gdb, we run into PR26880:
    ...
    $ gdb -q -batch test.c
    "0x7ffc87bfc8d0s": not in executable format: \
      file format not recognized
    ...

    The problem is caused by using %ps in combination with the error function
    (note that confusingly, it does work in combination with the warning
    function).

    Fix this by using plain "%s" instead.

    Tested on x86_64-linux.

    gdb/ChangeLog:

    2021-08-22  Tom de Vries  <tdevries@suse.de>

            PR gdb/26880
            * gdb/exec.c (exec_file_attach): Use %s instead of %ps in call to
            error function.

    gdb/testsuite/ChangeLog:

    2021-08-22  Tom de Vries  <tdevries@suse.de>

            PR gdb/26880
            * gdb.base/non-executable.exp: New file.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/26880] hex instead of filename printed in error message

[cvs-commit at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

--- Comment #10 from cvs-commit at gcc dot gnu.org <cvs-commit at gcc dot gnu.org> ---
The gdb-11-branch branch has been updated by Tom de Vries
<vries@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=2840a1862bc1b86ecd77e65608c0b8d819ed6901

commit 2840a1862bc1b86ecd77e65608c0b8d819ed6901
Author: Tom de Vries <tdevries@suse.de>
Date:   Mon Aug 23 21:08:51 2021 +0200

    [gdb] Fix 'not in executable format' error message

    With trying to load a non-executable file into gdb, we run into PR26880:
    ...
    $ gdb -q -batch test.c
    "0x7ffc87bfc8d0s": not in executable format: \
      file format not recognized
    ...

    The problem is caused by using %ps in combination with the error function
    (note that confusingly, it does work in combination with the warning
    function).

    Fix this by using plain "%s" instead.

    Tested on x86_64-linux.

    gdb/ChangeLog:

    2021-08-23  Tom de Vries  <tdevries@suse.de>

            PR gdb/26880
            * gdb/exec.c (exec_file_attach): Use %s instead of %ps in call to
            error function.

    gdb/testsuite/ChangeLog:

    2021-08-23  Tom de Vries  <tdevries@suse.de>

            PR gdb/26880
            * gdb.base/non-executable.exp: New file.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/26880] hex instead of filename printed in error message

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

Tom de Vries <vries at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
   Target Milestone|---                         |11.1
             Status|NEW                         |RESOLVED

--- Comment #11 from Tom de Vries <vries at gcc dot gnu.org> ---
Patch with test-case committed to master and gdb-11-branch, marking
resolved-fixed.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/26880] hex instead of filename printed in error message

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

--- Comment #12 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Jakub Kądziołka from comment #7)
> Hello. This is still happening; the bug has since made it into a release.

Thanks for the ping.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/26880] hex instead of filename printed in error message

[tromey at sourceware dot org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

--- Comment #13 from Tom Tromey <tromey at sourceware dot org> ---
(In reply to Tom Tromey from comment #6)
> (In reply to philippe.waroquiers from comment #5)
> 
> > Wondering what it would mean to support styled strings in error.
> 
> The main difficulty is that when the message is constructed,
> we don't know if it will be printed to a stream that allows
> styling.  However, we do have the machinery needed to strip
> the styling when printing, so maybe that would be an option.

I think the best approach would be to do this, and to reopen
this PR as a feature request to implement this idea.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

[Bug gdb/26880] hex instead of filename printed in error message

[vries at gcc dot gnu.org]

https://sourceware.org/bugzilla/show_bug.cgi?id=26880

--- Comment #14 from Tom de Vries <vries at gcc dot gnu.org> ---
(In reply to Tom Tromey from comment #13)
> (In reply to Tom Tromey from comment #6)
> > (In reply to philippe.waroquiers from comment #5)
> > 
> > > Wondering what it would mean to support styled strings in error.
> > 
> > The main difficulty is that when the message is constructed,
> > we don't know if it will be printed to a stream that allows
> > styling.  However, we do have the machinery needed to strip
> > the styling when printing, so maybe that would be an option.
> 
> I think the best approach would be to do this, and to reopen
> this PR as a feature request to implement this idea.

I think it's better to have a spinoff PR, and keep this PR closed:
- it makes sure the PR only refers to the "hex instead of filename" problem.
- it gives clarity about when the problem was fixed.

Filed spinoff PR28259 - "[gdb] Support %ps in error function".

-- 
You are receiving this mail because:
You are on the CC list for the bug.