diff mbox series

[fortran] Put workaround for broken C/Lapack wrappers behind flag

Message ID a5d2c281-126c-da4f-5a7f-a5638f455c16@netcologne.de
State New
Headers show
Series [fortran] Put workaround for broken C/Lapack wrappers behind flag | expand

Commit Message

Thomas Koenig May 18, 2019, 6:37 p.m. UTC 
Hello world,

the attached patch puts the workaround introduced by Jakub behind
a flag.  I debated with myself what the name should be.
-fbroken-c-interfaces-die-die-die came to mind, but that would
have penalized people who wanted to disable the option :-)
Therefore, I have settled on -fbroken-callers and -fno-broken-callers,
respectively.  If somebody has a better idea, please let the list know.

What we should do with this flag for gcc-10 is still up for discussion,
if we make it the default or not.

Regression-tested.  "make info", "make dvi" and "make pdf" all
passed.  OK for trunk?

Regards

	Thomas

2019-05-18  Thomas Koenig  <tkoenig@gcc.gnu.org>

	PR fortran/90329
	* invoke.texi: Document -fbroken-callers.
	* lang.opt: Add -fbroken-callers.
	* trans-decl.c (create_function_arglist): Only set
	DECL_HIDDEN_STRING_LENGTH if flag_broken_callers is set.

Comments

Steve Kargl May 18, 2019, 8:42 p.m. UTC | #1 
On Sat, May 18, 2019 at 08:37:06PM +0200, Thomas Koenig wrote:
> 
> the attached patch puts the workaround introduced by Jakub behind
> a flag.  I debated with myself what the name should be.
> -fbroken-c-interfaces-die-die-die came to mind, but that would
> have penalized people who wanted to disable the option :-)
> Therefore, I have settled on -fbroken-callers and -fno-broken-callers,
> respectively.  If somebody has a better idea, please let the list know.
> 
> What we should do with this flag for gcc-10 is still up for discussion,
> if we make it the default or not.
> 
> Regression-tested.  "make info", "make dvi" and "make pdf" all
> passed.  OK for trunk?
> 

Looks good to me.  I wonder if we should add a reference
to the option that can produce C prototypes for a Fortran
procedure in the description of -fbroken-caller.
Thomas Koenig May 19, 2019, 8:26 a.m. UTC | #2 
Hi Steve,

> Looks good to me.  I wonder if we should add a reference
> to the option that can produce C prototypes for a Fortran
> procedure in the description of -fbroken-caller.

Thanks, I added a reference to that option (see attached patch).
Committed as r271376.

Regards

	Thomas
Steve Kargl May 19, 2019, 3:56 p.m. UTC | #3 
On Sun, May 19, 2019 at 10:26:54AM +0200, Thomas Koenig wrote:
> Hi Steve,
> 
> > Looks good to me.  I wonder if we should add a reference
> > to the option that can produce C prototypes for a Fortran
> > procedure in the description of -fbroken-caller.
> 
> Thanks, I added a reference to that option (see attached patch).
> Committed as r271376.
> 

Thanks.
diff mbox series

Patch

Index: invoke.texi
===================================================================
--- invoke.texi	(Revision 271371)
+++ invoke.texi	(Arbeitskopie)
@@ -181,7 +181,7 @@  and warnings}.
 @item Code Generation Options
 @xref{Code Gen Options,,Options for code generation conventions}.
 @gccoptlist{-faggressive-function-elimination -fblas-matmul-limit=@var{n} @gol
--fbounds-check -fcheck-array-temporaries @gol
+-fbounds-check -fbroken-callers -fcheck-array-temporaries @gol
 -fcheck=@var{<all|array-temps|bounds|do|mem|pointer|recursion>} @gol
 -fcoarray=@var{<none|single|lib>} -fexternal-blas -ff2c
 -ffrontend-loop-interchange @gol
@@ -1617,6 +1617,30 @@  warnings for generated array temporaries.
 @c Note: This option is also referred in gcc's manpage
 Deprecated alias for @option{-fcheck=bounds}.
 
+@item -fbroken-callers
+@opindex @code{broken-callers}
+Some C interfaces to Fortran codes violate the gfortran ABI by
+omitting the hidden character length arguments as described in
+@xref{Argument passing conventions}.  This can lead to crashes
+because pushing arguments for tail calls can overflow the stack.
+
+To provide a workaround for existing binary packages, this option
+disables tail call optimization for gfortran procedures with character
+arguments.
+
+Using this option can lead to problems including crashes due to
+insufficient stack space.
+
+It is @emph{very strongly} recommended to fix the code in question.
+Support for this option will likely be withdrawn in a future release
+of gfortran.
+
+The negative form, @option{-fno-broken-callers}, can be used to
+disable this option.
+
+Default is currently @option{-fbroken-callers}, this will change
+in future releases.
+
 @item -fcheck-array-temporaries
 @opindex @code{fcheck-array-temporaries}
 Deprecated alias for @option{-fcheck=array-temps}.
Index: lang.opt
===================================================================
--- lang.opt	(Revision 271371)
+++ lang.opt	(Arbeitskopie)
@@ -397,6 +397,10 @@  fblas-matmul-limit=
 Fortran RejectNegative Joined UInteger Var(flag_blas_matmul_limit) Init(30)
 -fblas-matmul-limit=<n>	Size of the smallest matrix for which matmul will use BLAS.
 
+fbroken-callers
+Fortran Var(flag_broken_callers) Init(1)
+Disallow tail call optimization when a calling routine may have omitted character lenghts.
+
 fcheck-array-temporaries
 Fortran
 Produce a warning at runtime if a array temporary has been created for a procedure argument.
Index: trans-decl.c
===================================================================
--- trans-decl.c	(Revision 271371)
+++ trans-decl.c	(Arbeitskopie)
@@ -2516,7 +2516,11 @@  create_function_arglist (gfc_symbol * sym)
 	  DECL_ARG_TYPE (length) = len_type;
 	  TREE_READONLY (length) = 1;
 	  gfc_finish_decl (length);
-	  if (f->sym->ts.u.cl
+
+	  /* Marking the length DECL_HIDDEN_STRING_LENGTH will lead
+	     to tail calls being disabled.  Only do that if we
+	     potentially have broken callers.  */
+	  if (flag_broken_callers && f->sym->ts.u.cl
 	      && f->sym->ts.u.cl->length
 	      && f->sym->ts.u.cl->length->expr_type == EXPR_CONSTANT)
 	    DECL_HIDDEN_STRING_LENGTH (length) = 1;