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.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r16-266-ga1922f0252b3b0.
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 <dmalc...@redhat.com>
---
gcc/analyzer/engine.cc | 2 +-
gcc/analyzer/program-state.cc | 2 +-
gcc/analyzer/region-model.cc | 90 +++++++++++++++----
gcc/analyzer/region-model.h | 4 +-
gcc/analyzer/region.cc | 14 ++-
gcc/analyzer/sm-signal.cc | 2 +-
gcc/analyzer/state-purge.cc | 2 +-
.../c-c++-common/analyzer/hard-reg-1.c | 8 ++
gcc/testsuite/g++.dg/analyzer/nrvo-1.C | 18 ++++
gcc/testsuite/g++.dg/analyzer/nrvo-2.C | 26 ++++++
.../g++.dg/analyzer/nrvo-pr111536-1.C | 11 +++
.../g++.dg/analyzer/nrvo-pr111536-1b.C | 12 +++
.../g++.dg/analyzer/nrvo-pr111536-2.C | 10 +++
.../g++.dg/analyzer/nrvo-pr111536-2b.C | 13 +++
14 files changed, 186 insertions(+), 28 deletions(-)
create mode 100644 gcc/testsuite/c-c++-common/analyzer/hard-reg-1.c
create mode 100644 gcc/testsuite/g++.dg/analyzer/nrvo-1.C
create mode 100644 gcc/testsuite/g++.dg/analyzer/nrvo-2.C
create mode 100644 gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-1.C
create mode 100644 gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-1b.C
create mode 100644 gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-2.C
create mode 100644 gcc/testsuite/g++.dg/analyzer/nrvo-pr111536-2b.C
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 0917306fe38..c3e4800f70a 100644
--- 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 dbca0369b9a..21f78e5b5c3 100644
--- 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 fc586296ce5..1ee882c98ea 100644
--- 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 c1eb9468f4b..2c7f73795e2 100644
--- 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 4e56cd569d3..efbbca03c7f 100644
--- 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 bbd7d70a7bd..83f2808708e 100644
--- 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 d67802e7eea..7a93ceee9a1 100644
--- 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
index 00000000000..d22a5b570a1
--- /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
index 00000000000..146b80a3a0c
--- /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
index 00000000000..e0567c31ae2
--- /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
index 00000000000..de447ae7f47
--- /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
index 00000000000..681795da995
--- /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
index 00000000000..aa011e223b3
--- /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
index 00000000000..31df5a86ccc
--- /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" }
+}
--
2.26.3
