Segfault about the_silver_searcher HOT 14 CLOSED

Huh. Well, apparently it was a file called ag.txt or agtest.txt. I removed them, and it worked fine.

from the_silver_searcher.

Interestingly, this actually seems to be a problem with outputting to a file. Those two files were generated by

ag "test" > ag.txt

and

ag "test" > agtest.txt

Both of those throw segfaults.

This series of commands may help pin it down:

$ ack "test" > blah
$ ag "test" > blah2
$ ag "test" > agtest
Segmentation fault
$ ack "test" > agtest2

from the_silver_searcher.

Nice catch. I bet this is because buf is actually agtest.txt mmap()ed. If agtest.txt matches the search, then ag will try to print out results. The problem is that the redirected IO overwrites agtest.txt. That means the contents of buf change during printing. Also, buf_len doesn't change and we risk accessing invalid memory like you encountered.

from the_silver_searcher.

What operating system are you on? I can't reproduce this on OS X 10.7 or Ubuntu 11.10.

Also, can you try to reproduce this with the map_private branch?

from the_silver_searcher.

That was on the latest version of Snow Leopard. I ran into it at work, so I won't be able to get to it until Monday unfortunately.

from the_silver_searcher.

I was having trouble reproducing this as well, but I remembered that I compiled with GCC 4.7 at work rather than Apple's default. I just recompiled with 4.7 and had the segfault show up on OSX 10.8

from the_silver_searcher.

map_private doesn't fix it.

from the_silver_searcher.

The output from the two compilers is a bit different.

4.2:

src/search.c: In function ‘search_dir’:
src/search.c:187: warning: passing argument 3 of ‘scandir’ from incompatible pointer type
src/search.c:210: warning: passing argument 3 of ‘scandir’ from incompatible pointer type

4.7

src/search.c: In function ‘search_dir’:
src/search.c:187:5: warning: passing argument 3 of ‘scandir’ from incompatible pointer type [enabled by default]
In file included from src/search.h:4:0,
                 from src/search.c:1:
/usr/include/dirent.h:143:5: note: expected ‘int (*)(const struct dirent *)’ but argument is of type ‘int (*)(struct dirent *)’
src/search.c:210:5: warning: passing argument 3 of ‘scandir’ from incompatible pointer type [enabled by default]
In file included from src/search.h:4:0,
                 from src/search.c:1:
/usr/include/dirent.h:143:5: note: expected ‘int (*)(const struct dirent *)’ but argument is of type ‘int (*)(struct dirent *)’

from the_silver_searcher.

Aww, well it was worth a shot. Thanks for trying the branch.

I thought I had some #ifdefs to get rid of those warnings on OS X. I signed up for the Mac developer program yesterday, but I don't have access to 10.8 yet. Maybe the signature for scandir changed in Mountain Lion. Or it could be that you have glibc installed.

from the_silver_searcher.

OSX has always had functions that want a const struct dirent* instead of a struct dirent* (like Linux does). In previous projects I've done some OSX detection and used a #define:

#ifdef __APPLE__
  #define DIRENT const dirent*
#else
  #define DIRENT dirent*
#endif

from the_silver_searcher.

From man scandir on OS X 10.7:

 int
 scandir(const char *dirname, struct dirent ***namelist, int (*select)(struct dirent *), int (*compar)(const void *, const void *));

From man scandir on Ubuntu 11.10:

   int scandir(const char *dirp, struct dirent ***namelist,
          int (*filter)(const struct dirent *),
          int (*compar)(const struct dirent **, const struct dirent **));

I think you have it backwards. You definitely made me double-check my work, though. :)

from the_silver_searcher.

Yeah, I don't get those warnings on 10.6, so it must be something weird about 10.8 or my laptop.

from the_silver_searcher.

Resurrecting this thread.

This is a fundamental issue with modifying mmapped files while they're being read. If you can think of any solutions, I'd love to hear them. I wrote a few of my own, but I don't want to influence yours. Please take a couple minutes to think of solutions before reading any farther.

...

...

...

My ideas:

1. Locking every file. This seems inefficient. Every file has to be locked while it's searched, and every file with a match has to be locked until the matches are printed out. This also might require write permissions on the files being searched, or cause searching to hang while waiting for a file lock. I'm pretty sure most filesystems and OSes require write permissions to exclusively lock a file.
2. Adding a --no-mmap option. I'm not sure how useful this would be. I think most people who see a segfault would say, "This program sucks." and stop using it.
3. Default to reading files into buffers and add a --mmap option. This increases memory usage and hurts performance.

That's what I've thought of after a couple minutes. I'm split between option 2 and doing nothing. I welcome other ideas.

from the_silver_searcher.

I think the solution is to do nothing.

from the_silver_searcher.