[Bug ada/104767] New: GNAT.Serial_Communications windows package allows/causes multiple closing of the same windows handle.

[Bug ada/104767] New: GNAT.Serial_Communications windows package allows/causes multiple closing of the same windows handle.

[tornenvi at gmail dot com]

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

            Bug ID: 104767
           Summary: GNAT.Serial_Communications windows package
                    allows/causes multiple closing of the same windows
                    handle.
           Product: gcc
           Version: 10.3.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: ada
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tornenvi at gmail dot com
  Target Milestone: ---

Created attachment 52554
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52554&action=edit
Patch to set handle to -1 on close.

There is an error in the GNAT Ada package GNAT.Serial_Communications, as
implemented in the
file 
  gcc/ada/libgnat/g-sercom__mingw.adb
in the gcc git sources. Which is ultimately installed as
 10.3.1\adainclude\g-sercom.adb
on an windows installation.

This package does not correctly clean up the Port.H handle in the
Close procedure. The end result is that when Connect (or Close) is 
called for a second time it will silently close the previous window
handle again.

If the previous windows handle has been reassigned (this happens often under
windows 10 at least)
then whatever other random device,interface, file I/O will fail when
its handle is unexpectedly closed. This situation can be very difficult to
identify. I Also believe that the Ada implementation may use windows handles
to implement its task system, so this bug can lead to unexpected application
crashes.

The issue was detected in V10.3.1 of GCC but appears to be in the current HEAD
commit 
  ea4911c4fa629a97d22b6e48975d8f1c4e04549d

The attached patch implements the fix.

Note that this error appears to have been introduced when it was changed to
allow for some changes for the Linux implementation in commit
  55d4e2ba076049f88c24011f2f63aa226e6c87a0

Importantly the Linux implementation does not appear to have the same fault,
since it does not attempt to silently Close the serial port on Connect. So the
two implementations are different in behavior.

I would suggest that the package be changed to raise an exception in the
Connect function if the handle is not '-1' instead of silently closing the
handle, but such a change does break backwards compatibility.

[Bug ada/104767] GNAT.Serial_Communications windows package allows/causes multiple closing of the same windows handle.

[tornenvi at gmail dot com]

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

--- Comment #1 from tornenvi at gmail dot com ---
Bug report should be referring to calls to 'Open' instead of
'Connect'.

[Bug ada/104767] GNAT.Serial_Communications windows package allows/causes multiple closing of the same windows handle.

[tornenvi at gmail dot com]

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

--- Comment #2 from tornenvi at gmail dot com ---
Created attachment 52558
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52558&action=edit
Test program to demonstrate problem

Adding test program code gnatserialerrortest2.adb to highlight the problem.

The program requires two serial ports, the comport numbers 
are hard coded for simplicity, testers will have to 
modify the code with the comports for their own test environment.

[Bug ada/104767] [10/11/12 regression] GNAT.Serial_Communications does not properly close the port

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|GNAT.Serial_Communications  |[10/11/12 regression]
                   |windows package             |GNAT.Serial_Communications
                   |allows/causes multiple      |does not properly close the
                   |closing of the same windows |port
                   |handle.                     |
                 CC|                            |ebotcazou at gcc dot gnu.org
   Last reconfirmed|                            |2022-03-24
             Status|UNCONFIRMED                 |NEW
   Target Milestone|---                         |10.4
     Ever confirmed|0                           |1

--- Comment #3 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Confirmed, this happens on Linux too.

[Bug ada/104767] [10/11/12 regression] GNAT.Serial_Communications does not properly close the port

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Eric Botcazou <ebotcazou@gcc.gnu.org>:

https://gcc.gnu.org/g:d937c6e44ba64694c0fc88f40f42390149d1d624

commit r12-7796-gd937c6e44ba64694c0fc88f40f42390149d1d624
Author: Pascal Obry <obry@adacore.com>
Date:   Thu Mar 24 11:30:05 2022 +0100

    Properly reset the port handle when closing

    When the serial port is closed, we need to ensure that the port handle is
    properly reset for it to be detected as closed.

    gcc/ada/
            PR ada/104767
            * libgnat/g-sercom__mingw.adb (Close): Reset port handle to -1.
            * libgnat/g-sercom__linux.adb (Close): Likewise.

[Bug ada/104767] [10/11/12 regression] GNAT.Serial_Communications does not properly close the port

[cvs-commit at gcc dot gnu.org]

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

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Eric Botcazou
<ebotcazou@gcc.gnu.org>:

https://gcc.gnu.org/g:4c649dc71189f20ab8ed09b03a7353299f1022c8

commit r11-9691-g4c649dc71189f20ab8ed09b03a7353299f1022c8
Author: Pascal Obry <obry@adacore.com>
Date:   Thu Mar 24 11:30:05 2022 +0100

    Properly reset the port handle when closing

    When the serial port is closed, we need to ensure that the port handle is
    properly reset for it to be detected as closed.

    gcc/ada/
            PR ada/104767
            * libgnat/g-sercom__mingw.adb (Close): Reset port handle to -1.
            * libgnat/g-sercom__linux.adb (Close): Likewise.

[Bug ada/104767] [10/11/12 regression] GNAT.Serial_Communications does not properly close the port

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Eric Botcazou
<ebotcazou@gcc.gnu.org>:

https://gcc.gnu.org/g:d800a6f6c03e404a5f033299d9fd1c5caa3b7738

commit r10-10511-gd800a6f6c03e404a5f033299d9fd1c5caa3b7738
Author: Pascal Obry <obry@adacore.com>
Date:   Thu Mar 24 11:30:05 2022 +0100

    Properly reset the port handle when closing

    When the serial port is closed, we need to ensure that the port handle is
    properly reset for it to be detected as closed.

    gcc/ada/
            PR ada/104767
            * libgnat/g-sercom__mingw.adb (Close): Reset port handle to -1.
            * libgnat/g-sercom__linux.adb (Close): Likewise.

[Bug ada/104767] [10/11/12 regression] GNAT.Serial_Communications does not properly close the port

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #7 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Thanks for reporting the problem.