Re: [PATCH] PR 106003

[David Malcolm <dmalcolm@redhat.com>]

On Sat, 2022-07-02 at 19:34 +0530, Mir Immad wrote:
> From 62b7b7736975172f03b30783436fbc9217324223 Mon Sep 17 00:00:00 2001
> From: mir <mirimmad17@gmail.com>
> Date: Sat, 2 Jul 2022 15:04:37 +0530
> Subject: [PATCH] analyzer: implement five new warnings for misuse of
> POSIX
>  file descriptor APIs [PR106003].
> 
> This patch adds a new state machine to the analyzer for checking usage
> of
> POSIX file descriptor
> APIs with five new warnings.
> 
> It adds:
> - check for FD leaks (CWE 775).
> - check for double "close" of a FD (CWE-1341).
> - check for read/write of a closed file descriptor.
> - check whether a file descriptor was used without being checked for
> validity.
> - check for read/write of a descriptor opened for just writing/reading.
> 
> gcc/ChangeLog:
> PR analyzer/106003
> * Makefile.in (ANALYZER_OBJS): Add sm-fd.o.
> * doc/invoke.texi:  Add -Wanalyzer-fd-double-close, -Wanalyzer-fd-leak,
> -Wanalyzer-fd-access-mode-mismatch, -Wanalyzer-fd-use-without-check,
> -Wanalyzer-fd-use-after-close.
> 
> gcc/analyzer/ChangeLog:
> PR analyzer/106003
> * analyzer.opt (Wanalyzer-fd-leak): New option.
> (Wanalyzer-fd-access-mode-mismatch): New option.
> (Wanalyzer-fd-use-without-check): New option.
> (Wanalyzer-fd-double-close): New option.
> (Wanalyzer-fd-use-after-close): New option.
> * sm.h (make_fd_state_machine): New decl.
> * sm.cc (make_checkers): Call make_fd_state_machine.
> * sm-fd.cc: New file.
> 
> gcc/testsuite/ChangeLog:
> PR analyzer/106003
> * gcc.dg/analyzer/fd-1.c: New test.
> * gcc.dg/analyzer/fd-2.c: New test.
> * gcc.dg/analyzer/fd-3.c: New test.
> * gcc.dg/analyzer/fd-4.c: New test.

[...snip...]

Hi Immad.

Thanks for the updated patch.

For everyone else, we've been discussing this patch off-list.  We've
had some issues with gmail mangling patches; FWIW a pristine version of
the patch can be seen at:
  https://mirimmad.github.io/patch-02-07.txt

As discussed off-list, you've successfully bootstrapped this patch and
run the testsuite without regressions (and a bunch of extra PASSes), so
this patch is ready for you to push it to the "master" git branch (aka
trunk).  Please go ahead with that (or let me know if you need help
[1]).

Note that Tim's first analyzer patch is also ready to push, so there's
a chance that your patches might conflict with each other (though I
think you're touching different areas of the analyzer, so I'm hoping
that won't happen).

There's plenty of scope for followups, such as adding attributes for
parameters that expect an open file-descriptor, or for handling socket
APIs, etc.  Also, Murphy's Law means that there's sure to be at least
something we missed in review :/

Let's move followup patches to the gcc-patches mailing list, rather
than the "gcc" list.

Thanks
Dave

[1] though I'll only be checking email intermittently this weekend and
on Monday (which is a holiday here in the USA).