From 2221fb6f668a7edc8b8aad69772907aeabbbb0be Mon Sep 17 00:00:00 2001 From: Mark Wielaard 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/ChangeLog | 7 ++++ gcc/analyzer/sm-signal.cc | 42 +++++++++++++++++++-- gcc/testsuite/ChangeLog | 4 ++ gcc/testsuite/gcc.dg/analyzer/signal-exit.c | 23 +++++++++++ 4 files changed, 72 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-exit.c diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog index 5cd736385aa..d2c440a08e4 100644 --- a/gcc/analyzer/ChangeLog +++ b/gcc/analyzer/ChangeLog @@ -1,3 +1,10 @@ +2020-05-22 Mark Wielaard + + * 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". + 2020-04-28 David Malcolm PR analyzer/94816 diff --git a/gcc/analyzer/sm-signal.cc b/gcc/analyzer/sm-signal.cc index 5935e229e77..c0020321071 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/ChangeLog b/gcc/testsuite/ChangeLog index 1dbdc389ab9..bedaf9aa735 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,7 @@ +2020-05-22 Mark Wielaard + + * gcc.dg/analyzer/signal-exit.c: New testcase. + 2020-05-22 Uroš Bizjak PR target/95255 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 00000000000..a567124c7d4 --- /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 +#include + +extern void body_of_program(void); + +static void handler(int signum) +{ + exit(1); /* { dg-warning "call to 'exit' from within signal handler" "warning" } */ + /* { dg-message "note: '_exit' is a possible signal-safe alternative for 'exit'" "replacement note" { target *-*-* } .-1 } */ +} + +int main(int argc, const char *argv) +{ + signal(SIGINT, handler); /* { dg-message "registering 'handler' as signal handler" } */ + + body_of_program(); + + return 0; +} -- 2.30.2