On Tue, 2020-01-28 at 08:36 +0100, Jakub Jelinek wrote:
> 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.
What's motivating this is that I found a second place in the analyzer
that should be doing this check [1], which means it's needed in three
places in the code (calls.c, and the two places in the analyzer).
Here's an updated version of the patch which puts it inline in the
header. It does change the ordering of the checks in calls.c, in
that the status quo has a very early reject on
IDENTIFIER_LENGTH (name_decl) <= 11
which seems like a micro-optimization for calls.c that would break
things for the general case. I don't know how significant moving that
later is.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
OK for master? Or, given it's stage 4, should I simply add the new
helper function to the analyzer and leave a copy in calls.c, as a
way of fixing the analyzer bug w/o touching calls.c?
Thanks
Dave
[1] in function_set which is used e.g. for capturing
"all known async-signal-unsafe functions".
> 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
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.
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 (special_function_p): Split out the check for DECL_NAME
being non-NULL and fndecl being extern at file scope into a
new maybe_special_function_p and call it. Drop check for fndecl
being non-NULL that was after a usage of DECL_NAME (fndecl).
* tree.h (maybe_special_function_p): New inline function.
---
gcc/analyzer/analyzer.cc | 10 +-
gcc/analyzer/function-set.cc | 2 ++
gcc/calls.c | 14 ++
gcc/tree.h | 25 +
4 files changed, 30 insertions(+), 21 deletions(-)
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..d1c53171176 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -586,18 +586,8 @@ special_function_p (const_tree fndecl, int flags)
{
tree
