get_thread_name() conditionally encounters a memory leak (?)
- Dominant language
- C++
- Stars
- 1.9k
- Forks
- 280
- Avg merge
- 7m
- Merged PRs (30d)
- 4
Description
Experienced on a Tegra TK1 running Ubuntu 14.04 32-bit with GCC 4.8 compiler.
With -O2 optimizations turned on, we see a shift in bytes for the arguments passed on to snprintf() called in get_thread_name().
```
void get_thread_name(char* buffer, unsigned long long length, bool right_align_hext_id)
{
...
if (right_align_hext_id) {
snprintf(buffer, length, "%*X", length - 1, static_cast(thread_id));
} else {
snprintf(buffer, length, "%X", static_cast(thread_id));
}
}
```
The parameter `unsigned long long length` passed into the function is used in the snprintf() function; The `*` in the `%*X` is expecting a type int. This style of _va_args_ may under some conditions be misinterpreted (only the first half of the long long is taken, shoving the rest of the long long into the coming parameter). While later versions of GCC may be smarter about it, it may not always work with all compilers.
Suggested solution: prepare the type cast before the snprintf call. Example:
```
void get_thread_name(char* buffer, unsigned long long long_length, bool right_align_hext_id)
{
int length = static_cast(long_length);
```
Additional details:
(1) Valgrind reports "Conditional jump or move depends on uninitialised value(s)" around the loguru::get_thread_name
(2) Breakpointing and tracing the calls with GDB revealed this (emphasis added):
> Breakpoint 2, __GI__IO_padn (fp=fp@entry=0xbeffef38, pad=pad@entry=48, count=1) at iopadn.c:47
>
> Breakpoint 2, __GI__IO_padn (fp=fp@entry=0xbeffef38, pad=32, count=count@entry=1) at iopadn.c:47
>
> Breakpoint 2, __GI__IO_padn (fp=fp@entry=0xbeffef38, pad=pad@entry=32, count=count@entry=14) at iopadn.c:47
>
> Breakpoint 2, __GI__IO_padn (fp=fp@entry=0xbeffef38, pad=pad@entry=32, count=count@entry=3) at iopadn.c:47
>
> // ^ a series of consecutive calls to a glibc function called `_IO_padn`.
> // And then comes this, after the call to `get_thread_name`:
> Breakpoint 2, __GI__IO_padn (fp=fp@entry=0xbeffeef0, pad=pad@entry=32, count=count@entry=**1090522931**) at iopadn.c:47
Full backtrace:
```
(gdb) bt
#0 _IO_strn_overflow (fp=0xbeffeef0, c=32) at vsnprintf.c:66
#1 0xb6dfa1ee in __GI__IO_default_xsputn (f=0xbeffeef0, data=, n=16) at genops.c:480
#2 0xb6df238c in __GI__IO_padn (fp=fp@entry=0xbeffeef0, pad=pad@entry=32, count=count@entry=1090522931) at iopadn.c:59
#3 0xb6dddae6 in _IO_vfprintf_internal (s=s@entry=0xbeffeef0, format=format@entry=0x13994 "%*X", ap=..., ap@entry=...) at vfprintf.c:1660
#4 0xb6e47a9e in ___vsnprintf_chk (s=s@entry=0xbefff100 "A ", maxlen=, maxlen@entry=11, flags=flags@entry=1, slen=slen@entry=4294967295, format=format@entry=0x13994 "%*X", args=...) at vsnprintf_chk.c:63
#5 0xb6e47a40 in ___snprintf_chk (s=s@entry=0xbefff100 "A ", maxlen=maxlen@entry=11, flags=flags@entry=1, slen=slen@entry=4294967295, format=format@entry=0x13994 "%*X") at snprintf_chk.c:34
#6 0x0000d9a0 in snprintf (__fmt=0x13994 "%*X", __n=11, __s=0xbefff100 "A ") at /usr/include/arm-linux-gnueabihf/bits/stdio2.h:65
#7 loguru::get_thread_name (buffer=buffer@entry=0xbefff100 "A ", length=, right_align_hext_id=right_align_hext_id@entry=true) at loguru.hpp:1957
#8 0x0000db22 in loguru::print_preamble (out_buff=out_buff@entry=0xbefff15c "", verbosity=verbosity@entry=-2, file=file@entry=0x13b78 "main.cpp", line=line@entry=9, out_buff_size=128) at loguru.hpp:2113
#9 0x0000d24c in loguru::log_to_everywhere (stack_trace_skip=stack_trace_skip@entry=1, verbosity=verbosity@entry=-2, file=file@entry=0x13b78 "main.cpp", line=line@entry=9, prefix=prefix@entry=0x1378c "", buff=buff@entry=0x244c0 "Something has ceased to work.\n") at loguru.hpp:2257
#10 0x0000dfe6 in loguru::log (verbosity=-2, file=0x13b78 "main.cpp", line=9, format=0x137bc "%s") at loguru.hpp:2267
#11 0x0000e5aa in loguru::StreamLogger::~StreamLogger (this=0xbefff268, __in_chrg=) at loguru.hpp:2381
#12 0x0000ad12 in main () at main.cpp:9
```
And the address reveal of the va_list argument passed shows that there is a byte alignment issue:
```
(gdb) x/100dw 0xbeffef60
0xbeffef60: -1225102064 -1225102868 -1090523232 0
0xbeffef70: 2111285930 -1224842949 -1224761344 -1225100160
0xbeffef80: -1225102064 -1224741480 -1226257992 -1090523088
0xbeffef90: 0 -1224738620 -1224790084 -1224741480
0xbeffefa0: 0 0 1 119
0xbeffefb0: -1224756240 -1224753152 36981 -1225096944
0xbeffefc0: 35596 1 -1224756336 -1224738024
0xbeffefd0: -1224738464 36981 -1090523120 -1090523108
0xbeffefe0: 80276 -1090522880 1 -1224761344
0xbeffeff0: 11 -1226540479 80276 -1090523108
0xbefff000: -1090522880 -1090523108 -1224761344 10
0xbefff010: 0 55713 80276 -1090522932
0xbefff020: 10 0 -1224761344 802944595
```
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.