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
717 stars 110 forks source link

Shell scripts that are called by another scripts via sh foo.sh are not traced (./foo.sh is a workaround) #165

Open austin987 opened 7 years ago

austin987 commented 7 years ago

Hello,

I found kcov, and was very anxious to try it on my shell project (https://github.com/winetricks/winetricks). I built from source (HEAD 045d4e5d34f601b91c28d6fc68c8c268a512c4fd), and ran kcov with:

$ ~/src/kcov/build/src/kcov make check

that appeared to work, but got me coverage results for gmake. So, I switched to running the test wrapper script directly, and also tried bash/sh options (shebang is #/bin/sh), but that made no difference:

$ ~/src/kcov/build/src/kcov --bash-parser=/bin/sh --bash-handle-sh-invocation kcov/ ./tests/winetricks-test check 2>&1 | tee kcov_output.txt

So, I tried making a simple testcase: script 1:

#!/bin/sh
echo "I'm a test wrapper. I'll invoke script2"
sh script2

script 2:

#!/bin/sh
echo "Hi, script2 here!"

put both in same dir, and chmod +x, then try kcov:

$ ~/src/kcov/build/src/kcov --bash-parser=/bin/sh --bash-handle-sh-invocation kcov/ ./script1 
I'm a test wrapper. I'll invoke script2
Hi, script2 here!

So script2 was invoked, which is expected. But if you look at kcov/index.html, script2 is not listed (but script1 got 100% coverage, woohoo ;) ).

SimonKagstrom commented 7 years ago

Hi!

First, if /bin/sh is not bash (e.g., dash or similar), the --bash-parser= stuff will not work. You really should not have to use that, unless you want e.g., a specific version of bash to run with.

As to the second issue, that unfortunately is a bit tricky. I think it's the same problem as in Issue #64 . If /bin/sh is really a symlink to bash, I think it would work though. How does it look on your system?

austin987 commented 7 years ago

Hi @SimonKagstrom,

Fair point about the --bash stuff, consider that a debugging attempt ;)

On this system, /bin/sh in a link to bash: austin@austin2:~$ ls -l /bin/sh lrwxrwxrwx 1 root root 4 Dec 6 16:12 /bin/sh -> bash

Should sh be invoked with a full path rather than relative, maybe?

SimonKagstrom commented 7 years ago

OK, then it probably doesn't work when invoking that way. I think the reason is in Issue #51, bash scrubbing PS4 from the environment.

Another thing to try is to use the --bash-method=DEBUG option, which doesn't use PS4, although I'm not that hopeful that it will propagate to new bashes either.

austin987 commented 7 years ago

@SimonKagstrom so I found that if I used:

./foo

instead of:

sh foo

then it works. It seems to bail out early on large scripts though, winetricks is nearly 20k but kcov keeps capping out around 500 lines. Is that a known issue/user error, or should I file a new bug for that?

SimonKagstrom commented 7 years ago

OK, thanks for the test!

In my case, it works if I use "bash foo", but not "sh foo" as well. But then I also have /bin/sh linked to dash:

$ ls -l `which sh`
lrwxrwxrwx 1 root root 4 Oct 30  2014 /bin/sh -> dash

Kcov doesn't have a script size limitation as such, but the parser is stupid and can get lost in the bash complexities. A proper shell script parser would be needed for the general case, see Issue #145. If you can find the exact line in the winetricks script, it might be obvious what goes wrong and a bandaid might be applied.

I guess an alternative could be to make the parser "more stupid", and avoid trying to identify heredocs and other Bash peculiarities. Then it should not get lost as easily, but also mis-identify a lot of lines.

austin987 commented 7 years ago

The script aims to be POSIX, not bash, hence my trying to avoid bash workarounds. The sh syntax was an historical artifact, and ./ is fine for me to avoid the issue.

winetricks has some heredocs, but that's not a bashism :). There shouldn't be any bashisms in winetricks, as I regularly run shellcheck and checkbashisms on it.

That said, it definitely is a more complex script. I'll make a note to try seeing what breaks it, but I've got several plates in the air already, fyi.

SimonKagstrom commented 7 years ago

Sorry, what I mean is complexities in the regular shell syntax, not bash extensions. kcov doesn't understand shell script syntax through a real parser, but tries to keep track of e.g., heredocs and other multi-line constructions. Probably this is what happens here: It gets lost in a call to awk or something similar, and then thinks the rest of the file is non-code.

A test could be to modify bash-engine.cc to only disregard empty lines and full-comment lines. Then I think the script should be fully parsed, but with a lot of false uncovered code lines.

austin987 commented 7 years ago

From a quick glance of the output, it's breaking on a usage of w_ahk_do(), which is a function takes a multiple line input that it then runs through AutoHotKey. There are two earlier usages that it parsed okay.

 13250      w_try_cd "$W_PROGRAMS_X86_UNIX/Steam" 
 13260      w_ahk_do " 
 13270          SetTitleMatchMode 2 
 13280          SetWinDelay 500 
 13290          ; Run steam once until it finishes its initial update. 
 13300          ; For me, this exits at 26%. 
 13310          run steam.exe -applaunch $_W_steamid -login $W_STEAM_ID $W_STEAM_PASSWORD 
 13320          Loop 
 1333          { 
 13340              ifWinExist, Steam - Updating 
 1335              { 
 13360                  winwaitclose, Steam 
 13370                  process close, Steam.exe 
 13380                  sleep 1000 
 13390                  ; Run a second time; let it finish updating, then kill it. 
 13400                  run steam.exe 
 13410                  winwait Steam - Updating 
 13420                  winwaitclose 
 13430                  process close, Steam.exe 
 13440                  ; Run a third time, have it log in, wait until it has finished connecting 
 13450                  run steam.exe -applaunch $_W_steamid -login $W_STEAM_ID $W_STEAM_PASSWORD 
 1346              } 
 13470              ifWinExist, Steam Login 
 1348              { 
 13490                  break 
 1350              } 
 13510              sleep 500 
 1352          } 
 13530          ; wait for login window to close 
 13540          winwaitclose 
 1355   
 13560          winwait Steam  ; wait for small <<connecting>> window 
 1357          winwaitclose 
 1358      " 

kcov analyzes up until line 1356, then bails. However, it shows those lines as unexecuted (they were executed).

Thanks for the tip

SimonKagstrom commented 7 years ago

I've pushed a small patch which adds an option to run with a simpler bash parser. Run with

kcov --configure=bash-use-basic-parser=1 <other options>

that might give better results with more complex scripts like winetricks.

austin987 commented 7 years ago

Hi @SimonKagstrom, so that definitely gets the line count up. It appears to scan the entire file now (~12k lines, which looks about right once newlines and comments are excluded). Unfortunately, it misses large portions of the file, only 160 lines are counted as executed.

Most of the executed code appears to be in the main script, but not most functions (though a few pieces of code in smaller functions was executed). My guess is that nested functions or more complex shell constructs cause it to be ignored.

I've subscribed to #145 and will be watching that. Let me know if you need any testing done :)

SimonKagstrom commented 7 years ago

That's strange though. If you run bash with -x, does those lines show up? I.e.,

bash -x winetricks

kcov uses -x internally (unless --bash-method=DEBUG is passed on the command line).

SimonKagstrom commented 7 years ago

I must warn that #145 is not something I'm likely to work on in the short run. It's a big job and I personally mainly (almost exclusively) use kcov for compiled code.

austin987 commented 7 years ago

I figured as much, but thanks for explicitly confirming.

I'm going ahead and getting the infrastructure in place to test, mostly because I haven't found anything that works better. Do you know of any viable alternatives for sh code?

SimonKagstrom commented 7 years ago

There are a few other coverage collectors for bash/sh (including my own shcov), but I believe they all use PS4 (bash -x) internally, or perhaps the debug method as outlined above. Thus, if the basic/simple parser doesn't work reliably, I don't think you'll get better results with another collector.

Trying the --bash-method=DEBUG option could be good though, although I don't think it will make a difference.