Hi,
While trying out -fanalyzer on the bzip2 source code I noticed that it
did warn about some unsafe calls in the signal handler, but din't warn
about the exit call:
https://sourceware.org/pipermail/bzip2-devel/2020q2/000107.html
It was easy to add exit to the async_signal_unsafe_fns, but since
there is a signal safe _exit call (which is what you should use in a
signal handler, so no unsafe calls are made, like to the at_exit
handlers) I also wanted to add a fixit hint.
The fixit hint is emitted, but it is somewhat hard to see. Is there a
better way to do this for analyzer warnings so that it is a bit more
prominent?
This is how it currently looks:
/opt/local/gcc/bin/gcc -g -O2 -fanalyzer -c bzip2.c
bzip2.c: In function ‘mySIGSEGVorSIGBUScatcher’:
bzip2.c:874:4: warning: call to ‘exit’ from within signal handler [CWE-479]
[-Wanalyzer-unsafe-call-within-signal-handler]
874 | exit(exitValue);
| ^~~~~~~~~~~~~~~
| _exit
‘main’: events 1-2
|
| 1784 | IntNative main ( IntNative argc, Char *argv[] )
| | ^~~~
| | |
| | (1) entry to ‘main’
|......
| 1800 | smallMode = False;
| | ~~~~~~~~~
| | |
| | (2) registering ‘mySIGSEGVorSIGBUScatcher’ as signal handler
|
event 3
|
|cc1:
| (3): later on, when the signal is delivered to the process
|
+--> ‘mySIGSEGVorSIGBUScatcher’: events 4-5
|
| 816 | void mySIGSEGVorSIGBUScatcher ( IntNative n )
| | ^~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (4) entry to ‘mySIGSEGVorSIGBUScatcher’
|......
| 874 | exit(exitValue);
| | ~~~~~~~~~~~~~~~
| | |
| | (5) call to ‘exit’ from within signal handler
|
Thanks,
Mark
---
gcc/analyzer/sm-signal.cc | 34 +++++++++++++++++++++++++++++-----
1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/gcc/analyzer/sm-signal.cc b/gcc/analyzer/sm-signal.cc
index 5935e229e77c..99e87706af63 100644
--- a/gcc/analyzer/sm-signal.cc
+++ b/gcc/analyzer/sm-signal.cc
@@ -108,8 +108,9 @@ class signal_unsafe_call
{
public:
signal_unsafe_call (const signal_state_machine &sm, const gcall *unsafe_call,
- tree unsafe_fndecl)
- : m_sm (sm), m_unsafe_call (unsafe_call), m_unsafe_fndecl (unsafe_fndecl)
+ tree unsafe_fndecl, const char *replacement)
+ : m_sm (sm), m_unsafe_call (unsafe_call), m_unsafe_fndecl (unsafe_fndecl),
+ m_replacement (replacement)
{
gcc_assert (m_unsafe_fndecl);
}
@@ -126,6 +127,8 @@ public:
diagnostic_metadata m;
/* CWE-479: Signal Handler Use of a Non-reentrant Function. */
m.add_cwe (479);
+ if (m_replacement != NULL)
+ rich_loc->add_fixit_replace (m_replacement);
return warning_meta (rich_loc, m,
OPT_Wanalyzer_unsafe_call_within_signal_handler,
"call to %qD from within signal handler",
@@ -156,6 +159,7 @@ private:
const signal_state_machine &m_sm;
const gcall *m_unsafe_call;
tree m_unsafe_fndecl;
+ const char *m_replacement;
};
/* signal_state_machine's ctor. */
@@ -259,6 +263,7 @@ get_async_signal_unsafe_fns ()
// TODO: populate this list more fully
static const char * const async_signal_unsafe_fns[] = {
/* This array must be kept sorted. */
+ "exit",
"fprintf",
"free",
"malloc",
@@ -286,6 +291,20 @@ signal_unsafe_p (tree fndecl)
return fs.contains_decl_p (fndecl);
}
+/* Returns a replacement function as text if it exists. Currently
+ only "exit" has a signal-safe replacement "_exit", which does
+ slightly less, but can be used in a signal handler. */
+const char *
+replacement_fn (tree fndecl)
+{
+ gcc_assert (fndecl && DECL_P (fndecl));
+
+ if (strcmp ("exit", IDENTIFIER_POINTER (DECL_NAME (fndecl))) == 0)
+ return "_exit";
+
+ return NULL;
+}
+
/* Implementation of state_machine::on_stmt vfunc for signal_state_machine. */
bool
@@ -315,9 +334,14 @@ signal_state_machine::on_stmt (sm_context *sm_ctxt,
if (const gcall *call = dyn_cast <const gcall *> (stmt))
if (tree callee_fndecl = sm_ctxt->get_fndecl_for_call (call))
if (signal_unsafe_p (callee_fndecl))
- sm_ctxt->warn_for_state (node, stmt, NULL_TREE, m_in_signal_handler,
- new signal_unsafe_call (*this, call,
- callee_fndecl));
+ {
+ const char *replace = replacement_fn (callee_fndecl);
+ sm_ctxt->warn_for_state (node, stmt, NULL_TREE,
+ m_in_signal_handler,
+ new signal_unsafe_call (*this, call,
+ callee_fndecl,
+ replace));
+ }
}
return false;
--
2.20.1
