[Bug remote/30770] New: serial.c does not preserve errno correctly

[Bug remote/30770] New: serial.c does not preserve errno correctly

[tromey at sourceware dot org]

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

            Bug ID: 30770
           Summary: serial.c does not preserve errno correctly
           Product: gdb
           Version: HEAD
            Status: NEW
          Severity: normal
          Priority: P2
         Component: remote
          Assignee: unassigned at sourceware dot org
          Reporter: tromey at sourceware dot org
  Target Milestone: ---

Internally at AdaCore there is a test that randomly fails.
The error message differs, though:

Remote communication error.  Target disconnected.: Arg list too long.
Remote communication error.  Target disconnected.: No error.
Remote communication error.  Target disconnected.: Not a directory.

Looking into this, it seems to me that serial.c doesn't
always preserve the correct errno, but remote.c seems to assume
that it does.

For example, ser-base.c:generic_readchar does a lot of work
after do_readchar fails.

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

[Bug remote/30770] serial.c does not preserve errno correctly

[tromey at sourceware dot org]

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

--- Comment #1 from Tom Tromey <tromey at sourceware dot org> ---
I think this isn't really solveable, as the ser-mingw
code doesn't even use errno -- it makes Windows API calls.

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

[Bug remote/30770] serial.c does not preserve errno correctly

[tromey at sourceware dot org]

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

--- Comment #2 from Tom Tromey <tromey at sourceware dot org> ---
I see now that the Windows serial_open implementation just
sets errno, so perhaps that can be done elsewhere.

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

[Bug remote/30770] serial.c does not preserve errno correctly

[tromey at sourceware dot org]

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

--- Comment #3 from Tom Tromey <tromey at sourceware dot org> ---
I have some patches to try to do this, but now I realize
that I've only ever seen this error on Windows hosts.
On Windows I had my patch just do 'errno = EIO' rather than
try to translate Windows error codes, but I think that's
not very good.
Maybe the serial API has to change to throw exceptions instead.

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

[Bug remote/30770] serial.c does not preserve errno correctly

[tromey at sourceware dot org]

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

Tom Tromey <tromey at sourceware dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at sourceware dot org   |tromey at sourceware dot org

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

[Bug remote/30770] serial.c does not preserve errno correctly

[tromey at sourceware dot org]

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

--- Comment #4 from Tom Tromey <tromey at sourceware dot org> ---
https://sourceware.org/pipermail/gdb-patches/2023-September/202385.html

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

[Bug remote/30770] serial.c does not preserve errno correctly

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

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

commit 4dda9cc4b03788d1cf0416b39a3ab3780caf27fd
Author: Tom Tromey <tromey@adacore.com>
Date:   Fri Sep 1 11:04:58 2023 -0600

    Fix latent bug in ser_windows_send_break

    The ClearCommBreak documentation says:

        If the function fails, the return value is zero.

    ser_windows_send_break inverts this check.  This has never been
    noticed because the caller doesn't check the result.

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

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

[Bug remote/30770] serial.c does not preserve errno correctly

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

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

commit 602971b3863dcecf2daa5ffc0853a75a3131446c
Author: Tom Tromey <tromey@adacore.com>
Date:   Fri Sep 1 11:01:33 2023 -0600

    Introduce throw_winerror_with_name

    This introduces throw_winerror_with_name, a Windows analog of
    perror_with_name, and changes various places in gdb to call it.

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

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

[Bug remote/30770] serial.c does not preserve errno correctly

[cvs-commit at gcc dot gnu.org]

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

--- Comment #7 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

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

commit ad3cf8c64e6e4794fc48d28c90f20cbbfdc51ca4
Author: Tom Tromey <tromey@adacore.com>
Date:   Tue Aug 29 07:20:22 2023 -0600

    Change serial_setbaudrate to throw exception

    remote.c has this code:

          if (serial_setbaudrate (rs->remote_desc, baud_rate))
            {
              /* The requested speed could not be set.  Error out to
                 top level after closing remote_desc.  Take care to
                 set remote_desc to NULL to avoid closing remote_desc
                 more than once.  */
              serial_close (rs->remote_desc);
              rs->remote_desc = NULL;
              perror_with_name (name);

    The perror here cannot be correct, because if serial_setbaudrate did
    set errno, it may be obscured by serial_close.

    This patch changes serial_setbaudrate to throw an exception instead.

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

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

[Bug remote/30770] serial.c does not preserve errno correctly

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

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

commit a2e0acea420cca881296c6fcf58920f3d7c05a45
Author: Tom Tromey <tromey@adacore.com>
Date:   Fri Sep 1 12:11:37 2023 -0600

    Change serial "open" functions to throw exception

    remote.c assumes that a failure to open the serial connection will set
    errno.  This is somewhat true, because the Windows code tries to set
    errno appropriately -- but only somewhat, because it isn't clear that
    the "pex" code sets it, and the tcp code seems to do the wrong thing.
    It seems better to simply have the serial open functions throw on
    error.

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

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

[Bug remote/30770] serial.c does not preserve errno correctly

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

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

commit d69939bded50d76179f97284df35879a385cf8c0
Author: Tom Tromey <tromey@adacore.com>
Date:   Wed Sep 6 08:33:46 2023 -0600

    Change serial_send_break and serial_write to throw

    This changes serial_send_break and serial_write to throw exceptions
    rather than attempt to set errno and return an error indicator.  This
    lets us correctly report failures on Windows.

    Both functions had to be converted in a single patch because one
    implementation of send_break works via write.

    This also introduces remote_serial_send_break to handle error checking
    when attempting to send a break.  This was previously ignored.

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

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

[Bug remote/30770] serial.c does not preserve errno correctly

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

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

commit 0da23004a064e7149373b484fa671f2a2105ec9b
Author: Tom Tromey <tromey@adacore.com>
Date:   Tue Aug 29 08:22:09 2023 -0600

    Change serial_readchar to throw

    This changes serial_readchar to throw an exception rather than trying
    to set and preserve errno.

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

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

[Bug remote/30770] serial.c does not preserve errno correctly

[tromey at sourceware dot org]

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

Tom Tromey <tromey at sourceware dot org> changed:

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

--- Comment #11 from Tom Tromey <tromey at sourceware dot org> ---
Fixed.

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

[Bug remote/30770] serial.c does not preserve errno correctly

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from Sourceware Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Tom Tromey <tromey@sourceware.org>:

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

commit 74d8fa2df7e7844f9b1b8fac27fe7d0b82100ab0
Author: СеÑгей ЧеÑнов <klen_s@mail.ru>
Date:   Fri Jan 19 11:01:18 2024 -0700

    Fix remote serial read

    After closing "Bug 30770 - serial.c does not preserve errno correctly"
    https://sourceware.org/bugzilla/show_bug.cgi?id=30770
    remote debugging became impossible due to an attempt to recv() by a call
intended for the socket, and not for the character device file. The
    documentation implicitly states that it is possible to use the read() call
to work with a socket. But this does not mean in the general case that it is
    permissible to use recv in the case of a non-socket.

    condition:
    os: Distributor ID:    Ubuntu
    Description:    Ubuntu 23.10
    Release:    23.10
    Codename:    mantic

    libc:
    ldd (Ubuntu GLIBC 2.38-1ubuntu6) 2.38
    kernel:
    Linux klen-dev-um790pro 6.5.0-14-generic #14-Ubuntu SMP PREEMPT_DYNAMIC Tue
Nov 14 14:59:49 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
    gdb: build from trank at 15.0.50.20231226-git

    GDB output:
    $ arm-kgp-eabi-gdb
    GNU gdb
(Klen's_GNU_package_(KGP)_for_target::arm-kgp-eabi<rmprofile/lto>_host::x86_64-kgp-linux-gnu_znver4-avx512<<ílex>>)
    15.0.50.20231226-git
    ....
    (gdb) tar ext /dev/ttyACM1
    Remote debugging using /dev/ttyACM1
    Remote communication error.  Target disconnected: error while reading:
Socket operation on non-socket.
    (gdb)

    after fix gdb work fine

    $ arm-kgp-eabi-gdb -q
    (gdb) tar ext /dev/ttyACM0
    Remote debugging using /dev/ttyACM0
    (gdb) mon swd
    Target voltage: 0.0V
    Available Targets:
    No. Att Driver
           STM32F40x M4
    (gdb) att 1
    Attaching to Remote target
    warning: No executable has been specified and target does not support
    determining executable automatically.  Try using the "file" command.
    0x08020c80 in ?? ()
    (gdb)

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

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