Merge branch 'master' into handle-out-dir-correctly

.github/workflows/ci.yml

@@ -60,17 +60,16 @@ jobs:
 
 
   build-freebsd:
-    runs-on: macos-12
+    runs-on: ubuntu-22.04
     name: Build and test FreeBSD executable
     steps:
     - uses: actions/checkout@v3
     - name: FreeBSD
       id: test
-      uses: vmactions/freebsd-vm@v0
+      uses: vmactions/freebsd-vm@v1
       with:
         usesh: true
-        sync: rsync
-        prepare: pkg install -y binutils cmake elfutils python bash git
+        prepare: pkg install -y binutils cmake elfutils python bash git python2
         run: |
           chmod u+x .github/workflows/freebsd-build.sh .github/workflows/ci-run-tests.sh
           .github/workflows/freebsd-build.sh x86_64

.github/workflows/freebsd-build.sh

@@ -14,6 +14,10 @@ run () {
   cmake -DCMAKE_INSTALL_PREFIX=/usr/local ../tests || exit 64
   make || exit 64
   cd ..
+
+  chmod u+x .github/workflows/test-executable.sh
+  kcov --include-pattern=test-executable.sh coverage .github/workflows/test-executable.sh
+  cat coverage/test-executable.sh/coverage.json
 }
 
 run "$@"

.github/workflows/generic-build.sh

@@ -18,7 +18,7 @@ run () {
   apt-get update
   apt-mark hold php* google* libobjc* libpq* libidn* postgresql* python3-httplib2 samba* >/dev/null
   apt-get upgrade -y
-  apt-get install -y binutils-dev libcurl4-openssl-dev libdw-dev libiberty-dev gcc g++ make cmake libssl-dev git python3 $EXTRA_PACKAGES
+  apt-get install -y binutils-dev libcurl4-openssl-dev libdw-dev libiberty-dev gcc g++ make cmake libssl-dev git python3 python2 $EXTRA_PACKAGES
   export PATH="${PATH}:${HOME}/kcov/bin"
   mkdir build build-tests
   cd build

.github/workflows/setup-i386.sh

@@ -13,4 +13,4 @@ root-groups=root
 preserve-environment=true
 personality=linux32" >> /etc/schroot/schroot.conf
 
-debootstrap --variant=buildd --arch i386 focal /chroots/i386 http://archive.ubuntu.com/ubuntu/
+debootstrap --variant=buildd --arch i386 --components=main,universe jammy /chroots/i386 http://archive.ubuntu.com/ubuntu/

src/configuration.cc

@@ -5,6 +5,7 @@
 #include <getopt.h>
 #include <limits.h>
 #include <sys/stat.h>
+#include <sys/param.h>
 #include <sys/types.h>
 #include <unistd.h>
 #include <map>
@@ -13,10 +14,19 @@
 #include <stdexcept>
 #include <unordered_map>
 #include <iostream>
+
 using namespace kcov;
 
 extern "C" const char *kcov_version;
 
+// This function can't be defined in utils.cc due to the use of the singleton
+// pattern. It will cause a linker error when buildind tests, since the utils.cc
+// file is also included in the test source files.
+static bool is_supported_executable(const std::string path)
+{
+	return kcov::IParserManager::getInstance().matchParser(path) != 0;
+}
+
 class Configuration : public IConfiguration
 {
 public:
@@ -164,17 +174,10 @@ class Configuration : public IConfiguration
 
 		setupDefaults();
 
-		const char *path = getenv("PATH");
-
-		if (!path)
-			path = "";
-
-		std::vector<std::string> paths = split_string(path, ":");
-
 		setKey("kcov-binary-path", get_real_path(argv[0]));
 
-		/* Scan through the parameters for an ELF file: That will be the
-		 * second last argument in the list.
+		/* Scan through the parameters for a supported executable file: That
+		 * will be the second last argument in the list.
 		 *
 		 * After that it's arguments to the external program.
 		 */
@@ -206,9 +209,9 @@ class Configuration : public IConfiguration
 				if (S_ISREG(st.st_mode) == 0)
 					continue;
 
-				// Executable?
-				if ((st.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH)) == 0)
-					continue;
+			ret = look_path(arg, &path);
+			if (ret < 0)
+				continue;
 
 				if (IParserManager::getInstance().matchParser(get_real_path(cur)))
 				{
@@ -221,6 +224,7 @@ class Configuration : public IConfiguration
 
 			if (found)
 				break;
+			}
 		}
 
 		bool printUsage = false;

src/engines/bash-engine.cc

@@ -153,6 +153,8 @@ class BashEngine : public ScriptEngineBase
 			unsigned int argc = conf.getArgc();
 			int xtraceFd = 782; // Typical bash users use 3,4 etc but not high fd numbers (?)
 			{
+// This doesn't work on (recent?) FreeBSDs, so disable for now
+#ifndef __FreeBSD__
 				// However, the range of fd numbers that this process can use is limited by the caller of this process.
 				// The following code specification will require discussion with many kcov users.
 				struct rlimit rlim;
@@ -161,6 +163,7 @@ class BashEngine : public ScriptEngineBase
 					return false;
 				}
 				xtraceFd = rlim.rlim_cur / 4;
+#endif
 			}
 			const std::string command = conf.keyAsString("bash-command");
 			bool usePS4 = conf.keyAsInt("bash-use-ps4");

src/engines/python-helper.py

@@ -4,33 +4,52 @@
 # Based on http://pymotw.com/2/sys/tracing.html, "Tracing a program as it runs"
 # and http://hg.python.org/cpython/file/2.7/Lib/trace.py
 
-import imp
 import sys
 import os
 import struct
 
 fifo_file = None
-report_trace_real = None
 
-try:
-    # In Py 2.x, the builtins were in __builtin__
-    BUILTINS = sys.modules['__builtin__']
-except KeyError:
+if sys.version_info >= (3, 0):
     # In Py 3.x, they're in builtins
-    BUILTINS = sys.modules['builtins']
+    BUILTINS = sys.modules["builtins"]
+
+    def report_trace_real(file, line):
+        size = len(file) + 1 + 8 + 4 + 4
+        data = struct.pack(
+            ">QLL%dsb" % len(file),
+            0x6D6574616C6C6775,
+            size,
+            int(line),
+            bytes(file, "utf-8"),
+            0,
+        )
+
+        fifo_file.write(data)
+else:
+    # In Py 2.x, the builtins were in __builtin__
+    BUILTINS = sys.modules["__builtin__"]
+
+    def report_trace_real(file, line):
+        size = len(file) + 1 + 8 + 4 + 4
+        data = struct.pack(">QLL%dsb" % len(file), 0x6D6574616C6C6775, size, int(line), file, 0)
 
+        fifo_file.write(data)
 
-def report_trace3(file, line):
-    size = len(file) + 1 + 8 + 4 + 4
-    data = struct.pack(">QLL%dsb" % len(file), 0x6d6574616c6c6775, size, int(line), bytes(file, 'utf-8'), 0)
 
-    fifo_file.write(data)
+if sys.version_info >= (3, 5):
+    # The imp module has been deprecated since version 3.4 and removed in
+    # version 3.12.
+    import types
 
-def report_trace2(file, line):
-    size = len(file) + 1 + 8 + 4 + 4
-    data = struct.pack(">QLL%dsb" % len(file), 0x6d6574616c6c6775, size, int(line), file, 0)
+    def new_module(name):
+        return types.ModuleType(name)
+else:
+    import imp
+
+    def new_module(name):
+        return imp.new_module(name)
 
-    fifo_file.write(data)
 
 def report_trace(file, line):
     try:
@@ -40,17 +59,19 @@ def report_trace(file, line):
         # Ignore errors
         pass
 
+
 def trace_lines(frame, event, arg):
-    if event != 'line':
+    if event != "line":
         return
     co = frame.f_code
     func_name = co.co_name
     line_no = frame.f_lineno
     filename = co.co_filename
     report_trace(filename, line_no)
 
+
 def trace_calls(frame, event, arg):
-    if event != 'call':
+    if event != "call":
         return
     co = frame.f_code
     func_name = co.co_name
@@ -59,27 +80,24 @@ def trace_calls(frame, event, arg):
     report_trace(filename, line_no)
     return trace_lines
 
+
 def runctx(cmd, globals):
     sys.settrace(trace_calls)
     try:
         exec(cmd, globals)
     finally:
         sys.settrace(None)
 
-if __name__ == "__main__":
-    if sys.version_info >= (3, 0):
-        report_trace_real = report_trace3
-    else:
-        report_trace_real = report_trace2
 
+if __name__ == "__main__":
     prog_argv = sys.argv[1:]
 
     sys.argv = prog_argv
     progname = prog_argv[0]
     sys.path[0] = os.path.split(progname)[0]
 
     fifo_path = os.getenv("KCOV_PYTHON_PIPE_PATH")
-    if fifo_path == None:
+    if fifo_path is None:
         sys.stderr.write("the KCOV_PYTHON_PIPE_PATH environment variable is not set")
         sys.exit(127)
     try:
@@ -88,9 +106,9 @@ def runctx(cmd, globals):
         sys.stderr.write("Can't open fifo file")
         sys.exit(127)
 
-    main_mod = imp.new_module('__main__')
-    old_main_mod = sys.modules['__main__']
-    sys.modules['__main__'] = main_mod
+    main_mod = new_module("__main__")
+    old_main_mod = sys.modules["__main__"]
+    sys.modules["__main__"] = main_mod
     main_mod.__file__ = progname
     main_mod.__package__ = None
     main_mod.__builtins__ = BUILTINS
@@ -99,11 +117,11 @@ def runctx(cmd, globals):
         with open(progname) as fp:
             # try to emulate __main__ namespace as much as possible
 
-            code = compile(fp.read(), progname, 'exec')
+            code = compile(fp.read(), progname, "exec")
 
             runctx(code, main_mod.__dict__)
     except IOError:
         sys.stderr.write("Cannot run file %r" % (sys.argv[0]))
         sys.exit(127)
     finally:
-        sys.modules['__main__'] = old_main_mod
+        sys.modules["__main__"] = old_main_mod

src/include/utils.hh

@@ -223,3 +223,13 @@ static inline uint32_t hash_block(const void *buf, size_t len)
 }
 
 uint32_t hash_file(const std::string &filename);
+
+int find_executable(const std::string &file);
+
+// Searches for an executable named file in the directories named by the PATH
+// environment variable.
+// The named file is first tried directly without consulting PATH.
+//
+// Don't use this function when expanding the first argument of the exec()
+// family of functions, since it is insecure.
+int look_path(const std::string &file, std::string *out);

src/utils.cc

@@ -146,8 +146,9 @@ void *peek_file(size_t *out_size, const char *fmt, ...)
 	struct stat st;
 	if (lstat(path, &st) < 0)
 		return NULL;
-	// Regular file?
-	if (S_ISREG(st.st_mode) == 0)
+
+	// Not a directory?
+	if (S_ISDIR(st.st_mode) > 0)
 		return NULL;
 
 	// Read a little bit of the file    
@@ -775,3 +776,54 @@ uint32_t hash_file(const std::string &filename)
 
 	return out;
 }
+
+int find_executable(const std::string &file)
+{
+	struct stat st;
+
+	if (lstat(file.c_str(), &st) < 0)
+		return -1;
+
+	// Not a directory?
+	if (S_ISDIR(st.st_mode) > 0)
+		return -1;
+
+	// Executable?
+	if ((st.st_mode & (S_IXUSR | S_IXGRP | S_IXOTH)) == 0)
+		return -1;
+
+	return 0;
+}
+
+
+int look_path(const std::string &file, std::string *out)
+{
+	// First check if the relative file is an executable.
+	if (find_executable(file) == 0)
+	{
+		*out = file;
+
+		return 0;
+	}
+
+	const char *sys_path = getenv("PATH");
+	if (!sys_path)
+		return -1;
+
+	const std::vector<std::string> sys_paths = split_string(sys_path, ":");
+	for (std::vector<std::string>::const_iterator it = sys_paths.begin();
+			it != sys_paths.end(); ++it)
+	{
+		const std::string root = *it;
+		const std::string path = get_real_path(root + "/" + file);
+
+		if (find_executable(path) == 0)
+		{
+			*out = path;
+
+			return 0;
+		}
+	}
+
+	return -1;
+}

tests/CMakeLists.txt

@@ -143,7 +143,7 @@ add_executable(multi_1 merge-tests/file.c merge-tests/main_1.c)
 add_executable(multi_2 merge-tests/file.c merge-tests/main_2.c)
 
 add_executable(setpgid-kill setpgid-kill/setpgid-kill-main.cc ../src/utils.cc)
-target_link_libraries(setpgid-kill ${ZLIB_LIBRARIES} ${CURL_LIBRARIES})
+target_link_libraries(setpgid-kill ${ZLIB_LIBRARIES} CURL::libcurl)
 
 add_executable(issue31 daemon/test-issue31.cc)
 target_link_libraries(issue31 ${CMAKE_THREAD_LIBS_INIT})

tests/tools/python.py

@@ -24,6 +24,13 @@ def runTest(self):
 
         assert b"Cannot find Python parser 'python3'" not in o
 
+class python2_can_set_legal_parser(testbase.KcovTestCase):
+    def runTest(self):
+        self.setUp()
+        rv,o = self.do(testbase.kcov + " --python-parser=python2 " + testbase.outbase + "/kcov " + testbase.sources + "/tests/python/main 5")
+
+        assert b"Cannot find Python parser 'python2'" not in o
+
 class python_issue_368_can_handle_symlink_target(testbase.KcovTestCase):
     def runTest(self):
         self.setUp()
@@ -84,6 +91,10 @@ class python3_coverage(PythonBase):
     def runTest(self):
         self.doTest("--python-parser=python3")
 
+class python2_coverage(PythonBase):
+    def runTest(self):
+        self.doTest("--python-parser=python2")
+
 class python_tricky_single_line_string_assignment(testbase.KcovTestCase):
     def runTest(self):
         self.setUp()