[SOLVED] Use of uninitialised value from valgrind

MikeW · 2011-12-17 09:31:57

I think I'm missing something here since I don't see what the problem is. Anyone have any ideas?

The code is working correctly as far as I can tell, I'd just like to get rid of the valgrind errors if possible.

std::string timestamp()
{
    time_t rawtime;
    struct tm * timeinfo;

    time(&rawtime);
    timeinfo = localtime( &rawtime );  // This would be the line that valgrind is complaining about.

    char buffer[80]; 
    strftime(buffer, 80, "%b %d %X", timeinfo); 

    string result(buffer);
    return result;
}

==4937== Uninitialised value was created by a heap allocation
==4937== at 0x4C2893D: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==4937== by 0x5663FED: ??? (in /lib/libc-2.14.1.so)
==4937== by 0x5663732: ??? (in /lib/libc-2.14.1.so)
==4937== by 0x5663A1A: ??? (in /lib/libc-2.14.1.so)
==4937== by 0x4033BE: timestamp() (logger.cpp:16)
==4937== by 0x403539: Logger::write(std::string, loglevels) (logger.cpp:58)
==4937== by 0x403856: main (demo.cpp:59)

==4948== HEAP SUMMARY:
==4948== in use at exit: 0 bytes in 0 blocks
==4948== total heap usage: 14 allocs, 14 frees, 11,427 bytes allocated
==4948==
==4948== All heap blocks were freed -- no leaks are possible
==4948==
==4948== For counts of detected and suppressed errors, rerun with: -v
==4948== ERROR SUMMARY: 148 errors from 42 contexts (suppressed: 6 from 6)

Last edited by MikeW (2011-12-18 01:29:33)

lunar · 2011-12-17 16:05:49

I do not know the cause of this valgrind error, but I see some general issues in the code: Use the return value of "time()" instead of passing "rawtime" as pointer, and initialize "rawtime" and "timeinfo" right away. Al least check the return value of "strftime()" to detect truncation if "timestamp()" is called for a locale that creates dates longer than 80 characters (unlikely, but possible). Even better, call "strftime()" with a null pointer first to determine the length of the formatted string, and create a buffer with the right size right away (return value of "strftime()" + 1 for terminating null character). Use a vector instead of raw C arrays.

#include <iostream>
#include <ctime>
#include <vector>
#include <limits>

static const char TIMESTAMP_FORMAT[] = "%b %d %X";

std::string timestamp()
{
    using namespace std;
    const time_t rawtime = time(0);
    struct tm * timeinfo = localtime(&rawtime);

    const size_t n = strftime(0, numeric_limits<size_t>::max(), TIMESTAMP_FORMAT, timeinfo)+1;
    vector<char> buffer(n, '\0');
    strftime(buffer.data(), buffer.size(), "%b %d %X", timeinfo);
    return string(buffer.data(), n);
}

Or just use Boost:

#include <locale>
#include <sstream>
#include <boost/date_time/posix_time/posix_time.hpp>

static const char TIMESTAMP_FORMAT[] = "%b %d %X";

std::string timestamp()
{
    using namespace std;
    using namespace boost::posix_time;
    stringstream s;
    s.imbue(locale(locale("C"), new time_facet(TIMESTAMP_FORMAT)));
    s << microsec_clock::local_time();
    return s.str();
}

tavianator · 2011-12-17 17:26:26

You've only posted half the story; where does the USE of the uninitialised value occur?

MikeW · 2011-12-17 22:29:52

I think this may be some sort of false positive from valgrind. Both my code and yours cause the same errors. I got everything else out of the picture and the errors still get thrown. As far as I call tell its the call to localtime() that is triggering the errors.

#include <iostream>
#include <vector>
#include <string>
#include <sstream>
#include <limits>
#include <ctime>

using namespace std;

static const char TIMESTAMP_FORMAT[] = "%b %d %X";

std::string timestamp()
{
    const time_t rawtime = time(0);
    struct tm * timeinfo = localtime( &rawtime );

    const size_t n = strftime(0, numeric_limits<size_t>::max(), TIMESTAMP_FORMAT, timeinfo)+1;
    vector<char> buffer(n, '\0');
    strftime(buffer.data(), buffer.size(), "%b %d %X", timeinfo);
    return string(buffer.data(), n);
}

int main()
{
    cout << timestamp() << endl;
    return 0;
}

lunar · 2011-12-17 23:51:30

@MikeW: Might be related to the fact that "localtime()" allocates a static buffer and returns a pointer to this buffer. You may try "localtime_r()" with a pointer to a stack-allocated "tm" struct:

std::string timestamp()
{
    using namespace std;
    const time_t rawtime = time(0);
    tm timeinfo;
    localtime_r(&rawtime, &timeinfo);

    const size_t n = strftime(0, numeric_limits<size_t>::max(), TIMESTAMP_FORMAT, &timeinfo)+1;
    vector<char> buffer(n, '\0');
    strftime(buffer.data(), buffer.size(), "%b %d %X", &timeinfo);
    return string(buffer.data(), n);
}

Slightly more cumbersome, but avoids the static buffer from "localtime()" and also clarifies memory ownership.

But again, avoid C functions in C++ code and just use Boost Datetime. The parts of Boost Datetime shown in my previous posting are header-only, meaning that your binary doesn't get an additional shared library dependency.

Last edited by lunar (2011-12-17 23:54:53)

MikeW · 2011-12-18 01:28:53

Thanks lunar, I may just try boost out. I was avoiding adding more dependencies since I am just learning, or should I say relearning C++. I did some C++ back in my college (longer ago than I care to admit), but am really more familiar with C#. I decided to take on C++ mainly because it seems to be a better fit with Linux coding than mono does. I am also working my way through the C++ Primer book which has been a good read so far. Effective C++ may be the next one on the list.

The boost code does look a little cleaner, and it seems that its a very popular library (set of libraries actually). I am going to mark this as solved since I understand what the problem is now, and will likely go the boost route.

Thanks again for your explanations.

Allan · 2011-12-18 01:51:58

Your code runs fine in valgrind with glibc-2.14.1-3 which will appear in [testing] in a few hours.

MikeW · 2011-12-18 03:23:09

@Allan, thanks for the update. I was wondering if there was something up with the underlying libraries since I was able to reproduce similar errors using boost. I'm leaving this alone for now, I'll check it again once glibc-2.14.1-3 hits the mirrors.

lrm · 2011-12-19 15:50:44

There is something going on here. I ran into and spent a few hours trying to track down why valgrind was reporting this in my code. Boiling it down to a simplistic test case of calling localtime() showed the error. I tested the same code on the latest Ubuntu and saw no problem.

#include <time.h>

int main(void) {
    time_t now = time(NULL);
    struct tm* nowtm = localtime(&now);
    return 0;
}

Use of localtime_r() has no affect, same problem.

Last edited by lrm (2011-12-19 15:57:54)

lrm · 2011-12-20 14:14:50

Upgraded to glibc 2.14.1-4 and it appears to have fixed the issue.

Arch Linux

#1 2011-12-17 09:31:57

[SOLVED] Use of uninitialised value from valgrind

#2 2011-12-17 16:05:49

Re: [SOLVED] Use of uninitialised value from valgrind

#3 2011-12-17 17:26:26

Re: [SOLVED] Use of uninitialised value from valgrind

#4 2011-12-17 22:29:52

Re: [SOLVED] Use of uninitialised value from valgrind

#5 2011-12-17 23:51:30

Re: [SOLVED] Use of uninitialised value from valgrind

#6 2011-12-18 01:28:53

Re: [SOLVED] Use of uninitialised value from valgrind

#7 2011-12-18 01:51:58

Re: [SOLVED] Use of uninitialised value from valgrind

#8 2011-12-18 03:23:09

Re: [SOLVED] Use of uninitialised value from valgrind

#9 2011-12-19 15:50:44

Re: [SOLVED] Use of uninitialised value from valgrind

#10 2011-12-20 14:14:50

Re: [SOLVED] Use of uninitialised value from valgrind

Board footer