Closed Bug 538013 Opened 10 years ago Closed 10 years ago

fix valgrind warning initializing FPE exception handler

Categories

(Core :: General, defect, P3)

All
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla1.9.3a1
Tracking Status
status1.9.2 --- .9-fixed

People

(Reporter: dbaron, Assigned: dbaron)

References

Details

Attachments

(1 file)

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.
Attached patch patchSplinter Review
Attachment #420166 - Flags: review?
Attachment #420166 - Flags: review? → review?(gal)
Comment on attachment 420166 [details] [diff] [review]
patch

Thanks.
Attachment #420166 - Flags: review?(gal) → review+
Summary: fix valgrind warning in FPE exception handler → fix valgrind warning initializing FPE exception handler
http://hg.mozilla.org/mozilla-central/rev/cf068f95e2ab
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Priority: -- → P3
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a1
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.
Attachment #420166 - Flags: approval1.9.2.1?
Comment on attachment 420166 [details] [diff] [review]
patch

We'll take this in 1.9.2.3; simple one line fix.
Attachment #420166 - Flags: approval1.9.2.3?
Attachment #420166 - Flags: approval1.9.2.2?
Attachment #420166 - Flags: approval1.9.2.2-
Attachment #420166 - Flags: approval1.9.2.4? → approval1.9.2.8?
Comment on attachment 420166 [details] [diff] [review]
patch

Approved for 1.9.2.9, a=dveditz for release-drivers
Attachment #420166 - Flags: approval1.9.2.9? → approval1.9.2.9+
You need to log in before you can comment on or make changes to this bug.