[Bug other/105335] New: libiberty does not handle script exit codes correctly.

[Bug other/105335] New: libiberty does not handle script exit codes correctly.

[lh_mouse at 126 dot com]

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

            Bug ID: 105335
           Summary: libiberty does not handle script exit codes correctly.
           Product: gcc
           Version: 11.3.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: other
          Assignee: unassigned at gcc dot gnu.org
          Reporter: lh_mouse at 126 dot com
  Target Milestone: ---

Created attachment 52847
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52847&action=edit
proposed patch

A few days ago I was playing with linker plugins. I created a bug in the LTO
plugin that it segfaulted when the process was about to terminate. While I was
attempt to bootstrap GCC with this modified piece of code, it failed during
stage1 with a mysterious message:

    ```
    configure:3566:
/d/lh_mouse/GitHub/MINGW-packages-dev/mingw-w64-gcc-git/src/build-x86_64-w64-mingw32/./gcc/xgcc
   
-B/d/lh_mouse/GitHub/MINGW-packages-dev/mingw-w64-gcc-git/src/build-x86_64-w64-mingw32/./gcc/
-L/mingw64/x86_64-w64-mingw32/lib
    -L/mingw64/lib -isystem /mingw64/x86_64-w64-mingw32/include -isystem
/mingw64/include -B/mingw64/x86_64-w64-mingw32/bin/
    -B/mingw64/x86_64-w64-mingw32/lib/ -isystem
/mingw64/x86_64-w64-mingw32/include -isystem
/mingw64/x86_64-w64-mingw32/sys-include
    -fno-checking -o conftest -g -march=x86-64 -mtune=generic -O2 -pipe  -pipe
conftest.c  >&5
    C:\MSYS2\mingw64\x86_64-w64-mingw32\bin\ld.exe: cannot find -lgcc: No such
file or directory
    C:\MSYS2\mingw64\x86_64-w64-mingw32\bin\ld.exe: cannot find -lgcc_eh: No
such file or directory
    C:\MSYS2\mingw64\x86_64-w64-mingw32\bin\ld.exe: cannot find -lgcc: No such
file or directory
    C:\MSYS2\mingw64\x86_64-w64-mingw32\bin\ld.exe: cannot find -lgcc_eh: No
such file or directory
    configure:3669: error: C compiler cannot create executables
    ```

Later I found that it was because configure couldn't tell whether the stage1
compiler should fail, as it always returned zero, indicating success.

But how? Apparently ld failed with errors, and GCC should not have exited with
zero. I used GDB to dig for a while. There didn't seem anything wrong with GCC.
It was collect2 that returned zero.

collect2 seemed to invoke ld indirectly, so I set a breakpoint on
`GetExitCodeProcess()`. This of course got the exit code of ld (I thought so,
but it was wrong! see below), which was 2816, so it failed, but in liberty
`pex_win32_wait()` took merely its lowest byte and consider it the exit status,
which was zero, which was incorrect.

Later I set a breakpoint on `CreateProcessA()` and realized the aforementioned
process that exited with 2816 was actually created by `spawn_script()`, which
attempted to parse the shebang of `collect-ld`, which should denote `/bin/sh`.
I had a working MSYS2 shell at the moment, and it correctly detected the
segfault and returned 2816 = (11 << 8), indicating the signal 11 = SIGSEGV.


So far we can conclude that there is a bug in libiberty: It only takes the
lowest byte as the child process's exit status. If the child (bash) fails with
2816 then it is mistaken as having exited successfully.

Untested patch attached.

[Bug other/105335] libiberty does not handle script exit codes correctly.

[pinskia at gcc dot gnu.org]

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
The reason for the lower byte only is because posix says exit value is
truncated to char.
This seems like libiberty not knowing that win32 does not truncate to byte
unlike posix.

Note I thought the c standard says the same too.

[Bug other/105335] libiberty does not handle script exit codes correctly.

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu.org

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
The patch probably works but I'm not sure if it's the best we can do since I'm
not familiar with the pex internals.

[Bug other/105335] libiberty does not handle script exit codes correctly.

[lh_mouse at 126 dot com]

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

--- Comment #3 from LIU Hao <lh_mouse at 126 dot com> ---
Windows uses a full-width 32-bit exit status, but there is no standard
convention for them.

1. If a process has exited successfully, the status is zero. This matches
POSIX.
2. If a process has exited due to a fault, the status is an enumeration of
`NTSTATUS`, for example it's `0xC000001D` for illegal instructions, and
`0xC0000005` for invalid memory access, and so on. The guarantee of these codes
is that the lowest byte is never zero.
3. The MSYS2 bash (N.B. I don't know whether they have made such behavior
different from Cygwin, but I suspect not) checks whether a child process exits
successfully. If not, and the `-e` option is passed, it exits with `(SIGNAL <<
8) | (STATUS & 0xFF)`, where `SIGNAL` is the same with Linux, and in the case
of normal exit, `STATUS` is the lowest byte passed to `exit()`, which is also
the same with Linux. It's probably not necessary to make the exit status very
precise for describing the signal for this circumstance.

[Bug other/105335] libiberty does not handle script exit codes correctly.

[ebotcazou at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-04-23
     Ever confirmed|0                           |1
           Severity|normal                      |enhancement
             Status|UNCONFIRMED                 |NEW

--- Comment #4 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
I guess that a change like this needs a lot of testing...