{"thread":{"id":"60253","subject":"[PATCH] test-lib: set UBSAN_OPTIONS to match ASan","startedAt":"2023-09-21T20:48:59Z","lastAt":"2023-09-21T21:41:42Z","messageCount":2,"participants":["Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"482106","messageId":"20230921041825.GA2814583@coredump.intra.peff.net","threadId":"60253","inReplyTo":null,"subject":"[PATCH] test-lib: set UBSAN_OPTIONS to match ASan","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2023-09-21T04:18:25Z","receivedAt":"2023-09-21T20:48:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"For a long time we have used ASAN_OPTIONS to set abort_on_error. This is\nimportant because we want to notice detected problems even in programs\nwhich are expected to fail. But we never did the same for UBSAN_OPTIONS.\nThis means that our UBSan test suite runs might silently miss some\ncases.\n\nIt also causes a more visible effect, which is that t4058 complains\nabout unexpected \"fixes\" (and this is how I noticed the issue):\n\n  $ make SANITIZE=undefined CC=gcc && (cd t && ./t4058-*)\n  ...\n  ok 8 - git read-tree does not segfault # TODO known breakage vanished\n  ok 9 - reset --hard does not segfault # TODO known breakage vanished\n  ok 10 - git diff HEAD does not segfault # TODO known breakage vanished\n\nThe tests themselves aren't that interesting. We have a known bug where\nthese programs segfault, and they do when compiled without sanitizers.\nWith UBSan, when the test runs:\n\n  test_might_fail git read-tree --reset base\n\nit gets:\n\n  cache-tree.c:935:9: runtime error: member access within misaligned address 0x5a5a5a5a5a5a5a5a for type 'struct cache_entry', which requires 8 byte alignment\n\nSo that's garbage memory which would _usually_ cause us to segfault, but\nUBSan catches it and complains first about the alignment. That makes\nsense, but the weird thing is that UBSan then exits instead of aborting,\nso our test_might_fail call considers that an acceptable outcome and the\ntest \"passes\".\n\nCuriously, this historically seems to have aborted, because I've run\n\"make test\" with UBSan many times (and so did our CI) and we never saw\nthe problem. Even more curiously, I see an abort if I use clang with\nASan and UBSan together, like:\n\n  # this aborts!\n  make SANITIZE=undefined,address CC=clang\n\nBut not with just UBSan, and not with both when used with gcc:\n\n  # none of these do\n  make SANITIZE=undefined CC=gcc\n  make SANITIZE=undefined CC=clang\n  make SANITIZE=undefined,address CC=gcc\n\nLikewise moving to older versions of gcc (I tried gcc-11 and gcc-12 on\nmy Debian system) doesn't abort. Nor does moving around in Git's\nhistory. Neither this test nor the relevant code have been touched in a\nwhile, and going back to v2.41.0 produces the same outcome (even though\nmany UBSan CI runs have passed in the meantime).\n\nSo _something_ changed on my system (and likely will soon on other\npeople's, since this is stock Debian unstable), but I didn't track\nit further. I don't know why it ever aborted in the past, but we\ndefinitely should be explicit here and tell UBSan what we want to\nhappen.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/lib-httpd/apache.conf | 1 +\n t/test-lib.sh           | 3 +++\n 2 files changed, 4 insertions(+)\n\ndiff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf\nindex a22d138605..022276a6b9 100644\n--- a/t/lib-httpd/apache.conf\n+++ b/t/lib-httpd/apache.conf\n@@ -92,6 +92,7 @@ PassEnv GIT_VALGRIND_OPTIONS\n PassEnv GNUPGHOME\n PassEnv ASAN_OPTIONS\n PassEnv LSAN_OPTIONS\n+PassEnv UBSAN_OPTIONS\n PassEnv GIT_TRACE\n PassEnv GIT_CONFIG_NOSYSTEM\n PassEnv GIT_TEST_SIDEBAND_ALL\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 5ea5d1d62a..1656c9eed0 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -89,6 +89,9 @@ prepend_var LSAN_OPTIONS : $GIT_SAN_OPTIONS\n prepend_var LSAN_OPTIONS : fast_unwind_on_malloc=0\n export LSAN_OPTIONS\n \n+prepend_var UBSAN_OPTIONS : $GIT_SAN_OPTIONS\n+export UBSAN_OPTIONS\n+\n if test ! -f \"$GIT_BUILD_DIR\"/GIT-BUILD-OPTIONS\n then\n \techo >&2 'error: GIT-BUILD-OPTIONS missing (has Git been built?).'\n-- \n2.42.0.711.g968f0256ae\n"},{"id":"482115","messageId":"xmqqwmwj443q.fsf@gitster.g","threadId":"60253","inReplyTo":"20230921041825.GA2814583@coredump.intra.peff.net","subject":"Re: [PATCH] test-lib: set UBSAN_OPTIONS to match ASan","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-09-21T21:18:49Z","receivedAt":"2023-09-21T21:41:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> So that's garbage memory which would _usually_ cause us to segfault, but\n> UBSan catches it and complains first about the alignment. That makes\n> sense, but the weird thing is that UBSan then exits instead of aborting,\n> so our test_might_fail call considers that an acceptable outcome and the\n> test \"passes\".\n\nt4058 unexpectedly succeeding vaguely rings a bell to me, too, but\nit has been sporadic.  The changes are obviously the right thing to\ndo.\n\nThanks, queued.\n\n"}]}