contrib/kyua: Merge vendor import

* commit '5f174897f67783925f4ec69122673f9bad6ee6fe':
  vendor/kyua: Update to snapshot 84c8ec8
  Vendor import of freebsd/kyua@a0d44bb356
Approved by:    markj
Differential Revision:  https://reviews.freebsd.org/D43475

contrib/kyua/.cirrus.yml

@@ -0,0 +1,25 @@
+env:
+  ARCH: amd64
+
+task:
+  matrix:
+    - name: 14.0-RELEASE
+      freebsd_instance:
+        image_family: freebsd-14-0
+    - name: 14.0-STABLE
+      freebsd_instance:
+        image_family: freebsd-14-0-snap
+    - name: 13.2-RELEASE
+      freebsd_instance:
+        image_family: freebsd-13-2
+    - name: 13.2-STABLE
+      freebsd_instance:
+        image_family: freebsd-13-2-snap
+  env:
+    DO: distcheck
+  install_script:
+    - sed -i.bak -e 's,pkg+http://pkg.FreeBSD.org/\${ABI}/quarterly,pkg+http://pkg.FreeBSD.org/\${ABI}/latest,' /etc/pkg/FreeBSD.conf
+    - ASSUME_ALWAYS_YES=yes pkg bootstrap -f
+    - pkg install -y autoconf automake atf lutok pkgconf sqlite3
+  script:
+    - ./admin/travis-build.sh

contrib/kyua/AUTHORS

@@ -8,4 +8,5 @@
 #     * Name <email address>
 #     * Organization <optional email address>
 
+* The FreeBSD Foundation
 * Google Inc.

contrib/kyua/admin/travis-build.sh

@@ -61,7 +61,7 @@ syntax(2)
 parallelism = 4
 EOF
     [ "${UNPRIVILEGED_USER:-no}" = no ] || \
-        echo "unprivileged_user = 'travis'" >>kyua.conf
+        echo "unprivileged_user = 'nobody'" >>kyua.conf
 
     local f=
     f="${f} CFLAGS='${archflags}'"

contrib/kyua/cli/cmd_report_html.cpp

@@ -54,6 +54,7 @@
 #include "utils/fs/operations.hpp"
 #include "utils/fs/path.hpp"
 #include "utils/optional.ipp"
+#include "utils/text/operations.hpp"
 #include "utils/text/templates.hpp"
 
 namespace cmdline = utils::cmdline;
@@ -365,12 +366,12 @@ class html_hooks : public drivers::scan_results::base_hooks {
         {
             const std::string stdout_text = iter.stdout_contents();
             if (!stdout_text.empty())
-                templates.add_variable("stdout", stdout_text);
+                templates.add_variable("stdout", text::escape_xml(stdout_text));
         }
         {
             const std::string stderr_text = iter.stderr_contents();
             if (!stderr_text.empty())
-                templates.add_variable("stderr", stderr_text);
+                templates.add_variable("stderr", text::escape_xml(stderr_text));
         }
 
         generate(templates, "test_result.html",

contrib/kyua/doc/kyua.1.in

@@ -392,6 +392,11 @@ codes above 1 can be returned.
 .Xr atf 7 ,
 .Xr tests 7
 .Sh AUTHORS
+The original author of
+.Nm
+was
+.An Julio Merino .
+.Pp
 For more details on the people that made
 .Nm
 possible and the license terms, run:

contrib/kyua/integration/utils.sh

@@ -42,7 +42,7 @@
 # script that automatically does this.
 # CHECK_STYLE_DISABLE
 utils_strip_times='sed -E \
-    -e "s,( |\[|\")[0-9][0-9]*.[0-9][0-9][0-9](s]|s|\"),\1S.UUU\2,g" \
+    -e "s,( |\[|\")[0-9][0-9]*\.[0-9][0-9][0-9](s]|s|\"),\1S.UUU\2,g" \
     -e "s,[0-9]{8}-[0-9]{6}-[0-9]{6},YYYYMMDD-HHMMSS-ssssss,g" \
     -e "s,[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}Z,YYYY-MM-DDTHH:MM:SS.ssssssZ,g"'
 # CHECK_STYLE_ENABLE
@@ -55,7 +55,7 @@ utils_strip_times='sed -E \
 # replaced by the fixed string YYYY-MM-DDTHH:MM:SS.ssssssZ.
 # CHECK_STYLE_DISABLE
 utils_strip_times_but_not_ids='sed -E \
-    -e "s,( |\[|\")[0-9][0-9]*.[0-9][0-9][0-9](s]|s|\"),\1S.UUU\2,g" \
+    -e "s,( |\[|\")[0-9][0-9]*\.[0-9][0-9][0-9](s]|s|\"),\1S.UUU\2,g" \
     -e "s,[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}\.[0-9]{6}Z,YYYY-MM-DDTHH:MM:SS.ssssssZ,g"'
 # CHECK_STYLE_ENABLE
 

contrib/kyua/utils/datetime.cpp

@@ -590,11 +590,12 @@ datetime::timestamp::operator-=(const datetime::delta& other)
 datetime::delta
 datetime::timestamp::operator-(const datetime::timestamp& other) const
 {
-    if ((*this) < other) {
-        throw std::runtime_error(
-            F("Cannot subtract %s from %s as it would result in a negative "
-              "datetime::delta, which are not supported") % other % (*this));
-    }
+    /*
+     * XXX-BD: gettimeofday isn't necessarily monotonic so return the
+     * smallest non-zero delta if time went backwards.
+     */
+    if ((*this) < other)
+        return datetime::delta::from_microseconds(1);
     return datetime::delta::from_microseconds(to_microseconds() -
                                               other.to_microseconds());
 }

contrib/kyua/utils/datetime_test.cpp

@@ -532,11 +532,11 @@ ATF_TEST_CASE_BODY(timestamp__subtraction)
     ATF_REQUIRE_EQ(datetime::delta(100, 0), ts3 - ts1);
     ATF_REQUIRE_EQ(datetime::delta(99, 999988), ts3 - ts2);
 
-    ATF_REQUIRE_THROW_RE(
-        std::runtime_error,
-        "Cannot subtract 1291970850123456us from 1291970750123468us "
-        ".*negative datetime::delta.*not supported",
-        ts2 - ts3);
+    /*
+     * NOTE (ngie): behavior change for
+     * https://github.com/jmmv/kyua/issues/155 .
+     */
+    ATF_REQUIRE_EQ(datetime::delta::from_microseconds(1), ts2 - ts3);
 }
 
 

contrib/kyua/utils/process/Kyuafile

@@ -6,6 +6,7 @@ atf_test_program{name="child_test"}
 atf_test_program{name="deadline_killer_test"}
 atf_test_program{name="exceptions_test"}
 atf_test_program{name="executor_test"}
+atf_test_program{name="executor_pid_test"}
 atf_test_program{name="fdstream_test"}
 atf_test_program{name="isolation_test"}
 atf_test_program{name="operations_test"}

contrib/kyua/utils/process/Makefile.am.inc

@@ -83,6 +83,11 @@ utils_process_executor_test_SOURCES = utils/process/executor_test.cpp
 utils_process_executor_test_CXXFLAGS = $(UTILS_CFLAGS) $(ATF_CXX_CFLAGS)
 utils_process_executor_test_LDADD = $(UTILS_LIBS) $(ATF_CXX_LIBS)
 
+tests_utils_process_PROGRAMS += utils/process/executor_pid_test
+utils_process_executor_pid_test_SOURCES = utils/process/executor_pid_test.cpp
+utils_process_executor_pid_test_CXXFLAGS = $(UTILS_CFLAGS) $(ATF_CXX_CFLAGS)
+utils_process_executor_pid_test_LDADD = $(UTILS_LIBS) $(ATF_CXX_LIBS)
+
 tests_utils_process_PROGRAMS += utils/process/fdstream_test
 utils_process_fdstream_test_SOURCES = utils/process/fdstream_test.cpp
 utils_process_fdstream_test_CXXFLAGS = $(UTILS_CFLAGS) $(ATF_CXX_CFLAGS)

contrib/kyua/utils/process/executor.cpp

@@ -39,10 +39,12 @@ extern "C" {
 #include <signal.h>
 }
 
+#include <forward_list>
 #include <fstream>
 #include <map>
 #include <memory>
 #include <stdexcept>
+#include <utility>
 
 #include "utils/datetime.hpp"
 #include "utils/format/macros.hpp"
@@ -123,7 +125,7 @@ utils::process::executor::detail::setup_child(
 }
 
 
-/// Internal implementation for the exit_handle class.
+/// Internal implementation for the exec_handle class.
 struct utils::process::executor::exec_handle::impl : utils::noncopyable {
     /// PID of the process being run.
     int pid;
@@ -547,6 +549,9 @@ struct utils::process::executor::executor_handle::impl : utils::noncopyable {
     /// Mapping of PIDs to the data required at run time.
     exec_handles_map all_exec_handles;
 
+    /// Former members of all_exec_handles removed due to PID reuse.
+    std::forward_list<exec_handle> stale_exec_handles;
+
     /// Whether the executor state has been cleaned yet or not.
     ///
     /// Used to keep track of explicit calls to the public cleanup().
@@ -558,6 +563,8 @@ struct utils::process::executor::executor_handle::impl : utils::noncopyable {
         interrupts_handler(new signals::interrupts_handler()),
         root_work_directory(new fs::auto_directory(
             fs::auto_directory::mkdtemp_public(work_directory_template))),
+        all_exec_handles(),
+        stale_exec_handles(),
         cleaned(false)
     {
     }
@@ -603,6 +610,17 @@ struct utils::process::executor::executor_handle::impl : utils::noncopyable {
         }
         all_exec_handles.clear();
 
+        for (auto iter : stale_exec_handles) {
+            // The process already exited, so no need to kill and wait.
+            try {
+                fs::rm_r(iter.control_directory());
+            } catch (const fs::error& e) {
+                LE(F("Failed to clean up stale subprocess work directory "
+                    "%s: %s") % iter.control_directory() % e.what());
+            }
+        }
+        stale_exec_handles.clear();
+
         try {
             // The following only causes the work directory to be deleted, not
             // any of its contents, so we expect this to always succeed.  This
@@ -611,9 +629,9 @@ struct utils::process::executor::executor_handle::impl : utils::noncopyable {
             // subprocesses.
             root_work_directory->cleanup();
         } catch (const fs::error& e) {
-            LE(F("Failed to clean up executor work directory %s: %s; this is "
-                 "an internal error") % root_work_directory->directory()
-               % e.what());
+            LE(F("Failed to clean up executor work directory %s: %s; "
+                "this could be an internal error or a buggy test") %
+                root_work_directory->directory() % e.what());
         }
         root_work_directory.reset(NULL);
 
@@ -773,12 +791,16 @@ executor::executor_handle::spawn_post(
             timeout,
             unprivileged_user,
             detail::refcnt_t(new detail::refcnt_t::element_type(0)))));
-    INV_MSG(_pimpl->all_exec_handles.find(handle.pid()) ==
-            _pimpl->all_exec_handles.end(),
-            F("PID %s already in all_exec_handles; not properly cleaned "
-              "up or reused too fast") % handle.pid());;
-    _pimpl->all_exec_handles.insert(exec_handles_map::value_type(
-        handle.pid(), handle));
+    const auto value = exec_handles_map::value_type(handle.pid(), handle);
+    auto insert_pair = _pimpl->all_exec_handles.insert(value);
+    if (!insert_pair.second) {
+        LI(F("PID %s already in all_exec_handles") % handle.pid());
+        _pimpl->stale_exec_handles.push_front(insert_pair.first->second);
+        _pimpl->all_exec_handles.erase(insert_pair.first);
+        insert_pair = _pimpl->all_exec_handles.insert(value);
+        INV_MSG(insert_pair.second, F("PID %s still in all_exec_handles") %
+            handle.pid());
+    }
     LI(F("Spawned subprocess with exec_handle %s") % handle.pid());
     return handle;
 }
@@ -816,12 +838,16 @@ executor::executor_handle::spawn_followup_post(
             timeout,
             base.unprivileged_user(),
             base.state_owners())));
-    INV_MSG(_pimpl->all_exec_handles.find(handle.pid()) ==
-            _pimpl->all_exec_handles.end(),
-            F("PID %s already in all_exec_handles; not properly cleaned "
-              "up or reused too fast") % handle.pid());;
-    _pimpl->all_exec_handles.insert(exec_handles_map::value_type(
-        handle.pid(), handle));
+    const auto value = exec_handles_map::value_type(handle.pid(), handle);
+    auto insert_pair = _pimpl->all_exec_handles.insert(value);
+    if (!insert_pair.second) {
+        LI(F("PID %s already in all_exec_handles") % handle.pid());
+        _pimpl->stale_exec_handles.push_front(insert_pair.first->second);
+        _pimpl->all_exec_handles.erase(insert_pair.first);
+        insert_pair = _pimpl->all_exec_handles.insert(value);
+        INV_MSG(insert_pair.second, F("PID %s still in all_exec_handles") %
+            handle.pid());
+    }
     LI(F("Spawned subprocess with exec_handle %s") % handle.pid());
     return handle;
 }

contrib/kyua/utils/process/executor_pid_test.cpp

@@ -0,0 +1,208 @@
+/*-
+ * SPDX-License-Identifier: BSD-2-Clause
+ *
+ * Copyright (c) 2022 Dell Inc.
+ * Author: Eric van Gyzen
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
+ * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
+ * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#if 0
+
+1. Run some "bad" tests that prevent kyua from removing the work directory.
+   We use "chflags uunlink".  Mounting a file system from an md(4) device
+   is another common use case.
+2. Fork a lot, nearly wrapping the PID number space, so step 3 will re-use
+   a PID from step 1.  Running the entire FreeBSD test suite is a more
+   realistic scenario for this step.
+3. Run some more tests.  If the stars align, the bug is not fixed yet, and
+   kyua is built with debugging, kyua will abort with the following messages.
+   Without debugging, the tests in step 3 will reuse the context from step 1,
+   including stdout, stderr, and working directory, which are still populated
+   with stuff from step 1.  When I found this bug, step 3 was
+   __test_cases_list__, which expects a certain format in stdout and failed
+   when it found something completely unrelated.
+4. You can clean up with: chflags -R nouunlink /tmp/kyua.*; rm -rf /tmp/kyua.*
+
+$ cc -o pid_wrap -latf-c pid_wrap.c
+$ kyua test
+pid_wrap:leak_0  ->  passed  [0.001s]
+pid_wrap:leak_1  ->  passed  [0.001s]
+pid_wrap:leak_2  ->  passed  [0.001s]
+pid_wrap:leak_3  ->  passed  [0.001s]
+pid_wrap:leak_4  ->  passed  [0.001s]
+pid_wrap:leak_5  ->  passed  [0.001s]
+pid_wrap:leak_6  ->  passed  [0.001s]
+pid_wrap:leak_7  ->  passed  [0.001s]
+pid_wrap:leak_8  ->  passed  [0.001s]
+pid_wrap:leak_9  ->  passed  [0.001s]
+pid_wrap:pid_wrap  ->  passed  [1.113s]
+pid_wrap:pid_wrap_0  ->  passed  [0.001s]
+pid_wrap:pid_wrap_1  ->  passed  [0.001s]
+pid_wrap:pid_wrap_2  ->  passed  [0.001s]
+pid_wrap:pid_wrap_3  ->  *** /usr/src/main/contrib/kyua/utils/process/executor.cpp:779: Invariant check failed: PID 60876 already in all_exec_handles; not properly cleaned up or reused too fast
+*** Fatal signal 6 received
+*** Log file is /home/vangyzen/.kyua/logs/kyua.20221006-193544.log
+*** Please report this problem to kyua-discuss@googlegroups.com detailing what you were doing before the crash happened; if possible, include the log file mentioned above
+Abort trap (core dumped)
+
+#endif
+
+#include <sys/stat.h>
+
+#include <atf-c++.hpp>
+
+#include <fcntl.h>
+#include <signal.h>
+#include <unistd.h>
+
+#include <cerrno>
+#include <cstring>
+
+void
+leak_work_dir()
+{
+	int fd;
+
+	ATF_REQUIRE((fd = open("unforgettable", O_CREAT|O_EXCL|O_WRONLY, 0600))
+	    >= 0);
+	ATF_REQUIRE_EQ(0, fchflags(fd, UF_NOUNLINK));
+	ATF_REQUIRE_EQ(0, close(fd));
+}
+
+void
+wrap_pids()
+{
+	pid_t begin, current, target;
+	bool wrapped;
+
+	begin = getpid();
+	target = begin - 15;
+	if (target <= 1) {
+		target += 99999;    // PID_MAX
+		wrapped = true;
+	} else {
+		wrapped = false;
+	}
+
+	ATF_REQUIRE(signal(SIGCHLD, SIG_IGN) != SIG_ERR);
+
+	do {
+		current = vfork();
+		if (current == 0) {
+			_exit(0);
+		}
+		ATF_REQUIRE(current != -1);
+		if (current < begin) {
+			wrapped = true;
+		}
+	} while (!wrapped || current < target);
+}
+
+void
+test_work_dir_reuse()
+{
+	// If kyua is built with debugging, it would abort here before the fix.
+}
+
+void
+clean_up()
+{
+	(void)system("chflags -R nouunlink ../..");
+}
+
+ATF_TEST_CASE_WITHOUT_HEAD(leak_0);
+ATF_TEST_CASE_BODY(leak_0) { leak_work_dir(); }
+ATF_TEST_CASE_WITHOUT_HEAD(leak_1);
+ATF_TEST_CASE_BODY(leak_1) { leak_work_dir(); }
+ATF_TEST_CASE_WITHOUT_HEAD(leak_2);
+ATF_TEST_CASE_BODY(leak_2) { leak_work_dir(); }
+ATF_TEST_CASE_WITHOUT_HEAD(leak_3);
+ATF_TEST_CASE_BODY(leak_3) { leak_work_dir(); }
+ATF_TEST_CASE_WITHOUT_HEAD(leak_4);
+ATF_TEST_CASE_BODY(leak_4) { leak_work_dir(); }
+ATF_TEST_CASE_WITHOUT_HEAD(leak_5);
+ATF_TEST_CASE_BODY(leak_5) { leak_work_dir(); }
+ATF_TEST_CASE_WITHOUT_HEAD(leak_6);
+ATF_TEST_CASE_BODY(leak_6) { leak_work_dir(); }
+ATF_TEST_CASE_WITHOUT_HEAD(leak_7);
+ATF_TEST_CASE_BODY(leak_7) { leak_work_dir(); }
+ATF_TEST_CASE_WITHOUT_HEAD(leak_8);
+ATF_TEST_CASE_BODY(leak_8) { leak_work_dir(); }
+ATF_TEST_CASE_WITHOUT_HEAD(leak_9);
+ATF_TEST_CASE_BODY(leak_9) { leak_work_dir(); }
+
+ATF_TEST_CASE_WITHOUT_HEAD(pid_wrap);
+ATF_TEST_CASE_BODY(pid_wrap) { wrap_pids(); }
+
+ATF_TEST_CASE_WITHOUT_HEAD(pid_wrap_0);
+ATF_TEST_CASE_BODY(pid_wrap_0) { test_work_dir_reuse(); }
+ATF_TEST_CASE_WITHOUT_HEAD(pid_wrap_1);
+ATF_TEST_CASE_BODY(pid_wrap_1) { test_work_dir_reuse(); }
+ATF_TEST_CASE_WITHOUT_HEAD(pid_wrap_2);
+ATF_TEST_CASE_BODY(pid_wrap_2) { test_work_dir_reuse(); }
+ATF_TEST_CASE_WITHOUT_HEAD(pid_wrap_3);
+ATF_TEST_CASE_BODY(pid_wrap_3) { test_work_dir_reuse(); }
+ATF_TEST_CASE_WITHOUT_HEAD(pid_wrap_4);
+ATF_TEST_CASE_BODY(pid_wrap_4) { test_work_dir_reuse(); }
+ATF_TEST_CASE_WITHOUT_HEAD(pid_wrap_5);
+ATF_TEST_CASE_BODY(pid_wrap_5) { test_work_dir_reuse(); }
+ATF_TEST_CASE_WITHOUT_HEAD(pid_wrap_6);
+ATF_TEST_CASE_BODY(pid_wrap_6) { test_work_dir_reuse(); }
+ATF_TEST_CASE_WITHOUT_HEAD(pid_wrap_7);
+ATF_TEST_CASE_BODY(pid_wrap_7) { test_work_dir_reuse(); }
+ATF_TEST_CASE_WITHOUT_HEAD(pid_wrap_8);
+ATF_TEST_CASE_BODY(pid_wrap_8) { test_work_dir_reuse(); }
+ATF_TEST_CASE_WITHOUT_HEAD(pid_wrap_9);
+ATF_TEST_CASE_BODY(pid_wrap_9) { test_work_dir_reuse(); }
+
+ATF_TEST_CASE_WITHOUT_HEAD(really_clean_up);
+ATF_TEST_CASE_BODY(really_clean_up) { clean_up(); }
+
+ATF_INIT_TEST_CASES(tcs)
+{
+	ATF_ADD_TEST_CASE(tcs, leak_0);
+	ATF_ADD_TEST_CASE(tcs, leak_1);
+	ATF_ADD_TEST_CASE(tcs, leak_2);
+	ATF_ADD_TEST_CASE(tcs, leak_3);
+	ATF_ADD_TEST_CASE(tcs, leak_4);
+	ATF_ADD_TEST_CASE(tcs, leak_5);
+	ATF_ADD_TEST_CASE(tcs, leak_6);
+	ATF_ADD_TEST_CASE(tcs, leak_7);
+	ATF_ADD_TEST_CASE(tcs, leak_8);
+	ATF_ADD_TEST_CASE(tcs, leak_9);
+
+	ATF_ADD_TEST_CASE(tcs, pid_wrap);
+
+	ATF_ADD_TEST_CASE(tcs, pid_wrap_0);
+	ATF_ADD_TEST_CASE(tcs, pid_wrap_1);
+	ATF_ADD_TEST_CASE(tcs, pid_wrap_2);
+	ATF_ADD_TEST_CASE(tcs, pid_wrap_3);
+	ATF_ADD_TEST_CASE(tcs, pid_wrap_4);
+	ATF_ADD_TEST_CASE(tcs, pid_wrap_5);
+	ATF_ADD_TEST_CASE(tcs, pid_wrap_6);
+	ATF_ADD_TEST_CASE(tcs, pid_wrap_7);
+	ATF_ADD_TEST_CASE(tcs, pid_wrap_8);
+	ATF_ADD_TEST_CASE(tcs, pid_wrap_9);
+
+	ATF_ADD_TEST_CASE(tcs, really_clean_up);
+}