diff mbox

[RFC] warn about raw pointers that can be unique_ptr<T>

Message ID 20170507101919.21350-1-tbsaunde+gcc@tbsaunde.org
State New
Headers show

Commit Message

tbsaunde+gcc@tbsaunde.org May 7, 2017, 10:19 a.m. UTC
From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

Hi,

This is a start at warning about various resource allocation issues that can be
improved.  Currently it only warns about functions that call malloc and then
always pass the resulting pointer to free().

It should be pretty simple to extend this to new/delete and new[]/delete[], as
well as checking that in simple cases the pairing is correct.  However it
wasn't obvious to me how to tell if a function call is to a allocating operator
new.  We probably don't want to warn about placement new since complicated
things can be happening there.  There is the DECL_IS_OPERATOR_NEW() but that
doesn't seem to cover class specific operator new overloads, and maybe even
custom ones at global scope?

Other things that may be reasonable to try and include would be open / close
and locks.

It might also be good to warn about functions that take or return unique
ownership of resources, but I'm not quiet sure how to handle functions that
allocate or deallocate shared resources.

bootstrapped but not yet regtested other than the included test on
x86_64-linux-gnu.  All comments and suggestions welcome.

Trev


  ---
 gcc/Makefile.in                          |   1 +
 gcc/c-family/c.opt                       |   4 +
 gcc/gimple-owned-ptr.c                   | 214 +++++++++++++++++++++++++++++++
 gcc/passes.def                           |   1 +
 gcc/testsuite/g++.dg/warn/Wowning-ptr1.C |  76 +++++++++++
 gcc/tree-pass.h                          |   1 +
 6 files changed, 297 insertions(+)
 create mode 100644 gcc/gimple-owned-ptr.c
 create mode 100644 gcc/testsuite/g++.dg/warn/Wowning-ptr1.C

Comments

Marc Glisse May 7, 2017, 5:32 p.m. UTC | #1
On Sun, 7 May 2017, tbsaunde+gcc@tbsaunde.org wrote:

> This is a start at warning about various resource allocation issues that can be
> improved.  Currently it only warns about functions that call malloc and then
> always pass the resulting pointer to free().
>
> It should be pretty simple to extend this to new/delete and new[]/delete[], as
> well as checking that in simple cases the pairing is correct.  However it
> wasn't obvious to me how to tell if a function call is to a allocating operator
> new.  We probably don't want to warn about placement new since complicated
> things can be happening there.  There is the DECL_IS_OPERATOR_NEW() but that
> doesn't seem to cover class specific operator new overloads, and maybe even
> custom ones at global scope?
>
> Other things that may be reasonable to try and include would be open / close
> and locks.
>
> It might also be good to warn about functions that take or return unique
> ownership of resources, but I'm not quiet sure how to handle functions that
> allocate or deallocate shared resources.

Interesting.

1) How do you avoid warning for code that already uses unique_ptr? After 
inlining, it will look just like code calling new, and delete on all 
paths. Well, currently you only handle malloc/free, but if you provide an 
inline implementation of new/delete that forwards to malloc/free, the 
issue should already appear. Or maybe the pass is very early? But then the 
compiler will likely seldom notice that the argument to free is actually 
the return of malloc.

2) In some sense, we would want to warn when some path is missing the call 
to free, but it seems very hard to distinguish the cases where it is 
accidentally missing from those where it is done on purpose. Maybe detect 
cases where the only paths missing free are through exceptions?

3) This seems very similar to the conditions needed to transform a call to 
malloc into a stack allocation. I was experimenting with this some years 
ago, see for instance 
https://gcc.gnu.org/ml/gcc-patches/2013-11/msg01049.html (it is probably 
not the most recent message on the subject, since the main code moved to 
CCP afterwards 
http://geometrica.saclay.inria.fr/team/Marc.Glisse/tmp/heapstack, but I 
can't locate more recent messages at the moment, and it was more a 
prototype than a ready patch). Do you think the infrastructure could be 
shared between your warning and such an optimization? Or are the goals 
likely to diverge too soon for sharing to be practical?
Trevor Saunders May 7, 2017, 9:39 p.m. UTC | #2
On Sun, May 07, 2017 at 07:32:48PM +0200, Marc Glisse wrote:
> On Sun, 7 May 2017, tbsaunde+gcc@tbsaunde.org wrote:
> 
> > This is a start at warning about various resource allocation issues that can be
> > improved.  Currently it only warns about functions that call malloc and then
> > always pass the resulting pointer to free().
> > 
> > It should be pretty simple to extend this to new/delete and new[]/delete[], as
> > well as checking that in simple cases the pairing is correct.  However it
> > wasn't obvious to me how to tell if a function call is to a allocating operator
> > new.  We probably don't want to warn about placement new since complicated
> > things can be happening there.  There is the DECL_IS_OPERATOR_NEW() but that
> > doesn't seem to cover class specific operator new overloads, and maybe even
> > custom ones at global scope?
> > 
> > Other things that may be reasonable to try and include would be open / close
> > and locks.
> > 
> > It might also be good to warn about functions that take or return unique
> > ownership of resources, but I'm not quiet sure how to handle functions that
> > allocate or deallocate shared resources.
> 
> Interesting.
> 
> 1) How do you avoid warning for code that already uses unique_ptr? After
> inlining, it will look just like code calling new, and delete on all paths.

Currently by running early enough that no inline has happened.  I'm not
sure how valuable it would be to try and find things post inlining
instead.

> Well, currently you only handle malloc/free, but if you provide an inline
> implementation of new/delete that forwards to malloc/free, the issue should
> already appear. Or maybe the pass is very early? But then the compiler will
> likely seldom notice that the argument to free is actually the return of
> malloc.

I wonder how true this last bit is, sure we could find some more things
after more optimization, but its not obvious to me that it would be so
much to really be worth it.  Looking at gcc itself there's plenty of
places where this should already be enough with tracking places where
the pointer is coppied from one ssa name to another.  I feel like the
big areas of improvement are warning about functions that take or return
ownership of things.

> 2) In some sense, we would want to warn when some path is missing the call
> to free, but it seems very hard to distinguish the cases where it is
> accidentally missing from those where it is done on purpose. Maybe detect
> cases where the only paths missing free are through exceptions?

I didn't think about exceptions, but I'd been thinking of trying to warn
about variables we prove don't escape in alias analysis, but that would
probably be a small set of all allocations.

> 3) This seems very similar to the conditions needed to transform a call to
> malloc into a stack allocation. I was experimenting with this some years
> ago, see for instance
> https://gcc.gnu.org/ml/gcc-patches/2013-11/msg01049.html (it is probably not
> the most recent message on the subject, since the main code moved to CCP
> afterwards http://geometrica.saclay.inria.fr/team/Marc.Glisse/tmp/heapstack,
> but I can't locate more recent messages at the moment, and it was more a
> prototype than a ready patch). Do you think the infrastructure could be
> shared between your warning and such an optimization? Or are the goals
> likely to diverge too soon for sharing to be practical?

yeah, some of the issues are certainly similar.  I'm not sure if there
would be things worth sharing probably easiest to find out by just
trying it.

Trev

> 
> -- 
> Marc Glisse
Marc Glisse May 8, 2017, 8:28 a.m. UTC | #3
On Sun, 7 May 2017, Trevor Saunders wrote:

> On Sun, May 07, 2017 at 07:32:48PM +0200, Marc Glisse wrote:
>> On Sun, 7 May 2017, tbsaunde+gcc@tbsaunde.org wrote:
>>
>>> This is a start at warning about various resource allocation issues that can be
>>> improved.  Currently it only warns about functions that call malloc and then
>>> always pass the resulting pointer to free().
>>>
>>> It should be pretty simple to extend this to new/delete and new[]/delete[], as
>>> well as checking that in simple cases the pairing is correct.  However it
>>> wasn't obvious to me how to tell if a function call is to a allocating operator
>>> new.  We probably don't want to warn about placement new since complicated
>>> things can be happening there.  There is the DECL_IS_OPERATOR_NEW() but that
>>> doesn't seem to cover class specific operator new overloads, and maybe even
>>> custom ones at global scope?
>>>
>>> Other things that may be reasonable to try and include would be open / close
>>> and locks.
>>>
>>> It might also be good to warn about functions that take or return unique
>>> ownership of resources, but I'm not quiet sure how to handle functions that
>>> allocate or deallocate shared resources.
>>
>> Interesting.
>>
>> 1) How do you avoid warning for code that already uses unique_ptr? After
>> inlining, it will look just like code calling new, and delete on all paths.
>
> Currently by running early enough that no inline has happened.  I'm not
> sure how valuable it would be to try and find things post inlining
> instead.

Ah, right, for the warning it indeed makes sense to be pre-inlining, as it 
isn't clear you could use unique_ptr if the call to free was not directly 
in this function.

>> Well, currently you only handle malloc/free, but if you provide an inline
>> implementation of new/delete that forwards to malloc/free, the issue should
>> already appear. Or maybe the pass is very early? But then the compiler will
>> likely seldom notice that the argument to free is actually the return of
>> malloc.
>
> I wonder how true this last bit is, sure we could find some more things
> after more optimization, but its not obvious to me that it would be so
> much to really be worth it.  Looking at gcc itself there's plenty of
> places where this should already be enough with tracking places where
> the pointer is coppied from one ssa name to another.  I feel like the
> big areas of improvement are warning about functions that take or return
> ownership of things.

I fear that there are many cases where you need more PRE / DOM / SRA / ... 
for the SSA_NAME to end up in a place where malloc and free match, but if 
you already detect some cases, that's good.

>> 2) In some sense, we would want to warn when some path is missing the call
>> to free, but it seems very hard to distinguish the cases where it is
>> accidentally missing from those where it is done on purpose. Maybe detect
>> cases where the only paths missing free are through exceptions?
>
> I didn't think about exceptions, but I'd been thinking of trying to warn
> about variables we prove don't escape in alias analysis, but that would
> probably be a small set of all allocations.
>
>> 3) This seems very similar to the conditions needed to transform a call to
>> malloc into a stack allocation. I was experimenting with this some years
>> ago, see for instance
>> https://gcc.gnu.org/ml/gcc-patches/2013-11/msg01049.html (it is probably not
>> the most recent message on the subject, since the main code moved to CCP
>> afterwards http://geometrica.saclay.inria.fr/team/Marc.Glisse/tmp/heapstack,
>> but I can't locate more recent messages at the moment, and it was more a
>> prototype than a ready patch). Do you think the infrastructure could be
>> shared between your warning and such an optimization? Or are the goals
>> likely to diverge too soon for sharing to be practical?
>
> yeah, some of the issues are certainly similar.  I'm not sure if there
> would be things worth sharing probably easiest to find out by just
> trying it.

It seems that the differences will be larger than I originally expected, 
so it isn't clear the gain would be worth it.


While I remember, one (easy) case you may want to handle is:

p = malloc(n);
if (p != 0) ...

where the else branch does not need to contain a call to free.

(maybe one can add strdup and similar functions later, but better start 
with few functions for now)
Trevor Saunders May 8, 2017, 10:57 a.m. UTC | #4
On Mon, May 08, 2017 at 10:28:13AM +0200, Marc Glisse wrote:
> On Sun, 7 May 2017, Trevor Saunders wrote:
> 
> > On Sun, May 07, 2017 at 07:32:48PM +0200, Marc Glisse wrote:
> > > On Sun, 7 May 2017, tbsaunde+gcc@tbsaunde.org wrote:
> > > 
> > > > This is a start at warning about various resource allocation issues that can be
> > > > improved.  Currently it only warns about functions that call malloc and then
> > > > always pass the resulting pointer to free().
> > > > 
> > > > It should be pretty simple to extend this to new/delete and new[]/delete[], as
> > > > well as checking that in simple cases the pairing is correct.  However it
> > > > wasn't obvious to me how to tell if a function call is to a allocating operator
> > > > new.  We probably don't want to warn about placement new since complicated
> > > > things can be happening there.  There is the DECL_IS_OPERATOR_NEW() but that
> > > > doesn't seem to cover class specific operator new overloads, and maybe even
> > > > custom ones at global scope?
> > > > 
> > > > Other things that may be reasonable to try and include would be open / close
> > > > and locks.
> > > > 
> > > > It might also be good to warn about functions that take or return unique
> > > > ownership of resources, but I'm not quiet sure how to handle functions that
> > > > allocate or deallocate shared resources.
> > > 
> > > Interesting.
> > > 
> > > 1) How do you avoid warning for code that already uses unique_ptr? After
> > > inlining, it will look just like code calling new, and delete on all paths.
> > 
> > Currently by running early enough that no inline has happened.  I'm not
> > sure how valuable it would be to try and find things post inlining
> > instead.
> 
> Ah, right, for the warning it indeed makes sense to be pre-inlining, as it
> isn't clear you could use unique_ptr if the call to free was not directly in
> this function.

well, you probably could, you'd just need to pass it around some.

> > > Well, currently you only handle malloc/free, but if you provide an inline
> > > implementation of new/delete that forwards to malloc/free, the issue should
> > > already appear. Or maybe the pass is very early? But then the compiler will
> > > likely seldom notice that the argument to free is actually the return of
> > > malloc.
> > 
> > I wonder how true this last bit is, sure we could find some more things
> > after more optimization, but its not obvious to me that it would be so
> > much to really be worth it.  Looking at gcc itself there's plenty of
> > places where this should already be enough with tracking places where
> > the pointer is coppied from one ssa name to another.  I feel like the
> > big areas of improvement are warning about functions that take or return
> > ownership of things.
> 
> I fear that there are many cases where you need more PRE / DOM / SRA / ...
> for the SSA_NAME to end up in a place where malloc and free match, but if
> you already detect some cases, that's good.

So, I've been using a hacked up version of this to find places to use
auto_bitmap.  I ended up following places where one ssa name is assigned
to another, and moving this pass after the ccp within
all_early_optimizations.  The pass location change was because at the
earlier location there was places where the ssa name from the allocation
was assigned to a vAR_DECL, that was ssa'd away at the later point.  So
it may actually be that optimization helps more than I thought, but I
couldn't see an immediate reason we needed to be still using variables
instead of ssa names in the one spot I looked at.

> > > 2) In some sense, we would want to warn when some path is missing the call
> > > to free, but it seems very hard to distinguish the cases where it is
> > > accidentally missing from those where it is done on purpose. Maybe detect
> > > cases where the only paths missing free are through exceptions?
> > 
> > I didn't think about exceptions, but I'd been thinking of trying to warn
> > about variables we prove don't escape in alias analysis, but that would
> > probably be a small set of all allocations.
> > 
> > > 3) This seems very similar to the conditions needed to transform a call to
> > > malloc into a stack allocation. I was experimenting with this some years
> > > ago, see for instance
> > > https://gcc.gnu.org/ml/gcc-patches/2013-11/msg01049.html (it is probably not
> > > the most recent message on the subject, since the main code moved to CCP
> > > afterwards http://geometrica.saclay.inria.fr/team/Marc.Glisse/tmp/heapstack,
> > > but I can't locate more recent messages at the moment, and it was more a
> > > prototype than a ready patch). Do you think the infrastructure could be
> > > shared between your warning and such an optimization? Or are the goals
> > > likely to diverge too soon for sharing to be practical?
> > 
> > yeah, some of the issues are certainly similar.  I'm not sure if there
> > would be things worth sharing probably easiest to find out by just
> > trying it.
> 
> It seems that the differences will be larger than I originally expected, so
> it isn't clear the gain would be worth it.

yeah, it seems to me there's a couple parts an iterator over all
statements that matches if a statement meets some requirements.  That
may be generalizable?  Then there's a function that checks if you can
get from one block to another without going through a given set of
blocks.  There's also a thing to find where a function is called with
some arguments which I'm not sure if that's generally useful.  Finally
there's some glue holding specific checkers and the rest together.

> While I remember, one (easy) case you may want to handle is:
> 
> p = malloc(n);
> if (p != 0) ...
> 
> where the else branch does not need to contain a call to free.

yeah, that's another thing to try and cover, though I suspect there's a
lot of xmalloc like things out there that will have the !p case end in a
noreturn block, which would already skip them from the analysis.

> (maybe one can add strdup and similar functions later, but better start with
> few functions for now)

yeah, I think there's a bunch of things to explore in the space, but we
should probably start by getting some of it finished ;)

Trev

> 
> -- 
> Marc Glisse
Joseph Myers May 8, 2017, 3:49 p.m. UTC | #5
Note that the new option will need documenting in invoke.texi for any 
patch version actually proposed for inclusion.
diff mbox

Patch

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index f675e073ecc..d33e8591b03 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1294,6 +1294,7 @@  OBJS = \
 	gimple-laddress.o \
 	gimple-low.o \
 	gimple-pretty-print.o \
+	gimple-owned-ptr.o \
 	gimple-ssa-backprop.o \
 	gimple-ssa-isolate-paths.o \
 	gimple-ssa-nonnull-compare.o \
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 8697eb13108..6f12c3ac5eb 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -428,6 +428,10 @@  Wctor-dtor-privacy
 C++ ObjC++ Var(warn_ctor_dtor_privacy) Warning
 Warn when all constructors and destructors are private.
 
+Wowning-ptr
+C++ ObjC++ Var(warn_owning_ptr) Warning
+Warn about pointers that can be marked as owning the resource they refer to.
+
 Wdangling-else
 C ObjC C++ ObjC++ Var(warn_dangling_else) Warning LangEnabledBy(C ObjC C++ ObjC++,Wparentheses)
 Warn about dangling else.
diff --git a/gcc/gimple-owned-ptr.c b/gcc/gimple-owned-ptr.c
new file mode 100644
index 00000000000..f1283940920
--- /dev/null
+++ b/gcc/gimple-owned-ptr.c
@@ -0,0 +1,214 @@ 
+/* Detection of allocations that are owned by a single function.
+   Copyright (C) 2015-2017 Free Software Foundation, Inc.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it under
+the terms of the GNU General Public License as published by the Free
+Software Foundation; either version 3, or (at your option) any later
+version.
+
+GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+WARRANTY; without even the implied warranty of MERCHANTABILITY or
+FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+<http://www.gnu.org/licenses/>.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "backend.h"
+#include "tree.h"
+#include "gimple.h"
+#include "basic-block.h"
+#include "options.h"
+#include "flags.h"
+#include "stmt.h"
+#include "gimple-iterator.h"
+#include "tree-cfg.h"
+#include "tree-pass.h"
+#include "ssa.h"
+#include "diagnostic-core.h"
+
+/*  Return true if gcall calls a function in the malloc family where the memory
+ *  can be released by calling free().  */
+
+static bool
+is_malloc_alloc (gcall *call)
+{
+  if (!gimple_call_builtin_p (call))
+    return false;
+
+  tree callee = gimple_call_fndecl (call);
+  built_in_function builtin = DECL_FUNCTION_CODE (callee);
+  if (builtin != BUILT_IN_MALLOC
+      && builtin != BUILT_IN_CALLOC
+      && builtin != BUILT_IN_ALIGNED_ALLOC)
+    return false;
+
+  return true;
+}
+
+/* A free_fn_p implementation for free().  We don't need to check what's being
+   passed to free() because the resource in question is used in the statement,
+   and free() should only use one thing.  */
+
+static bool
+is_malloc_free (gcall *call, tree)
+{
+      return gimple_call_builtin_p (call, BUILT_IN_FREE);
+}
+
+namespace {
+
+class pass_owned_ptrs : public gimple_opt_pass
+{
+public:
+  pass_owned_ptrs (gcc::context *ctxt) : gimple_opt_pass (data, ctxt) {}
+
+  static const pass_data data;
+  virtual bool
+  gate (function *)
+  {
+    return true;
+  }
+  virtual unsigned int
+  execute (function *fun);
+
+private:
+  /* Should return true if the call is to a function that allocates some sort
+     of resource.  It is assumed the lhs of the call is a reference to the
+     resource.  */
+  typedef bool (*alloc_fn_p)(gcall *);
+
+  /* Should return true if the call releases the resource.  */
+  typedef bool (*free_fn_p)(gcall *, tree);
+
+  void collect_frees (free_fn_p is_free, tree var,
+		      bitmap freeing_blocks) const;
+  bool always_reaches_free (basic_block bb, const bitmap freeing_blocks) const;
+  void detect_owned (alloc_fn_p is_alloc, free_fn_p is_free) const;
+
+  function *m_fun;
+};
+
+unsigned int
+pass_owned_ptrs::execute (function *fun)
+      {
+	m_fun = fun;
+	detect_owned (is_malloc_alloc, is_malloc_free);
+	return 0;
+      }
+
+void
+pass_owned_ptrs::detect_owned (alloc_fn_p is_alloc, free_fn_p is_free) const
+{
+	basic_block bb;
+	FOR_EACH_BB_FN (bb, m_fun)
+	  {
+	    for (gimple_stmt_iterator gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+	      {
+		gcall *call = dyn_cast<gcall *> (gsi_stmt (gsi));
+		if (!call || !is_alloc (call))
+		  continue;
+
+		tree lhs = gimple_call_lhs (call);
+		if (!lhs)
+		  continue;
+
+		auto_bitmap freeing_blocks;
+		collect_frees (is_free, lhs, freeing_blocks);
+		if (bitmap_empty_p (freeing_blocks))
+		  continue;
+
+		if (!always_reaches_free (bb, freeing_blocks))
+		  continue;
+
+		warning_at (gimple_location (call), OPT_Wowning_ptr, "result of allocation can be owned by this function");
+	      }
+	  }
+      }
+
+/* Collect the set of places where the value in var is passed to free.  */
+
+void
+pass_owned_ptrs::collect_frees (free_fn_p is_free, tree var,
+				bitmap freeing_blocks) const
+{
+  imm_use_iterator iter;
+  use_operand_p use;
+  FOR_EACH_IMM_USE_FAST (use, iter, var)
+    {
+      gcall *use_stmt = dyn_cast<gcall *> (USE_STMT (use));
+      if (!use_stmt || !is_free (use_stmt, var))
+	continue;
+      if (!gimple_call_builtin_p (use_stmt, BUILT_IN_FREE))
+	continue;
+
+      basic_block bb = gimple_bb (use_stmt);
+      bitmap_set_bit (freeing_blocks, bb->index);
+    }
+}
+
+/* Return true if allocating in bb means we must free the resource in one of
+ * the blocks where it is freed, and false if there is a path on which it is
+   not freed.
+
+   This is the same as checking if the set of freeing blocks as a whole post
+   domminates bb.  However there isn't an immediately obvious way to check that
+   so we just do a bredth first search from bb looking for the exit block and
+   stopping any path that goes through a block in freeing_blocks.  */
+
+bool
+pass_owned_ptrs::always_reaches_free (basic_block bb, const bitmap freeing_blocks) const
+{
+  auto_bitmap worklist, visited;
+  bitmap_set_bit (worklist, bb->index);
+  while (!bitmap_empty_p (worklist)) {
+      unsigned int bb_idx = bitmap_first_set_bit (worklist);
+      bitmap_clear_bit (worklist, bb_idx);
+      if (bb_idx == EXIT_BLOCK)
+	return false;
+
+      if (bitmap_bit_p (freeing_blocks, bb_idx))
+	continue;
+
+      basic_block current = BASIC_BLOCK_FOR_FN (m_fun, bb_idx);
+      edge_iterator ei;
+      edge e;
+      FOR_EACH_EDGE (e, ei, current->succs)
+	{
+	  unsigned int succ_idx = e->dest->index;
+	  if (bitmap_bit_p (visited, succ_idx))
+	    continue;
+
+	  bitmap_set_bit (worklist, succ_idx);
+	  bitmap_set_bit (visited, succ_idx);
+	}
+  }
+
+      return true;
+}
+
+const pass_data pass_owned_ptrs::data = {
+  GIMPLE_PASS,		       /* type */
+  "owned_ptrs", /* name */
+  OPTGROUP_NONE,	       /* optinfo_flags */
+  TV_NONE,		       /* tv_id */
+  (PROP_cfg),		       /* properties_required */
+  0,			       /* properties_provided */
+  0,			       /* properties_destroyed */
+  0,			       /* todo_flags_start */
+  0,	     /* todo_flags_finish */
+};
+
+} // anon namespace
+
+gimple_opt_pass *
+make_pass_warn_owned_ptrs (gcc::context *ctxt)
+{
+  return new pass_owned_ptrs (ctxt);
+}
diff --git a/gcc/passes.def b/gcc/passes.def
index 6b0f05b07bd..1236eb96ec0 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -62,6 +62,7 @@  along with GCC; see the file COPYING3.  If not see
       NEXT_PASS (pass_early_warn_uninitialized);
       NEXT_PASS (pass_nothrow);
       NEXT_PASS (pass_rebuild_cgraph_edges);
+  NEXT_PASS (pass_warn_owned_ptrs);
   POP_INSERT_PASSES ()
 
   NEXT_PASS (pass_chkp_instrumentation_passes);
diff --git a/gcc/testsuite/g++.dg/warn/Wowning-ptr1.C b/gcc/testsuite/g++.dg/warn/Wowning-ptr1.C
new file mode 100644
index 00000000000..7064ba40b47
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wowning-ptr1.C
@@ -0,0 +1,76 @@ 
+// { dg-do compile }
+// { dg-options "-Wowning-ptr" }
+
+int *
+foo ()
+{
+	int *x = (int *) __builtin_malloc (sizeof (int));
+	return x;
+}
+
+void
+bar ()
+{
+	void *x = __builtin_malloc (4); // { dg-warning "result of allocation can be owned by this function" }
+	__builtin_free (x);
+}
+
+void
+baz (bool b)
+{
+	void *x = __builtin_malloc (4); // { dg-warning "result of allocation can be owned by this function" }
+	if (b)
+	{
+		bar ();
+		__builtin_free (x);
+	return;
+	}
+
+	__builtin_free(x);
+}
+
+void
+foofoo (bool b)
+{
+	void *x = __builtin_malloc (4);
+	if (b)
+	{
+		__builtin_free (x);
+	}
+}
+
+struct s { void *p; };
+
+void
+barbar (s *ptr)
+{
+	ptr->p = __builtin_malloc (4);
+}
+
+s
+qux (bool b)
+{
+	s c = { 0 };
+	void *x = __builtin_malloc (4);
+	if (b)
+		__builtin_free (x);
+	else
+		c.p = x;
+
+	return c;
+}
+
+void something (int *);
+
+void
+blah ()
+{
+	int *x = (int *) __builtin_malloc (sizeof (int) * 100); // { dg-warning "result of allocation can be owned" }
+	for (int i = 0; i < 100; i++)
+	{
+		something (x);
+		something (&x[i]);
+	}
+
+	__builtin_free (x);
+}
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index 85bfba7ac28..b7d6202ffda 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -426,6 +426,7 @@  extern gimple_opt_pass *make_pass_post_ipa_warn (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_stdarg (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_early_warn_uninitialized (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_late_warn_uninitialized (gcc::context *ctxt);
+extern gimple_opt_pass *make_pass_warn_owned_ptrs (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_cse_reciprocals (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_cse_sincos (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_optimize_bswap (gcc::context *ctxt);