SimonKagstrom / kcov

Code coverage tool for compiled programs, Python and Bash which uses debugging information to collect and report data without special compilation options
http://simonkagstrom.github.io/kcov/
GNU General Public License v2.0
710 stars 109 forks source link

Command line parsing fooled by symlink to coverage target #368

Closed rpoyner-tri closed 2 years ago

rpoyner-tri commented 2 years ago

Thanks for providing a fabulous tool; we use it all the time.

While trying to integrate kcov into our builds to collect python coverage, I noticed it can mis-parse command lines where the coverage target executable is a symbolic link.

For example, given a simple directory containing echo.py (it just prints sys.argv) and a symlink to it called link_echo.py, these command lines appear to work correctly:

$ kcov /tmp/kcov echo.py
$ kcov /tmp/kcov echo.py foo
$ kcov /tmp/kcov echo.py --foo
$ kcov /tmp/kcov link_echo.py
$ kcov /tmp/kcov link_echo.py foo

This one, however, confuses kcov since it tries to interpret the "flag" argument:

$ kcov /tmp/kcov link_echo.py --foo

From a quick debug session with kcov master, it looks like the first thing that goes wrong is that peek_file() refuses to read bytes out of the symlink file, and things go downhill from there.

I was able to improve things a bit with a one-line patch:

--- a/src/configuration.cc
+++ b/src/configuration.cc
@@ -177,7 +177,7 @@ public:
                 */
                for (lastArg = 1; lastArg < argc; lastArg++)
                {
-                       if (IParserManager::getInstance().matchParser(argv[lastArg]))
+                       if (IParserManager::getInstance().matchParser(get_real_path(argv[lastArg])))
                                break;

                        bool found = false;

Of course, I didn't consider any cases where this change might break things, nor did I write any tests. I'm hoping you can reproduce the issue and give it a proper treatment when you have time.

I originally discovered the problem in kcov 38, and did my experiments and patching on very recent master.

SimonKagstrom commented 2 years ago

Good analysis of the problem! I think your patch is correct, so feel free to submit a pull request. There are some automated CI tests in tests/, so I think they would find a regression (which is probably unlikely here). You could also add a Python test there (i.e., a symlink to another file) which would trigger this problem, but the testsuite needs to be run like in .github/workflows/ci-run-tests.sh , so somewhat error-prone :-)

Anyway, thanks for both analysing and providing a fix for the problem!