public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] analyzer: Impose recursion limit on indirect calls.
@ 2021-08-25  8:09 Ankur Saini
  2021-08-25 13:22 ` David Malcolm
  0 siblings, 1 reply; 9+ messages in thread
From: Ankur Saini @ 2021-08-25  8:09 UTC (permalink / raw)
  To: gcc Patches

[-- Attachment #1: Type: text/plain, Size: 178 bytes --]

This should also fix the failing regression found in PR analyzer/101980.

- The patch is in sync with current master
- successfully bootstrapped and tested on x86_64-linux-gnu.


[-- Attachment #2: fix.patch --]
[-- Type: application/octet-stream, Size: 1533 bytes --]

From 8e6c04e508d88184780af10f5027b92ce4caa5ae Mon Sep 17 00:00:00 2001
From: Ankur Saini <arsenic@sourceware.org>
Date: Wed, 25 Aug 2021 12:33:06 +0530
Subject: [PATCH] analyzer: Impose recursion limit on indirect calls.

2021-08-25  Ankur Saini  <arsenic@sourceware.org>

gcc/analyzer/ChangeLog:
	PR analyzer/101980
	* engine.cc (exploded_graph::maybe_create_dynamic_call): Don't create
	calls if max recursion limit is reached.
---
 gcc/analyzer/engine.cc | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 4ee92794941..9c604d1eb8c 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -3059,6 +3059,20 @@ exploded_graph::maybe_create_dynamic_call (const gcall *call,
 
       new_point.push_to_call_stack (sn_exit,
                                     next_point.get_supernode());
+
+      /* Impose a maximum recursion depth and don't analyze paths
+         that exceed it further.
+         This is something of a blunt workaround, but it only
+         applies to recursion (and mutual recursion), not to
+         general call stacks.  */
+      if (new_point.get_call_string ().calc_recursion_depth ()
+          > param_analyzer_max_recursion_depth)
+      {
+        if (logger)
+          logger->log ("rejecting call edge: recursion limit exceeded");
+        return false;
+      }
+
       next_state.push_call (*this, node, call, uncertainty);
 
       if (next_state.m_valid)
-- 
2.32.0


[-- Attachment #3: Type: text/plain, Size: 17 bytes --]



Thanks 
- Ankur

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
  2021-08-25  8:09 [PATCH] analyzer: Impose recursion limit on indirect calls Ankur Saini
@ 2021-08-25 13:22 ` David Malcolm
  2021-08-25 13:54   ` Martin Liška
  0 siblings, 1 reply; 9+ messages in thread
From: David Malcolm @ 2021-08-25 13:22 UTC (permalink / raw)
  To: Ankur Saini, gcc Patches

On Wed, 2021-08-25 at 13:39 +0530, Ankur Saini wrote:
> This should also fix the failing regression found in PR
> analyzer/101980.
> 
> - The patch is in sync with current master
> - successfully bootstrapped and tested on x86_64-linux-gnu.
> 

The patch is OK for trunk.

Thanks for fixing this
Dave



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
  2021-08-25 13:22 ` David Malcolm
@ 2021-08-25 13:54   ` Martin Liška
       [not found]     ` <9C51062C-3457-4345-9BDC-20F7FE4BFA1E@gmail.com>
  0 siblings, 1 reply; 9+ messages in thread
From: Martin Liška @ 2021-08-25 13:54 UTC (permalink / raw)
  To: David Malcolm, Ankur Saini, gcc Patches

On 8/25/21 15:22, David Malcolm via Gcc-patches wrote:
> On Wed, 2021-08-25 at 13:39 +0530, Ankur Saini wrote:
>> This should also fix the failing regression found in PR
>> analyzer/101980.
>>
>> - The patch is in sync with current master
>> - successfully bootstrapped and tested on x86_64-linux-gnu.
>>
> 
> The patch is OK for trunk.
> 
> Thanks for fixing this
> Dave
> 
> 

Hello.

Quite accidentally, but I noticed the patch violates GNU coding style:

$ git show 43a5d46feabd93ba78983919234f05f5fc9a0982 | ./contrib/check_GNU_style.py -

=== ERROR type #1: blocks of 8 spaces should be replaced with tabs (8 error(s)) ===

gcc/analyzer/engine.cc:3064:0:████████ that exceed it further.

gcc/analyzer/engine.cc:3065:0:████████ This is something of a blunt workaround, but it only

gcc/analyzer/engine.cc:3066:0:████████ applies to recursion (and mutual recursion), not to

gcc/analyzer/engine.cc:3067:0:████████ general call stacks.  */

gcc/analyzer/engine.cc:3069:0:████████  > param_analyzer_max_recursion_depth)

gcc/analyzer/engine.cc:3071:0:████████if (logger)

gcc/analyzer/engine.cc:3072:0:████████  logger->log ("rejecting call edge: recursion limit exceeded");

gcc/analyzer/engine.cc:3073:0:████████return false;


Martin

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
       [not found]     ` <9C51062C-3457-4345-9BDC-20F7FE4BFA1E@gmail.com>
@ 2021-08-25 15:05       ` David Malcolm
  2021-08-25 15:55         ` Ankur Saini
  0 siblings, 1 reply; 9+ messages in thread
From: David Malcolm @ 2021-08-25 15:05 UTC (permalink / raw)
  To: Ankur Saini, Martin Liška; +Cc: gcc Patches

On Wed, 2021-08-25 at 20:31 +0530, Ankur Saini wrote:
> 
> 
> > On 25-Aug-2021, at 7:24 PM, Martin Liška <mliska@suse.cz> wrote:
> > 
> > On 8/25/21 15:22, David Malcolm via Gcc-patches wrote:
> > > On Wed, 2021-08-25 at 13:39 +0530, Ankur Saini wrote:
> > > > This should also fix the failing regression found in PR
> > > > analyzer/101980.
> > > > 
> > > > - The patch is in sync with current master
> > > > - successfully bootstrapped and tested on x86_64-linux-gnu.
> > > > 
> > > The patch is OK for trunk.
> > > Thanks for fixing this
> > > Dave
> > 
> > Hello.
> > 
> > Quite accidentally, but I noticed the patch violates GNU coding
> > style:
> > 
> > $ git show 43a5d46feabd93ba78983919234f05f5fc9a0982 |
> > ./contrib/check_GNU_style.py -
> > 
> > === ERROR type #1: blocks of 8 spaces should be replaced with tabs
> > (8 error(s)) ===
> > 
> > gcc/analyzer/engine.cc:3064:0:████████ that exceed it further.
> > 
> > gcc/analyzer/engine.cc:3065:0:████████ This is something of a blunt
> > workaround, but it only
> > 
> > gcc/analyzer/engine.cc:3066:0:████████ applies to recursion (and
> > mutual recursion), not to
> > 
> > gcc/analyzer/engine.cc:3067:0:████████ general call stacks.  */
> > 
> > gcc/analyzer/engine.cc:3069:0:████████  >
> > param_analyzer_max_recursion_depth)
> > 
> > gcc/analyzer/engine.cc:3071:0:████████if (logger)
> > 
> > gcc/analyzer/engine.cc:3072:0:████████  logger->log ("rejecting
> > call edge: recursion limit exceeded");
> > 
> > gcc/analyzer/engine.cc:3073:0:████████return false;
> > 
> 
> Looks like my editor again converted all the tabs to spaces.
> 
> btw, I can also see a lot of other places where 8 spaces are not
> begin converted to tabs, should I also change that accordingly or
> leave them the way it is and just update this patch ?

It's usually best to split out bugfixes from formatting/whitespace
changes, so maybe do it as two patches:

(1) an updated version of this patch to fix the recursion issue, using
the correct whitespace for the lines that are touched

(2) a patch that fixes whitespaces issues, but doesn't change the
behavior of the code


Dave


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
  2021-08-25 15:05       ` David Malcolm
@ 2021-08-25 15:55         ` Ankur Saini
  2021-08-25 17:15           ` David Malcolm
  0 siblings, 1 reply; 9+ messages in thread
From: Ankur Saini @ 2021-08-25 15:55 UTC (permalink / raw)
  To: David Malcolm; +Cc: Martin Liška, gcc Patches



> On 25-Aug-2021, at 8:35 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> 
> On Wed, 2021-08-25 at 20:31 +0530, Ankur Saini wrote:
>> 
>> 
>>> On 25-Aug-2021, at 7:24 PM, Martin Liška <mliska@suse.cz> wrote:
>>> 
>>> On 8/25/21 15:22, David Malcolm via Gcc-patches wrote:
>>>> On Wed, 2021-08-25 at 13:39 +0530, Ankur Saini wrote:
>>>>> This should also fix the failing regression found in PR
>>>>> analyzer/101980.
>>>>> 
>>>>> - The patch is in sync with current master
>>>>> - successfully bootstrapped and tested on x86_64-linux-gnu.
>>>>> 
>>>> The patch is OK for trunk.
>>>> Thanks for fixing this
>>>> Dave
>>> 
>>> Hello.
>>> 
>>> Quite accidentally, but I noticed the patch violates GNU coding
>>> style:
>>> 
>>> $ git show 43a5d46feabd93ba78983919234f05f5fc9a0982 |
>>> ./contrib/check_GNU_style.py -
>>> 
>>> === ERROR type #1: blocks of 8 spaces should be replaced with tabs
>>> (8 error(s)) ===
>>> 
>>> gcc/analyzer/engine.cc:3064:0:████████ that exceed it further.
>>> 
>>> gcc/analyzer/engine.cc:3065:0:████████ This is something of a blunt
>>> workaround, but it only
>>> 
>>> gcc/analyzer/engine.cc:3066:0:████████ applies to recursion (and
>>> mutual recursion), not to
>>> 
>>> gcc/analyzer/engine.cc:3067:0:████████ general call stacks.  */
>>> 
>>> gcc/analyzer/engine.cc:3069:0:████████  >
>>> param_analyzer_max_recursion_depth)
>>> 
>>> gcc/analyzer/engine.cc:3071:0:████████if (logger)
>>> 
>>> gcc/analyzer/engine.cc:3072:0:████████  logger->log ("rejecting
>>> call edge: recursion limit exceeded");
>>> 
>>> gcc/analyzer/engine.cc:3073:0:████████return false;
>>> 
>> 
>> Looks like my editor again converted all the tabs to spaces.
>> 
>> btw, I can also see a lot of other places where 8 spaces are not
>> begin converted to tabs, should I also change that accordingly or
>> leave them the way it is and just update this patch ?
> 
> It's usually best to split out bugfixes from formatting/whitespace
> changes, so maybe do it as two patches:
> 
> (1) an updated version of this patch to fix the recursion issue, using
> the correct whitespace for the lines that are touched

unfortunately, I already checked in and pushed the changes to the master before this was pointed out. But I can add them to the whitespace issue patch.

> 
> (2) a patch that fixes whitespaces issues, but doesn't change the
> behavior of the code
> 
> 
> Dave


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
  2021-08-25 15:55         ` Ankur Saini
@ 2021-08-25 17:15           ` David Malcolm
  2021-08-27 12:44             ` Ankur Saini
  0 siblings, 1 reply; 9+ messages in thread
From: David Malcolm @ 2021-08-25 17:15 UTC (permalink / raw)
  To: Ankur Saini; +Cc: Martin Liška, gcc Patches

On Wed, 2021-08-25 at 21:25 +0530, Ankur Saini wrote:
> 
> 
> > On 25-Aug-2021, at 8:35 PM, David Malcolm <dmalcolm@redhat.com>
> > wrote:
> > 
> > On Wed, 2021-08-25 at 20:31 +0530, Ankur Saini wrote:
> > > 
> > > 
> > > > On 25-Aug-2021, at 7:24 PM, Martin Liška <mliska@suse.cz>
> > > > wrote:
> > > > 
> > > > On 8/25/21 15:22, David Malcolm via Gcc-patches wrote:
> > > > > On Wed, 2021-08-25 at 13:39 +0530, Ankur Saini wrote:
> > > > > > This should also fix the failing regression found in PR
> > > > > > analyzer/101980.
> > > > > > 
> > > > > > - The patch is in sync with current master
> > > > > > - successfully bootstrapped and tested on x86_64-linux-gnu.
> > > > > > 
> > > > > The patch is OK for trunk.
> > > > > Thanks for fixing this
> > > > > Dave
> > > > 
> > > > Hello.
> > > > 
> > > > Quite accidentally, but I noticed the patch violates GNU coding
> > > > style:
> > > > 
> > > > $ git show 43a5d46feabd93ba78983919234f05f5fc9a0982 |
> > > > ./contrib/check_GNU_style.py -
> > > > 
> > > > === ERROR type #1: blocks of 8 spaces should be replaced with
> > > > tabs
> > > > (8 error(s)) ===
> > > > 
> > > > gcc/analyzer/engine.cc:3064:0:████████ that exceed it further.
> > > > 
> > > > gcc/analyzer/engine.cc:3065:0:████████ This is something of a
> > > > blunt
> > > > workaround, but it only
> > > > 
> > > > gcc/analyzer/engine.cc:3066:0:████████ applies to recursion
> > > > (and
> > > > mutual recursion), not to
> > > > 
> > > > gcc/analyzer/engine.cc:3067:0:████████ general call stacks.  */
> > > > 
> > > > gcc/analyzer/engine.cc:3069:0:████████  >
> > > > param_analyzer_max_recursion_depth)
> > > > 
> > > > gcc/analyzer/engine.cc:3071:0:████████if (logger)
> > > > 
> > > > gcc/analyzer/engine.cc:3072:0:████████  logger->log ("rejecting
> > > > call edge: recursion limit exceeded");
> > > > 
> > > > gcc/analyzer/engine.cc:3073:0:████████return false;
> > > > 
> > > 
> > > Looks like my editor again converted all the tabs to spaces.
> > > 
> > > btw, I can also see a lot of other places where 8 spaces are not
> > > begin converted to tabs, should I also change that accordingly or
> > > leave them the way it is and just update this patch ?
> > 
> > It's usually best to split out bugfixes from formatting/whitespace
> > changes, so maybe do it as two patches:
> > 
> > (1) an updated version of this patch to fix the recursion issue,
> > using
> > the correct whitespace for the lines that are touched
> 
> unfortunately, I already checked in and pushed the changes to the
> master before this was pointed out. 

Fair enough.

> But I can add them to the whitespace issue patch.

Sounds good.
Dave

> 
> > 
> > (2) a patch that fixes whitespaces issues, but doesn't change the
> > behavior of the code
> > 
> > 
> > Dave
> 



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
  2021-08-25 17:15           ` David Malcolm
@ 2021-08-27 12:44             ` Ankur Saini
  2021-08-27 12:58               ` Martin Liška
  0 siblings, 1 reply; 9+ messages in thread
From: Ankur Saini @ 2021-08-27 12:44 UTC (permalink / raw)
  To: David Malcolm; +Cc: Martin Liška, gcc Patches

[-- Attachment #1: Type: text/plain, Size: 402 bytes --]

While working on patch to convert most of the 8 whitespace characters to tabs in the analyzer’s source, I see this weird behaviour where at some places indentation looks weird when viewed in patch file ( generated via “git format-patch” )  but looks alright when viewed in text editor.


Here is the current patch file, Can someone look at this and see if they also experience the same ?



[-- Attachment #2: fix.patch --]
[-- Type: application/octet-stream, Size: 22798 bytes --]

From 36ce00faa492de780cbc7e2e1e440cd4dad50b2e Mon Sep 17 00:00:00 2001
From: Ankur Saini <arsenic@sourceware.org>
Date: Thu, 26 Aug 2021 10:33:38 +0530
Subject: [PATCH] analyzer: whitespace fix

---
 gcc/analyzer/call-string.cc        |   6 +-
 gcc/analyzer/diagnostic-manager.cc |  62 ++++----
 gcc/analyzer/engine.cc             | 242 ++++++++++++++---------------
 gcc/analyzer/exploded-graph.h      |  14 +-
 gcc/analyzer/program-state.cc      |  28 ++--
 gcc/analyzer/region-model.cc       |  16 +-
 gcc/analyzer/region-model.h        |   8 +-
 7 files changed, 188 insertions(+), 188 deletions(-)

diff --git a/gcc/analyzer/call-string.cc b/gcc/analyzer/call-string.cc
index 1e652a08a5a..91a01e2db20 100644
--- a/gcc/analyzer/call-string.cc
+++ b/gcc/analyzer/call-string.cc
@@ -248,11 +248,11 @@ call_string::cmp (const call_string &a,
       const call_string::element_t a_node_pair = a[i];
       const call_string::element_t b_node_pair = b[i];
       int src_cmp 
-      	= a_node_pair.m_callee->m_index - b_node_pair.m_callee->m_index;
+	= a_node_pair.m_callee->m_index - b_node_pair.m_callee->m_index;
       if (src_cmp)
 	return src_cmp;
       int dest_cmp 
-      	= a_node_pair.m_caller->m_index - b_node_pair.m_caller->m_index;
+	= a_node_pair.m_caller->m_index - b_node_pair.m_caller->m_index;
       if (dest_cmp)
 	return dest_cmp;
       i++;
@@ -297,7 +297,7 @@ call_string::validate () const
     if (i > 0)
     {
       gcc_assert (e->get_caller_function () == 
-      		  m_elements[i - 1].get_callee_function ());
+		  m_elements[i - 1].get_callee_function ());
     }
 }
 
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 7ffe0004356..6989f166f47 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -2100,18 +2100,18 @@ diagnostic_manager::prune_for_sm_diagnostic (checker_path *path,
 	    callsite_expr expr;
 
 	    tree caller_var;
-            if(event->m_sedge)
-              {
-                const callgraph_superedge& cg_superedge
-                  = event->get_callgraph_superedge ();
-                if (cg_superedge.m_cedge)
-	          caller_var
-	            = cg_superedge.map_expr_from_callee_to_caller (callee_var,
-                                                                   &expr);
-                else
-                  caller_var = caller_model->get_representative_tree (sval);
-              }
-            else
+	    if(event->m_sedge)
+	      {
+		const callgraph_superedge& cg_superedge
+		  = event->get_callgraph_superedge ();
+		if (cg_superedge.m_cedge)
+		  caller_var
+		    = cg_superedge.map_expr_from_callee_to_caller (callee_var,
+								   &expr);
+		else
+		  caller_var = caller_model->get_representative_tree (sval);
+	      }
+	    else
 	      caller_var = caller_model->get_representative_tree (sval);
 
 	    if (caller_var)
@@ -2135,27 +2135,27 @@ diagnostic_manager::prune_for_sm_diagnostic (checker_path *path,
 	    if (sval)
 	      {
 		return_event *event = (return_event *)base_event;
-                const region_model *caller_model
-                  = event->m_eedge.m_dest->get_state ().m_region_model;
-                tree caller_var = caller_model->get_representative_tree (sval);
-                const region_model *callee_model
-                  = event->m_eedge.m_src->get_state ().m_region_model;
+		const region_model *caller_model
+		  = event->m_eedge.m_dest->get_state ().m_region_model;
+		tree caller_var = caller_model->get_representative_tree (sval);
+		const region_model *callee_model
+		  = event->m_eedge.m_src->get_state ().m_region_model;
 		callsite_expr expr;
 
-                tree callee_var;
-                if (event->m_sedge)
-                  {
-                    const callgraph_superedge& cg_superedge
-                      = event->get_callgraph_superedge ();
-                    if (cg_superedge.m_cedge)
-                      callee_var
-                        = cg_superedge.map_expr_from_caller_to_callee (caller_var,
-                                                                       &expr);
-                    else
-                      callee_var = callee_model->get_representative_tree (sval);
-                  }
-                else
-                  callee_var = callee_model->get_representative_tree (sval);
+		tree callee_var;
+		if (event->m_sedge)
+		  {
+		    const callgraph_superedge& cg_superedge
+		      = event->get_callgraph_superedge ();
+		    if (cg_superedge.m_cedge)
+		      callee_var
+			= cg_superedge.map_expr_from_caller_to_callee (caller_var,
+								       &expr);
+		    else
+		      callee_var = callee_model->get_representative_tree (sval);
+		  }
+		else
+		  callee_var = callee_model->get_representative_tree (sval);
 
 		if (callee_var)
 		  {
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 9c604d1eb8c..2894a3f16cf 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -1658,16 +1658,16 @@ dynamic_call_info_t::add_events_to_path (checker_path *emission_path,
 
   if (m_is_returning_call)
     emission_path->add_event (new return_event (eedge, (m_dynamic_call
-	                   			        ? m_dynamic_call->location
-	           	   		                : UNKNOWN_LOCATION),
-	          	      dest_point.get_fndecl (),
-	          	      dest_stack_depth));
+							? m_dynamic_call->location
+							: UNKNOWN_LOCATION),
+			      dest_point.get_fndecl (),
+			      dest_stack_depth));
   else
     emission_path->add_event (new call_event (eedge, (m_dynamic_call
-	                   			      ? m_dynamic_call->location
-	           	   		              : UNKNOWN_LOCATION),
-	          	      src_point.get_fndecl (),
-	          	      src_stack_depth));
+						      ? m_dynamic_call->location
+						      : UNKNOWN_LOCATION),
+			      src_point.get_fndecl (),
+			      src_stack_depth));
 
 }
 
@@ -3035,12 +3035,12 @@ state_change_requires_new_enode_p (const program_state &old_state,
 
 bool
 exploded_graph::maybe_create_dynamic_call (const gcall *call,
-                                           tree fn_decl,
-                                           exploded_node *node,
-                                           program_state next_state,
-                                           program_point &next_point,
-                                           uncertainty_t *uncertainty,
-                                           logger *logger)
+					   tree fn_decl,
+					   exploded_node *node,
+					   program_state next_state,
+					   program_point &next_point,
+					   uncertainty_t *uncertainty,
+					   logger *logger)
 {
   LOG_FUNC (logger);
 
@@ -3053,44 +3053,44 @@ exploded_graph::maybe_create_dynamic_call (const gcall *call,
       supernode *sn_exit = sg.get_node_for_function_exit (fun);
 
       program_point new_point
-        = program_point::before_supernode (sn_entry,
-                                           NULL,
-                                           this_point->get_call_string ());
+	= program_point::before_supernode (sn_entry,
+					   NULL,
+					   this_point->get_call_string ());
 
       new_point.push_to_call_stack (sn_exit,
-                                    next_point.get_supernode());
+				    next_point.get_supernode());
 
       /* Impose a maximum recursion depth and don't analyze paths
-         that exceed it further.
-         This is something of a blunt workaround, but it only
-         applies to recursion (and mutual recursion), not to
-         general call stacks.  */
+	 that exceed it further.
+	 This is something of a blunt workaround, but it only
+	 applies to recursion (and mutual recursion), not to
+	 general call stacks.  */
       if (new_point.get_call_string ().calc_recursion_depth ()
-          > param_analyzer_max_recursion_depth)
+	  > param_analyzer_max_recursion_depth)
       {
-        if (logger)
-          logger->log ("rejecting call edge: recursion limit exceeded");
-        return false;
+	if (logger)
+	  logger->log ("rejecting call edge: recursion limit exceeded");
+	return false;
       }
 
       next_state.push_call (*this, node, call, uncertainty);
 
       if (next_state.m_valid)
-        {
-          if (logger)
-            logger->log ("Discovered call to %s [SN: %i -> SN: %i]",
-                          function_name(fun),
-                          this_point->get_supernode ()->m_index,
-                          sn_entry->m_index);
-
-          exploded_node *enode = get_or_create_node (new_point,
-                                                     next_state,
-                                                     node);
-          if (enode)
-            add_edge (node,enode, NULL,
-                      new dynamic_call_info_t (call));
-          return true;
-        }
+	{
+	  if (logger)
+	    logger->log ("Discovered call to %s [SN: %i -> SN: %i]",
+			  function_name(fun),
+			  this_point->get_supernode ()->m_index,
+			  sn_entry->m_index);
+
+	  exploded_node *enode = get_or_create_node (new_point,
+						     next_state,
+						     node);
+	  if (enode)
+	    add_edge (node,enode, NULL,
+		      new dynamic_call_info_t (call));
+	  return true;
+	}
     }
   return false;
 }
@@ -3289,8 +3289,8 @@ exploded_graph::process_node (exploded_node *node)
       break;
     case PK_AFTER_SUPERNODE:
       {
-        bool found_a_superedge = false;
-        bool is_an_exit_block = false;
+	bool found_a_superedge = false;
+	bool is_an_exit_block = false;
 	/* If this is an EXIT BB, detect leaks, and potentially
 	   create a function summary.  */
 	if (point.get_supernode ()->return_p ())
@@ -3334,53 +3334,53 @@ exploded_graph::process_node (exploded_node *node)
 	    program_state next_state (state);
 	    uncertainty_t uncertainty;
 
-            /* Make use the current state and try to discover and analyse
-               indirect function calls (a call that doesn't have an underlying
-               cgraph edge representing call).
-
-               Some examples of such calls are virtual function calls
-               and calls that happen via a function pointer.  */
-            if (succ->m_kind == SUPEREDGE_INTRAPROCEDURAL_CALL
-            	&& !(succ->get_any_callgraph_edge ()))
-              {
-                const gcall *call
-                  = point.get_supernode ()->get_final_call ();
-
-                impl_region_model_context ctxt (*this,
-                                                node,
-                                                &state,
-                                                &next_state,
-                                                &uncertainty,
-                                                point.get_stmt());
-
-                region_model *model = state.m_region_model;
-                bool call_discovered = false;
-
-                if (tree fn_decl = model->get_fndecl_for_call(call,&ctxt))
-                  call_discovered = maybe_create_dynamic_call (call,
-                                                               fn_decl,
-                                                               node,
-                                                               next_state,
-                                                               next_point,
-                                                               &uncertainty,
-                                                               logger);
-                if (!call_discovered)
-                  {
-                     /* An unknown function or a special function was called 
-                        at this point, in such case, don't terminate the 
-                        analysis of the current function.
-
-                        The analyzer handles calls to such functions while
-                        analysing the stmt itself, so the the function call
-                        must have been handled by the anlyzer till now.  */
-                     exploded_node *next
-                       = get_or_create_node (next_point,
-                                             next_state,
-                                             node);
-                     if (next)
-                       add_edge (node, next, succ);
-                  }
-              }
+	    /* Make use the current state and try to discover and analyse
+	       indirect function calls (a call that doesn't have an underlying
+	       cgraph edge representing call).
+
+	       Some examples of such calls are virtual function calls
+	       and calls that happen via a function pointer.  */
+	    if (succ->m_kind == SUPEREDGE_INTRAPROCEDURAL_CALL
+		&& !(succ->get_any_callgraph_edge ()))
+	      {
+		const gcall *call
+		  = point.get_supernode ()->get_final_call ();
+
+		impl_region_model_context ctxt (*this,
+						node,
+						&state,
+						&next_state,
+						&uncertainty,
+						point.get_stmt());
+
+		region_model *model = state.m_region_model;
+		bool call_discovered = false;
+
+		if (tree fn_decl = model->get_fndecl_for_call(call,&ctxt))
+		  call_discovered = maybe_create_dynamic_call (call,
+							       fn_decl,
+							       node,
+							       next_state,
+							       next_point,
+							       &uncertainty,
+							       logger);
+		if (!call_discovered)
+		  {
+		     /* An unknown function or a special function was called
+			at this point, in such case, don't terminate the
+			analysis of the current function.
+
+			The analyzer handles calls to such functions while
+			analysing the stmt itself, so the the function call
+			must have been handled by the anlyzer till now.  */
+		     exploded_node *next
+		       = get_or_create_node (next_point,
+					     next_state,
+					     node);
+		     if (next)
+		       add_edge (node, next, succ);
+		  }
+	      }
 
 	    if (!node->on_edge (*this, succ, &next_point, &next_state,
 				&uncertainty))
@@ -3396,37 +3396,37 @@ exploded_graph::process_node (exploded_node *node)
 	      add_edge (node, next, succ);
 	  }
 
-        /* Return from the calls which doesn't have a return superedge.
-    	   Such case occurs when GCC's middle end didn't knew which function to
-    	   call but analyzer did.  */
-        if((is_an_exit_block && !found_a_superedge)
-           && (!point.get_call_string ().empty_p ()))
-          {
-            const call_string cs = point.get_call_string ();
-            program_point next_point
-              = program_point::before_supernode (cs.get_caller_node (),
-                                                 NULL,
-                                                 cs);
-            program_state next_state (state);
-            uncertainty_t uncertainty;
-
-            const gcall *call
-              = next_point.get_supernode ()->get_returning_call ();
-
-            if(call)
-              next_state.returning_call (*this, node, call, &uncertainty);
-
-            if (next_state.m_valid)
-              {
-                next_point.pop_from_call_stack ();
-                exploded_node *enode = get_or_create_node (next_point,
-                                                           next_state,
-                                                           node);
-                if (enode)
-                  add_edge (node, enode, NULL,
-                            new dynamic_call_info_t (call, true));
-              }
-          }
+	/* Return from the calls which doesn't have a return superedge.
+	   Such case occurs when GCC's middle end didn't knew which function to
+	   call but analyzer did.  */
+	if((is_an_exit_block && !found_a_superedge)
+	   && (!point.get_call_string ().empty_p ()))
+	  {
+	    const call_string cs = point.get_call_string ();
+	    program_point next_point
+	      = program_point::before_supernode (cs.get_caller_node (),
+						 NULL,
+						 cs);
+	    program_state next_state (state);
+	    uncertainty_t uncertainty;
+
+	    const gcall *call
+	      = next_point.get_supernode ()->get_returning_call ();
+
+	    if(call)
+	      next_state.returning_call (*this, node, call, &uncertainty);
+
+	    if (next_state.m_valid)
+	      {
+		next_point.pop_from_call_stack ();
+		exploded_node *enode = get_or_create_node (next_point,
+							   next_state,
+							   node);
+		if (enode)
+		  add_edge (node, enode, NULL,
+			    new dynamic_call_info_t (call, true));
+	      }
+	  }
       }
       break;
     }
diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded-graph.h
index 6890e84f985..b27de80c6d0 100644
--- a/gcc/analyzer/exploded-graph.h
+++ b/gcc/analyzer/exploded-graph.h
@@ -369,7 +369,7 @@ class dynamic_call_info_t : public exploded_edge::custom_info_t
 {
 public:
   dynamic_call_info_t (const gcall *dynamic_call,
-  		       const bool is_returning_call = false)
+		       const bool is_returning_call = false)
   : m_dynamic_call (dynamic_call), 
     m_is_returning_call (is_returning_call)
   {}
@@ -817,12 +817,12 @@ public:
   void process_node (exploded_node *node);
 
   bool maybe_create_dynamic_call (const gcall *call,
-                                  tree fn_decl,
-                                  exploded_node *node,
-                                  program_state next_state,
-                                  program_point &next_point,
-                                  uncertainty_t *uncertainty,
-                                  logger *logger);
+				  tree fn_decl,
+				  exploded_node *node,
+				  program_state next_state,
+				  program_point &next_point,
+				  uncertainty_t *uncertainty,
+				  logger *logger);
 
   exploded_node *get_or_create_node (const program_point &point,
 				     const program_state &state,
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index ea53c61f497..5e100d48d9f 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -1040,19 +1040,19 @@ program_state::on_edge (exploded_graph &eg,
    the call ( like call via a function pointer )  */
 void
 program_state::push_call (exploded_graph &eg,
-                          exploded_node *enode,
-                          const gcall *call_stmt,
-                          uncertainty_t *uncertainty)
+			  exploded_node *enode,
+			  const gcall *call_stmt,
+			  uncertainty_t *uncertainty)
 {
   /* Update state.  */
   const program_point &point = enode->get_point ();
   const gimple *last_stmt = point.get_supernode ()->get_last_stmt ();
 
   impl_region_model_context ctxt (eg, enode,
-                                  &enode->get_state (),
-                                  this,
-                                  uncertainty,
-                                  last_stmt);
+				  &enode->get_state (),
+				  this,
+				  uncertainty,
+				  last_stmt);
   m_region_model->update_for_gcall (call_stmt, &ctxt);
 }
 
@@ -1062,19 +1062,19 @@ program_state::push_call (exploded_graph &eg,
    the return */
 void
 program_state::returning_call (exploded_graph &eg,
-                               exploded_node *enode,
-                               const gcall *call_stmt,
-                               uncertainty_t *uncertainty)
+			       exploded_node *enode,
+			       const gcall *call_stmt,
+			       uncertainty_t *uncertainty)
 {
   /* Update state.  */
   const program_point &point = enode->get_point ();
   const gimple *last_stmt = point.get_supernode ()->get_last_stmt ();
 
   impl_region_model_context ctxt (eg, enode,
-                                  &enode->get_state (),
-                                  this,
-                                  uncertainty,
-                                  last_stmt);
+				  &enode->get_state (),
+				  this,
+				  uncertainty,
+				  last_stmt);
   m_region_model->update_for_return_gcall (call_stmt, &ctxt);
 }
 
diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc
index 787f2ed33c0..8bbd52f9bff 100644
--- a/gcc/analyzer/region-model.cc
+++ b/gcc/analyzer/region-model.cc
@@ -1843,8 +1843,8 @@ region_model::get_rvalue_1 (path_var pv, region_model_context *ctxt) const
       }
     case OBJ_TYPE_REF:
       {
-        tree expr = OBJ_TYPE_REF_EXPR (pv.m_tree);
-        return get_rvalue (expr, ctxt);
+	tree expr = OBJ_TYPE_REF_EXPR (pv.m_tree);
+	return get_rvalue (expr, ctxt);
       }
     }
 }
@@ -3232,11 +3232,11 @@ region_model::update_for_return_gcall (const gcall *call_stmt,
   if (lhs)
     {
       /* Normally we access the top-level frame, which is:
-         path_var (expr, get_stack_depth () - 1)
-         whereas here we need the caller frame, hence "- 2" here.  */
+	 path_var (expr, get_stack_depth () - 1)
+	 whereas here we need the caller frame, hence "- 2" here.  */
       gcc_assert (get_stack_depth () >= 2);
       result_dst_reg = get_lvalue (path_var (lhs, get_stack_depth () - 2),
-           			   ctxt);
+				   ctxt);
     }
 
   pop_frame (result_dst_reg, NULL, ctxt);
@@ -5368,10 +5368,10 @@ test_widening_constraints ()
        {i_23: (WIDENED (at phi, 0, 1) + 1); WIDENED <= 255}
      i_11 = PHI(); merge with state at phi above
        {i_11: WIDENED (at phi, 0, 1); WIDENED <= 256}
-         [changing meaning of "WIDENED" here]
+	 [changing meaning of "WIDENED" here]
      if (i_11 <= 255)
-        T: {i_11: WIDENED (at phi, 0, 1); WIDENED <= 255}; cache hit
-        F: {i_11: 256}
+	T: {i_11: WIDENED (at phi, 0, 1); WIDENED <= 255}; cache hit
+	F: {i_11: 256}
  */
 
 static void
diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h
index f2c82b0dd80..06f14e5a983 100644
--- a/gcc/analyzer/region-model.h
+++ b/gcc/analyzer/region-model.h
@@ -614,11 +614,11 @@ class region_model
 			      rejected_constraint **out);
 
   void update_for_gcall (const gcall *call_stmt,
-                         region_model_context *ctxt,
-                         function *callee = NULL);
-  
+			 region_model_context *ctxt,
+			 function *callee = NULL);
+
   void update_for_return_gcall (const gcall *call_stmt,
-                                region_model_context *ctxt);
+				region_model_context *ctxt);
 
   const region *push_frame (function *fun, const vec<const svalue *> *arg_sids,
 			    region_model_context *ctxt);
-- 
2.32.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
  2021-08-27 12:44             ` Ankur Saini
@ 2021-08-27 12:58               ` Martin Liška
  2021-09-20 12:36                 ` Ankur Saini
  0 siblings, 1 reply; 9+ messages in thread
From: Martin Liška @ 2021-08-27 12:58 UTC (permalink / raw)
  To: Ankur Saini, David Malcolm; +Cc: gcc Patches

On 8/27/21 14:44, Ankur Saini wrote:
> While working on patch to convert most of the 8 whitespace characters to tabs in the analyzer’s source, I see this weird behaviour where at some places indentation looks weird when viewed in patch file ( generated via “git format-patch” )  but looks alright when viewed in text editor.

Yes, I can confirm the patch fixed the white spaces, you can verify that with:
$ git diff | ./contrib/check_GNU_style.py -
...

Note there are some other formatting issues reported by the script.

Martin

> 
> 
> Here is the current patch file, Can someone look at this and see if they also experience the same ?
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] analyzer: Impose recursion limit on indirect calls.
  2021-08-27 12:58               ` Martin Liška
@ 2021-09-20 12:36                 ` Ankur Saini
  0 siblings, 0 replies; 9+ messages in thread
From: Ankur Saini @ 2021-09-20 12:36 UTC (permalink / raw)
  To: Martin Liška; +Cc: David Malcolm, gcc Patches

[ Sorry for very late response ]

> On 27-Aug-2021, at 6:28 PM, Martin Liška <mliska@suse.cz> wrote:
> 
> On 8/27/21 14:44, Ankur Saini wrote:
>> While working on patch to convert most of the 8 whitespace characters to tabs in the analyzer’s source, I see this weird behaviour where at some places indentation looks weird when viewed in patch file ( generated via “git format-patch” )  but looks alright when viewed in text editor.
> 
> Yes, I can confirm the patch fixed the white spaces, you can verify that with:
> $ git diff | ./contrib/check_GNU_style.py -
> ...
> 
> Note there are some other formatting issues reported by the script.

Yes, I see that too, and have fixed most of them 

Unfortunately, I can’t seem to find a way to avoid the following report.
---
=== ERROR type #1: lines should not exceed 80 characters (1 error(s)) ===
gcc/analyzer/diagnostic-manager.cc:2152:80:                        = cg_superedge.map_expr_from_caller_to_callee (caller_var,
—

Also, does these kinds of patches require some special changelog or should I proceed in usual fashion ?

==
Here is updated patch : 

==

Thanks
- Ankur 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-09-20 12:36 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-25  8:09 [PATCH] analyzer: Impose recursion limit on indirect calls Ankur Saini
2021-08-25 13:22 ` David Malcolm
2021-08-25 13:54   ` Martin Liška
     [not found]     ` <9C51062C-3457-4345-9BDC-20F7FE4BFA1E@gmail.com>
2021-08-25 15:05       ` David Malcolm
2021-08-25 15:55         ` Ankur Saini
2021-08-25 17:15           ` David Malcolm
2021-08-27 12:44             ` Ankur Saini
2021-08-27 12:58               ` Martin Liška
2021-09-20 12:36                 ` Ankur Saini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).