analyzer: handle NRVO and DECL_BY_REFERENCE [PR111536]

The analyzer was issuing false warnings about uninitialized variables
in C++ in places where NRVO was marking DECL_RESULT with
DECL_BY_REFERENCE.

Fixed thusly.

gcc/analyzer/ChangeLog:
	PR analyzer/111536
	* engine.cc (maybe_update_for_edge): Update for new call_stmt
	param to region_model::push_frame.
	* program-state.cc (program_state::push_frame): Likewise.
	* region-model.cc (region_model::update_for_gcall): Likewise.
	(region_model::push_frame): Add "call_stmt" param.
	Handle DECL_RESULT with DECL_BY_REFERENCE set on it by stashing
	the region of the lhs of the call_stmt in the caller frame,
	and writing a reference to it within the "result" in the callee
	frame.
	(region_model::pop_frame): Don't write back to the LHS for
	DECL_BY_REFERENCE results.
	(selftest::test_stack_frames): Update for new call_stmt param to
	region_model::push_frame.
	(selftest::test_get_representative_path_var): Likewise.
	(selftest::test_state_merging): Likewise.
	(selftest::test_alloca): Likewise.
	* region-model.h (region_model::push_frame): Add "call_stmt"
	param.
	* region.cc: Include "tree-ssa.h".
	(region::can_have_initial_svalue_p): Use ssa_defined_default_def_p
	for ssa names, rather than special-casing it for just parameters.
	This should now also cover DECL_RESULT with DECL_BY_REFERENCE and
	hard registers.
	* sm-signal.cc (update_model_for_signal_handler): Update for new
	call_stmt param to region_model::push_frame.
	* state-purge.cc (state_purge_per_decl::process_worklists):
	Likewise.

gcc/testsuite/ChangeLog:
	PR analyzer/111536
	* c-c++-common/analyzer/hard-reg-1.c: New test.
	* g++.dg/analyzer/nrvo-1.C: New test.
	* g++.dg/analyzer/nrvo-2.C: New test.
	* g++.dg/analyzer/nrvo-pr111536-1.C: New test.
	* g++.dg/analyzer/nrvo-pr111536-1b.C: New test.
	* g++.dg/analyzer/nrvo-pr111536-2.C: New test.
	* g++.dg/analyzer/nrvo-pr111536-2b.C: New test.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 0917306fe38c42d3461c6d493f49922f0de141fb..c3e4800f70adc45f635a5147b1048e47ff8b1428 100644 (file)
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -5343,7 +5343,7 @@ maybe_update_for_edge (logger *logger,
                      == PK_BEFORE_SUPERNODE);
          function *fun = eedge->m_dest->get_function ();
          gcc_assert (fun);
-         m_model.push_frame (*fun, NULL, ctxt);
+         m_model.push_frame (*fun, nullptr, nullptr, ctxt);
          if (logger)
            logger->log ("  pushing frame for %qD", fun->decl);
        }

diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index dbca0369b9a15a45910c090845dd969388b1e0f9..21f78e5b5c3c7ebacfa557e46be2f01d0edb29ee 100644 (file)
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -1231,7 +1231,7 @@ void
 program_state::push_frame (const extrinsic_state &ext_state ATTRIBUTE_UNUSED,
                           const function &fun)
 {
-  m_region_model->push_frame (fun, NULL, NULL);
+  m_region_model->push_frame (fun, nullptr, nullptr, nullptr);
 }
 
 /* Get the current function of this state.  */

diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index fc586296ce5b4c849e9dd81a9746e763dd5f3844..1ee882c98ea5899ac22aa61bb7a9f0e71b19977f 100644 (file)
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -6270,7 +6270,7 @@ region_model::update_for_gcall (const gcall &call_stmt,
   }
 
   gcc_assert (callee);
-  push_frame (*callee, &arg_svals, ctxt);
+  push_frame (*callee, &call_stmt, &arg_svals, ctxt);
 }
 
 /* Pop the top-most frame_region from the stack, and copy the return
@@ -6744,6 +6744,10 @@ region_model::on_top_level_param (tree param,
 /* Update this region_model to reflect pushing a frame onto the stack
    for a call to FUN.
 
+   If CALL_STMT is non-NULL, this is for the interprocedural case where
+   we already have an execution path into the caller.  It can be NULL for
+   top-level entrypoints into the analysis, or in selftests.
+
    If ARG_SVALS is non-NULL, use it to populate the parameters
    in the new frame.
    Otherwise, the params have their initial_svalues.
@@ -6752,14 +6756,32 @@ region_model::on_top_level_param (tree param,
 
 const region *
 region_model::push_frame (const function &fun,
+                         const gcall *call_stmt,
                          const vec<const svalue *> *arg_svals,
                          region_model_context *ctxt)
 {
-  m_current_frame = m_mgr->get_frame_region (m_current_frame, fun);
+  tree fndecl = fun.decl;
   if (arg_svals)
     {
+      /* If the result of the callee is DECL_BY_REFERENCE, then
+        we'll need to store a reference to the caller's lhs of
+        CALL_STMT within callee's result.
+        If so, determine the region of CALL_STMT's lhs within
+        the caller's frame before updating m_current_frame.  */
+      const region *caller_return_by_reference_reg = nullptr;
+      if (tree result = DECL_RESULT (fndecl))
+       if (DECL_BY_REFERENCE (result))
+         {
+           gcc_assert (call_stmt);
+           tree lhs = gimple_call_lhs (call_stmt);
+           gcc_assert (lhs);
+           caller_return_by_reference_reg = get_lvalue (lhs, ctxt);
+         }
+
+      /* Update m_current_frame.  */
+      m_current_frame = m_mgr->get_frame_region (m_current_frame, fun);
+
       /* Arguments supplied from a caller frame.  */
-      tree fndecl = fun.decl;
       unsigned idx = 0;
       for (tree iter_parm = DECL_ARGUMENTS (fndecl); iter_parm;
           iter_parm = DECL_CHAIN (iter_parm), ++idx)
@@ -6787,13 +6809,39 @@ region_model::push_frame (const function &fun,
                                         va_arg_idx);
          set_value (var_arg_reg, arg_sval, ctxt);
        }
+
+      /* If the result of the callee is DECL_BY_REFERENCE, then above
+        we should have determined the region within the
+        caller's frame that the callee will be writing back to.
+        Use this now to initialize the reference in callee's frame.  */
+      if (tree result = DECL_RESULT (fndecl))
+       if (DECL_BY_REFERENCE (result))
+         {
+           /* Get reference to the caller lhs.  */
+           gcc_assert (caller_return_by_reference_reg);
+           const svalue *ref_sval
+             = m_mgr->get_ptr_svalue (TREE_TYPE (result),
+                                      caller_return_by_reference_reg);
+
+           /* Get region for default val of DECL_RESULT within the
+              callee.  */
+           tree result_default_ssa = get_ssa_default_def (fun, result);
+           gcc_assert (result_default_ssa);
+           const region *callee_result_reg
+             = get_lvalue (result_default_ssa, ctxt);
+
+           /* Set the callee's reference to refer to the caller's lhs.  */
+           set_value (callee_result_reg, ref_sval, ctxt);
+         }
     }
   else
     {
       /* Otherwise we have a top-level call within the analysis.  The params
         have defined but unknown initial values.
         Anything they point to has escaped.  */
-      tree fndecl = fun.decl;
+
+      /* Update m_current_frame.  */
+      m_current_frame = m_mgr->get_frame_region (m_current_frame, fun);
 
       /* Handle "__attribute__((nonnull))".   */
       tree fntype = TREE_TYPE (fndecl);
@@ -6946,7 +6994,11 @@ region_model::pop_frame (tree result_lvalue,
   /* Pop the frame.  */
   m_current_frame = m_current_frame->get_calling_frame ();
 
-  if (result_lvalue && retval)
+  if (result_lvalue
+      && retval
+      /* Don't write back for DECL_BY_REFERENCE; the writes
+        should have happened within the callee already.  */
+      && !DECL_BY_REFERENCE (result))
     {
       gcc_assert (eval_return_svalue);
 
@@ -8925,7 +8977,7 @@ test_stack_frames ()
   /* Push stack frame for "parent_fn".  */
   const region *parent_frame_reg
     = model.push_frame (*DECL_STRUCT_FUNCTION (parent_fndecl),
-                       NULL, &ctxt);
+                       nullptr, nullptr, &ctxt);
   ASSERT_EQ (model.get_current_frame (), parent_frame_reg);
   ASSERT_TRUE (model.region_exists_p (parent_frame_reg));
   const region *a_in_parent_reg = model.get_lvalue (a, &ctxt);
@@ -8940,7 +8992,8 @@ test_stack_frames ()
 
   /* Push stack frame for "child_fn".  */
   const region *child_frame_reg
-    = model.push_frame (*DECL_STRUCT_FUNCTION (child_fndecl), NULL, &ctxt);
+    = model.push_frame (*DECL_STRUCT_FUNCTION (child_fndecl),
+                       nullptr, nullptr, &ctxt);
   ASSERT_EQ (model.get_current_frame (), child_frame_reg);
   ASSERT_TRUE (model.region_exists_p (child_frame_reg));
   const region *x_in_child_reg = model.get_lvalue (x, &ctxt);
@@ -9033,7 +9086,8 @@ test_get_representative_path_var ()
   for (int depth = 0; depth < 5; depth++)
     {
       const region *frame_n_reg
-       = model.push_frame (*DECL_STRUCT_FUNCTION (fndecl), NULL, &ctxt);
+       = model.push_frame (*DECL_STRUCT_FUNCTION (fndecl),
+                           nullptr, nullptr, &ctxt);
       const region *parm_n_reg = model.get_lvalue (path_var (n, depth), &ctxt);
       parm_regs.safe_push (parm_n_reg);
 
@@ -9279,9 +9333,11 @@ test_state_merging ()
     region_model model0 (&mgr);
     region_model model1 (&mgr);
     ASSERT_EQ (model0.get_stack_depth (), 0);
-    model0.push_frame (*DECL_STRUCT_FUNCTION (test_fndecl), NULL, &ctxt);
+    model0.push_frame (*DECL_STRUCT_FUNCTION (test_fndecl),
+                      nullptr, nullptr, &ctxt);
     ASSERT_EQ (model0.get_stack_depth (), 1);
-    model1.push_frame (*DECL_STRUCT_FUNCTION (test_fndecl), NULL, &ctxt);
+    model1.push_frame (*DECL_STRUCT_FUNCTION (test_fndecl),
+                      nullptr, nullptr, &ctxt);
 
     placeholder_svalue test_sval (mgr.alloc_symbol_id (),
                                  integer_type_node, "test sval");
@@ -9373,7 +9429,8 @@ test_state_merging ()
   /* Pointers: non-NULL and non-NULL: ptr to a local.  */
   {
     region_model model0 (&mgr);
-    model0.push_frame (*DECL_STRUCT_FUNCTION (test_fndecl), NULL, NULL);
+    model0.push_frame (*DECL_STRUCT_FUNCTION (test_fndecl),
+                      nullptr, nullptr, nullptr);
     model0.set_value (model0.get_lvalue (p, NULL),
                      model0.get_rvalue (addr_of_a, NULL), NULL);
 
@@ -9512,12 +9569,14 @@ test_state_merging ()
      frame points to a local in a more recent stack frame.  */
   {
     region_model model0 (&mgr);
-    model0.push_frame (*DECL_STRUCT_FUNCTION (test_fndecl), NULL, NULL);
+    model0.push_frame (*DECL_STRUCT_FUNCTION (test_fndecl),
+                      nullptr, nullptr, nullptr);
     const region *q_in_first_frame = model0.get_lvalue (q, NULL);
 
     /* Push a second frame.  */
     const region *reg_2nd_frame
-      = model0.push_frame (*DECL_STRUCT_FUNCTION (test_fndecl), NULL, NULL);
+      = model0.push_frame (*DECL_STRUCT_FUNCTION (test_fndecl),
+                          nullptr, nullptr, nullptr);
 
     /* Have a pointer in the older frame point to a local in the
        more recent frame.  */
@@ -9544,7 +9603,8 @@ test_state_merging ()
   /* Verify that we can merge a model in which a local points to a global.  */
   {
     region_model model0 (&mgr);
-    model0.push_frame (*DECL_STRUCT_FUNCTION (test_fndecl), NULL, NULL);
+    model0.push_frame (*DECL_STRUCT_FUNCTION (test_fndecl),
+                      nullptr, nullptr, nullptr);
     model0.set_value (model0.get_lvalue (q, NULL),
                      model0.get_rvalue (addr_of_y, NULL), NULL);
 
@@ -10076,7 +10136,7 @@ test_alloca ()
   /* Push stack frame.  */
   const region *frame_reg
     = model.push_frame (*DECL_STRUCT_FUNCTION (fndecl),
-                       NULL, &ctxt);
+                       nullptr, nullptr, &ctxt);
   /* "p = alloca (n * 4);".  */
   const svalue *size_sval = model.get_rvalue (n_times_4, &ctxt);
   const region *reg = model.create_region_for_alloca (size_sval, &ctxt);

diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index c1eb9468f4bd1f6c1fe5812bb718209c99009f23..2c7f73795e242708c86be6f76d9f02f494bd9f95 100644 (file)
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -391,7 +391,9 @@ class region_model
   void update_for_return_gcall (const gcall &call_stmt,
                                 region_model_context *ctxt);
 
-  const region *push_frame (const function &fun, const vec<const svalue *> *arg_sids,
+  const region *push_frame (const function &fun,
+                           const gcall *call_stmt,
+                           const vec<const svalue *> *arg_sids,
                            region_model_context *ctxt);
   const frame_region *get_current_frame () const { return m_current_frame; }
   const function *get_current_function () const;

diff --git a/gcc/analyzer/region.cc b/gcc/analyzer/region.cc
index 4e56cd569d39b4910f5d6b778757e62521b73e21..efbbca03c7fc8a068cf7f1093539b30a344ed1b0 100644 (file)
--- a/gcc/analyzer/region.cc
+++ b/gcc/analyzer/region.cc
@@ -27,6 +27,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "digraph.h"
 #include "sbitmap.h"
 #include "fold-const.h"
+#include "tree-ssa.h"
 
 #include "analyzer/analyzer-logging.h"
 #include "analyzer/supergraph.h"
@@ -546,15 +547,12 @@ region::can_have_initial_svalue_p () const
 
            case SSA_NAME:
              {
+               /* Some SSA names have an implicit default defined value.  */
                tree ssa_name = decl;
-               /* SSA names that are the default defn of a PARM_DECL
-                  have initial_svalues; other SSA names don't.  */
-               if (SSA_NAME_IS_DEFAULT_DEF (ssa_name)
-                   && SSA_NAME_VAR (ssa_name)
-                   && TREE_CODE (SSA_NAME_VAR (ssa_name)) == PARM_DECL)
-                 return true;
-               else
-                 return false;
+               if (SSA_NAME_IS_DEFAULT_DEF (ssa_name))
+                 return ssa_defined_default_def_p (ssa_name);
+               /* Others don't.  */
+               return false;
              }
            }
        }

diff --git a/gcc/analyzer/sm-signal.cc b/gcc/analyzer/sm-signal.cc
index bbd7d70a7bd5d340714e70ea0044dd73c523a509..83f2808708e8b32c35df701379cd9dcf18a26f0f 100644 (file)
--- a/gcc/analyzer/sm-signal.cc
+++ b/gcc/analyzer/sm-signal.cc
@@ -196,7 +196,7 @@ update_model_for_signal_handler (region_model *model,
   gcc_assert (model);
   /* Purge all state within MODEL.  */
   *model = region_model (model->get_manager ());
-  model->push_frame (handler_fun, NULL, NULL);
+  model->push_frame (handler_fun, nullptr, nullptr, nullptr);
 }
 
 /* Custom exploded_edge info: entry into a signal-handler.  */

diff --git a/gcc/analyzer/state-purge.cc b/gcc/analyzer/state-purge.cc
index d67802e7eea12f174cca12af98bc625e2dcd3ccd..7a93ceee9a1d9bece5d69c49b8fdae453001f73f 100644 (file)
--- a/gcc/analyzer/state-purge.cc
+++ b/gcc/analyzer/state-purge.cc
@@ -730,7 +730,7 @@ state_purge_per_decl::process_worklists (const state_purge_map &map,
       worklist.safe_push (iter);
 
     region_model model (mgr);
-    model.push_frame (get_function (), NULL, NULL);
+    model.push_frame (get_function (), nullptr, nullptr, nullptr);
 
     /* Process worklist by walking backwards until we reach a stmt
        that fully overwrites the decl.  */

diff --git a/gcc/testsuite/c-c++-common/analyzer/hard-reg-1.c b/gcc/testsuite/c-c++-common/analyzer/hard-reg-1.c
new file mode 100644 (file)
index 0000000..d22a5b5
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/analyzer/hard-reg-1.c
@@ -0,0 +1,8 @@
+/* { dg-do compile { target x86_64-*-* } } */
+
+void *
+get_from_hard_reg (void)
+{
+  register void *sp asm ("sp");
+  return sp;
+}

diff --git a/gcc/testsuite/g++.dg/analyzer/nrvo-1.C b/gcc/testsuite/g++.dg/analyzer/nrvo-1.C
new file mode 100644 (file)
index 0000000..146b80a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/analyzer/nrvo-1.C
@@ -0,0 +1,18 @@
+#include "../../gcc.dg/analyzer/analyzer-decls.h"
+
+struct s1
+{
+  s1 (int x) : m_x (x) {}
+  int m_x;
+};
+
+s1 make_s1 (int x)
+{
+  return s1 (x);
+}
+
+void test_1 (int x)
+{
+  s1 s = make_s1 (x);
+  __analyzer_eval (s.m_x == x); // { dg-warning "TRUE" }
+}

diff --git a/gcc/testsuite/g++.dg/analyzer/nrvo-2.C b/gcc/testsuite/g++.dg/analyzer/nrvo-2.C
new file mode 100644 (file)
index 0000000..e0567c3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/analyzer/nrvo-2.C
@@ -0,0 +1,26 @@
+#include "../../gcc.dg/analyzer/analyzer-decls.h"
+
+// { dg-do compile { target c++11 } }
+
+struct s1
+{
+  int t;
+  s1() { t = 42; }
+  s1(const s1& other) { t = other.t; }
+};
+
+s1 inner()
+{
+  return s1{}; // { dg-bogus "uninitialized" }
+}
+
+s1 middle()
+{
+  return inner();
+}
+
+void outer()
+{
+  s1 obj = middle();
+  __analyzer_eval (obj.t == 42); // { dg-warning "TRUE" }
+}

diff --git a/gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-1.C b/gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-1.C
new file mode 100644 (file)
index 0000000..de447ae
--- /dev/null
+++ b/gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-1.C
@@ -0,0 +1,11 @@
+struct Guard {
+  int i;
+  ~Guard() {}
+};
+Guard lock() {
+  return Guard(); // { dg-bogus "uninitialized" }
+}
+void bar() {
+  Guard foo = lock();
+}
+

diff --git a/gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-1b.C b/gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-1b.C
new file mode 100644 (file)
index 0000000..681795d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-1b.C
@@ -0,0 +1,12 @@
+// { dg-do compile { target c++11 } }
+
+struct Guard {
+  int i;
+  ~Guard() {}
+};
+Guard lock() {
+  return Guard(); // { dg-bogus "uninitialized" }
+}
+void bar() {
+  auto foo = lock();
+}

diff --git a/gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-2.C b/gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-2.C
new file mode 100644 (file)
index 0000000..aa011e2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-2.C
@@ -0,0 +1,10 @@
+struct g
+{
+  int t;
+  g();
+};
+
+g foo1()
+{
+  return g(); // { dg-bogus "uninitialized" }
+}

diff --git a/gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-2b.C b/gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-2b.C
new file mode 100644 (file)
index 0000000..31df5a8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-2b.C
@@ -0,0 +1,13 @@
+// { dg-do compile { target c++11 } }
+
+struct g
+{
+  int t;
+  g();
+  g(const g&);
+};
+
+g foo1()
+{
+  return g{}; // { dg-bogus "uninitialized" }
+}