CODEWITHSUNDEEP
clinuxvalgrindlibc
Alex Reynolds
Alex Reynolds

Reputation: 96937

Do I need to worry about Valgrind reporting errors outside the scope of my application?

When running Valgrind's memcheck tool, I often get many hundreds of thousands (or more, since Valgrind cuts off at 100K) of small invalid read statements, e.g.:

==32027== Invalid read of size 1
==32027==    at 0x3AB426E26A: _IO_default_xsputn (in /lib64/libc-2.5.so)
==32027==    by 0x3AB426CF70: _IO_file_xsputn@@GLIBC_2.2.5 (in /lib64/libc-2.5.so)
==32027==    by 0x3AB42621FA: fwrite (in /lib64/libc-2.5.so)
==32027==    by 0x4018CA: STARCH_gzip_deflate (in /home/areynolds/trunk/utility/applications/bed/starch/bin/starch)
==32027==    by 0x401F48: compressFileWithGzip (in /home/areynolds/trunk/utility/applications/bed/starch/bin/starch)
==32027==    by 0x4028B5: transformInput (in /home/areynolds/trunk/utility/applications/bed/starch/bin/starch)
==32027==    by 0x402F12: main (in /home/areynolds/trunk/utility/applications/bed/starch/bin/starch)
==32027==  Address 0x7febb9b3c is on thread 1's stack

These statements refer to calls to functions outside of my application ("starch") and which appear to be part of libc. Is this something I need to be concerned with?

EDIT

If I modify the fwrite call to remove one byte, then my gzip stream gets corrupted. Here's the original code:

int STARCH_gzip_deflate(FILE *source, FILE *dest, int level) {                                                                                                                                                                                                              

    int ret, flush;                                                                                                                                                                                                                                                         
    unsigned have;                                                                                                                                                                                                                                                          
    z_stream strm;                                                                                                                                                                                                                                                          
    unsigned char in[STARCH_Z_CHUNK];                                                                                                                                                                                                                                       
    unsigned char out[STARCH_Z_CHUNK];                                                                                                                                                                                                                                      

    /* initialize deflate state */                                                                                                                                                                                                                                            
    strm.zalloc = Z_NULL;                                                                                                                                                                                                                                                   
    strm.zfree = Z_NULL;                                                                                                                                                                                                                                                    
    strm.opaque = Z_NULL;                                                                                                                                                                                                                                                   

    /* deflateInit2 allows creation of archive with gzip header, i.e. a gzip file */                                                                                                                                                                                        
    /* cf. http://www.zlib.net/manual.html */                                                                                                                                                                                                                               
    ret = deflateInit2(&strm, level, Z_DEFLATED, (15+16), 8, Z_DEFAULT_STRATEGY);                                                                                                                                                                                           
    if (ret != Z_OK)                                                                                                                                                                                                                                                        
        return ret;                                                                                                                                                                                                                                                         

    /* compress until end of file */                                                                                                                                                                                                                                        
    do {                                                                                                                                                                                                                                                                    
        strm.avail_in = fread(in, 1, STARCH_Z_CHUNK, source);                                                                                                                                                                                                               
        if (ferror(source)) {                                                                                                                                                                                                                                               
            (void)deflateEnd(&strm);                                                                                                                                                                                                                                        
            return Z_ERRNO;                                                                                                                                                                                                                                                 
        }                                                                                                                                                                                                                                                                   
        flush = feof(source) ? Z_FINISH : Z_NO_FLUSH;                                                                                                                                                                                                                       
        strm.next_in = in;                                                                                                                                                                                                                                                  

        do {                                                                                                                                                                                                                                                                
            strm.avail_out = STARCH_Z_CHUNK;                                                                                                                                                                                                                                
            strm.next_out = out;                                                                                                                                                                                                                                            
            ret = deflate(&strm, flush);                                                                                                                                                                                                                                    
            assert(ret != Z_STREAM_ERROR);                                                                                                                                                                                                                                  
            have = STARCH_Z_CHUNK - strm.avail_out;     

            /* invalid read happens here */                                                                                                                                                                                                                    
            if (fwrite(out, 1, have, dest) != have || ferror(dest)) {                                                                                                                                                                                                       
                (void)deflateEnd(&strm);                                                                                                                                                                                                                                    
                return Z_ERRNO;                                                                                                                                                                                                                                             
            }                                                                                                                                                                                                                                                               
        } while (strm.avail_out == 0);                                                                                                                                                                                                                                      
        assert(strm.avail_in == 0);                                                                                                                                                                                                                                         

    } while (flush != Z_FINISH);                                                                                                                                                                                                                                            
    assert(ret == Z_STREAM_END);                                                                                                                                                                                                                                            

    /* clean up and return */                                                                                                                                                                                                                                               
    (void)deflateEnd(&strm);                                                                                                                                                                                                                                                
    return Z_OK;                                                                                                                                                                                                                                                            
}

EDIT 2

I think I see the problem. I have in[STARCH_Z_CHUNK] and not in[STARCH_Z_CHUNK + 1] (and likewise for out[]). If I adjust both of the fread and fwrite statements by -1, I don't seem to get those Invalid read of size 1 statements, although I still see a lot of Invalid read of size 4 and 8 which are specific to zlib:

==32624== Invalid read of size 4
==32624==    at 0x3AB5206455: deflateInit2_ (in /usr/lib64/libz.so.1.2.3)
==32624==    by 0x40180E: STARCH_gzip_deflate (in /home/areynolds/trunk/utility/applications/bed/starch/bin/starch)
==32624==    by 0x401F48: compressFileWithGzip (in /home/areynolds/trunk/utility/applications/bed/starch/bin/starch)
==32624==    by 0x402C03: transformInput (in /home/areynolds/trunk/utility/applications/bed/starch/bin/starch)
==32624==    by 0x402F12: main (in /home/areynolds/trunk/utility/applications/bed/starch/bin/starch)
==32624==  Address 0x7feafde38 is on thread 1's stack

EDIT 3

I am recompiling with -g which, as mentioned, does associate line numbers with errors.

But I'm just doing a straightforward strncpy of argv[] variables, e.g.:

strncpy(uniqTag, argv[2], strlen(argv[2]) + 1);

This should copy over the null-terminated argv[2] string to uniqTag, but valgrind still marks this as an error.

EDIT 4

Here's the error message:

==3682== Invalid read of size 1
==3682==    at 0x4A081C1: strncpy (mc_replace_strmem.c:329)
==3682==    by 0x4022F1: parseCommandLineInputs (starch.c:589)
==3682==    by 0x402F20: main (starch.c:46)
==3682==  Address 0x7fedffe11 is on thread 1's stac

Here are the two relevant lines; valgrind is saying the second line is an invalid read:

uniqTag = (char *)malloc(strlen(argv[2]) + 1); 
strncpy(uniqTag, argv[2], strlen(argv[2]) + 1);

Because strlen(argv[2]) + 1 > strlen(argv[2]), this should result in a null-terminated uniqTag.

Upvotes: 4

Views: 1358

Answers (2)

thkala
thkala

Reputation: 86333

In this case I'd say that you do. The libc function arguments come from your program. I'd hazard a guess and say that you have an off by one error in your code that leads fwrite to read one byte past the end of its source buffer.

EDIT:

By the way, such a small error can often remain unseen (i.e. your code does not crash) because both the compiler and the memory allocator usually allocate memory blocks in specific sizes and align them at word edges. This means that many times there is a small region past the requested buffer end that you can access without triggering the memory protection code. Of course your code might just break if you change compiler, libc, platform or bitness (e.g. go from 64 to 32 bit).

Valgrind has suppression lists for expected errors in libc, which you can usually find at /usr/lib64/valgrind/default.supp or /usr/lib/valgrind/default.supp. There are quite a few issues that valgrind detects in libc, many of them intentional in an effort to optimise the code, but due to the suppresions in 99% of the cases it's the tested code that causes the problem.

EDIT2:

Keep in mind that, like most debugging tools, Valgrind will output infinitely more useful information on the issues it detects if you compile your code with debugging symbols. It will be able to point you to specific lines of code that are related to the issue - even if quite often they are not where the actual issue lies. If you use GCC just add -g to its options to compile your code with debugging symbols. In a production release, though, please remember to remove that flag!

Upvotes: 5

nategoose
nategoose

Reputation: 12382

You should follow down the call stack until you get to some code that is yours and look for the error's origin. In this case, STARCH_gzip_deflate appears to be calling fwrite with something that is bad (probably a bad FILE * or the buffer you are attempting to write out) which is causing the valgrind to bark at you.

It is possible that this is not actually an error or that it is not your error, though. But it probably is.

Upvotes: 1

Related Questions