Skip to content

GHC build/testsuite failures with process 1.6.13.0 #210

@wz1000

Description

@wz1000
Contributor

There are a couple of warnings with the new refactor in #208 (promoted to errors with -Werror):

libraries/process/cbits/posix/posix_spawn.c:68:1: error:
     error: control reaches end of non-void function [-Werror=return-type]
     }
     ^
   |
68 | }
   | ^
cc1: all warnings being treated as errors
`cc' failed in phase `C Compiler'. (Exit code: 1)
libraries/process/ghc.mk:4: recipe for target 'libraries/process/dist-install/build/cbits/posix/posix_spawn.o' failed
make[1]: *** [libraries/process/dist-install/build/cbits/posix/posix_spawn.o] Error 1
make[1]: *** Waiting for unfinished jobs....
libraries/process/cbits/posix/find_executable.c: In function ‘find_executable’:
libraries/process/cbits/posix/find_executable.c:31:48: error:
     error: ‘__s’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
             const int tmp_len = filename_len + 1 + strlen(path) + 1;
                                                    ^~~~~~~~~~~~
   |
31 |         const int tmp_len = filename_len + 1 + strlen(path) + 1;
   |                                                ^
libraries/process/cbits/posix/find_executable.c:26:14: error:
     note: ‘__s’ was declared here
     static char *find_in_search_path(char *search_path, const char *filename) {
                  ^~~~~~~~~~~~~~~~~~~
   |
26 | static char *find_in_search_path(char *search_path, const char *filename) {
   |              ^
cc1: all warnings being treated as errors
`cc' failed in phase `C Compiler'. (Exit code: 1)
libraries/process/cbits/posix/fork_exec.c: In function ‘do_spawn_fork’:
libraries/process/cbits/posix/fork_exec.c:222:19: error:
     error: suggest parentheses around comparison in operand of ‘&’ [-Werror=parentheses]
             if (flags & RESET_INT_QUIT_HANDLERS != 0) {
                       ^
    |
222 |         if (flags & RESET_INT_QUIT_HANDLERS != 0) {
    |                   ^
libraries/process/cbits/posix/fork_exec.c: In function ‘setup_std_handle_fork’:
libraries/process/cbits/posix/fork_exec.c:89:1: error:
     error: control reaches end of non-void function [-Werror=return-type]
     }
     ^
   |
89 | }
   | ^
cc1: all warnings being treated as errors

In addition, there are two testsuite failures once -Werror is no longer in effect.

T3994 - waitForProcess now fails with UserInterrupt when it is expected to complete without failures

Wrong exit code for T3994(threaded1)(expected 0 , actual 130 )
Stdout ( T3994 ):
start
*** unexpected failure for T3994(threaded1)
main :: IO ()
main = do (_,Just hout,_,p) <- createProcess (proc "./T3994app" ["start", "10000"])
                            { std_out = CreatePipe, create_group = True }
          start <- hGetLine hout
          putStrLn start
          interruptProcessGroupOf p
          t <- myThreadId
               -- timeout
          forkIO $ do
            threadDelay 5000000
            putStrLn "Interrupting a Running Process Failed"
            hFlush stdout
            killThread t
          waitForProcess p
       -- ^^^^^^^^^^^^^^^ this fails with `UserInterrupt`
          putStrLn "end"
          return ()

process010 fails with a benign error message change which can probably be accepted.

+++ "/builds/ghc/ghc/tmp/ghctest-3wgma_aq/test   spaces/libraries/process/tests/process010.run/process010.run.stdout.normalised"	2021-07-12 19:36:34.037437016 +0000
@@ -1,4 +1,4 @@
 ExitSuccess
 ExitFailure 1
-Exc: /non/existent: rawSystem: posix_exec: illegal operation (Inappropriate ioctl for device)
+Exc: /non/existent: rawSystem: posix_exec: does not exist (No such file or directory)
 Done

In general, we need a better story for the GHC tests included in process. Ideally we would be able to run them in process CI as well.

Unfortunately, this means there will need to be another release soon.

/cc @bgamari

Activity

bgamari

bgamari commented on Jul 13, 2021

@bgamari
Contributor

I'm investigating although this is very strange: I cannot confirm the T3993 failure locally.

bgamari

bgamari commented on Jul 13, 2021

@bgamari
Contributor

@wz1000, can you confirm which libc (glibc or musl, and which version) and kernel you are using?

bgamari

bgamari commented on Jul 13, 2021

@bgamari
Contributor

libraries/process/cbits/posix/find_executable.c: In function ‘find_executable’:
libraries/process/cbits/posix/find_executable.c:31:48: error:
error: ‘__s’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

This is very strange. It looks like this __s symbol was introduced by a macro substitution (or similar) from the definition of strlen. Perhaps this is a compiler bug?

libraries/process/cbits/posix/posix_spawn.c:68:1: error:
error: control reaches end of non-void function [-Werror=return-type]

This is also very strange. The switch indeed lacks a default case but this case should be unreachable as hdl->behavior is an enum std_handle_behavior; the switch is total with respect to the variants of this type.

Which gcc version are you compiling with?

process010 fails with a benign error message change which can probably be accepted.

Indeed it looks like we will need to normalise this difference away since it is environment-dependent.

T3994 - waitForProcess now fails with UserInterrupt when it is expected to complete without failures

I have found a configuration where I can reproduce this failure. Thanks @wz1000!

bgamari

bgamari commented on Jul 13, 2021

@bgamari
Contributor

I see what happened here. Patch coming.

bgamari

bgamari commented on Jul 14, 2021

@bgamari
Contributor

libraries/process/cbits/posix/find_executable.c: In function ‘find_executable’:
libraries/process/cbits/posix/find_executable.c:31:48: error:
error: ‘__s’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

I've spent quite a while trying to understand this warning. At this point, I am quite confident that this is a gcc bug which sadly is only exhibited in the compiler used in Debian 9:

  • gcc 4.8.5 doesn't throw the warning
  • gcc 4.9.4 doesn't
  • gcc 6.3.0 throws the warning
  • gcc 6.5.0 doesn't
  • gcc 7.5.0 doesn't
  • gcc 8.4.0 doesn't
  • gcc 9.3.0 doesn't
  • gcc 10.3.0 doesn't
  • gcc 11.1.0 doesn't
  • clang 7.1.0 doesn't

Sadly, I think the only way to avoid this is to disable the uninitialized variable warning in the relevant part program.

bgamari

bgamari commented on Jul 15, 2021

@bgamari
Contributor

N.B. Disabling the warning results in an "unknown warning" warning being thrown in later compiler versions. I'll need to guard the pragma on the compiler version. Oof.

3kyro

3kyro commented on Jul 18, 2021

@3kyro

edit: sorry, it's already reported

andrewthad

andrewthad commented on Jul 18, 2021

@andrewthad

An application I use with process as a dependency fails with:

Building library for process-1.6.13.0..
[1 of 5] Compiling System.Process.Common ( System/Process/Common.hs, dist/build/System/Process/Common.o )
[2 of 5] Compiling System.Process.Posix ( System/Process/Posix.hs, dist/build/System/Process/Posix.o )
[3 of 5] Compiling System.Process.Internals ( System/Process/Internals.hs, dist/build/System/Process/Internals.o )
[4 of 5] Compiling System.Process   ( System/Process.hs, dist/build/System/Process.o )
[5 of 5] Compiling System.Cmd       ( System/Cmd.hs, dist/build/System/Cmd.o )

cbits/posix/find_executable.c:12:10: error:
     fatal error: common.h: No such file or directory
     #include "common.h"
              ^~~~~~~~~~
   |
12 | #include "common.h"
   |          ^

Ubuntu 18.04, GHC 8.6.5. I think it would be beneficial to mark process-1.6.13.0 be marked as deprecated on hackage so that cabal stops trying to use it in build plans.

3kyro

3kyro commented on Jul 18, 2021

@3kyro

Hi @andrewthad, see #211 for this. I believe an update will come shortly

snoyberg

snoyberg commented on Jul 19, 2021

@snoyberg
Collaborator

In the meanwhile, I've marked process-1.6.13.0 as deprecated so that cabal will hopefully not select it going forward.

bgamari

bgamari commented on Jul 19, 2021

@bgamari
Contributor

Yes, 1.6.13.0 is certainly broken. Apologies for the delay, everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

    Development

    Participants

    @snoyberg@bgamari@wz1000@andrewthad@3kyro

    Issue actions

      GHC build/testsuite failures with process 1.6.13.0 · Issue #210 · haskell/process