This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix segfault with inlining


On Fri, Sep 13, 2013 at 4:29 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> 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?

I've looked at the C++ testcase

int foo (int &x)
{
  try {
    return x;
  }
  catch (...)
  {
    return 0;
  }
}

which exhibits exactly the behavior you quote - return x is considered throwing
an exception.  The C++ FE doesn't arrange for TREE_THIS_NOTRAP to be
set here (maybe due to this issue you quote?).

Other than that the patch looks reasonable (I suppose you need
is_parameter_of only because as we recursively handle the trees
PARM_DECLs from the destination could already have leaked into
the tree we recurse into?)

Thanks,
Richard.

>
> 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 Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]