diff mbox series

calls.c: refactor special_function_p for use by analyzer

Message ID 20200128020937.18455-1-dmalcolm@redhat.com
State New
Headers show
Series calls.c: refactor special_function_p for use by analyzer | expand

Commit Message

David Malcolm Jan. 28, 2020, 2:09 a.m. UTC 
On Wed, 2020-01-22 at 20:40 +0100, Jakub Jelinek wrote:
> On Wed, Jan 22, 2020 at 02:35:13PM -0500, David Malcolm wrote:
> > PR analyzer/93316 reports various testsuite failures where I
> > accidentally relied on properties of x86_64-pc-linux-gnu.
> > 
> > The following patch fixes them on sparc-sun-solaris2.11 (gcc211 in
> > the
> > GCC compile farm), and, I hope, the other configurations showing
> > failures.
> > 
> > There may still be other failures for pattern-test-2.c, which I'm
> > tracking separately as PR analyzer/93291.
> > 
> > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
> > tested on stage 1 on sparc-sun-solaris2.11.
> > 
> > gcc/analyzer/ChangeLog:
> > 	PR analyzer/93316
> > 	* analyzer.cc (is_setjmp_call_p): Check for "setjmp" as well as
> > 	"_setjmp".
> 
> Please see calls.c (special_function_p), you should treat certainly
> also sigsetjmp as a setjmp call, and similarly to special_function_p,
> skip over _ or __ prefixes before the setjmp or sigsetjmp name.
> Similarly for longjmp/siglongjmp.
> 
> 	Jakub

This patch refactors some code in special_function_p that checks for
the function being sane to match by name, splitting it out into a new
maybe_special_function_p, and using it it two places in the analyzer.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
OK for master?

Thanks
Dave

gcc/analyzer/ChangeLog:
	* analyzer.cc (is_named_call_p): Replace tests for fndecl being
	extern at file scope and having a non-NULL DECL_NAME with a call
	to maybe_special_function_p.
	* function-set.cc (function_set::contains_decl_p): Add call to
	maybe_special_function_p.

gcc/ChangeLog:
	* calls.c (maybe_special_function_p): New function, splitting out
	the check for DECL_NAME being non-NULL and fndecl being extern at
	file scope from...
	(special_function_p): ...here.  Drop check for fndecl being
	non-NULL that was after a usage of DECL_NAME (fndecl).
	* tree.h (maybe_special_function_p): New decl.
---
 gcc/analyzer/analyzer.cc     | 10 +--------
 gcc/analyzer/function-set.cc |  2 ++
 gcc/calls.c                  | 40 +++++++++++++++++++++++++-----------
 gcc/tree.h                   |  2 ++
 4 files changed, 33 insertions(+), 21 deletions(-)

Comments

Jakub Jelinek Jan. 28, 2020, 7:36 a.m. UTC | #1 
On Mon, Jan 27, 2020 at 09:09:37PM -0500, David Malcolm wrote:
> > Please see calls.c (special_function_p), you should treat certainly
> > also sigsetjmp as a setjmp call, and similarly to special_function_p,
> > skip over _ or __ prefixes before the setjmp or sigsetjmp name.
> > Similarly for longjmp/siglongjmp.
> 
> This patch refactors some code in special_function_p that checks for
> the function being sane to match by name, splitting it out into a new
> maybe_special_function_p, and using it it two places in the analyzer.
> 
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu;
> OK for master?

Not sure it is worth it to factor out the
      DECL_NAME (fndecl)
      && (DECL_CONTEXT (fndecl) == NULL_TREE
          || TREE_CODE (DECL_CONTEXT (fndecl)) == TRANSLATION_UNIT_DECL)
      && TREE_PUBLIC (fndecl)
check, that seems like simple enough that it could be duplicated.
And, even if there is a strong reason not to, at least it ought to be
defined inline in the header, not everyone will use LTO and without LTO it
will need to be an out of line call.
Ack on removing the fndecl && check from special_function_p, the callers
ensure it is non-NULL already, and even if they didn't, after the if (fndecl
&& ...) guarded if there is unconditional dereferencing of fndecl.

	Jakub
diff mbox series

Patch

diff --git a/gcc/analyzer/analyzer.cc b/gcc/analyzer/analyzer.cc
index 1b5e4c9ecf8..5cf745ea632 100644
--- a/gcc/analyzer/analyzer.cc
+++ b/gcc/analyzer/analyzer.cc
@@ -65,18 +65,10 @@  is_named_call_p (tree fndecl, const char *funcname)
   gcc_assert (fndecl);
   gcc_assert (funcname);
 
-  /* Exclude functions not at the file scope, or not `extern',
-     since they are not the magic functions we would otherwise
-     think they are.  */
-  if (!((DECL_CONTEXT (fndecl) == NULL_TREE
-	 || TREE_CODE (DECL_CONTEXT (fndecl)) == TRANSLATION_UNIT_DECL)
-	&& TREE_PUBLIC (fndecl)))
+  if (!maybe_special_function_p (fndecl))
     return false;
 
   tree identifier = DECL_NAME (fndecl);
-  if (identifier == NULL)
-    return false;
-
   const char *name = IDENTIFIER_POINTER (identifier);
   const char *tname = name;
 
diff --git a/gcc/analyzer/function-set.cc b/gcc/analyzer/function-set.cc
index 6ed15ae95ad..1b6b5d9f9c1 100644
--- a/gcc/analyzer/function-set.cc
+++ b/gcc/analyzer/function-set.cc
@@ -59,6 +59,8 @@  bool
 function_set::contains_decl_p (tree fndecl) const
 {
   gcc_assert (fndecl && DECL_P (fndecl));
+  if (!maybe_special_function_p (fndecl))
+    return false;
   return contains_name_p (IDENTIFIER_POINTER (DECL_NAME (fndecl)));
 }
 
diff --git a/gcc/calls.c b/gcc/calls.c
index 1336f49ea5e..447a36c3707 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -572,22 +572,18 @@  emit_call_1 (rtx funexp, tree fntree ATTRIBUTE_UNUSED, tree fndecl ATTRIBUTE_UNU
     anti_adjust_stack (gen_int_mode (n_popped, Pmode));
 }
 
-/* Determine if the function identified by FNDECL is one with
-   special properties we wish to know about.  Modify FLAGS accordingly.
-
-   For example, if the function might return more than one time (setjmp), then
-   set ECF_RETURNS_TWICE.
+/* Determine if the function identified by FNDECL is one that
+   makes sense to match by name, for those places where we detect
+   "magic" functions by name.
 
-   Set ECF_MAY_BE_ALLOCA for any memory allocation function that might allocate
-   space from the stack such as alloca.  */
+   Return true if FNDECL has a name and is an extern fndecl at file scope.
+   FNDECL must be a non-NULL decl.  */
 
-static int
-special_function_p (const_tree fndecl, int flags)
+bool
+maybe_special_function_p (const_tree fndecl)
 {
   tree name_decl = DECL_NAME (fndecl);
-
-  if (fndecl && name_decl
-      && IDENTIFIER_LENGTH (name_decl) <= 11
+  if (name_decl
       /* Exclude functions not at the file scope, or not `extern',
 	 since they are not the magic functions we would otherwise
 	 think they are.
@@ -598,6 +594,26 @@  special_function_p (const_tree fndecl, int flags)
       && (DECL_CONTEXT (fndecl) == NULL_TREE
 	  || TREE_CODE (DECL_CONTEXT (fndecl)) == TRANSLATION_UNIT_DECL)
       && TREE_PUBLIC (fndecl))
+    return true;
+  return false;
+}
+
+/* Determine if the function identified by FNDECL is one with
+   special properties we wish to know about.  Modify FLAGS accordingly.
+
+   For example, if the function might return more than one time (setjmp), then
+   set ECF_RETURNS_TWICE.
+
+   Set ECF_MAY_BE_ALLOCA for any memory allocation function that might allocate
+   space from the stack such as alloca.  */
+
+static int
+special_function_p (const_tree fndecl, int flags)
+{
+  tree name_decl = DECL_NAME (fndecl);
+
+  if (maybe_special_function_p (fndecl)
+      && IDENTIFIER_LENGTH (name_decl) <= 11)
     {
       const char *name = IDENTIFIER_POINTER (name_decl);
       const char *tname = name;
diff --git a/gcc/tree.h b/gcc/tree.h
index 93422206b63..b7db69267c2 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -5611,6 +5611,8 @@  builtin_decl_declared_p (enum built_in_function fncode)
 	  && builtin_info[uns_fncode].declared_p);
 }
 
+extern bool maybe_special_function_p (const_tree fndecl);
+
 /* Return true if T (assumed to be a DECL) is a global variable.
    A variable is considered global if its storage is not automatic.  */