2

I have the following program http://ideone.com/1RPs8E . It uses a variadic function tlog that will print a line in a log file. It receives a level for the line printed, a file name, a line and a function for debug information and a format for printf and a list o arguments.

void tlog(int level, const char *file, int line, const char *fun, const char *fmt, ...)

I also use a LOGL macro that calls tlog function for the current file, line and function.

#define LOGL(level, fmt, ...) tlog(level, __FILENAME__, __LINE__, __FUNCTION__, fmt, ##__VA_ARGS__)

And various macros that use LOGL macro for example:

#define DEBUGEX(fmt, ...) LOGL(LDEBUGEX, fmt, ##__VA_ARGS__)
#define DEBUG(fmt, ...) LOGL(LDEBUG, fmt, ##__VA_ARGS__)
#define INFO(fmt, ...)  LOGL(LINFO, fmt, ##__VA_ARGS__)

When running the program in valgrind:

$ valgrind -v --track-origins=yes ./t

I receive the following error: "Uninitialised value was created by a stack allocation" on the line 150 (when I use the DEBUGEX macro). Full log here: http://pastebin.com/rZu4nkHd

What is the problem with the code? For me it seems to be ok. If I remove the level parameter from the tlog function and all the macros that call it the error does not occur anymore.

Tested with gcc 4.8.2 on Archlinux, and gcc 4.6.3 on Ubuntu 12.04.3

tshepang
  • 12,111
  • 21
  • 91
  • 136
Victor Dodon
  • 1,796
  • 3
  • 18
  • 27

2 Answers2

4
#define nmemb 1
#define size 32

...
void *_nptr_ = calloc((nmemb), (size));
LOGL(LDEBUGEX, "CALLOC: %#lX nmemb %ld, size %ld", _nptr_, (nmemb), (size));

The conversion specifiers are wrong all three.

The latter two conversion specifiers cause Valgrind to complain, as the amount of data passed in (int) does not match what the format expects (long).

The format shall look like: 

..., "%p ... %d ... %d", _nptr_, ...

However a "nicer" approach would be:

#define nmemb ((size_t) 1)
#define size ((size_t) 32)

...

..., "%p ... %zu ... %zu", _nptr_, nmemb, size);
alk
  • 69,737
  • 10
  • 105
  • 255
  • 2
    While you're entirely correct, only the second and third lead to reading of uninitialised data. (Of course, on other platforms, the first could break too, so I don't recommend leaving it like it is.) –  Dec 07 '13 at 10:19
  • +1, but as stated already the cast to `void*` is superflouous in that case – Jens Gustedt Dec 07 '13 at 10:20
  • @hwd, no the log clearly says that it is the initialization of `_nptr_`. Probably `long` is 64 bit on that machine and pointers are 32. – Jens Gustedt Dec 07 '13 at 10:21
  • @JensGustedt The log doesn't say "nptr" anywhere, and the line numbers don't point to the line where `_nptr_` is initialised either. –  Dec 07 '13 at 10:23
3

You're using %ld to print a constant of type int, and presumably you're on a platform there int and long do not have the same size. The fact that it happens in _itoa_word is a good hint.

The reason it does not crash or print garbage is because the extra bytes that are getting read happen to be zero, so the code that reads the value gets the right value, the wrong way.