Re: Restore functional DONT_USE_BUILTIN_SETJMP support

Re: Restore functional DONT_USE_BUILTIN_SETJMP support

[Dominique d'Humières]

> The attached patch replaces it with an ad-hoc definition of setjmp, …

This cause

% /opt/gcc/gcc8w/bin/g++ -std=c++11 -O2 -fnon-call-exceptions /opt/gcc/work/gcc/testsuite/g++.dg/pr62079.C -c -m32
/opt/gcc/work/gcc/testsuite/g++.dg/pr62079.C: In function 'int main()':
/opt/gcc/work/gcc/testsuite/g++.dg/pr62079.C:78:1: error: non-cold basic block 5 reachable only by paths crossing the cold partition
 }
 ^
during RTL pass: split2
/opt/gcc/work/gcc/testsuite/g++.dg/pr62079.C:78:1: internal compiler error: verify_flow_info failed

on x86_64-apple-darwin16.

TIA

Dominique

Re: Restore functional DONT_USE_BUILTIN_SETJMP support

[Eric Botcazou]

> This cause
> 
> % /opt/gcc/gcc8w/bin/g++ -std=c++11 -O2 -fnon-call-exceptions
> /opt/gcc/work/gcc/testsuite/g++.dg/pr62079.C -c -m32
> /opt/gcc/work/gcc/testsuite/g++.dg/pr62079.C: In function 'int main()':
> /opt/gcc/work/gcc/testsuite/g++.dg/pr62079.C:78:1: error: non-cold basic
> block 5 reachable only by paths crossing the cold partition }
>  ^
> during RTL pass: split2
> /opt/gcc/work/gcc/testsuite/g++.dg/pr62079.C:78:1: internal compiler error:
> verify_flow_info failed
> 
> on x86_64-apple-darwin16.

Really sure?  The patch is a no-op except for IA-64 and Aarch64.

-- 
Eric Botcazou

Re: Restore functional DONT_USE_BUILTIN_SETJMP support

[Dominique d'Humières]

> Le 10 oct. 2017 à 11:53, Eric Botcazou <ebotcazou@adacore.com> a écrit :
> 
>> This cause
>> 
>> % /opt/gcc/gcc8w/bin/g++ -std=c++11 -O2 -fnon-call-exceptions
>> /opt/gcc/work/gcc/testsuite/g++.dg/pr62079.C -c -m32
>> /opt/gcc/work/gcc/testsuite/g++.dg/pr62079.C: In function 'int main()':
>> /opt/gcc/work/gcc/testsuite/g++.dg/pr62079.C:78:1: error: non-cold basic
>> block 5 reachable only by paths crossing the cold partition }
>> ^
>> during RTL pass: split2
>> /opt/gcc/work/gcc/testsuite/g++.dg/pr62079.C:78:1: internal compiler error:
>> verify_flow_info failed
>> 
>> on x86_64-apple-darwin16.
> 
> Really sure?  The patch is a no-op except for IA-64 and Aarch64.

After verification, you are right: the problem is older but requires the compiler to be configured with 
--enable-checking=yes.

Sorry for the noise.

Dominique

> 
> -- 
> Eric Botcazou

Re: Restore functional DONT_USE_BUILTIN_SETJMP support

[Eric Botcazou]

> After verification, you are right: the problem is older but requires the
> compiler to be configured with --enable-checking=yes.

Thanks for confirming.  The problem is present on x86-64/Linux too, I'll have 
a quick look later today.

-- 
Eric Botcazou

Restore functional DONT_USE_BUILTIN_SETJMP support

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

DONT_USE_BUILTIN_SETJMP is an old configuration macro that instructs the 
compiler to use the usual setjmp/longjmp routines instead of their built-in 
variants to implement SJLJ exceptions.  It's less efficient but, sometimes, 
the setjmp/longjmp routines have specific features that cannot be easily 
implemented by the built-in.  Only IA-64 and Aarch64 define it.

The support is currently broken at compile time (because of a broken CFG) and 
at link time (an unresolved symbol).  The attached small patch restores it.

Bootstrapped & tested on Aarch64/Linux with --enable-sjlj-exceptions, applied 
on the mainline as obvious.


2017-10-07  Eric Botcazou  <ebotcazou@adacore.com>

	* builtins.def (BUILT_IN_SETJMP): Declare as library builtin instead
	of GCC builtin if DONT_USE_BUILTIN_SETJMP is defined.
	* except.c (sjlj_emit_function_enter): If DONT_USE_BUILTIN_SETJMP is
	defined, force the creation of a new block for a dispatch label.

-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 2096 bytes --]

Index: builtins.def
===================================================================
--- builtins.def	(revision 253506)
+++ builtins.def	(working copy)
@@ -890,7 +890,11 @@ DEF_LIB_BUILTIN        (BUILT_IN_REALLOC
 DEF_GCC_BUILTIN        (BUILT_IN_RETURN, "return", BT_FN_VOID_PTR, ATTR_NORETURN_NOTHROW_LEAF_LIST)
 DEF_GCC_BUILTIN        (BUILT_IN_RETURN_ADDRESS, "return_address", BT_FN_PTR_UINT, ATTR_LEAF_LIST)
 DEF_GCC_BUILTIN        (BUILT_IN_SAVEREGS, "saveregs", BT_FN_PTR_VAR, ATTR_NULL)
+#ifdef DONT_USE_BUILTIN_SETJMP
+DEF_LIB_BUILTIN        (BUILT_IN_SETJMP, "setjmp", BT_FN_INT_PTR, ATTR_RT_NOTHROW_LEAF_LIST)
+#else
 DEF_GCC_BUILTIN        (BUILT_IN_SETJMP, "setjmp", BT_FN_INT_PTR, ATTR_RT_NOTHROW_LEAF_LIST)
+#endif
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRFMON, "strfmon", BT_FN_SSIZE_STRING_SIZE_CONST_STRING_VAR, ATTR_FORMAT_STRFMON_NOTHROW_3_4)
 DEF_LIB_BUILTIN        (BUILT_IN_STRFTIME, "strftime", BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_TM_PTR, ATTR_FORMAT_STRFTIME_NOTHROW_3_0)
 DEF_GCC_BUILTIN        (BUILT_IN_TRAP, "trap", BT_FN_VOID, ATTR_NORETURN_NOTHROW_LEAF_COLD_LIST)
Index: except.c
===================================================================
--- except.c	(revision 253506)
+++ except.c	(working copy)
@@ -1209,6 +1209,28 @@ sjlj_emit_function_enter (rtx_code_label
 	  fn_begin_outside_block = false;
       }
 
+#ifdef DONT_USE_BUILTIN_SETJMP
+  if (dispatch_label)
+    {
+      /* The sequence contains a branch in the middle so we need to force
+	 the creation of a new basic block by means of BB_SUPERBLOCK.  */
+      if (fn_begin_outside_block)
+	{
+	  basic_block bb
+	    = split_edge (single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun)));
+	  if (JUMP_P (BB_END (bb)))
+	    emit_insn_before (seq, BB_END (bb));
+	  else
+	    emit_insn_after (seq, BB_END (bb));
+	}
+      else
+	emit_insn_after (seq, fn_begin);
+
+      single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun))->flags |= BB_SUPERBLOCK;
+      return;
+    }
+#endif
+
   if (fn_begin_outside_block)
     insert_insn_on_edge (seq, single_succ_edge (ENTRY_BLOCK_PTR_FOR_FN (cfun)));
   else

Re: Restore functional DONT_USE_BUILTIN_SETJMP support

[Andreas Schwab]

On Okt 08 2017, Eric Botcazou <ebotcazou@adacore.com> wrote:

> 	* builtins.def (BUILT_IN_SETJMP): Declare as library builtin instead
> 	of GCC builtin if DONT_USE_BUILTIN_SETJMP is defined.

This breaks gcc.dg/plugin/must-tail-call-2.c, gcc.dg/torture/pr81083.c
and gcc.dg/torture/pr82264.c:

warning: conflicting types for built-in function 'setjmp' [-Wbuiltin-declaration-mismatch]

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Restore functional DONT_USE_BUILTIN_SETJMP support

[Joseph Myers]

On Sun, 8 Oct 2017, Andreas Schwab wrote:

> On Okt 08 2017, Eric Botcazou <ebotcazou@adacore.com> wrote:
> 
> > 	* builtins.def (BUILT_IN_SETJMP): Declare as library builtin instead
> > 	of GCC builtin if DONT_USE_BUILTIN_SETJMP is defined.
> 
> This breaks gcc.dg/plugin/must-tail-call-2.c, gcc.dg/torture/pr81083.c
> and gcc.dg/torture/pr82264.c:
> 
> warning: conflicting types for built-in function 'setjmp' [-Wbuiltin-declaration-mismatch]

It also breaks the glibc testsuite for AArch64 and IA64 
(setjmp/check-installed-headers-cxx).

https://sourceware.org/ml/libc-testresults/2017-q4/msg00057.html

In file included from ../include/setjmp.h:2:0,
                 from /tmp/cih_test_epu39R.cc:8:
../setjmp/setjmp.h:49:12: error: declaration of 'int setjmp(__jmp_buf_tag*)' conflicts with built-in declaration 'int setjmp(void*)' [-Werror=builtin-declaration-mismatch]
 extern int setjmp (jmp_buf __env) __THROWNL;
            ^~~~~~
cc1plus: all warnings being treated as errors

That is, the C++ front end does not consider this built-in declaration 
compatible with <setjmp.h>'s declaration of setjmp (GCC of course does not 
know the exact type of jmp_buf in libc, so can't know the exact argument 
type for the library function, so when you use DEF_LIB_BUILTIN instead of 
DEF_GCC_BUILTIN you get this conflict; the C front end is apparently more 
lenient about this particular built-in type conflict).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: Restore functional DONT_USE_BUILTIN_SETJMP support

[Andreas Schwab]

On Okt 08 2017, Eric Botcazou <ebotcazou@adacore.com> wrote:

> 	* builtins.def (BUILT_IN_SETJMP): Declare as library builtin instead
> 	of GCC builtin if DONT_USE_BUILTIN_SETJMP is defined.
> 	* except.c (sjlj_emit_function_enter): If DONT_USE_BUILTIN_SETJMP is
> 	defined, force the creation of a new block for a dispatch label.

This also breaks gcc.c-torture/compile/951222-1.c:

during GIMPLE pass: lower
/opt/gcc/gcc-20171008/gcc/testsuite/gcc.c-torture/compile/951222-1.c:4:1: internal compiler error: in gimple_call_arg, at gimple.h:3159
0x118ab1b gimple_call_arg
        ../../gcc/gimple.h:3159
0x118ab1b gimple_call_arg
        ../../gcc/gimple.h:3167
0x118ab1b lower_stmt
        ../../gcc/gimple-low.c:327
0x118ab1b lower_sequence
        ../../gcc/gimple-low.c:205
0x1189df3 lower_stmt
        ../../gcc/gimple-low.c:274
0x1189df3 lower_sequence
        ../../gcc/gimple-low.c:205
0x118997f lower_gimple_bind
        ../../gcc/gimple-low.c:441
0x118afff lower_function_body
        ../../gcc/gimple-low.c:109
0x118afff execute
        ../../gcc/gimple-low.c:183

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: Restore functional DONT_USE_BUILTIN_SETJMP support

[Eric Botcazou]

> This also breaks gcc.c-torture/compile/951222-1.c:
> 
> during GIMPLE pass: lower
> /opt/gcc/gcc-20171008/gcc/testsuite/gcc.c-torture/compile/951222-1.c:4:1:
> internal compiler error: in gimple_call_arg, at gimple.h:3159 0x118ab1b
> gimple_call_arg

You're right, not clear why I didn't spot it on Aarch64.  Hunk reverted.

-- 
Eric Botcazou

Re: Restore functional DONT_USE_BUILTIN_SETJMP support

[Eric Botcazou]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

> You're right, not clear why I didn't spot it on Aarch64.  Hunk reverted.

The attached patch replaces it with an ad-hoc definition of setjmp, like the 
ones used for the coverage routines.  Tested on Aarch64/Linux with and without 
--enable-sjlj-exceptions and on IA-64/Linux without --enable-sjlj-exceptions.

Applied on the mainline as obvious.


2017-10-09  Eric Botcazou  <ebotcazou@adacore.com>

	* except.c (setjmp_fn): New global variable.
	(init_eh): Initialize it if DONT_USE_BUILTIN_SETJMP is defined.
	(sjlj_emit_function_enter): Call it instead of BUILTIN_SETJMP if
	DONT_USE_BUILTIN_SETJMP is defined.

-- 
Eric Botcazou

[-- Attachment #2: p.diff --]
[-- Type: text/x-patch, Size: 1526 bytes --]

Index: except.c
===================================================================
--- except.c	(revision 253530)
+++ except.c	(working copy)
@@ -147,7 +147,9 @@ along with GCC; see the file COPYING3.
 
 static GTY(()) int call_site_base;
 
-static GTY (()) hash_map<tree_hash, tree> *type_to_runtime_map;
+static GTY(()) hash_map<tree_hash, tree> *type_to_runtime_map;
+
+static GTY(()) tree setjmp_fn;
 
 /* Describe the SjLj_Function_Context structure.  */
 static GTY(()) tree sjlj_fc_type_node;
@@ -331,6 +333,16 @@ init_eh (void)
       sjlj_fc_jbuf_ofs
 	= (tree_to_uhwi (DECL_FIELD_OFFSET (f_jbuf))
 	   + tree_to_uhwi (DECL_FIELD_BIT_OFFSET (f_jbuf)) / BITS_PER_UNIT);
+
+#ifdef DONT_USE_BUILTIN_SETJMP
+      tmp = build_function_type_list (integer_type_node, TREE_TYPE (f_jbuf),
+				      NULL);
+      setjmp_fn = build_decl (BUILTINS_LOCATION, FUNCTION_DECL,
+			      get_identifier ("setjmp"), tmp);
+      TREE_PUBLIC (setjmp_fn) = 1;
+      DECL_EXTERNAL (setjmp_fn) = 1;
+      DECL_ASSEMBLER_NAME (setjmp_fn);
+#endif
     }
 }
 
@@ -1176,8 +1188,7 @@ sjlj_emit_function_enter (rtx_code_label
       addr = convert_memory_address (ptr_mode, addr);
       tree addr_tree = make_tree (ptr_type_node, addr);
 
-      tree fn = builtin_decl_implicit (BUILT_IN_SETJMP);
-      tree call_expr = build_call_expr (fn, 1, addr_tree);
+      tree call_expr = build_call_expr (setjmp_fn, 1, addr_tree);
       rtx x = expand_call (call_expr, NULL_RTX, false);
 
       emit_cmp_and_jump_insns (x, const0_rtx, NE, 0,