From: David Malcolm <dmalcolm@redhat.com>
To: Szabolcs Nagy <szabolcs.nagy@arm.com>
Cc: Mir Immad <mirimnan017@gmail.com>,
gcc@gcc.gnu.org, libc-alpha@sourceware.org
Subject: Re: Adding file descriptor attribute(s) to gcc and glibc
Date: Wed, 13 Jul 2022 08:57:20 -0400 [thread overview]
Message-ID: <ed74e45beca885b5a681facb73d3b2794a2f1806.camel@redhat.com> (raw)
In-Reply-To: <Ys6ENUYqC2Qm5MUL@arm.com>
On Wed, 2022-07-13 at 09:37 +0100, Szabolcs Nagy wrote:
> The 07/12/2022 18:25, David Malcolm via Libc-alpha wrote:
> > On Tue, 2022-07-12 at 18:16 -0400, David Malcolm wrote:
> > > On Tue, 2022-07-12 at 23:03 +0530, Mir Immad wrote:
> > > GCC's attribute syntax here:
> > > https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html
> > > allows for a parenthesized list of parameters for the attribute,
> > > which
> > > can be:
> > > (a) An identifier
> > > (b) An identifier followed by a comma and a non-empty comma-
> > > separated
> > > list of expressions
> > > (c) A possibly empty comma-separated list of expressions
> > >
> > > I'd hoped to have an argument number, with an optional extra param
> > > describing the direction of the access, but syntax (b) puts the
> > > identifier first, alas.
> > >
> > > Here's one possible way of doing it with a single attribute, via
> > > syntax
> > > (b):
> > > e.g.
> > > __attribute__((fd_argument (access, 1))
> > > __attribute__((fd_argument (read, 1))
> > > __attribute__((fd_argument (write, 1))
> > >
> > > meaning that argument 1 of the function is expected to be an open
> > > file-
> > > descriptor, and that it must be possible to read from/write to that
> > > fd
> > > for cases 2 and 3.
> > >
> > > Here are some possible examples of how glibc might use this syntax:
> > >
> > > int dup (int oldfd)
> > > __attribute((fd_argument (access, 1));
> > >
> > > int ftruncate (int fd, off_t length)
> > > __attribute((fd_argument (access, 1));
> > >
> > > ssize_t pread(int fd, void *buf, size_t count, off_t offset)
> > > __attribute((fd_argument (read, 1));
> > >
> > > ssize_t pwrite(int fd, const void *buf, size_t count,
> > > off_t offset);
> > > __attribute((fd_argument (write, 1));
> > >
> > > ...but as I said, I'm most interested in input from glibc
> > > developers on
> > > this.
>
> note that glibc headers have to be namespace clean so it
> would be more like
>
> __attribute__((__fd_argument (__access, 1)))
> __attribute__((__fd_argument (__read, 1)))
> __attribute__((__fd_argument (__write, 1)))
>
> so it would be even shorter to write
>
> __attribute__((__fd_argument_access (1)))
> __attribute__((__fd_argument_read (1)))
> __attribute__((__fd_argument_write (1)))
As I understand it, you'd use a macro for this, but this made me think
of the following attributes that GCC could provide:
__attribute__ ((fd_arg(N)))
__attribute__ ((fd_arg_read(N)))
__attribute__ ((fd_arg_write(N)))
(since GCC already has "__attribute__((format_arg(N)))")
It looks like you define your attribute macros in:
https://sourceware.org/git/?p=glibc.git;a=blob_plain;f=misc/sys/cdefs.h;hb=HEAD
which presumably could be extended to add something like:
#if __GNUC_PREREQ (13, 0)
# define __attr_fd_arg(argno) __attribute__ ((fd_arg(argno)))
# define __attr_fd_arg_read(argno) __attribute__ ((fd_arg_read(argno)))
# define __attr_fd_arg_write(argno) __attribute__ ((fd_arg_write(argno)))
#else
# define __attr_fd_arg(argno)
# define __attr_fd_arg_read(argno)
# define __attr_fd_arg_write(argno)
#endif
if I've got my syntax correct.
(Or maybe "readable" and "writable"?)
>
> I just realized that the attribute could accept both the single integer
> argument number (syntax (c)) for the "don't care about access
> direction" case, or the ({read|write}, N) of syntax (b) above, giving
> e.g.:
>
> int dup (int oldfd)
> __attribute((fd_argument (1));
>
> int ftruncate (int fd, off_t length)
> __attribute((fd_argument (1));
>
> ssize_t pread(int fd, void *buf, size_t count, off_t offset)
> __attribute((fd_argument (read, 1));
>
> ssize_t pwrite(int fd, const void *buf, size_t count,
> off_t offset);
> __attribute((fd_argument (write, 1));
>
> for the above examples.
>
> How does that look?
> Dave
i think fd in ftruncate should be open for writing.
Agreed.
So with the above macros, this might look like:
int dup (int oldfd)
__attr_fd_arg(1);
int ftruncate (int fd, off_t length)
__attr_fd_arg_write(1);
ssize_t pread(int fd, void *buf, size_t count, off_t offset)
__attr_fd_arg_read(1);
ssize_t pwrite(int fd, const void *buf, size_t count,
off_t offset);
__attr_fd_arg_write(1);
to be honest, i'd expect interesting fd bugs to be
dynamic and not easy to statically analyze.
the use-after-unchecked-open maybe useful. i would
not expect the access direction to catch many bugs.
One goal of -fanalyzer is to help detect problems as code is written,
before it ever leaves the developer's workstation. So for instance it
might save a few seconds helping catch silly bugs where a developer is
working with two different FDs and gets the read and write FDs the
wrong way around.
Dave
prev parent reply other threads:[~2022-07-13 12:57 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-12 17:31 [PATCH] filedescriptor attribute Immad Mir
2022-07-12 17:33 ` Mir Immad
2022-07-12 22:16 ` Adding file descriptor attribute(s) to gcc and glibc David Malcolm
2022-07-12 22:25 ` David Malcolm
2022-07-13 8:37 ` Szabolcs Nagy
2022-07-13 8:46 ` Andreas Schwab
2022-07-13 12:05 ` Florian Weimer
2022-07-13 13:33 ` David Malcolm
2022-07-13 14:01 ` Florian Weimer
2022-07-13 16:55 ` David Malcolm
2022-07-14 8:30 ` Szabolcs Nagy
2022-07-14 15:22 ` David Malcolm
2022-07-14 17:07 ` Paul Eggert
2022-07-13 16:56 ` Mir Immad
2022-07-13 19:29 ` David Malcolm
2022-07-13 12:57 ` 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=ed74e45beca885b5a681facb73d3b2794a2f1806.camel@redhat.com \
--to=dmalcolm@redhat.com \
--cc=gcc@gcc.gnu.org \
--cc=libc-alpha@sourceware.org \
--cc=mirimnan017@gmail.com \
--cc=szabolcs.nagy@arm.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).