diff mbox

Fix for PR 59039

Message ID BF230D13CA30DD48930C31D4099330003A49CB1F@FMSMSX101.amr.corp.intel.com
State New
Headers show

Commit Message

Iyer, Balaji V Nov. 8, 2013, 4:48 p.m. UTC 
Hello Everyone,
	Attached, please find a patch that will fix a crash in a function in libcilkrts when optimization was turned on. The issue was that the longjump and setjmp were called in the same function.  This patch should fix that issue. The change is not in the compiler side but only in the Cilk Library. If it is OK with everyone, I will check this in this afternoon.

Thanks,

Balaji V. Iyer.

Comments

H.J. Lu Nov. 8, 2013, 6:11 p.m. UTC | #1 
On Fri, Nov 8, 2013 at 8:48 AM, Iyer, Balaji V <balaji.v.iyer@intel.com> wrote:
> Hello Everyone,
>         Attached, please find a patch that will fix a crash in a function in libcilkrts when optimization was turned on. The issue was that the longjump and setjmp were called in the same function.  This patch should fix that issue. The change is not in the compiler side but only in the Cilk Library. If it is OK with everyone, I will check this in this afternoon.
>

It looks good to me.
diff mbox

Patch

Index: libcilkrts/runtime/cilk_fiber-unix.cpp
===================================================================
--- libcilkrts/runtime/cilk_fiber-unix.cpp	(revision 204546)
+++ libcilkrts/runtime/cilk_fiber-unix.cpp	(working copy)
@@ -44,24 +44,27 @@ 
 #include <cstdio>
 #include <cstdlib>
 
+#include <errno.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+// You'd think that getting a defintion for alloca would be easy.  But you'd
+// be wrong. Here's a variant on what's recommended in the autoconf doc.  I've
+// remove the Windows portion since this is Unix-specific code.
 #if defined HAVE_ALLOCA_H
-# include <alloca.h>
+#   include <alloca.h>
 #elif defined __GNUC__
-# define alloca __builtin_alloca
+#   define alloca __builtin_alloca
 #elif defined _AIX
-# define alloca __alloca
+#   define alloca __alloca
 #else
-# include <stddef.h>
-# ifdef  __cplusplus
+#   include <stddef.h>
+#   ifdef  __cplusplus
 extern "C"
-# endif
+#   endif
 void *alloca (size_t);
 #endif
 
-#include <errno.h>
-#include <sys/mman.h>
-#include <unistd.h>
-
 // MAP_ANON is deprecated on Linux, but seems to be required on Mac...
 #ifndef MAP_ANONYMOUS
 #define MAP_ANONYMOUS MAP_ANON
@@ -163,8 +166,15 @@ 
     __cilkrts_bug("Should not get here");
 }
 
-#pragma GCC push_options
-#pragma GCC optimize ("-O0")
+// GCC doesn't allow us to call __builtin_longjmp in the same function that
+// calls __builtin_setjmp, so create a new function to house the call to
+// __builtin_longjmp
+static void __attribute__((noinline))
+do_cilk_longjmp(__CILK_JUMP_BUFFER jmpbuf)
+{
+    CILK_LONGJMP(jmpbuf);
+}
+
 NORETURN cilk_fiber_sysdep::run()
 {
     // Only fibers created from a pool have a proc method to run and execute. 
@@ -201,7 +211,11 @@ 
         // switching to for any temporaries required for this run()
         // function.
         JMPBUF_SP(m_resume_jmpbuf) = m_stack_base - frame_size;
-        CILK_LONGJMP(m_resume_jmpbuf);
+
+        // GCC doesn't allow us to call __builtin_longjmp in the same function
+        // that calls __builtin_setjmp, so it's been moved into it's own
+        // function that cannot be inlined.
+        do_cilk_longjmp(m_resume_jmpbuf);
     }
 
     // Note: our resetting of the stack pointer is valid only if the
@@ -228,7 +242,6 @@ 
     // User proc should never return.
     __cilkrts_bug("Should not get here");
 }
-#pragma GCC pop_options
 
 void cilk_fiber_sysdep::make_stack(size_t stack_size)
 {
Index: libcilkrts/ChangeLog
===================================================================
--- libcilkrts/ChangeLog	(revision 204546)
+++ libcilkrts/ChangeLog	(working copy)
@@ -1,3 +1,9 @@ 
+2013-11-08  Balaji V. Iyer  <balaji.v.iyer@intel.com>
+
+	PR c/59039
+	* runtime/cilk_fiber-unix.cpp: Fixed a crash in run() function
+	when optimization is turned on.
+
 2013-11-04  Balaji V. Iyer  <balaji.v.iyer@intel.com>
 
 	PR bootstrap/58951