201 Commits

Author SHA1 Message Date
Thiago Macieira
105c05dfb2 QProcess/Unix: restore the signal mask before re-enabling cancellations
That way we will restore the thread's signal mask even if a thread
cancellation is pending and thus forces an immediate stack unwind when
we re-enable them.

Change-Id: I36589f84c5f4d8929129fffdbb77b2020c6fa5a6
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
(cherry picked from commit 7400db0f82632a4bc41c1e6fc628ae4d4c1b9385)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
2024-09-11 03:20:02 +00:00
Alexey Edelev
a4c6635077 Add the missing include of termios.h
The include unlocks the usage of the TIOCNOTTY constant.

Fixes: QTBUG-124642
Change-Id: Ife2475626ca61efe38ea7928a9b61a8aded1701d
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
2024-04-26 01:45:56 +02:00
Thiago Macieira
0f56502fb6 QProcess/Unix: fix close() on invalid file descriptor
Commit 90bc0ad41f9937f9cba801b3166635f6f55e0678 ("QProcess/Unix: add
failChildProcessModifier()") added this line that set childStartedPipe
so that the failChildProcess() callback had something to write to. But
we left it set on exit from QProcessPrivate::startDetached(), which
caused the QProcess destructor to try and close it.

Noticed when debugging the issue for QTBUG-123083.

Pick-to: 6.7 6.7.0
Task-number: QTBUG-123083
Change-Id: I6818d78a57394e37857bfffd17bbc41c8400270f
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
2024-03-12 22:21:13 -07:00
Thiago Macieira
418dcf88f8 QProcess/Unix: fix improper restoration of signal mask and cancel state
By just moving the handling of the child process' desired target
directory below the initialization of either the signal mask and PThread
cancel state, without that "return".

Commit 52ed6af5277100ed5b9a4f4231b94013ce539a2c ("QProcess/Unix: merge
some code from startProcess() and startDetached()") introduced
QChildProcess and merged the functionality of PThreadCancelGuard into
it. But it added that "return;" to the code path failing to opendirfd()
the target directory, meaning that the QChildProcess constructor could
exit without calling disableThreadCancellations(), but the destructor
would still run restoreThreadCancellations() every time the opening
failed. And we have tests for that: setNonExistentWorkingDirectory and
detachedSetNonExistentWorkingDirectory.

For the cancel state, the uninitialized variable we ended up passing to
pthread_setcancelstate() was probably harmless, because the cancellation
state is almost always active and the variable would have been non-zero.
And we don't test pthread cancellation, so we would never notice the
problem.

But commit bd32c7d7055b436b8c33486a5b5ce1c29db77fd4 ("QProcess/Unix:
block all Unix signals between vfork() and exec()") introduced a block
of the Unix signals with the same uninitialized variable problem. Unlike
the PThread cancellation state, the original signal mask would usually
be empty, so the "restoration" would actually mask signals we wanted.
And one such important signal is SIGCHLD, used by QProcess/forkfd when
*not* using vfork semantics. This meant that tests that had a child
process modifier (meaning, they wouldn't use vfork semantics) would end
up timing out because we'd never get the SIGCHLD that told us the child
had exited.

Fixes: QTBUG-123083
Pick-to: 6.7 6.7.0
Change-Id: I1362eb554b97dc012d02eab2dbca90b06728460e
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by:  Alexey Edelev <alexey.edelev@qt.io>
2024-03-12 04:38:57 -07:00
Thiago Macieira
b653e05c92 QProcess: set the error message and state if qt_create_pipe() fails
It can only happen on resource exhaustion, which is an unlikely
situation. That's probably why we didn't handle this case. But let's do
it now.

This necessitated that qt_create_pipe() return with errno/GetLastError()
properly set, so save it and restore around functions that may change it
(like qErrnoWarning()).

Drive-by update of some warning messages.

Testing this is not practical and is fragile, because it requires
causing the resource exhaustion problem.

Pick-to: 6.6 6.7
Change-Id: I76ffba14ece04f24b43efffd17aafe4f11f54c21
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
2024-02-26 23:29:40 -08:00
Thiago Macieira
79e1389fb9 QProcess: align treatment of early start errors
There are a couple of possible runtime errors that could happen before
the state was changed to QProcess::Starting. This aligns the Unix code
with Windows, which has the state transition at the top, and with the
documentation which says we will enter QProcess::Starting state.

Complements commit 956b2495285251e4840ec32885ffa2cfbb7bd79c, repeating
what it did for Unix (removing the overwriting of the error message that
openChannel() sets) on Windows. We also need to ensure cleanup() is
always called.

Pick-to: 6.6 6.7
Change-Id: I76ffba14ece04f24b43efffd17aafdd47f908bf1
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
2024-01-22 21:44:21 -08:00
Thiago Macieira
7c4e271fe7 QProcess/Unix: detect ASan and TSan dynamically
Fixes: QTBUG-117533
Fixes: QTBUG-117954
Task-number: QTBUG-104493
Pick-to: 6.7 6.6
Change-Id: I09c3950e719e4b259bc7fffd1793ee472c5d5a9a
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
2024-01-14 12:45:15 -08:00
Marc Mutz
f96a17225f Rename EINTR_LOOP -> QT_EINTR_LOOP
This non-namespaced macro was defined in a header, and while that
header is private, we shouldn't define non-namespaced macros in our
headers.

The macro also clashed with one of the same name defined in forkfd.c,
which broke unity-builds including the forkfd_qt.cpp TU. This rename
fixes that, too, so we can now remove forkfd_qt.cpp from
NO_UNITY_BUILD_SOURCES.

Pick-to: 6.6 6.5
Change-Id: Ic4bb4e4d7a632ca87905e48913db788a7c202314
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-11-30 22:52:11 +01:00
Marc Mutz
635774fc32 QProcess: work around GCC 13 -Wmaybe-uninitialized in -unity-build
When in a unity-build GCC 13 sees the implementation of
forkfd_wait4(), called dfrom forkfd_wait(), it can prove that there
are paths that do not initialize *info: Those paths are the returns
following fcntl() in system_forkfs_wait() in forkd_linux.c and read()
in forkfd_wait4().

To work around the issue and unbreak unity-builds, initialize info = {}.

Task-number: QTBUG-119081
Pick-to: 6.6 6.5
Change-Id: I1b3504e7f83c766ebccc851233d4c3e677bf2acd
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
2023-11-19 13:10:29 +01:00
Thiago Macieira
a0a29695ab QProcess/Unix: also ban vfork() under TSan
Complements 94ec17436cbdab52ed85e15ea197b538541b14ca for the ASan logic
(52ed6af5277100ed5b9a4f4231b94013ce539a2c for the code change).

Fixes: QTBUG-117533
Pick-to: 6.6
Change-Id: I09c3950e719e4b259bc7fffd1793d59dbdbfe82a
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Jonas Karlsson <jonas.karlsson@qt.io>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-11-02 20:49:17 -07:00
Ahmad Samir
ceee7acf43 qcore_unix: port qt_safe_poll to QDeadlineTimer
Remove qt_poll_msecs() since the "forever" state can be simply expressed
with a QDeadlineTimer::Forever arg, instead of passing a nullptr
timespec, and the negative timeouts treated as "run forever" is also
encapsulated by QDealineTimer.

Use the QDealineTimer(qint64) constructor in the call sites where
the timeout could be negative, so that it creates a Forever timer (the
QDeadlineTimer(chrono::duration) constructor uses
setRemainingTime(duration) which handles negative timeouts by creating
expired timers).

Remove qt_gettime() (and do_gettime()).

Drive-by changes:
- Fix a narrowing conversion warning, qt_make_pollfd() takes an int
- Remove an unused include

Change-Id: I096319af5e191e28c3d39295fb1aafe9d69841e6
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
2023-10-24 18:29:45 +03:00
Giuseppe D'Angelo
91dcc76fc1 QProcess: zero-fill a struct sigaction
Otherwise valgrind (rightfully) complains:

==75693== Syscall param rt_sigaction(act->sa_flags) points to uninitialised byte(s)
==75693==    at 0x597E5F3: __libc_sigaction (libc_sigaction.c:58)
==75693==    by 0x5105CE4: QtVforkSafe::change_sigpipe(void (*)(int)) (qprocess_unix.cpp:145)
==75693==    by 0x51083AA: (anonymous namespace)::QChildProcess::startProcess() const (qprocess_unix.cpp:911)
==75693==    by 0x5106442: (anonymous namespace)::QChildProcess::startProcess(void*) (qprocess_unix.cpp:330)
==75693==    by 0x5A61BB3: clone (clone.S:100)
==75693==    by 0x1FFEFFE3DF: ???
==75693==    by 0x857E86F: ???
==75693==    by 0x1FFEFFE3EF: ???
==75693==    by 0x5102C09: std::tuple_element<0ul, std::tuple<QProcessPrivate::UnixExtras*, std::default_delete<QProcessPrivate::UnixExtras> > >::type const& std::get<0ul, QProcessPrivate::UnixExtras*, std::default_delete<QProcessPrivate::UnixExtras> >(std::tuple<QProcessPrivate::UnixExtras*, std::default_delete<QProcessPrivate::UnixExtras> > const&) (tuple:1796)
==75693==  Address 0x1ffeffe148 is on thread 1's stack
==75693==  in frame #0, created by __libc_sigaction (libc_sigaction.c:43)
==75693==

Change-Id: Ib5a18fb76f2b7c2c1604e73ff64a251a0418c63c
Pick-to: 6.6 6.5
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
2023-09-05 12:28:42 +02:00
Thiago Macieira
271901c5cf QProcess/Unix: add a simple way to reset the UID and GID for the child
This is done as one of the last steps inside QProcess itself, so the
child modifier and all other tasks still run with the parent process'
permissions. On Linux, setting the UID to non-zero will also
automatically clear the effective capabilities(7) set.

This feature is only useful for setuid or setgid applications, so this
commit updates the QCoreApplication::setSetuidAllowed() documentation to
mention the QProcess flag.

Change-Id: I3e3bfef633af4130a03afffd175e940c0668d244
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
2023-07-08 15:03:23 -07:00
Thiago Macieira
13a1995e9d QProcess/Unix: add a few, basic session & terminal management flags
Doing setsid() and disconnecting from the controlling terminal are, in
addition to resetting the standard file descriptors to /dev/null, a
common task that daemons do. These options allow a QProcess to force a
child to be a daemon.

QProcess ensures that the operations are done in the correct order.

Change-Id: I3e3bfef633af4130a03afffd175e9451d2716d7a
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-07-08 15:03:23 -07:00
Ahmad Samir
22c540a66d QProcess: make UnixProcessFlag an enum class
See: https://lists.qt-project.org/pipermail/development/2023-May/043804.html

Not being an enum class looks more of an oversight, in most places usage
of the enumerators was already prefixed with QProcess::UnixProcessFlag.

Change-Id: Ie37d74e0039d3f65f90af560cb85bb11b77ae20c
Pick-to: 6.6
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
2023-07-08 01:08:43 +03:00
Thiago Macieira
bd32c7d705 QProcess/Unix: block all Unix signals between vfork() and exec()
This is similar to and extends the prevention of thread cancellation
introduced by commit ba05af82d3d8b7cbc6e22f93cbf1e3d1575afefe. This
prevents the situation in which a signal gets delivered (usually because
of a crash) and the parent process' handler is run, doing things it
shouldn't between vfork() and execve().

Most C libraries (all that I've investigated) unblock SIGABRT on
abort(), so this doesn't affect them. Likewise, on most OSes, crashes
ignore the signal block and terminate the application -- Darwin appears
to be an exception, but vfork() is not enabled there. Both situations
are tested by terminateInChildProcessModifier().

Task-number: QTBUG-113822
Change-Id: Ib5ce7a497e034ebabb2cfffd17628ca33969b7af
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-06-28 00:16:31 -07:00
Thiago Macieira
bf67518b53 QProcess/Unix: don't set the SIGPIPE signal handler twice
If the user is asking us to ignore it, don't set to the default
behavior.

Change-Id: Ib5ce7a497e034ebabb2cfffd17628562f745358d
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-06-21 11:12:42 -07:00
Thiago Macieira
52ed6af527 QProcess/Unix: merge some code from startProcess() and startDetached()
... into a new local class called QChildProcess. This groups all the
information that the child process will need between the fork()/vfork()
call and the eventual execve(). That currently includes:
- the argv array, including resolving the program name to a path
- the envp array, possibly a null
- the working directory file descriptor
- the disabled thread cancellation state

We also move the fork() and vfork() calls to inside of this class,
eliminating the the nested lambda was passed to vforkfd(). This
duplicates the trick of calling a lambda in the child side of vfork()
now for the non-file descriptor version too.

None of this should have a side effect for the application. You may be
able to tell apart only in system-call tracing tools like strace(1) or
truss(1).

Change-Id: Ib5ce7a497e034ebabb2cfffd176284edfdd71b32
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
2023-06-21 11:12:42 -07:00
Thiago Macieira
94ec17436c QProcess/Unix: disable vfork() under ASan
Pick-to: 6.5 6.6
Change-Id: I443cf0c8a76243eead33fffd1768ee771eca2d56
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
2023-06-21 11:12:42 -07:00
Thiago Macieira
c4a0a76dec QProcess/Unix: fix setting SIGPIPE to SIG_IGN where SIG_DFL was intended
And take the opportunity to clarify what the QtVforkSafe namespace is
doing. Amends commit e71c226d6f188abd811b28d3cb7529343f52d61f.

Pick-to: 6.6
Change-Id: I443cf0c8a76243eead33fffd1767f3fa390a7cdd
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
2023-06-21 11:12:42 -07:00
Thiago Macieira
166e792269 QProcess/FreeBSD: remove the workaround allowing undefined symbols
On FreeBSD, the environ variable doesn't come from libc, but is instead
inserted during linking. See ccf74b592809e0c5a613eff27d6431a4c659e368
(5.6) for more information.

But instead of allowing undefined symbols in QtCore, let's use a weakref
to environ so this one symbol is allowed to be undefined. It won't be,
but the linker doesn't know.

FreeBSD appears to be the only BSD to require this. We used to apply the
same linker option to OpenBSD in Qt 5, but neither the OpenBSD or nor
the NetBSD ports trees[1][2] carry a patch for this, so I don't think
it's necessary.

[1] https://github.com/openbsd/ports/tree/master/x11/qt6/qtbase/patches
[2] https://github.com/NetBSD/pkgsrc/tree/trunk/x11/qt6-qtbase/patches

Pick-to: 6.6
Change-Id: I63b988479db546dabffcfffd17661c839014771a
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-06-16 01:07:33 +00:00
Marc Mutz
30f87c86b4 QProcess/Unix: fix unsafe strncpy() use
GCC 11 complains:

    qprocess_unix.cpp:672:12: error: ‘char* strncpy(char*, const char*, size_t)’ specified bound 508 equals destination size [-Werror=stringop-truncation]
      672 |     strncpy(error.function, description, sizeof(error.function));
          |     ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

And it's correct: if description is longer than
sizeof(error.function), then error.function will not be
NUL-terminated.

While a quick check suggests that the user of the field performs a
qstrnlen(), thus avoiding falling off the unterminated end of
error.function, it's safer to always NUL-terminate. A single added
qDebug() << error.function would already be UB.

Fix by using _q_strncpy(), which is also more efficient, as it doesn't
write 0.5KiB of NULs in the likely case that description is short.

Amends 90bc0ad41f9937f9cba801b3166635f6f55e0678.

Change-Id: If5c2cb80fc4a3c92b8e78b680a635045bb14a30d
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
2023-06-14 07:12:02 +02:00
Thiago Macieira
921bf4a11a QProcess/Unix: capture the child process modifier exception's what()
Change-Id: I5f7f427ded124479baa6fffd175ffb017b6cd13c
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-06-09 17:32:35 -07:00
Thiago Macieira
90bc0ad41f QProcess/Unix: add failChildProcessModifier()
QProcess detects other types of failures from inside the modifier as
successful starts, because the childStartedPipe gets closed without an
error condition getting written. The new method allows a reporting as a
proper failure-to-start.

Added tests for both cases.

[ChangeLog][QtCore][QProcess] Added failChildProcessModifier().

Change-Id: Icfe44ecf285a480fafe4fffd174da2b10306d3c2
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
2023-06-09 17:32:33 -07:00
Thiago Macieira
abd2ffc149 QProcess/Unix: update the close-file-descriptors feature with a minimum
So that one can pass a few extra file descriptors to the child while
still closing all the rest.

strace -f of this test showed on Linux:

[pid 117952] dup3(4, 0, 0)              = 0
[pid 117952] dup3(9, 1, 0)              = 1
[pid 117952] dup3(11, 2, 0)             = 2
[pid 117952] close(12)                  = 0
[pid 117952] dup2(100, 3)               = 3
[pid 117952] close_range(4, 2147483647, 0) = 0
[pid 117952] execve("testUnixProcessParameters/testUnixProcessParameters", ["testUnixProcessParameters/testUn"..., "file-descriptors2", "3", "100"], 0x561793dc87d0 /* 120 vars */ <unfinished ...>

Pick-to: 6.6
Change-Id: I3e3bfef633af4130a03afffd175e984bf50b558d
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
2023-06-09 17:27:48 -07:00
Thiago Macieira
e71c226d6f QProcess/Unix: call the internal version of sigaction()
I've investigated the functions we call in the child side of a vfork()
for implementations that do more than simply place the system
call. Where wrappers exist, they are usually related to handling of Unix
signals or PThread cancellation. The implementations investigated are:
- Bionic (Android)
- FreeBSD
- glibc (Linux)
- MUSL (Linux)
- NetBSD
- OpenBSD

Relating to thread cancellation, NetBSD implements it with an internal
API that does not include Unix signals and Bionic doesn't implement
thread cancellation at all. Their wrapper functions are harmless.

The rest do use Unix signals to implement thread cancellations (called
SIGCANCEL everywhere except OpenBSD, where it's SIGTHR). Therefore, they
all block the application attempts to mask this signal or change its
handler (if they're not buggy). FreeBSD's and MUSL's do some locking in
their implementations[1][2] we really want to bypass, therefore we must
bypass their sigaction() wrappers.

The investigation also showed that the glibc[3] and NetBSD[4] abort()
implementations to be slightly unsafe, but we don't use them
ourselves. We're also adding QProcess::failChildProcessModifier() so
users won't have to resort to abort().

[1] https://github.com/bminor/musl/blob/master/src/signal/sigaction.c
[2] https://github.com/freebsd/freebsd-src/blob/main/lib/libthr/thread/thr_sig.c
[3] https://codebrowser.dev/glibc/glibc/stdlib/abort.c.html
[4] https://github.com/NetBSD/src/blob/trunk/lib/libc/stdlib/abort.c

Task-number: QTBUG-113822
Pick-to: 6.6
Change-Id: I9201d9ecf52f4146bb04fffd17651123800e15a4
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-06-09 17:27:47 -07:00
Thiago Macieira
062b2ac71b QProcess/Unix: reset the signal block if ResetSignalHandlers requested
This amends commit f9c87cfd44bcf4b90cb45354252ef19f647b0469 to reset the
signal block mask too, not just the signal handlers. For this, SIGPIPE
is not treated specially.

Pick-to: 6.6
Change-Id: Ib5ce7a497e034ebabb2cfffd17627289614bf315
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-06-09 17:27:46 -07:00
Amir Masoud Abdol
69b69a7bcc Add [[maybe_unused]] to avoid compilation warnings on Android
When cross compiling for Android, these need to be marked with
[[maybe_unused]] as they might break the build due to Android not having
pthread cancellation feature.

Pick-to: 6.5
Change-Id: I3b3fb58b5e6df8ce37c2d8d9b1b8e8295f0aebb0
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
2023-05-25 08:18:45 +02:00
Thiago Macieira
c5c7712910 QProcess/Unix: allow startDetached() to also use vfork()
The same arguments that applied to the regular process-starting code in
commit e1a787a76ed462e4ed49db78a40c6d7e272182d7 (6.4) apply here too.

Task-number: QTBUG-104493
Change-Id: Icfe44ecf285a480fafe4fffd174da1b05d0054bc
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Marcus Tillmanns <marcus.tillmanns@qt.io>
2023-05-23 18:41:43 -07:00
Thiago Macieira
815bb2a4fb QProcess/Darwin: disable use of vfork() on this OS
The CI is failing running all the startDetached() tests, despite this
passing on my machine (both macOS 12 and 13). We don't know why it's
happening, so we just disable use of vfork(). This doesn't affect the
regular startProcess() because forkfd() requires full fork() in OSes
without pidfd or pdfork, but it does prevent changing startDetached()
behavior, starting with the next commit.

Change-Id: Ib5ce7a497e034ebabb2cfffd1761d91c456f20e0
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
2023-05-23 18:41:41 -07:00
Thiago Macieira
c5221f6be0 QProcess/Linux: add a flag to re-enable the vfork()-like semantics
Commit 29b2fe40dc778ec73da7e5643fcfd8979d8ecebc disabled it by reverting
commit d6bf71123d3ef073f25610345cb5dc920e4fb783. We now add the promised
flag to opt-in. The flag is added to all Unix systems, but it really
only applies to Linux right now.

No ChangeLog because the whole UnixProcessParameters structure is new
and has its own changelog.

Task-number: QTBUG-104493
Task-number: QTBUG-111243
Task-number: QTBUG-111964
Change-Id: Icfe44ecf285a480fafe4fffd174d4effd3382495
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Paul Wicking <paul.wicking@qt.io>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
2023-05-22 10:43:53 -07:00
Thiago Macieira
f9c87cfd44 QProcess/Unix: add setUnixProcessParameters()
This commit adds those three flags that are either frequent enough or
difficult to do: close all file descriptors above stderr and reset the
signal handlers. Setting SIGPIPE to be ignored isn't critical, but is
required when the ResetSignalHandlers flag is used, as this is run
after the user child process modifier.

[ChangeLog][QtCore][QProcess] Added setUnixProcessParameters() function
that can be used to modify certain settings of the child process,
without the need to provide a callback using setChildProcessModifier().

Change-Id: Icfe44ecf285a480fafe4fffd174d0d1d63840403
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-05-22 10:43:50 -07:00
Tor Arne Vestbø
bc4e9e432a Fix unused variable 'FakeErrnoForThrow' [-Wunused-const-variable]
Amends ba05af82d3d8b7cbc6e22f93cbf1e3d1575afefe.

Change-Id: I979d43af3e937edd69b75c9138f1b24ebdc8fac3
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
2023-05-16 21:54:40 +00:00
Thiago Macieira
48b6c8503a QProcess/Unix: enable setChildProcessModifier for startDetached
Do this by making the actual child-execution code common between
startProcess() and startDetached(). It does mean we've moved the chdir()
operation from the child to the grandchild process, though.

[ChangeLog][QtCore][QProcess] The modifier function set with
setChildProcessModifier() will now also be executed when the process is
started with startDetached().

Change-Id: Icfe44ecf285a480fafe4fffd174d9aa57dd7dfff
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-05-15 19:10:03 +02:00
Thiago Macieira
fb40737b0d QProcess/Unix: improve the error message if the child modifier throws
Functionality added for 6.5, but after translatable string freeze.

Change-Id: Icfe44ecf285a480fafe4fffd174d984c5349e0cb
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-05-15 19:10:03 +02:00
Thiago Macieira
ba05af82d3 QProcess/Unix: protect against stack unwinding in the child process stub
There are two types of stack unwinding that can happen on Unix systems:
C++ exceptions and PThread cancellations (on some systems, like Linux,
PThread cancellations can be caught in catch(...) statements). We call a
variety of PThread cancellation functions from inside the child stub,
like close(). To avoid problems, we disable PThread cancellations
completely before fork() or vfork().

The C++ exception case is simpler, because we can be sure of catching
them with the catch (...) statement and simply transform them into an
error message. This is also testable, which the PThread cancellation
isn't.

The error message isn't ideal because we're string-frozen. I'll improve
it for 6.6.

Pick-to: 6.5
Change-Id: Icfe44ecf285a480fafe4fffd174d97a475c93ff1
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-05-15 10:10:03 -07:00
Thiago Macieira
49eb9021e3 QProcess/Unix: use open() + fchdir() to change directories
This means we have more system calls (2 more in the parent), but we
can now detect non-existent or inaccessible directories before fork().

Pick-to: 6.5
Change-Id: Icfe44ecf285a480fafe4fffd174d1003581bff59
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-05-15 00:12:43 +02:00
Thiago Macieira
6b561ccf44 QProcess/Unix: use pid_t for the pid
Qt 5 and earlier versions used to share this member with Windows, where
we needed to store a pointer. We had the Q_PID public type, which was
removed in commit b73d5a0511bed8c3ccc504e74c52a61d4d3749b4 (6.0). That
commit made the QProcess::processId() public API use qint64, which is
fine. But we don't need to store more bits than the OS actually
requires.

This further reduces QProcessPrivate's size to 688 bytes on 64-bit Unix,
with 5 bytes of tail padding.

Pick-to: 6.5
Change-Id: Icfe44ecf285a480fafe4fffd174d3fa9345872c0
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
2023-05-15 00:12:41 +02:00
Thiago Macieira
29b2fe40dc Revert commit "don't ever force fork() instead of forkfd()"
This reverts commit d6bf71123d3ef073f25610345cb5dc920e4fb783 and the
docs from commit 82b75570f099911076ad0e144927862e8e359fbd
("QProcess/Linux: fix file descriptor leak in case of failed child
start").

Despite the title of the commit being reverted, the actual consequence
is slightly different: we always use the forkfd() function, but we
change whether we force the use of the fork() library function by use of
the FFD_USE_FORK flag.

Commit 97645478de3ceffce11f58eab140c4c775e48be5 (5.15) first added the
FFD_USE_FORK flag with a hack to detect whether the setupChild() virtual
might have been overwritten. A configure-time feature to force the flag
was added in commit 2ed99ff5ca338ac02f71c347b1449d4662e6c221 (6.0).
Before the 6.0 release, commit d6bf71123d3ef073f25610345cb5dc920e4fb783
removed the conditional use of FFD_USE_FORK, with the changelog message
saying "pthread_atfork() callbacks are consistently not invoked".

We've also since added vfork()-like behavior. We tried it for Qt 5.15
and reverted shortly afterwards because we had got the memory semantics
wrong. Commit e1a787a76ed462e4ed49db78a40c6d7e272182d7 (6.5) finally got
it right, for Linux, which revealed another set of problems with
functions used in the child process modifier.

Therefore, we're going to make vfork() and clone() usage opt-in if the
child process modifier is active. This commit is the first part:
disabling their use by default. The flag to opt in will come in Qt 6.6.

[ChangeLog][QtCore][QProcess] Reverted a change from Qt 6.0 that made
the childProcessModifier() callback be run in a child created by means
other than a real fork() library call, a situation in which certain
other library functions would be unusable, unreliable, or cause
deadlocks. A flag to opt in to the solution with better performance will
be added to Qt 6.6.

Task-number: QTBUG-104493
Fixes: QTBUG-111243
Fixes: QTBUG-111964
Pick-to: 6.5
Change-Id: Icfe44ecf285a480fafe4fffd174d3e66843e5a29
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
2023-05-15 00:12:41 +02:00
Thiago Macieira
2b98dd7645 QProcessPrivate: repack and reorganize the members
Reduce the number of #ifdef blocks and use quint8 for the enums that
don't need more than 8 bits anyway (none of them do). Plus move the
std::function callback to an indirect block, as most users of QProcess
won't set them and this type is 4 pointers with libstdc++ and libc++.

After this, QProcessPrivate on 64-bit Unix is 688 bytes, of which:
- 392 bytes from QIODevicePrivate
- 295 bytes of own data
- 3x56 bytes per Channel (which have 5 bytes of tail padding each)
- 1 byte of tail padding and no middle padding

Pick-to: 6.5
Change-Id: Icfe44ecf285a480fafe4fffd174d188a0821d060
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
2023-05-15 00:12:39 +02:00
Thiago Macieira
c257e518c1 QProcessPrivate: remove the member variable "crashed"
It was only used to later set exitStatus = CrashExit, so simply do it
early.

Drive-by removal of a magic numeric literal in the middle of the source
code. It's still magic, but at least we avoid accidentally making typos.

Pick-to: 6.5
Change-Id: Icfe44ecf285a480fafe4fffd174d4176a5d87641
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
2023-05-13 20:25:27 +00:00
Thiago Macieira
956b249528 QProcess/Unix: don't overwrite openChannels() error message
Pick-to: 6.5
Change-Id: Icfe44ecf285a480fafe4fffd174d0fa4701b0076
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
2023-04-25 21:53:30 -04:00
Ahmad Samir
7191b8fe38 Fix some narrowing conversion warnings
ioctl call for FIONREAD takes an int arg, it won't work correctly
otherwise. Cast the return of the read() call to int, because it won't
read more than buffSize (which is an int).

Change-Id: I130202a732684257bbb0e79c9358b60a61010c46
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
2023-04-08 13:24:04 +02:00
Edward Welbourne
db5279825c Corelib: s/Q_OS_MAC/Q_OS_DARWIN/wg except for doc and definition
I got tired of being told off by the inanity 'bot for faithfully
reflecting existing #if-ery in new #if-ery. Retain only the
documentation and definition of the deprecated define.

Change-Id: I47f47b76bd239a360f27ae5afe593dfad8746538
Reviewed-by: Ahmad Samir <a.samirh78@gmail.com>
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
2023-03-20 19:18:52 +01:00
Thiago Macieira
82b75570f0 QProcess/Linux: fix file descriptor leak in case of failed child start
If the child failed to start, execChild() would (unnecessarily) store -1
in childStartedPipe[1] after writing the failure message to it and
closing that pipe. This had worked for the previous 20 years of QProcess
existence, because that was run in the child process. However, with 6.5
commit e1a787a76ed462e4ed49db78a40c6d7e272182d7 (cherry-picked to 6.4)
we implemented vfork-like behavior, meaning the child would share memory
with the parent and ran before the parent, so startProcess() failed to
close it:

    // parent
    // close the ends we don't use and make all pipes non-blocking
    qt_safe_close(childStartedPipe[1]);

Also updated the docs to account for the fact that this is a vfork()
environment and, moreover, not using the vfork() function from glibc on
Linux.

[ChangeLog][QtCore][QProcess] Fixed a file descriptor leak in QProcess
when running on Linux 5.4 or later, if the executable being run failed
to start (for example, the file for the executable didn't exist).

Pick-to: 6.4.3 6.4 6.5
Task-number: QTBUG-111243
Change-Id: I7f354474adce419ca6c2fffd17481002e4853cc3
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
2023-03-16 16:26:47 -08:00
Thiago Macieira
e1a787a76e forkfd: implement vfork(2)-like support on Linux
fork() works by implementing Copy-On-Write for all pages that either the
parent or the child process write to. So if the parent process continues
running while the child is between fork(2) and execve(2), then it will
keep causing page faults and requiring the OS to duplicate those pages,
which may be expensive (page table updates, TLB flushes, etc.). This
problem is aggravated if the parent process is multithreaded, as the
simple act of running in the parent will cause those threads' stacks to
cause page faults.

The BSD solution for that was vfork(), which has two differences in
behavior: (1) it blocks the parent from running and (2) it shares memory
with it. But it's always been tricky, so POSIX.1-2001 deprecated it and
2008 removed its definition completely. Still, it is available somewhat
widely, and on Linux that can be achieved with clone(2) and the
CLONE_VFORK and CLONE_VM flags, for those two behaviors respectively.

Because of (2), we can't return from the forkfd() function in the child
(as that would trash the stack in the parent process), so to implement
this functionality vforkfd() adds a callback of the same signature as
glibc's clone(2) wrapper (something that hadn't occurred to me when we
attempted to use CLONE_VFORK last time).

On Linux, (1) is no problem, as clone(2) has native forkfd support. But
on other OSes, forkfd() requires the parent to run before the child
execve()s, in order to save the child PID in the list of children we're
going to handle SIGCHLD for in a non-racy way. Investigating if it is
possible to use vfork() anyway is left as an exercise for the reader.

Matching OpenDCDiag pull request:
https://github.com/opendcdiag/opendcdiag/pull/94

Pick-to: 6.4
Fixes: QTBUG-104493
Change-Id: Id0fb9ab0089845ee8843fffd16fa63c7c6f7dd1c
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
Reviewed-by: Milian Wolff <milian.wolff@kdab.com>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
2022-07-16 10:50:47 -07:00
Thiago Macieira
5e60b2ab16 QProcess: re-enable pdfork() on FreeBSD
The forkfd_pidfd is a Linux feature, but we ended up disabling the
equivalent functionality on FreeBSD.

Pick-to: 6.3 6.4
Change-Id: I6d3880c7d99d4fc494c8fffd16fabfbc38865f94
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
2022-06-27 17:39:35 -07:00
Lucie Gérard
05fc3aef53 Use SPDX license identifiers
Replace the current license disclaimer in files by
a SPDX-License-Identifier.
Files that have to be modified by hand are modified.
License files are organized under LICENSES directory.

Task-number: QTBUG-67283
Change-Id: Id880c92784c40f3bbde861c0d93f58151c18b9f1
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Lars Knoll <lars.knoll@qt.io>
Reviewed-by: Jörg Bornemann <joerg.bornemann@qt.io>
2022-05-16 16:37:38 +02:00
Marc Mutz
80b6bcc385 Short live Q_CONSTINIT!
It expands to the first available of

- constinit (C++20)
- [[clang::require_constant_initialization]] (Clang)
- __constinit (GCC >= 10)

Use it around the code (on and near static QBasicAtomic; this patch
makes no attempt to find all statics in qtbase).

[ChangeLog][QtCore][QtGlobal] Added macro Q_CONSTINIT.

Fixes: QTBUG-100484
Change-Id: I11e0363a7acb3464476859d12ec7f94319d82be7
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
2022-03-26 17:15:02 +01:00
Sona Kurazyan
b625195893 QtCore: Replace remaining uses of QLatin1String with QLatin1StringView
Task-number: QTBUG-98434
Change-Id: Ib7c5fc0aaca6ef33b93c7486e99502c555bf20bc
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Marc Mutz <marc.mutz@qt.io>
2022-03-26 01:44:05 +01:00