public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
From: David Malcolm <dmalcolm@redhat.com>
To: Jay K <jayk123@hotmail.com>, gcc <gcc@gcc.gnu.org>,
	"mirimmad17@gmail.com" <mirimmad17@gmail.com>
Subject: Re: analyzer: implement five new warnings for misuse of POSIX
Date: Sun, 03 Jul 2022 15:47:12 -0400	[thread overview]
Message-ID: <d52b1c5ea3038799e22386f9ed28b04a7b8654a7.camel@redhat.com> (raw)
In-Reply-To: <MWHPR1401MB1951899916C1EEBFF16A23CBE6BF9@MWHPR1401MB1951.namprd14.prod.outlook.com>

On Sun, 2022-07-03 at 02:46 +0000, Jay K wrote:
>  > check for double "close" of a FD (CWE-1341).
>  > check for read/write of a closed file descriptor 
>  
>  These sound good but kinda non general or incomplete to me. 
>  I mean, isn't the "right" thing, to disallow passing 
>  a closed fd to "almost any" function?  
> 
>  But I realize "almost any" is difficult to pin down. 
>   fd = open(); 
>   close(fd); 
>   printf("%d", fd);  
> 
> is often ok (assuming nobody reads the output, string to int,
> back to close/read/write). It is any path leading to,
> a long list, like close, read, write, ioctl, send, recv, etc.
> and I don't know if "path leading to" is possible to model here,
> haven't looked, sorry.

Jay: right now, the only function calls that these FD warnings know
about are open, close, read, and write: the only "use" that -Wanalyzer-
fd-use-after-close currently cares about is "read" and "write" (and
double-close is its own diagnostic).

I put together an example on Compiler Explorer here which tries to show
as many of the new warnings as possible:
  https://godbolt.org/z/rsc51PYoK  [1]

I think we're going to want to add a new function attribute that
signifies
  "this 'int' parameter is expected to be an open file descriptor"
so that e.g. glibc's headers can use the attribute to mark up all the
places expecting such FDs, and thus -fanalyzer can warn about misuses,
lacking checks, etc.

Probably some sort of support for dup, dup2, etc.


Immad: we might want some test coverage that Jay's example doesn't
generate a warning from -fanalyzer.


Hope this makes sense
Dave

[1] I also put in a bounds-check bug in the first "write" call, which
sadly we don't complain about yet.  I've added a note about that to 
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106000




      reply	other threads:[~2022-07-03 19:47 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <mailman.394.1656804030.3030988.gcc-patches@gcc.gnu.org>
2022-07-03  2:46 ` Jay K
2022-07-03 19:47   ` David Malcolm [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d52b1c5ea3038799e22386f9ed28b04a7b8654a7.camel@redhat.com \
    --to=dmalcolm@redhat.com \
    --cc=gcc@gcc.gnu.org \
    --cc=jayk123@hotmail.com \
    --cc=mirimmad17@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).