[RFC] analyzer: Add exit, and _exit replacement, to sm-signal.

[Mark Wielaard]

On Mon, May 18, 2020 at 05:24:58PM +0200, Mark Wielaard wrote:
> On Mon, May 18, 2020 at 11:09:18AM -0400, David Malcolm wrote:
> > Overall, I like it, but it looks like there's a problem with the
> > location of the fix-it hint: it looks like it might be replacing the
> > whole of the underlined section of the call, argument, parentheses and
> > all, with "_exit", rather than just the "exit" token.  I wonder if the
> > location of that token is still available in the middle-end by the time
> > the analyzer runs.
> > 
> > What does -fdiagnostics-generate-patch emit?
> 
> --- bzip2.c
> +++ bzip2.c
> @@ -871,7 +871,7 @@
>        terribly wrong. Trying to clean up might fail spectacularly. */
>  
>     if (opMode == OM_Z) setExit(3); else setExit(2);
> -   exit(exitValue);
> +   _exit;
>  }
> 
> Hmmm, back to the drawing board it seems.

It seems it is impossible to untangle the gimple call. We do have the
function decl, but this is associated with the declaration of the
function. Since we know the call starts with a known symbol identifier
(that is all on one line), it seems we should be able to truncate the
source_range for the call location to just that prefix. But it is
incredibly hard to manipulate locations. Neither seems it simple to
get the actual text of the location or a range to sanity check what we
are doing.

So I am afraid we have to drop the actual fixit. But we can still add
the note itself. The diagnostic now looks as follows:

/opt/local/install/gcc/bin/gcc -g -O2 -fanalyzer -fanalyzer-fine-grained -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);
      |    ^~~~~~~~~~~~~~~
  ‘main’: events 1-2
    |
    | 1784 | IntNative main ( IntNative argc, Char *argv[] )
    |      |           ^~~~
    |      |           |
    |      |           (1) entry to ‘main’
    |......
    | 1816 |    signal (SIGSEGV, mySIGSEGVorSIGBUScatcher);
    |      |    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |    |
    |      |    (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
           |
bzip2.c:874:4: note: ‘_exit’ is a possible signal-safe alternative for ‘exit’
  874 |    exit(exitValue);
      |    ^~~~~~~~~~~~~~~

What do you think of the attached patch?

Thanks,

Mark
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-analyzer-Add-exit-and-_exit-replacement-to-sm-signal.patch
Type: text/x-diff
Size: 4173 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20200519/e7ee9ae8/attachment.bin>