In bug 533035 comment 97, I wrote: > Created an attachment (id=419074) [details] > fix valgrind warning > > While starting Firefox under valgrind, I saw the warning: > > ==30253== Syscall param rt_sigaction(act->sa_mask) points to uninitialised > byte(s) > ==30253== at 0x4E392AE: __libc_sigaction (sigaction.c:67) > ==30253== by 0x5075D48: InstallSignalHandlers(char const*) > (nsSigHandlers.cpp:307) > ==30253== by 0x5064C58: SetupErrorHandling(char const*) > (nsAppRunner.cpp:3801) > ==30253== by 0x5069B8F: XRE_main (nsAppRunner.cpp:2739) > ==30253== by 0x40110C: main (nsBrowserApp.cpp:158) > ==30253== Address 0x7fefff058 is on thread 1's stack > > > According to man sigaction(2): > sa_mask specifies a mask of signals which should be blocked (i.e., > added to the signal mask of the thread in which the signal handler is > invoked) during execution of the signal handler. In addition, the sig‐ > nal which triggered the handler will be blocked, unless the SA_NODEFER > flag is used. > > It doesn't seem particularly great to leave this uninitialized, although I'm > not sure it's all *that* horrible. > > Here's a patch to initialize to the empty set. In bug 533035 comment 111 Julian Seward wrote: > Could someone please commit Dave Baron's patch in Comment #97 ? > Passing an uninitialised .sa_mask to the kernel is .. well, in > this case, as Dave says, probably harmless, but it's ungood and > adds unnecessarily to the Valgrind noise level. In bug 533035 comment #113 Benjamin Smedberg wrote: > In order to aid trunk and branch tracking, please file new bugs for the > followup issues and request review there. This bug is long and confusing > enough, and is status1.9.2 final-fixed.
Comment on attachment 420166 [details] [diff] [review] patch Thanks.
Comment on attachment 420166 [details] [diff] [review] patch Low risk patch to silence a valgrind warning (and maybe make behavior slightly more deterministic, although unlikely); fixes regression from a patch landed very late in the 1.9.2 cycle.
Comment on attachment 420166 [details] [diff] [review] patch We'll take this in 18.104.22.168; simple one line fix.
Comment on attachment 420166 [details] [diff] [review] patch Approved for 22.214.171.124, a=dveditz for release-drivers