Hi,
On Mon, May 18, 2020 at 07:30:58PM -0400, Marek Polacek wrote:
> > + /* 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 *
> > + get_replacement_fn ()
> > + {
> > + gcc_assert (m_unsafe_fndecl && DECL_P (m_unsafe_fndecl));
> > +
> > + if (strcmp ("exit",
> > + IDENTIFIER_POINTER (DECL_NAME (m_unsafe_fndecl))) == 0)
>
> Instead of strcmp, you should be able to use id_equal here, making this a bit
> simpler.
That does make it a little easier to read.
How about the attached?
Thanks,
Mark
>From 2406c5a70b407052bc4099a80ecddd4ed3d62385 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <m...@klomp.org>
Date: Sun, 17 May 2020 23:50:41 +0200
Subject: [PATCH] analyzer: Add exit, and _exit replacement, to sm-signal.
Warn about using exit in signal handler and suggest _exit as alternative.
gcc/analyzer/ChangeLog:
* sm-signal.cc(signal_unsafe_call::emit): Possibly add
gcc_rich_location note for replacement.
(signal_unsafe_call::get_replacement_fn): New private function.
(get_async_signal_unsafe_fns): Add "exit".
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/signal-exit.c: New testcase.
---
gcc/analyzer/sm-signal.cc | 42 +++++++++++++++++++--
gcc/testsuite/gcc.dg/analyzer/signal-exit.c | 23 +++++++++++
2 files changed, 61 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-exit.c
diff --git a/gcc/analyzer/sm-signal.cc b/gcc/analyzer/sm-signal.cc
index 5935e229e77c..c0020321071e 100644
--- a/gcc/analyzer/sm-signal.cc
+++ b/gcc/analyzer/sm-signal.cc
@@ -123,13 +123,32 @@ public:
bool emit (rich_location *rich_loc) FINAL OVERRIDE
{
+ auto_diagnostic_group d;
diagnostic_metadata m;
/* CWE-479: Signal Handler Use of a Non-reentrant Function. */
m.add_cwe (479);
- return warning_meta (rich_loc, m,
- OPT_Wanalyzer_unsafe_call_within_signal_handler,
- "call to %qD from within signal handler",
- m_unsafe_fndecl);
+ if (warning_meta (rich_loc, m,
+ OPT_Wanalyzer_unsafe_call_within_signal_handler,
+ "call to %qD from within signal handler",
+ m_unsafe_fndecl))
+ {
+ /* If we know a possible alternative function, add a note
+ suggesting the replacement. */
+ if (const char *replacement = get_replacement_fn ())
+ {
+ location_t note_loc = gimple_location (m_unsafe_call);
+ /* It would be nice to add a fixit, but the gimple call
+ location covers the whole call expression. It isn't
+ currently possible to cut this down to just the call
+ symbol. So the fixit would replace too much.
+ note_rich_loc.add_fixit_replace (replacement); */
+ inform (note_loc,
+ "%qs is a possible signal-safe alternative for %qD",
+ replacement, m_unsafe_fndecl);
+ }
+ return true;
+ }
+ return false;
}
label_text describe_state_change (const evdesc::state_change &change)
@@ -156,6 +175,20 @@ private:
const signal_state_machine &m_sm;
const gcall *m_unsafe_call;
tree m_unsafe_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 *
+ get_replacement_fn ()
+ {
+ gcc_assert (m_unsafe_fndecl && DECL_P (m_unsafe_fndecl));
+
+ if (id_equal ("exit", DECL_NAME (m_unsafe_fndecl)))
+ return "_exit";
+
+ return NULL;
+ }
};
/* signal_state_machine's ctor. */
@@ -259,6 +292,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",
diff --git a/gcc/testsuite/gcc.dg/analyzer/signal-exit.c b/gcc/testsuite/gcc.dg/analyzer/signal-exit.c
new file mode 100644
index 000000000000..ed1583d4a920
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/signal-exit.c
@@ -0,0 +1,23 @@
+/* Example of a bad call within a signal handler with replacement
+ alternative. 'handler' calls 'exit', and 'exit' is not allowed
+ from a signal handler. But '_exit' is allowed. */
+
+#include <signal.h>
+#include <stdlib.h>
+
+extern void body_of_program(void);
+
+static void handler(int signum)
+{
+ exit(1); /* { dg-warning "call to 'exit' from within signal handler" } */
+ /* { dg-message "note: '_exit' is a possible signal-safe alternative for 'exit'" "" { target *-*-* } 12 } */
+}
+
+int main(int argc, const char *argv)
+{
+ signal(SIGINT, handler); /* { dg-message "registering 'handler' as signal handler" } */
+
+ body_of_program();
+
+ return 0;
+}
--
2.20.1
