analyzer: fix accessing wrong stack frame on interprocedural return [PR104979]

PR analyzer/104979 reports a leak false positive when handling an
interprocedural return to a caller:

  LHS = CALL(ARGS);

where the LHS is a certain non-trivial compound expression.

The root cause is that parts of the LHS were being erroneously
evaluated with respect to the stack frame of the called function,
rather than tha of the caller.  When LHS contained a local variable
within the caller as part of certain nested expressions, this local
variable was looked for within the called frame, rather than that of the
caller.  This lookup in the wrong stack frame led to the local variable
being treated as uninitialized, and thus the write to LHS was considered
as writing to a garbage location, leading to the return value being
lost, and thus being considered as a leak.

The region_model code uses the analyzer's path_var class to try to
extend the tree type with stack depth information.  Based on the above,
I think that the path_var class is fundamentally broken, but it's used
in a few other places in the analyzer, so I don't want to rip it out
until the next stage 1.

In the meantime, this patch reworks how region_model::pop_frame works so
that the destination region for an interprocedural return value is
computed after the frame is popped, so that the region_model has the
stack frame for the *caller* at that point.  Doing so fixes the issue.

I attempted a more ambitious fix which moved the storing of the return
svalue into the destination region from region_model::pop_region into
region_model::update_for_return_gcall, with pop_frame returning the
return svalue.  Unfortunately, this regressed g++.dg/analyzer/pr93212.C,
which returns a pointer into a stale frame.
unbind_region_and_descendents and poison_any_pointers_to_descendents are
only set up to poison regions with bindings into the stale frame, not
individual svalues, and updating that became more invasive than I'm
comfortable with in stage 4.

The patch also adds assertions to verify that we have the correct
function when looking up locals/SSA names in a stack frame.  There
doesn't seem to be a general-purpose way to get at the function of an
SSA name, so the assertions go from SSA name to def-stmt to basic_block,
and from there use the analyzer's supergraph to get the function from
the basic_block.  If there's a simpler way to do this, please let me know.

gcc/analyzer/ChangeLog:
	PR analyzer/104979
	* engine.cc (impl_run_checkers): Create the engine after the
	supergraph, and pass the supergraph to the engine.
	* region-model.cc (region_model::get_lvalue_1): Pass ctxt to
	frame_region::get_region_for_local.
	(region_model::update_for_return_gcall): Pass the lvalue for the
	result to pop_frame as a tree, rather than as a region.
	(region_model::pop_frame): Update for above change, determining
	the destination region after the frame is popped and thus with
	respect to the caller frame rather than the called frame.
	Likewise, set the value of the region to the return value after
	the frame is popped.
	(engine::engine): Add supergraph pointer.
	(selftest::test_stack_frames): Set the DECL_CONTECT of PARM_DECLs.
	(selftest::test_get_representative_path_var): Likewise.
	(selftest::test_state_merging): Likewise.
	* region-model.h (region_model::pop_frame): Convert first param
	from a const region * to a tree.
	(engine::engine): Add param "sg".
	(engine::m_sg): New field.
	* region.cc: Include "analyzer/sm.h" and
	"analyzer/program-state.h".
	(frame_region::get_region_for_local): Add "ctxt" param.
	Add assertions that VAR_DECLs are locals, and that expr is for the
	correct function.
	* region.h (frame_region::get_region_for_local): Add "ctxt" param.

gcc/testsuite/ChangeLog:
	PR analyzer/104979
	* gcc.dg/analyzer/boxed-malloc-1-29.c: Deleted test, moving the
	now fixed test_29 to...
	* gcc.dg/analyzer/boxed-malloc-1.c: ...here.
	* gcc.dg/analyzer/stale-frame-1.c: Add test coverage.

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

1 files changed, 2 insertions, 2 deletions

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index caa8796b494..0f40e064a22 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -5717,11 +5717,11 @@ impl_run_checkers (logger *logger)
   FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
     node->get_untransformed_body ();
 
-  engine eng (logger);
-
   /* Create the supergraph.  */
   supergraph sg (logger);
 
+  engine eng (&sg, logger);
+
   state_purge_map *purge_map = NULL;
 
   if (flag_analyzer_state_purge)