Bug 10247 - liboil should use sigsetjmp when testing for CPU.
Summary: liboil should use sigsetjmp when testing for CPU.
Status: RESOLVED FIXED
Alias: None
Product: liboil
Classification: Unclassified
Component: unknown (show other bugs)
Version: HEAD
Hardware: All All
: medium normal
Assignee: David Schleef
QA Contact: David Schleef
URL:
Whiteboard:
Keywords:
: 11997 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-03-10 13:06 UTC by didier
Modified: 2007-08-15 14:22 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description didier 2007-03-10 13:06:31 UTC
from setjmp manual:
POSIX  does  not  specify whether setjmp() will save the signal context.

Example of strace output on a G3 running linux Ubuntu 2.6.20

strace -f gnome-sound-properties without a ~/.gstreamer-0.10
directory.
...
32021 open("/usr/lib/liboil-0.3.so.0", O_RDONLY) = 5
32021 rt_sigaction(SIGILL, {0xe0a3120, [], 0}, {SIG_DFL}, 8) = 0
...
32021 --- SIGILL (Illegal instruction) @ 0 (0) ---
           no altivec
32021 rt_sigaction(SIGILL, {SIG_DFL}, NULL, 8) = 0
32021 rt_sigaction(SIGILL, {0xe0a3120, [], 0}, {SIG_DFL}, 8) = 0
32021 rt_sigaction(SIGILL, {SIG_DFL}, NULL, 8) = 0
32021 rt_sigaction(SIGSEGV, {SIG_DFL}, NULL, 8) = 0
...
32021 open("/usr/lib/libvisual-0.4.so.0", O_RDONLY) = 5
....
32021 rt_sigaction(SIGILL, {0xe03b884, [ILL], SA_RESTART}, {SIG_DFL}, 8) = 0
                     SIGILL is now blocked...
32021 rt_sigprocmask(SIG_BLOCK, NULL, [ILL], 8) = 0
32021 --- SIGILL (Illegal instruction) @ 0 (0) ---
                undefined behaviour...
32020 <... read resumed> "", 1)         = 0
32020 --- SIGCHLD (Child exited) @ 0 (0) ---
32020 close(3)                          = 0

Sure it's also a bug in libvisual but IMO a library should change signal set if it doesn't need it.
Comment 1 didier 2007-03-10 13:40:22 UTC
Can't create an attachment but the patch is obvious

diff -Nru /tmp/f3RzZU4qya/liboil-0.3.10/liboil/liboilcpu.c /tmp/6RumtZJFBp/liboil-0.3.10/liboil/liboilcpu.c
--- /tmp/f3RzZU4qya/liboil-0.3.10/liboil/liboilcpu.c	2007-03-09 12:38:34.000000000 +0100
+++ /tmp/6RumtZJFBp/liboil-0.3.10/liboil/liboilcpu.c	2007-03-09 12:38:37.000000000 +0100
@@ -142,7 +142,7 @@
 illegal_instruction_handler (int num)
 {
   if (in_try_block) {
-    longjmp (jump_env, 1);
+    siglongjmp (jump_env, 1);
   } else {
     abort ();
   }
@@ -196,7 +196,7 @@
   int ret;
 
   in_try_block = 1;
-  ret = setjmp (jump_env);
+  ret = sigsetjmp (jump_env,1);
   if (!ret) {
     func (priv);
   }
Comment 2 David Schleef 2007-03-16 14:46:34 UTC
What does this fix?  Liboil code doesn't change any signal masks, so it's not really important to save them or not.
Comment 3 didier 2007-03-17 02:47:43 UTC
(In reply to comment #2)
> What does this fix?  Liboil code doesn't change any signal masks, so it's not
> really important to save them or not.
> 

Yes it does, SIGILL is blocked in the signal handler, and the longjmp doesn't unblock it. In the strace output there's no call to sigset.
Comment 4 David Schleef 2007-03-17 11:54:15 UTC
That's not in liboil, it's in libvisual.
Comment 5 didier 2007-03-22 10:07:10 UTC
(In reply to comment #4)
> That's not in liboil, it's in libvisual.
> 
If you compile the following code oil_cpu_test.c:

#include <stdio.h>
main()
{
_oil_cpu_init();
_oil_cpu_init();
printf("done\n");
}

gcc oil_cpu_test.c -o oil_cpu /usr/lib/liboil-0.3.a -lm

On a G3 for some systems (kernel/libc) it will dump core in the second _oil_cpu_init
Comment 6 David Schleef 2007-03-22 14:07:12 UTC
Ok, I see what the problem is now.  Both the kernel and liboil have changed subtlely since this was tested heavily.
Comment 7 Bastien Nocera 2007-08-14 08:43:18 UTC
*** Bug 11997 has been marked as a duplicate of this bug. ***
Comment 8 David Schleef 2007-08-14 10:54:35 UTC
*** Bug 11997 has been marked as a duplicate of this bug. ***
Comment 9 David Schleef 2007-08-14 10:57:11 UTC
Checked in, finally.
Comment 10 Bastien Nocera 2007-08-15 01:28:29 UTC
Doesn't compile:
liboilcpu.c:207:28: error: macro "sigsetjmp" requires 2 arguments, but only 1 given
liboilcpu.c: In function 'oil_cpu_fault_check_try':
liboilcpu.c:207: error: 'sigsetjmp' undeclared (first use in this function)
liboilcpu.c:207: error: (Each undeclared identifier is reported only once
liboilcpu.c:207: error: for each function it appears in.)
make[4]: *** [liboil_0.3_la-liboilcpu.lo] Error 1

That's on i686 with glibc-2.6.90-8 (Fedora Rawhide)
Comment 11 Bastien Nocera 2007-08-15 01:31:48 UTC
-  ret = sigsetjmp (jump_env);
+  ret = sigsetjmp (jump_env, 1);

That should do.
Comment 12 David Schleef 2007-08-15 14:22:16 UTC
Er, I'm an idiot.

First of all, for checking in code that trivially doesn't compile.

Second, because I fixed the problem without using sigsetjmp/siglongjmp() by restoring the signal mask by hand.  Restoring the mask by hand has the advantage of being slightly more thread-friendly, since it's an atomic unmasking of SIGILL.  I think.  Also, I think I was worried that siglongjmp() isn't supported on ${some_platform}.

Anyway, I'm leaving it as sigsetjmp/siglongjmp() because these function calls are designed to do exactly what we're doing.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.