souzaonofre / fabricate

Automatically exported from code.google.com/p/fabricate
0 stars 0 forks source link

Make strace more robust -- handle fchdir calls #23

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
On the some servers strace shows stat calls as "stat(...)" instead of the 
usual "stat64(...)". I don't know why the difference. The strace version is 
on there is 4.5.13. So we'll have to figure out a portable way to handle 
this or fix it some other way.

As Berwyn said in an email (9 Sep 2009):

"Good find. Looks like we may need to make it more robust.

"Stat -e trace=file covers all the file operations (which also covers lstat 
which the man page says it would bet we'll forget).  And trace=process for 
all process operations, so presumably we'd want to do 
trace=file,process,chdir,mkdir.  trace=process may also cover exit_group 
which is used to get the exit code: I don't know.

"And then we have to write parsers for all of them, of course.  Including 
lstat and fstat (see their man page entries)."

Original issue reported on code.google.com by benh...@gmail.com on 6 Nov 2009 at 5:34

GoogleCodeExporter commented 9 years ago
Fixed (at least for this stat/stat64 special case) in r89.

Original comment by benh...@gmail.com on 1 Apr 2010 at 8:30

GoogleCodeExporter commented 9 years ago
Well, fixed, but I had forgotten about that trace=file thing Ber mentioned. 
Needs 
improvement.

Original comment by benh...@gmail.com on 6 Apr 2010 at 2:29

GoogleCodeExporter commented 9 years ago

Original comment by benh...@gmail.com on 6 Apr 2010 at 2:29

GoogleCodeExporter commented 9 years ago
I don't know if this is a good place to note miscellaneous unhandled syscalls, 
but I
happened to encounter one recently: fchdir. This looks like a rather tricky one 
to
fix, since the file descriptor would need to be mapped to a directory that was 
opened
at some earlier point.

Original comment by kevingoo...@gmail.com on 9 May 2010 at 9:12

GoogleCodeExporter commented 9 years ago
Interesting find, Kevin. Yes, that would be tricky to fix -- we'll probably put 
it in 
the too-hard basket for now. What was the use case, application, or script that 
showed 
this problem up?

Original comment by benh...@gmail.com on 10 May 2010 at 8:05

GoogleCodeExporter commented 9 years ago
The case with fchdir was a crazy hack invoking vim with a string of command
arguments. The purpose was to capture the output from :highlight using each of 
about
130 color schemes and different sets of options, which required some serious
persuasion. (Good ol' vim, you can script it to do just about anything, but 
you'd
better have a high tolerance for frustration.) Definitely not the kind of thing 
a lot
of people are likely to try.

However, It's worth noting that this produces rather confusing results. Since
fabricate gets the current directory wrong, it can get dependencies completely 
wrong
(leaving things out because they appear to be outside the build directory for
example). While handling it correctly is difficult, detecting fchdir and warning
about it is rather easy, and may be worth considering.

This also has me pondering: tracking file descriptors would probably be 
necessary to
handle this. I wonder if there is anything else interesting or useful that this 
would
enable.

Original comment by kevingoo...@gmail.com on 11 May 2010 at 4:49

GoogleCodeExporter commented 9 years ago
Good find.  A patch would be accepted.

I guess it wouldn't be too hard: you'd have to keep a dictionary of file 
descriptors 
against filenames.  I don't think it should monitor file descriptor syscalls 
themselves (except fchdir, of course) because it could cause a huge amount of 
strace 
output and slow things down. However, we wouldn't need to monitor file 
descriptor 
syscalls: just  ftrace and the filename syscalls we're already monitoring.

While you're at it, would you like to implement the "-e trace=process and -e 
trace=file" and remove the need for get_strace_version() to test for 32-bit or 
64-bit 
version of strace?  Might need to find strace source on google code to 
double-check 
that clone is included in trace=process (since the man page doesn't say).

Original comment by berh...@gmail.com on 11 May 2010 at 10:25

GoogleCodeExporter commented 9 years ago
Just found the missing lstat support crop up when trying to fix issue 32.

To reproduce run 'fabricate.py tar czvf foo.tar.gz a.c b.c' as in issue 32 but 
ensure one or both of a.c and b.c are empty files. Checking the .deps file 
shows that the empty files are not logged as dependencies of the command. 

Review of the strace output shows the reason is the empty files are only 
checked with lstat. tar must be optimised to skip opening and reading of empty 
files. This obviously means the command is not rerun when the input files are 
changed.

Original comment by simon.al...@gmail.com on 5 Apr 2013 at 9:30

GoogleCodeExporter commented 9 years ago
I have added support for lstat and lstat64 to the StraceRunner.

The remaining issue is now the fchdir support. We need a test case for this, 
which may require writing a small C program to force the behaviour, as we don't 
know what the vim command to cause the fchdir call was.

Original comment by simon.al...@gmail.com on 8 Apr 2013 at 12:35

GoogleCodeExporter commented 9 years ago
why C? how about:

import os
import os.path

if __name__ == '__main__':
    if not os.path.exists('tmp'): os.mkdir('tmp')
    d = os.open('tmp', os.O_RDONLY)
    os.fchdir(d)
    open('foo','w').write("\nfoodata\n")

Original comment by pjimen...@gmail.com on 4 Jun 2013 at 8:34

GoogleCodeExporter commented 9 years ago
Proper handling of fstat requires many syscalls. Currently, the strace output 
on my tar has stuff like:

[pid 13032] newfstatat(3, "kix", {st_mode=S_IFREG|0644, st_size=8, ...}, 
AT_SYMLINK_NOFOLLOW) = 0

Original comment by nuutti.k...@gmail.com on 6 Jul 2013 at 6:17