This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix segfault with inlining
- From: Eric Botcazou <ebotcazou at adacore dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Fri, 13 Sep 2013 16:29:48 +0200
- Subject: [PATCH] Fix segfault with inlining
- Authentication-results: sourceware.org; auth=none
Hi,
in Ada parameters can be passed by reference: in this case, the address of the
argument is directly passed to the callee, which dereferences it to access the
argument; now Ada also enables -fexceptions -fnon-call-exceptions, which means
that any pointer dereference is seen as the potential source of an exception,
which can quickly block the optimizer. That's why we put TREE_THIS_NOTRAP on
the dereferences associated with a parameter.
This works fine as long as the function isn't inlined, because it may happen
that the argument is itself the dereference of a pointer, properly guarded by
a null check on the pointer. When the function is inlined, the dereference in
the caller is replaced with that of the callee, which is TREE_THIS_NOTRAP and
thus can be moved ahead of the null pointer check, for example by LIM.
The patch ensures that this cannot happen by clearing TREE_THIS_NOTRAP in the
inliner. I think that this affects only the Ada compiler in practice.
Tested on x86_64-suse-linux, OK for the mainline?
2013-09-13 Eric Botcazou <ebotcazou@adacore.com>
* tree-inline.h (struct copy_body_data): Add transform_parameter.
* tree-inline.c (is_parameter_of): New predicate.
(remap_gimple_op_r): Do not propagate TREE_THIS_NOTRAP on MEM_REF if
a parameter has been remapped.
(copy_tree_body_r): Likewise on INDIRECT_REF and MEM_REF.
(optimize_inline_calls): Initialize transform_parameter.
(unsave_expr_now): Likewise.
(copy_gimple_seq_and_replace_locals): Likewise.
(tree_function_versioning): Likewise.
(maybe_inline_call_in_expr): Likewise.
2013-09-13 Eric Botcazou <ebotcazou@adacore.com>
* gnat.dg/opt27.adb: New test.
* gnat.dg/opt27_pkg.ad[sb]: New helper.
--
Eric Botcazou
Index: tree-inline.c
===================================================================
--- tree-inline.c (revision 202431)
+++ tree-inline.c (working copy)
@@ -751,6 +751,21 @@ copy_gimple_bind (gimple stmt, copy_body
return new_bind;
}
+/* Return true if DECL is a parameter or a SSA_NAME for a parameter of
+ function FNDECL. */
+
+static bool
+is_parameter_of (tree decl, tree fndecl)
+{
+ if (TREE_CODE (decl) == SSA_NAME)
+ {
+ decl = SSA_NAME_VAR (decl);
+ if (!decl)
+ return false;
+ }
+
+ return (TREE_CODE (decl) == PARM_DECL && DECL_CONTEXT (decl) == fndecl);
+}
/* Remap the GIMPLE operand pointed to by *TP. DATA is really a
'struct walk_stmt_info *'. DATA->INFO is a 'copy_body_data *'.
@@ -840,20 +855,25 @@ remap_gimple_op_r (tree *tp, int *walk_s
if (TREE_CODE (*tp) == MEM_REF)
{
- tree ptr = TREE_OPERAND (*tp, 0);
- tree type = remap_type (TREE_TYPE (*tp), id);
- tree old = *tp;
-
/* We need to re-canonicalize MEM_REFs from inline substitutions
that can happen when a pointer argument is an ADDR_EXPR.
Recurse here manually to allow that. */
+ tree ptr = TREE_OPERAND (*tp, 0);
+ tree type = remap_type (TREE_TYPE (*tp), id);
+ tree old = *tp;
walk_tree (&ptr, remap_gimple_op_r, data, NULL);
- *tp = fold_build2 (MEM_REF, type,
- ptr, TREE_OPERAND (*tp, 1));
- TREE_THIS_NOTRAP (*tp) = TREE_THIS_NOTRAP (old);
+ *tp = fold_build2 (MEM_REF, type, ptr, TREE_OPERAND (*tp, 1));
TREE_THIS_VOLATILE (*tp) = TREE_THIS_VOLATILE (old);
TREE_SIDE_EFFECTS (*tp) = TREE_SIDE_EFFECTS (old);
TREE_NO_WARNING (*tp) = TREE_NO_WARNING (old);
+ /* We cannot propagate the TREE_THIS_NOTRAP flag if we have
+ remapped a parameter as the property might be valid only
+ for the parameter itself. */
+ if (TREE_THIS_NOTRAP (old)
+ && (!is_parameter_of (TREE_OPERAND (old, 0), id->src_fn)
+ || (!id->transform_parameter
+ && is_parameter_of (ptr, id->dst_fn))))
+ TREE_THIS_NOTRAP (*tp) = 1;
*walk_subtrees = 0;
return NULL;
}
@@ -1043,45 +1063,45 @@ copy_tree_body_r (tree *tp, int *walk_su
/* Get rid of *& from inline substitutions that can happen when a
pointer argument is an ADDR_EXPR. */
tree decl = TREE_OPERAND (*tp, 0);
- tree *n;
-
- n = (tree *) pointer_map_contains (id->decl_map, decl);
+ tree *n = (tree *) pointer_map_contains (id->decl_map, decl);
if (n)
{
- tree new_tree;
- tree old;
/* If we happen to get an ADDR_EXPR in n->value, strip
it manually here as we'll eventually get ADDR_EXPRs
which lie about their types pointed to. In this case
build_fold_indirect_ref wouldn't strip the INDIRECT_REF,
but we absolutely rely on that. As fold_indirect_ref
does other useful transformations, try that first, though. */
- tree type = TREE_TYPE (TREE_TYPE (*n));
- if (id->do_not_unshare)
- new_tree = *n;
- else
- new_tree = unshare_expr (*n);
- old = *tp;
- *tp = gimple_fold_indirect_ref (new_tree);
+ tree type = TREE_TYPE (*tp);
+ tree ptr = id->do_not_unshare ? *n : unshare_expr (*n);
+ tree old = *tp;
+ *tp = gimple_fold_indirect_ref (ptr);
if (! *tp)
{
- if (TREE_CODE (new_tree) == ADDR_EXPR)
+ if (TREE_CODE (ptr) == ADDR_EXPR)
{
- *tp = fold_indirect_ref_1 (EXPR_LOCATION (new_tree),
- type, new_tree);
+ *tp
+ = fold_indirect_ref_1 (EXPR_LOCATION (ptr), type, ptr);
/* ??? We should either assert here or build
a VIEW_CONVERT_EXPR instead of blindly leaking
incompatible types to our IL. */
if (! *tp)
- *tp = TREE_OPERAND (new_tree, 0);
+ *tp = TREE_OPERAND (ptr, 0);
}
else
{
- *tp = build1 (INDIRECT_REF, type, new_tree);
+ *tp = build1 (INDIRECT_REF, type, ptr);
TREE_THIS_VOLATILE (*tp) = TREE_THIS_VOLATILE (old);
TREE_SIDE_EFFECTS (*tp) = TREE_SIDE_EFFECTS (old);
TREE_READONLY (*tp) = TREE_READONLY (old);
- TREE_THIS_NOTRAP (*tp) = TREE_THIS_NOTRAP (old);
+ /* We cannot propagate the TREE_THIS_NOTRAP flag if we
+ have remapped a parameter as the property might be
+ valid only for the parameter itself. */
+ if (TREE_THIS_NOTRAP (old)
+ && (!is_parameter_of (decl, id->src_fn)
+ || (!id->transform_parameter
+ && is_parameter_of (ptr, id->dst_fn))))
+ TREE_THIS_NOTRAP (*tp) = 1;
}
}
*walk_subtrees = 0;
@@ -1090,20 +1110,25 @@ copy_tree_body_r (tree *tp, int *walk_su
}
else if (TREE_CODE (*tp) == MEM_REF)
{
- tree ptr = TREE_OPERAND (*tp, 0);
- tree type = remap_type (TREE_TYPE (*tp), id);
- tree old = *tp;
-
/* We need to re-canonicalize MEM_REFs from inline substitutions
that can happen when a pointer argument is an ADDR_EXPR.
Recurse here manually to allow that. */
+ tree ptr = TREE_OPERAND (*tp, 0);
+ tree type = remap_type (TREE_TYPE (*tp), id);
+ tree old = *tp;
walk_tree (&ptr, copy_tree_body_r, data, NULL);
- *tp = fold_build2 (MEM_REF, type,
- ptr, TREE_OPERAND (*tp, 1));
- TREE_THIS_NOTRAP (*tp) = TREE_THIS_NOTRAP (old);
+ *tp = fold_build2 (MEM_REF, type, ptr, TREE_OPERAND (*tp, 1));
TREE_THIS_VOLATILE (*tp) = TREE_THIS_VOLATILE (old);
TREE_SIDE_EFFECTS (*tp) = TREE_SIDE_EFFECTS (old);
TREE_NO_WARNING (*tp) = TREE_NO_WARNING (old);
+ /* We cannot propagate the TREE_THIS_NOTRAP flag if we have
+ remapped a parameter as the property might be valid only
+ for the parameter itself. */
+ if (TREE_THIS_NOTRAP (old)
+ && (!is_parameter_of (TREE_OPERAND (old, 0), id->src_fn)
+ || (!id->transform_parameter
+ && is_parameter_of (ptr, id->dst_fn))))
+ TREE_THIS_NOTRAP (*tp) = 1;
*walk_subtrees = 0;
return NULL;
}
@@ -4443,6 +4468,7 @@ optimize_inline_calls (tree fn)
id.transform_call_graph_edges = CB_CGE_DUPLICATE;
id.transform_new_cfg = false;
id.transform_return_to_modify = true;
+ id.transform_parameter = true;
id.transform_lang_insert_block = NULL;
id.statements_to_fold = pointer_set_create ();
@@ -4748,6 +4774,8 @@ copy_gimple_seq_and_replace_locals (gimp
id.transform_call_graph_edges = CB_CGE_DUPLICATE;
id.transform_new_cfg = false;
id.transform_return_to_modify = false;
+ id.transform_parameter = false;
+ id.transform_parameter = false;
id.transform_lang_insert_block = NULL;
/* Walk the tree once to find local labels. */
@@ -5207,6 +5235,7 @@ tree_function_versioning (tree old_decl,
= update_clones ? CB_CGE_MOVE_CLONES : CB_CGE_MOVE;
id.transform_new_cfg = true;
id.transform_return_to_modify = false;
+ id.transform_parameter = false;
id.transform_lang_insert_block = NULL;
old_entry_block = ENTRY_BLOCK_PTR_FOR_FUNCTION
@@ -5431,6 +5460,7 @@ maybe_inline_call_in_expr (tree exp)
id.transform_call_graph_edges = CB_CGE_DUPLICATE;
id.transform_new_cfg = false;
id.transform_return_to_modify = true;
+ id.transform_parameter = true;
id.transform_lang_insert_block = NULL;
/* Make sure not to unshare trees behind the front-end's back
Index: tree-inline.h
===================================================================
--- tree-inline.h (revision 202431)
+++ tree-inline.h (working copy)
@@ -97,6 +97,10 @@ typedef struct copy_body_data
by manipulating the CFG rather than a statement. */
bool transform_return_to_modify;
+ /* True if the parameters of the source function are transformed.
+ Only true for inlining. */
+ bool transform_parameter;
+
/* True if this statement will need to be regimplified. */
bool regimplify;
generic
type Element_T is private;
package Opt27_Pkg is
type Node_T is private;
type List_T is private;
function Is_Null (Node : in Node_T) return Boolean;
generic
type Template_T is private;
with function Is_Match
(Element : in Element_T;
Template : in Template_T) return Boolean is <>;
function Find_Elem (Template : Template_T; List : List_T) return Node_T;
private
type Node_Rec_T;
type Node_T is access Node_Rec_T;
type List_T is record
First_Node : Node_T := null;
Last_Node : Node_T := null;
end record;
end Opt27_Pkg;
package body Opt27_Pkg is
type Node_Rec_T is record
Element : Element_T;
Left : Node_T;
Right : Node_T;
end record;
function Is_Null (Node : in Node_T) return Boolean is
begin
return (Node = null);
end Is_Null;
function Find_Elem (Template : Template_T; List : List_T) return Node_T is
Element_Found : Boolean := False;
Node_Walker : Node_T := null;
begin
Node_Walker := List.First_Node;
while not Element_Found and (Node_Walker /= null) loop
if Is_Match (Node_Walker.Element, Template) then
Element_Found := True;
else
Node_Walker := Node_Walker.Right;
end if;
end loop;
return Node_Walker;
end;
end Opt27_Pkg;
-- { dg-do run }
-- { dg-options "-O" }
with Opt27_Pkg;
procedure Opt27 is
type Rec_T is record
A, B, C, D, E : Integer;
end record;
package List is new Opt27_Pkg (Rec_T);
My_List : List.List_T;
function Is_Match (Element : Rec_T; Template : Integer) return Boolean is
begin
return (Element.C = Template);
end;
function Find_Int is new List.Find_Elem (Integer, Is_Match);
Node : List.Node_T := Find_Int (10, My_List);
begin
if not List.Is_Null (Node) then
raise Program_Error;
end if;
end;