GNOME Bugzilla – Bug 775510
testing with -fsanitize=undefined reports various undefined behaviour
Last modified: 2016-12-02 19:12:27 UTC
While trying to debug some OSTree/libsoup issues, I tried compiling and testing GLib with gcc -g -Og -fsanitize=address -fsanitize=undefined. The OSTree and GLib test suites report various instances of undefined behaviour in GLib. I have not attempted to address memory leaks yet (I'm running the tests with ASAN_OPTIONS=detect_leaks=0).
Created attachment 341219 [details] [review] GParam: make G_PARAM_USER_MASK unsigned UBSan considers left-shifting a negative number to be undefined behaviour (per <http://www.open-std.org/jtc1/sc22/wg14/www/docs/dr_081.html> it is implementation-defined in C89, but according to <https://llvm.org/bugs/show_bug.cgi?id=17490> it is undefined in C99).
Created attachment 341220 [details] [review] Avoid calling Standard C string/array functions with NULL arguments glibc string.h declares memcpy() with attribute(nonnull(1,2)), causing calls with NULL arguments to be treated as undefined behaviour. This is consistent with ISO C99 and C11, which state that passing 0 to string functions as an array length does not remove the requirement that the pointer to the array is a valid pointer. gcc -fsanitize=undefined catches this while running OSTree's test suite. Similarly, running the GLib test suite reports similar issues for qsort(), memmove(), memcmp().
Created attachment 341221 [details] [review] gdbus-serialization test: don't left-shift a negative number -2LL<<34 is undefined, because left-shifting a negative number is undefined (it was implementation-defined behaviour in C99, but is formally undefined in C11). The undefined behaviour sanitizer picks this up.
Created attachment 341222 [details] [review] g_unichar_iswide_cjk: add a special case for U+0000 bsearch() is defined to search for a non-null key, so we can't search for NULL. The undefined behaviour sanitizer picks this up.
Created attachment 341223 [details] [review] g_hostname_is_ip_address: detect integer overflow Signed integer overflow is undefined behaviour, which the undefined behaviour sanitizer detects. Previously, if the compiler had implemented this in the obvious way (overflowing signed multiplication wraps around mod 2**32), we would have incorrectly classified addresses where one octet was, for example, (2**32 + 42) as valid IP addresses, by treating that octet as though it was 42.
Created attachment 341224 [details] [review] type-test: do not rely on signed integer overflow wrapping around Signed integer overflow is undefined behaviour: if a compiler detects signed integer overflow, it is free to compile it to absolutely anything.
(In reply to Simon McVittie from comment #6) > type-test: do not rely on signed integer overflow wrapping around > > Signed integer overflow is undefined behaviour: if a compiler > detects signed integer overflow, it is free to compile it to absolutely > anything. This does still assume two's complement arithmetic, but I think that's implementation-defined (and done by all practical compilers and CPUs in practice), not undefined.
I still get a memory leak report that makes the gschema-compile test fail: GLib-GIO:ERROR:/home/smcv/src/glib/gio/tests/gschema-compile.c:52:test_schema: child process (/gschema/range-missing-min/subprocess/do_compile [28957]) failed unexpectedly PASS: gschema-compile 7 /gschema/range-wrong-type # child process (/gschema/range-missing-min/subprocess/do_compile [28957]) stdout: "" # child process (/gschema/range-missing-min/subprocess/do_compile [28957]) stderr: "\n=================================================================\n==28957==ERROR: LeakSanitizer: detected memory leaks\n\nDirect leak of 168 byte(s) in 1 object(s) allocated from:\n #0 0x7f2f54d66d28 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1d28)\n #1 0x7f2f54645b04 in g_try_malloc /home/smcv/src/glib/glib/gmem.c:242\n\nDirect leak of 16 byte(s) in 1 object(s) allocated from:\n #0 0x7f2f54d66d28 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1d28)\n #1 0x7f2f54645965 in g_malloc /home/smcv/src/glib/glib/gmem.c:94\n\nIndirect leak of 73 byte(s) in 1 object(s) allocated from:\n #0 0x7f2f54d66d28 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc1d28)\n #1 0x7f2f54645965 in g_malloc /home/smcv/src/glib/glib/gmem.c:94\n\nSUMMARY: AddressSanitizer: 257 byte(s) leaked in 3 allocation(s).\n" Aborted (core dumped) which I would have expected would be silenced by ASAN_OPTIONS=detect_leaks=0, but for some reason it isn't.
Review of attachment 341219 [details] [review]: Ok.
Review of attachment 341220 [details] [review]: Yep.
Review of attachment 341221 [details] [review]: OK
Review of attachment 341222 [details] [review]: OK, I probably would have put it before the _iswide() call but it doesn't matter of course.
Review of attachment 341223 [details] [review]: That's a nice -fsanitize=undefined catch.
Review of attachment 341224 [details] [review]: OK.
Fixed in git for 2.51.1, thanks.