Fully initialize the epoll_data structure.

[Lukasa]

While running a simple libdispatch-using application on Linux under Valgrind, I noticed that Valgrind complained that the struct epoll_event initialized in this function was not fully initialized:

==3103== Syscall param epoll_ctl(event) points to uninitialised byte(s)
==3103==    at 0x5DBE63A: epoll_ctl (syscall-template.S:81)
==3103==    by 0x4075493: _dispatch_timeout_program (in /usr/lib/swift/linux/libdispatch.so)
==3103==    by 0x407534F: _dispatch_event_loop_timer_arm (in /usr/lib/swift/linux/libdispatch.so)
==3103==    by 0x4072391: _dispatch_timers_program (in /usr/lib/swift/linux/libdispatch.so)
==3103==    by 0x40712D9: _dispatch_mgr_invoke (in /usr/lib/swift/linux/libdispatch.so)
==3103==    by 0x407118D: _dispatch_mgr_thread (in /usr/lib/swift/linux/libdispatch.so)

While this is unlikely to cause any bugs today (none of those fields are accessed anywhere else), we can save ourselves the risk of future pain by initialising those portions of the structure appropriately before we pass it to the kernel.

[MadCoder]

This just can't be the cause for the bug, as C initializer will 0-initialize non explicitly initialized fields already.

[Lukasa]

You're right that the fix is wrong, but the why was a bit of a surprise to me. I went back and checked the man page for epoll and noticed that epoll_data is a union, not a struct (my mistake), and that union type contains 64-bit members. The compiler output for _dispatch_timeout_program initialises .data like this:

   4f3d7:       4c 8d 3d f2 58 04 00    lea    0x458f2(%rip),%r15        # 94cd0 <_dispatch_epoll_timeout>
   4f3de:       43 0f b7 44 e7 04       movzwl 0x4(%r15,%r12,8),%eax
   4f3e4:       89 44 24 04             mov    %eax,0x4(%rsp)

Note that the initialisation is done with movzwl, which initialises only first 4 bytes of the 8 byte union. Nowhere else zero-initialises the epoll_event structure, so the trailing 4 bytes of the structure are in fact uninitialized.

As noted before, this is probably not an issue, but it may be better to split the initialisation up. If we changed the code to look like this:

struct epoll_event ev = {
    .events = EPOLLONESHOT | EPOLLIN
};
ev.data.u32 = timer->det_ident;

This should correctly initialise all bytes of the union, and silence this warning. I'll double-check this by recompiling Swift and confirming that this fixes up the assembly, and that this no longer annoys valgrind, and then open a new PR.

[MadCoder]

Ok, this makes more sense, I like the code the way you just wrote it. I reopened the pull request, please make this patch instead.

[MadCoder]

actually there's even simpler, which is to use u64 instead in the dispatch codebase, given that it's the larger member of the union, it'll DTRT.

[Lukasa]

Yup, that fix works. I'll open a new PR.

[MadCoder]

you can reuse this one and force push it should work. (I think).