Patchwork [fortran] PR46079 ABI for empty stop statement broken

login
register
mail settings
Submitter Jerry DeLisle
Date Oct. 19, 2010, 2:45 a.m.
Message ID <4CBD0631.7090603@frontier.com>
Download mbox | patch
Permalink /patch/68266/
State New
Headers show

Comments

Jerry DeLisle - Oct. 19, 2010, 2:45 a.m.
Hi,

I discovered this minor bug while testing for the revamp of the type enumerators.

I think the ChangeLog is clear enough.

Regression tested on x86-64-linux-gnu. No new test case needed.

OK for trunk?

Jerry

2010-10-18  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR fortran/46079
	* trans_stmt.c (gfc_trans_stop): If the STOP statement is empty,
	build a call to the new function stop_empty.  If integer, build a call
	to stop_string or error_stop_string with a NULL pointer and the
	integer value as the length.
	* trans.h: Add declaration for gfor_fndecl_stop_empty.
	* trans-decl.c (gfc_build_builtin_function_decls): Build declaration
	for stop_empty.
	
2010-10-18  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libgfortran/46079
	* runtime/stop.c (stop_empty): New function. (stop_numeric): Restore
	to previous behavior. (stop_string, error_stop_string): Use string
	pointer to signal emitting numeric only.  Use string length as the
	stop code.
Mikael Morin - Oct. 19, 2010, 7:45 p.m.
On Tuesday 19 October 2010 04:45:05 Jerry DeLisle wrote:
> Hi,
> 
> I discovered this minor bug while testing for the revamp of the type
> enumerators.
> 
> I think the ChangeLog is clear enough.
> 
> Regression tested on x86-64-linux-gnu. No new test case needed.
> 
> OK for trunk?
> 
> Jerry
> 
> 2010-10-18  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
> 
> 	PR fortran/46079
> 	* trans_stmt.c (gfc_trans_stop): If the STOP statement is empty,
> 	build a call to the new function stop_empty.  If integer, build a call
> 	to stop_string or error_stop_string with a NULL pointer and the
> 	integer value as the length.
> 	* trans.h: Add declaration for gfor_fndecl_stop_empty.
> 	* trans-decl.c (gfc_build_builtin_function_decls): Build declaration
> 	for stop_empty.
> 
> 2010-10-18  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
> 
> 	PR libgfortran/46079
> 	* runtime/stop.c (stop_empty): New function. (stop_numeric): Restore
> 	to previous behavior. (stop_string, error_stop_string): Use string
> 	pointer to signal emitting numeric only.  Use string length as the
> 	stop code.

Let's keep error_stop_string care about strings only. (Please!)

The current stop_numeric function doesn't escape -1 (and doesn't need to, for 
new binaries), but we have to provide the previous stop_numeric that did. 
That's the problem, right ?

Why not keep stop_numeric for compatibility and add a new function (called 
let's say stop_numeric_{new,plain,noworkaround,jerry} or whatever) and 
generate calls for that new function only?
That's probably some code duplication, but should save some obfuscation ;-)


Mikael
Jerry DeLisle - Oct. 20, 2010, 12:33 a.m.
On 10/19/2010 12:45 PM, Mikael Morin wrote:
> On Tuesday 19 October 2010 04:45:05 Jerry DeLisle wrote:
>> Hi,
>>
>> I discovered this minor bug while testing for the revamp of the type
>> enumerators.
>>
>> I think the ChangeLog is clear enough.
>>
>> Regression tested on x86-64-linux-gnu. No new test case needed.
>>
>> OK for trunk?
>>
>> Jerry
>>
>> 2010-10-18  Jerry DeLisle<jvdelisle@gcc.gnu.org>
>>
>> 	PR fortran/46079
>> 	* trans_stmt.c (gfc_trans_stop): If the STOP statement is empty,
>> 	build a call to the new function stop_empty.  If integer, build a call
>> 	to stop_string or error_stop_string with a NULL pointer and the
>> 	integer value as the length.
>> 	* trans.h: Add declaration for gfor_fndecl_stop_empty.
>> 	* trans-decl.c (gfc_build_builtin_function_decls): Build declaration
>> 	for stop_empty.
>>
>> 2010-10-18  Jerry DeLisle<jvdelisle@gcc.gnu.org>
>>
>> 	PR libgfortran/46079
>> 	* runtime/stop.c (stop_empty): New function. (stop_numeric): Restore
>> 	to previous behavior. (stop_string, error_stop_string): Use string
>> 	pointer to signal emitting numeric only.  Use string length as the
>> 	stop code.
>
> Let's keep error_stop_string care about strings only. (Please!)
>
Thanks for bringing this up.  It was bothering me too  I will gladly do this 
diffently and will re-submit the patch.

Jerry

Patch

Index: gcc/fortran/trans-stmt.c
===================================================================
--- gcc/fortran/trans-stmt.c	(revision 165674)
+++ gcc/fortran/trans-stmt.c	(working copy)
@@ -600,18 +600,24 @@  gfc_trans_stop (gfc_code *code, bool error_stop)
 
   if (code->expr1 == NULL)
     {
-      tmp = build_int_cst (gfc_int4_type_node, 0);
-      tmp = build_call_expr_loc (input_location,
-			     	 error_stop ? gfor_fndecl_error_stop_string
-				 : gfor_fndecl_stop_string,
-			     	 2, build_int_cst (pchar_type_node, 0), tmp);
+      if (error_stop)
+        {
+	  tmp = build_int_cst (gfc_int4_type_node, 0);
+	  tmp = build_call_expr_loc (input_location,
+				     gfor_fndecl_error_stop_string,
+				     2, build_int_cst (pchar_type_node, 0),
+				     tmp);
+	}
+      else
+	tmp = build_call_expr_loc (input_location, gfor_fndecl_stop_empty, 0);
     }
   else if (code->expr1->ts.type == BT_INTEGER)
     {
       gfc_conv_expr (&se, code->expr1);
       tmp = build_call_expr_loc (input_location,
-      				 error_stop ? gfor_fndecl_error_stop_numeric
-			   	 : gfor_fndecl_stop_numeric, 1,
+      				 error_stop ? gfor_fndecl_error_stop_string
+			   	 : gfor_fndecl_stop_string, 2,
+				 build_int_cst (pchar_type_node, 0),
 				 fold_convert (gfc_int4_type_node, se.expr));
     }
   else
Index: gcc/fortran/trans.h
===================================================================
--- gcc/fortran/trans.h	(revision 165674)
+++ gcc/fortran/trans.h	(working copy)
@@ -588,6 +588,7 @@  void gfc_omp_firstprivatize_type_sizes (struct gim
 /* Runtime library function decls.  */
 extern GTY(()) tree gfor_fndecl_pause_numeric;
 extern GTY(()) tree gfor_fndecl_pause_string;
+extern GTY(()) tree gfor_fndecl_stop_empty;
 extern GTY(()) tree gfor_fndecl_stop_numeric;
 extern GTY(()) tree gfor_fndecl_stop_string;
 extern GTY(()) tree gfor_fndecl_error_stop_numeric;
Index: gcc/fortran/trans-decl.c
===================================================================
--- gcc/fortran/trans-decl.c	(revision 165674)
+++ gcc/fortran/trans-decl.c	(working copy)
@@ -86,6 +86,7 @@  tree gfc_static_ctors;
 
 tree gfor_fndecl_pause_numeric;
 tree gfor_fndecl_pause_string;
+tree gfor_fndecl_stop_empty;
 tree gfor_fndecl_stop_numeric;
 tree gfor_fndecl_stop_string;
 tree gfor_fndecl_error_stop_numeric;
@@ -2796,6 +2797,12 @@  gfc_build_builtin_function_decls (void)
 {
   tree gfc_int4_type_node = gfc_get_int_type (4);
 
+  gfor_fndecl_stop_empty = gfc_build_library_function_decl (
+	get_identifier (PREFIX("stop_empty")),
+	void_type_node, 1, void_type_node);
+  /* STOP doesn't return.  */
+  TREE_THIS_VOLATILE (gfor_fndecl_stop_empty) = 1;
+  
   gfor_fndecl_stop_numeric = gfc_build_library_function_decl (
 	get_identifier (PREFIX("stop_numeric")),
 	void_type_node, 1, gfc_int4_type_node);
Index: libgfortran/runtime/stop.c
===================================================================
--- libgfortran/runtime/stop.c	(revision 165674)
+++ libgfortran/runtime/stop.c	(working copy)
@@ -26,6 +26,18 @@  see the files COPYING3 and COPYING.RUNTIME respect
 #include "libgfortran.h"
 #include <string.h>
 
+/* An empty STOP statement.  */
+
+extern void stop_empty (void)
+  __attribute__ ((noreturn));
+export_proto(stop_empty);
+
+void
+stop_empty (void)
+{
+  sys_exit (0);
+}
+
 /* A numeric STOP statement.  */
 
 extern void stop_numeric (GFC_INTEGER_4)
@@ -35,7 +47,11 @@  export_proto(stop_numeric);
 void
 stop_numeric (GFC_INTEGER_4 code)
 {
-  st_printf ("STOP %d\n", (int)code);
+  if (code == -1)
+    code = 0;
+  else
+    st_printf ("STOP %d\n", (int)code);
+
   sys_exit (code);
 }
 
@@ -44,13 +60,17 @@  stop_numeric (GFC_INTEGER_4 code)
 void
 stop_string (const char *string, GFC_INTEGER_4 len)
 {
+  st_printf ("STOP ");
+
   if (string)
     {
-      st_printf ("STOP ");
       while (len--)
 	st_printf ("%c", *(string++));
-      st_printf ("\n");
     }
+  else
+    st_printf ("%d", (int)len);
+
+  st_printf ("\n");
   sys_exit (0);
 }
 
@@ -68,10 +88,15 @@  void
 error_stop_string (const char *string, GFC_INTEGER_4 len)
 {
   st_printf ("ERROR STOP ");
-  while (len--)
-    st_printf ("%c", *(string++));
+  if (string)
+    {
+      while (len--)
+	st_printf ("%c", *(string++));
+    }
+  else
+    st_printf ("%d", (int)len);
+
   st_printf ("\n");
-
   sys_exit (1);
 }
 
Index: libgfortran/gfortran.map
===================================================================
--- libgfortran/gfortran.map	(revision 165674)
+++ libgfortran/gfortran.map	(working copy)
@@ -1141,6 +1141,7 @@  GFORTRAN_1.4 {
     _gfortran_parity_l8;
     _gfortran_parity_l16;
     _gfortran_selected_real_kind2008;
+    _gfortran_stop_empty;
     _gfortran_transfer_array_write;
     _gfortran_transfer_character_write;
     _gfortran_transfer_character_wide_write;