glommio icon indicating copy to clipboard operation
glommio copied to clipboard

Make `msan` happy

Open vmingchen opened this issue 2 years ago • 2 comments

This is a false positive of memory sanitizer because the CQE should have been set (at least to NULL) by kernel here: https://elixir.bootlin.com/linux/latest/source/tools/io_uring/queue.c#L19

Use MaybeUninit::zeroed instead to make msan happy.

Here is the msan message:

==189722==WARNING: MemorySanitizer: use-of-uninitialized-value
error: address range table at offset 0x1b0100 has a premature terminator entry at offset 0x1b0110
error: address range table at offset 0x2130e0 has a premature terminator entry at offset 0x213160
    #0 0xaaaae5f20d4c in glommio::iou::completion_queue::CompletionQueue::peek_for_cqe::he650cead72246369 /home/ubuntu/github/glommio/glommio/src/iou/completion_queue.rs:37:16
    #1 0xaaaae8450c68 in glommio::iou::IoUring::peek_for_cqe::h2419e4a32ec453e5 /home/ubuntu/github/glommio/glommio/src/iou/mod.rs:215:9
    #2 0xaaaae84a3e64 in _$LT$glommio..sys..uring..SleepableRing$u20$as$u20$glommio..sys..uring..UringCommon$GT$::consume_one_event::h433849c0cd1b14aa /home/ubuntu/github/glommio/glommio/src/sys/uring.rs:1202:13
    #3 0xaaaae905e9fc in glommio::sys::uring::UringCommon::consume_completion_queue::h74c2f2693444c020 /home/ubuntu/github/glommio/glommio/src/sys/uring.rs:728:19
    #4 0xaaaae9107be4 in glommio::sys::uring::Reactor::wait::h1e73eff475d38cbd /home/ubuntu/github/glommio/glommio/src/sys/uring.rs:1845:9
    #5 0xaaaae96e22c4 in glommio::reactor::Reactor::react::he276c7e1c29eea29 /home/ubuntu/github/glommio/glommio/src/reactor.rs:866:15
    #6 0xaaaae8383dbc in glommio::parking::Inner::park::h85b46b0d01a267c8 /home/ubuntu/github/glommio/glommio/src/parking.rs:87:9
    #7 0xaaaae834d09c in glommio::parking::Parker::poll_io::h4ff7b1c593735c49 /home/ubuntu/github/glommio/glommio/src/parking.rs:64:9

What does this PR do?

A brief description of the change being made with this pull request.

Motivation

What inspired you to submit this pull request?

Related issues

A list of issues either fixed, containing architectural discussions, otherwise relevant for this Pull Request.

Additional Notes

Anything else we should know when reviewing?

Checklist

[] I have added unit tests to the code I am submitting [] My unit tests cover both failure and success scenarios [] If applicable, I have discussed my architecture

vmingchen avatar Apr 18 '22 18:04 vmingchen

Hum this is a bit sad given the kernel guarantees the CQE is initialized if the syscall doesn't return an error code. This code is rather hot so I wonder if there is a way to mute the false positive instead?

HippoBaro avatar Apr 18 '22 20:04 HippoBaro

We can disable the sanitizer for this particular function with the #[no_sanitize(memory)] attribute. But that is an experimental feature and we need to add #![feature(no_sanitizer) to the create attributes to enable it. Considering the sanitizer support itself (https://github.com/rust-lang/rust/issues/39699) is still WIP, I will not do so for now.

I will just keep this PR open so that people are aware of this in case they ran memory sanitizer and encountered this.

Hope the large Rust sanitizer work will be ready soon. After that, I will come back to this. I will also probably add a check of test run with sanitizer support enabled.

vmingchen avatar Apr 20 '22 13:04 vmingchen