travis: enable Memory Sanitizer (MSan) #11730
Conversation
af10cbe to
d5bf98c
Compare
6cfd02a to
f557350
Compare
|
@mrc0mmand thank you! Ideally, all the dependencies should be instrumented because of https://clang.llvm.org/docs/MemorySanitizer.html#limitations but I guess getting around false positives one by one as in 3e180a2 should be fine too. Having taken a look at https://travis-ci.org/systemd/systemd/jobs/494149490, I was going to write that it would be better to add something like
That's what I do :-) I put kludges until it works :-) I'll take a closer look on Monday. (I'd also add the "don't-merge label" and open a couple of issues regarding the tests failing under MSan). |
f557350 to
5bf4122
Compare
For some strange reason adding |
|
As far as I can tell, it's just a variation of mesonbuild/meson#4542. The only difference is that |
6a16afd to
4fb258c
Compare
|
Sorry. I should have mentioned that even with the "-fsanitize=memory" kludge, |
Ahh, I see, thanks! |
4fb258c to
5db9450
Compare
12419e6 to
37958c2
Compare
|
@mrc0mmand it looks suspicious that there're a lot of tests that are just skipped. Could you try to set |
37958c2 to
67f17a7
Compare
The test output now definitely looks more complete, thanks! |
67f17a7 to
02781d4
Compare
|
So, instead of trying to workaround Travis log limits, I ran the |
02781d4 to
80cd304
Compare
4f8b306 to
843a258
Compare
With something like this, non-sudo tests pass locally on Fedora.
test-helper.c predates tests.h, but nowadays the latter is more appropriate.
Even after disabling various uninstrumented dependencies, there are some failing tests, because of uninstrumented mandatory dependencies. Let's use the crude approach of skipping those tests. The skipping is opt-in — so anyone running the tests under msan will get those failures, unless they specify the env var. Now all tests pass under sudo on Fedora. Let's see if this is enough for Travis. Hopefully closes systemd#11738.
843a258 to
d3d1031
Compare
|
Just to provide a quick update: almost all issues (so far) have been resolved. The last one is #12825. |
VALGRIND=1 was intended as a catch-all to avoid memory access patterns that are OK on current hardware but undefined behaviour according to strict language rules. It applies to msan tests as well.
It's just duct tape and popsicle sticks for now
Uninitialized bytes in __interceptor_strcmp at offset 0 inside [0x7010000007d0, 5)
==24332==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x7fce2e98315c in g_str_equal (/lib64/libglib-2.0.so.0+0x3e15c)
#1 0x7fce2e982408 in g_hash_table_lookup (/lib64/libglib-2.0.so.0+0x3d408)
#2 0x7fce2e9d950e in g_variant_type_info_get (/lib64/libglib-2.0.so.0+0x9450e)
#3 0x7fce2e9d1f2c (/lib64/libglib-2.0.so.0+0x8cf2c)
#4 0x7fce2e9cc53f in g_variant_new_dict_entry (/lib64/libglib-2.0.so.0+0x8753f)
#5 0x7fce2e80f65f (/lib64/libgio-2.0.so.0+0xcc65f)
#6 0x7fce2e80f8c4 (/lib64/libgio-2.0.so.0+0xcc8c4)
#7 0x7fce2e811794 in g_dbus_message_new_from_blob (/lib64/libgio-2.0.so.0+0xce794)
#8 0x49a3dc in main /home/fsumsal/repos/systemd/build/../src/libsystemd/sd-bus/test-bus-marshal.c:216:21
#9 0x7fce2e3b7f32 in __libc_start_main (/lib64/libc.so.6+0x23f32)
#10 0x41c61d in _start (/home/fsumsal/repos/systemd/build/test-bus-marshal+0x41c61d)
Uninitialized value was stored to memory at
#0 0x42c43c in strncpy (/home/fsumsal/repos/systemd/build/test-bus-marshal+0x42c43c)
#1 0x7fce2e9b415b in g_strndup (/lib64/libglib-2.0.so.0+0x6f15b)
Uninitialized value was created by a heap allocation
#0 0x425dc4 in __interceptor_malloc (/home/fsumsal/repos/systemd/build/test-bus-marshal+0x425dc4)
#1 0x7fce2e999dc5 in g_malloc (/lib64/libglib-2.0.so.0+0x54dc5)
SUMMARY: MemorySanitizer: use-of-uninitialized-value (/lib64/libglib-2.0.so.0+0x3e15c) in g_str_equal
Uninitialized bytes in __interceptor_strcmp at offset 4 inside [0x7010000001b0, 5)
==27105==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x7fcc0947a15c in g_str_equal (/lib64/libglib-2.0.so.0+0x3e15c)
#1 0x7fcc09478dea (/lib64/libglib-2.0.so.0+0x3cdea)
#2 0x7fcc094d094f in g_variant_type_info_unref (/lib64/libglib-2.0.so.0+0x9494f)
#3 0x7fcc094cf966 in g_variant_serialised_is_normal (/lib64/libglib-2.0.so.0+0x93966)
#4 0x7fcc094cfca8 in g_variant_serialised_is_normal (/lib64/libglib-2.0.so.0+0x93ca8)
#5 0x7fcc094cfeb2 in g_variant_serialised_is_normal (/lib64/libglib-2.0.so.0+0x93eb2)
#6 0x7fcc094cfca8 in g_variant_serialised_is_normal (/lib64/libglib-2.0.so.0+0x93ca8)
#7 0x7fcc094d008c in g_variant_serialised_is_normal (/lib64/libglib-2.0.so.0+0x9408c)
#8 0x7fcc094cfca8 in g_variant_serialised_is_normal (/lib64/libglib-2.0.so.0+0x93ca8)
#9 0x7fcc094c999e in g_variant_is_normal_form (/lib64/libglib-2.0.so.0+0x8d99e)
#10 0x49dbd3 in test_marshal /home/fsumsal/repos/systemd/build/../src/libsystemd/sd-bus/test-bus-gvariant.c:190:17
#11 0x495495 in main /home/fsumsal/repos/systemd/build/../src/libsystemd/sd-bus/test-bus-gvariant.c:222:16
#12 0x7fcc09106f32 in __libc_start_main (/lib64/libc.so.6+0x23f32)
#13 0x41c46d in _start (/home/fsumsal/repos/systemd/build/test-bus-gvariant+0x41c46d)
Uninitialized value was created by a heap allocation
#0 0x425c14 in __interceptor_malloc (/home/fsumsal/repos/systemd/build/test-bus-gvariant+0x425c14)
#1 0x7fcc09490dc5 in g_malloc (/lib64/libglib-2.0.so.0+0x54dc5)
SUMMARY: MemorySanitizer: use-of-uninitialized-value (/lib64/libglib-2.0.so.0+0x3e15c) in g_str_equal
Uninitialized bytes in __interceptor_strcmp at offset 13 inside [0x701000000020, 14)
==27715==WARNING: MemorySanitizer: use-of-uninitialized-value
#0 0x7ff0a9ba915c in g_str_equal (/lib64/libglib-2.0.so.0+0x3e15c)
#1 0x7ff0a9ba7dea (/lib64/libglib-2.0.so.0+0x3cdea)
#2 0x7ff0a9bff94f in g_variant_type_info_unref (/lib64/libglib-2.0.so.0+0x9494f)
#3 0x7ff0a9bf8025 in g_variant_unref (/lib64/libglib-2.0.so.0+0x8d025)
#4 0x49dd84 in test_marshal /home/fsumsal/repos/systemd/build/../src/libsystemd/sd-bus/test-bus-gvariant.c:168:17
#5 0x495495 in main /home/fsumsal/repos/systemd/build/../src/libsystemd/sd-bus/test-bus-gvariant.c:224:16
#6 0x7ff0a9835f32 in __libc_start_main (/lib64/libc.so.6+0x23f32)
#7 0x41c46d in _start (/home/fsumsal/repos/systemd/build/test-bus-gvariant+0x41c46d)
Uninitialized value was created by a heap allocation
#0 0x425c14 in __interceptor_malloc (/home/fsumsal/repos/systemd/build/test-bus-gvariant+0x425c14)
#1 0x7ff0a9bbfdc5 in g_malloc (/lib64/libglib-2.0.so.0+0x54dc5)
SUMMARY: MemorySanitizer: use-of-uninitialized-value (/lib64/libglib-2.0.so.0+0x3e15c) in g_str_equal
d3d1031 to
23691f7
Compare
|
@mrc0mmand now that libcap and libmount are skipped, could you revert 7916762?
I think that's exactly what we should do. I'm frankly still feeling somewhat uneasy about rolling out this PR :-) |
|
@evverx I played around with package rebuild on both Debian and Arch Linux and I'm strongly inclining to Arch. On Debian, each package has the configuration scattered around multiple files and there's no way (at least known to me) how to tell On Arch, the only thing you need to do is add following lines to and then rebuild each package with If this is a viable way to go, I'd add a yet another TravisCI manager for Arch (for a start). We'd also have to make use of the Travis CI cache, so we don't have to rebuild all dependencies in every PR. |
|
@mrc0mmand before moving forward, I think it would make sense to take a look at https://github.com/google/oss-fuzz/tree/master/infra/base-images/base-msan-builder and especially at how, for example, In general I think it would be useful to utilize MSan but given that it's relatively hard to maintain I'm not sure who is going to do that and how long it's going to last. Plus I'd say MSan is the next level and it would probably be better to deal with the low-hanging fruit like systemd/systemd-centos-ci#117 first. |
|
@evverx it would indeed make sense to first sort out the outstanding issues with ASan and UBSan, before we jump at MSan. I was just pushing this one forward, as I can't do basically anything for systemd/systemd-centos-ci#117 due to my absolute lack of networking knowledge in systemd (and more or less networking in general).
Thanks for the link, will definitely check it out! |
|
Now that #13281 is merged all the fuzzers are run under MSan (in addition to ASan and UBsan) against the corpora OSS-Fuzz and Fuzzit have been accumulating for more than a year when PRs are opened. |
|
Should we have another go at this? |
Few weeks ago @evverx hinted (#11592 (comment)) that adding MSan to our collection of sanitizers would be nice, so there it is. It definitely needs some tweaking, but at least it finally compiles without any issues.
As stated in documentation, the MSan has to run separately from other sanitizers, hence the separate job. Also, it suffers from the same issue as other sanitizers under clang, but the workaround introduced by @evverx works in this case as well.
Feedback is, of course, more than welcome!