From: David Malcolm <dmalcolm@redhat.com>
To: Mir Immad <mirimnan017@gmail.com>
Cc: gcc@gcc.gnu.org, libc-alpha@sourceware.org
Subject: Re: Adding file descriptor attribute(s) to gcc and glibc
Date: Tue, 12 Jul 2022 18:25:42 -0400 [thread overview]
Message-ID: <5769682d0d17579cbd72f72a4001bfa8444b80a8.camel@redhat.com> (raw)
In-Reply-To: <260f0b41c663133cea96eb64bd85e8ba16d8a936.camel@redhat.com>
On Tue, 2022-07-12 at 18:16 -0400, David Malcolm wrote:
> On Tue, 2022-07-12 at 23:03 +0530, Mir Immad wrote:
>
> [cross-posting to the glibc development mailing list; updating subject
> accordingly]
>
> > Hi everyone,
>
> Hi Immad, GCC developers, and glibc developers.
>
> glibc developers: Immad is a GSoC student, he's been adding checks for
> file-descriptor-based APIs to gcc's -fanalyzer:
>
> https://gcc.gnu.org/git/?p=gcc.git;a=commitdiff;h=97baacba963c06e3d0e33cde04e7e687671e60e7
>
> >
> > This is an in-progress patch for adding three new function attributes
> > to
> > GCC for static analysis of file descriptor APIs (which is a part of
> > my
> > GSoC
> > project)
>
> As you say, the patch is "in-progress".
>
> Immad and I discussed this off-list and considered a few different
> ideas about this; I'm especially hoping for feedback from glibc
> developers on this idea, as I think the attributes could potentially be
> used in dozens of places in glibc's headers.
>
> >
> > 1) __attribute__((accesses_fd(N)))
> > This attribute expected argument N of a function to be an open file
> > descriptor. (see test_1 of fd-5.c)
>
> The patch is missing a few things, of which I think the most important
> for a discussion is documentation: what do we want these attributes to
> mean?
>
> Presumably: __attribute__((accesses_fd(N))) means something like:
>
> This function attribute documents to human and automated readers of the
> code that the function expects the argument with the given index to be
> a valid, open file-descriptor.
>
> If -fanalyzer is enabled, it will complain with:
> * -Wanalyzer-fd-use-without-check if the given argument is the result
> of an "open" call that hasn't yet been checked for validity
> * -Wanalyzer-fd-use-after-close if the given argument is a closed FD
>
> The argument must be of "int" type.
>
>
> ...or somesuch.
>
> >
> > 2) __attribute__((reads_from_fd(N)))
> > This attribute expects arguments N of a function to not be a write-
> > only
> > FD.
> > (see test_2 of fd-5.c)
>
> Maybe reword to: this is equivalent to "accesses_fd", but with the
> additional check that the file descriptor must not have been opened
> write-only; the analyzer will complain with -Wanalyzer-fd-access-mode-
> mismatch if this is the case.
>
>
> >
> > 3) __attribute__((writes_to_fd(N)))
> > This attribute expects arguments N of a function to not be a read-
> > only
> > FD.
> > (see test_3 of fd-6.c)
>
> As above, but must not have been opened read-only.
>
>
> FWIW I'm not sure whether I prefer having three attributes, or one
> attribute with an optional access direction parameter.
>
> One drawback of the "three attributes" approach is that they're
> alphabetically distant from each other, obscuring their close
> relationship.
>
> 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.
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
>
> Hope this is constructive
> Dave
>
> >
> > Thanks.
> > Immad
> >
> >
> >
> >
> >
> > On Tue, Jul 12, 2022 at 11:02 PM Immad Mir <mirimmad@outlook.com>
> > wrote:
> >
> > > ---
> > > gcc/analyzer/sm-fd.cc | 231
> > > ++++++++++++++++++++++++-
> > > --
> > > gcc/c-family/c-attribs.cc | 63 ++++++++
> > > gcc/testsuite/gcc.dg/analyzer/fd-5.c | 44 +++++
> > > gcc/testsuite/gcc.dg/analyzer/fd-6.c | 4 +
> > > 4 files changed, 322 insertions(+), 20 deletions(-)
> > > create mode 100644 gcc/testsuite/gcc.dg/analyzer/fd-5.c
> > > create mode 100644 gcc/testsuite/gcc.dg/analyzer/fd-6.c
> > >
> > > diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
> > > index 8e4300b06e2..bdc807160cc 100644
> > > --- a/gcc/analyzer/sm-fd.cc
> > > +++ b/gcc/analyzer/sm-fd.cc
> > > @@ -39,10 +39,13 @@ along with GCC; see the file COPYING3. If not
> > > see
> > > #include "analyzer/analyzer-selftests.h"
> > > #include "tristate.h"
> > > #include "selftest.h"
> > > +#include "stringpool.h"
> > > +#include "attribs.h"
> > > #include "analyzer/call-string.h"
> > > #include "analyzer/program-point.h"
> > > #include "analyzer/store.h"
> > > #include "analyzer/region-model.h"
> > > +#include "bitmap.h"
> > >
> > > #if ENABLE_ANALYZER
> > >
> > > @@ -59,6 +62,13 @@ enum access_mode
> > > WRITE_ONLY
> > > };
> > >
> > > +enum fd_access_direction
> > > +{
> > > + DIR_READ_WRITE,
> > > + DIR_READ,
> > > + DIR_WRITE
> > > +};
> > > +
> > > class fd_state_machine : public state_machine
> > > {
> > > public:
> > > @@ -104,6 +114,8 @@ public:
> > > bool is_readonly_fd_p (state_t s) const;
> > > bool is_writeonly_fd_p (state_t s) const;
> > > enum access_mode get_access_mode_from_flag (int flag) const;
> > > + void inform_filedescriptor_attribute (tree callee_fndecl, int
> > > arg,
> > > + fd_access_direction
> > > fd_dir)
> > > const;
> > >
> > > /* State for a constant file descriptor (>= 0) */
> > > state_t m_constant_fd;
> > > @@ -146,7 +158,7 @@ private:
> > > void check_for_open_fd (sm_context *sm_ctxt, const supernode
> > > *node,
> > > const gimple *stmt, const gcall *call,
> > > const tree callee_fndecl,
> > > - enum access_direction access_fn) const;
> > > + enum fd_access_direction access_fn)
> > > const;
> > >
> > > void make_valid_transitions_on_condition (sm_context *sm_ctxt,
> > > const supernode *node,
> > > @@ -156,6 +168,12 @@ private:
> > > const supernode
> > > *node,
> > > const gimple *stmt,
> > > const svalue *lhs)
> > > const;
> > > + bitmap get_fd_attrs (const char *attr_name, tree callee_fndecl)
> > > const;
> > > + void check_for_fd_attrs (sm_context *sm_ctxt, const supernode
> > > *node,
> > > + const gimple *stmt, const gcall *call,
> > > + const tree callee_fndecl, bitmap argmap,
> > > + fd_access_direction fd_attr_access_dir)
> > > const;
> > > +
> > > };
> > >
> > > /* Base diagnostic class relative to fd_state_machine. */
> > > @@ -294,10 +312,10 @@ class fd_access_mode_mismatch : public
> > > fd_diagnostic
> > > {
> > > public:
> > > fd_access_mode_mismatch (const fd_state_machine &sm, tree arg,
> > > - enum access_direction fd_dir,
> > > - const tree callee_fndecl)
> > > + enum fd_access_direction fd_dir,
> > > + const tree callee_fndecl, bool attr,
> > > int
> > > arg_idx)
> > > : fd_diagnostic (sm, arg), m_fd_dir (fd_dir),
> > > - m_callee_fndecl (callee_fndecl)
> > > + m_callee_fndecl (callee_fndecl), m_attr (attr), m_arg_idx
> > > (arg_idx)
> > >
> > > {
> > > }
> > > @@ -317,19 +335,27 @@ public:
> > > bool
> > > emit (rich_location *rich_loc) final override
> > > {
> > > + bool warned;
> > > switch (m_fd_dir)
> > > {
> > > case DIR_READ:
> > > - return warning_at (rich_loc, get_controlling_option (),
> > > + warned = warning_at (rich_loc, get_controlling_option (),
> > > "%qE on %<read-only%> file descriptor
> > > %qE",
> > > m_callee_fndecl, m_arg);
> > > + break;
> > > case DIR_WRITE:
> > > - return warning_at (rich_loc, get_controlling_option (),
> > > + warned = warning_at (rich_loc, get_controlling_option (),
> > > "%qE on %<write-only%> file descriptor
> > > %qE",
> > > m_callee_fndecl, m_arg);
> > > + break;
> > > default:
> > > gcc_unreachable ();
> > > }
> > > + if (warned && m_attr)
> > > + {
> > > + m_sm.inform_filedescriptor_attribute (m_callee_fndecl,
> > > m_arg_idx,
> > > m_fd_dir);
> > > + }
> > > + return warned;
> > > }
> > >
> > > bool
> > > @@ -359,8 +385,10 @@ public:
> > > }
> > >
> > > private:
> > > - enum access_direction m_fd_dir;
> > > + enum fd_access_direction m_fd_dir;
> > > const tree m_callee_fndecl;
> > > + bool m_attr;
> > > + int m_arg_idx;
> > > };
> > >
> > > class double_close : public fd_diagnostic
> > > @@ -422,8 +450,9 @@ class fd_use_after_close : public fd_diagnostic
> > > {
> > > public:
> > > fd_use_after_close (const fd_state_machine &sm, tree arg,
> > > - const tree callee_fndecl)
> > > - : fd_diagnostic (sm, arg), m_callee_fndecl (callee_fndecl)
> > > + const tree callee_fndecl, bool attr, int
> > > arg_idx)
> > > + : fd_diagnostic (sm, arg), m_callee_fndecl (callee_fndecl),
> > > m_attr
> > > (attr),
> > > + m_arg_idx (arg_idx)
> > > {
> > > }
> > >
> > > @@ -439,12 +468,27 @@ public:
> > > return OPT_Wanalyzer_fd_use_after_close;
> > > }
> > >
> > > + bool
> > > + subclass_equal_p (const pending_diagnostic &base_other) const
> > > override
> > > + {
> > > + const fd_use_after_close &sub_other
> > > + = (const fd_use_after_close &)base_other;
> > > + return (same_tree_p (m_arg, sub_other.m_arg)
> > > + && m_callee_fndecl == sub_other.m_callee_fndecl
> > > + && m_attr == sub_other.m_attr
> > > + && m_arg_idx == sub_other.m_arg_idx);
> > > + }
> > > +
> > > bool
> > > emit (rich_location *rich_loc) final override
> > > {
> > > - return warning_at (rich_loc, get_controlling_option (),
> > > + bool warned;
> > > + warned = warning_at (rich_loc, get_controlling_option (),
> > > "%qE on closed file descriptor %qE",
> > > m_callee_fndecl,
> > > m_arg);
> > > + if (warned && m_attr)
> > > + m_sm.inform_filedescriptor_attribute (m_callee_fndecl,
> > > m_arg_idx,
> > > DIR_READ_WRITE);
> > > + return warned;
> > > }
> > >
> > > label_text
> > > @@ -466,17 +510,19 @@ public:
> > > describe_final_event (const evdesc::final_event &ev) final
> > > override
> > > {
> > > if (m_first_close_event.known_p ())
> > > - return ev.formatted_print (
> > > - "%qE on closed file descriptor %qE; %qs was at %@",
> > > m_callee_fndecl,
> > > - m_arg, "close", &m_first_close_event);
> > > - else
> > > - return ev.formatted_print ("%qE on closed file descriptor
> > > %qE",
> > > - m_callee_fndecl, m_arg);
> > > + return ev.formatted_print (
> > > + "%qE on closed file descriptor %qE; %qs was at %@",
> > > m_callee_fndecl,
> > > + m_arg, "close", &m_first_close_event);
> > > + else
> > > + return ev.formatted_print ("%qE on closed file descriptor
> > > %qE",
> > > + m_callee_fndecl, m_arg);
> > > }
> > >
> > > private:
> > > diagnostic_event_id_t m_first_close_event;
> > > const tree m_callee_fndecl;
> > > + bool m_attr;
> > > + int m_arg_idx;
> > > };
> > >
> > > class unchecked_use_of_fd : public fd_diagnostic
> > > @@ -543,8 +589,38 @@ public:
> > > private:
> > > diagnostic_event_id_t m_first_open_event;
> > > const tree m_callee_fndecl;
> > > +
> > > };
> > >
> > > +/* Issue a note regarding misuse of a file descriptor.
> > > + ARG_IDX here is 0-based.
> > > + */
> > > +void
> > > +fd_state_machine::inform_filedescriptor_attribute (
> > > + tree callee_fndecl, int arg_idx, fd_access_direction fd_dir)
> > > const
> > > +{
> > > + switch (fd_dir)
> > > + {
> > > + case READ_WRITE:
> > > + inform (DECL_SOURCE_LOCATION (callee_fndecl),
> > > + "argument %d of %qD must be an open file
> > > descriptor",
> > > arg_idx + 1,
> > > + callee_fndecl);
> > > + break;
> > > + case DIR_WRITE:
> > > + inform (DECL_SOURCE_LOCATION (callee_fndecl),
> > > + "argument %d of %qD must be a read-only file
> > > descriptor",
> > > + arg_idx + 1, callee_fndecl);
> > > + break;
> > > + case DIR_READ:
> > > + inform (DECL_SOURCE_LOCATION (callee_fndecl),
> > > + "argument %d of %qD must be a write-only file
> > > descriptor",
> > > + arg_idx + 1, callee_fndecl);
> > > + break;
> > > + }
> > > +}
> > > +
> > > +
> > > +
> > > fd_state_machine::fd_state_machine (logger *logger)
> > > : state_machine ("file-descriptor", logger),
> > > m_constant_fd (add_state ("fd-constant")),
> > > @@ -647,11 +723,124 @@ fd_state_machine::on_stmt (sm_context
> > > *sm_ctxt,
> > > const supernode *node,
> > > on_read (sm_ctxt, node, stmt, call, callee_fndecl);
> > > return true;
> > > } // "read"
> > > +
> > > + // Handle __attribute__((filedescriptor))
> > > + {
> > > + bitmap argmap = get_fd_attrs ("accesses_fd",
> > > callee_fndecl);
> > > + if (argmap)
> > > + check_for_fd_attrs (sm_ctxt, node, stmt, call,
> > > callee_fndecl, argmap, DIR_READ_WRITE);
> > > +
> > > + bitmap read_argmap = get_fd_attrs ("reads_from_fd",
> > > callee_fndecl);
> > > + if(read_argmap)
> > > + check_for_fd_attrs (sm_ctxt, node, stmt, call,
> > > callee_fndecl, read_argmap, DIR_READ);
> > > +
> > > + bitmap write_argmap = get_fd_attrs ("writes_to_fd",
> > > callee_fndecl);
> > > + if (write_argmap)
> > > + check_for_fd_attrs (sm_ctxt, node, stmt, call,
> > > callee_fndecl, write_argmap, DIR_WRITE);
> > > +
> > > + return true;
> > > + }
> > > +
> > > }
> > >
> > > return false;
> > > }
> > >
> > > +void
> > > +fd_state_machine::check_for_fd_attrs (sm_context *sm_ctxt,
> > > + const supernode *node, const
> > > gimple
> > > *stmt,
> > > + const gcall *call,
> > > + const tree callee_fndecl,
> > > bitmap
> > > argmap,
> > > + fd_access_direction
> > > fd_attr_access_dir) const
> > > +{
> > > + for (unsigned i = 0; i < gimple_call_num_args (stmt); i++)
> > > + {
> > > + unsigned int arg_idx = i;
> > > + tree arg = gimple_call_arg (stmt, i);
> > > + tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
> > > + state_t state = sm_ctxt->get_state (stmt, arg);
> > > + if (TREE_CODE (TREE_TYPE (arg)) != INTEGER_TYPE)
> > > + continue;
> > > + if (bitmap_bit_p (argmap, arg_idx)) // check if the arg is
> > > any
> > > of
> > > the file
> > > + // descriptor attributes
> > > + {
> > > +
> > > + if (is_closed_fd_p (state))
> > > + {
> > > +
> > > + sm_ctxt->warn (node, stmt, arg,
> > > + new fd_use_after_close (*this,
> > > diag_arg,
> > > +
> > > callee_fndecl,
> > > true,
> > > + arg_idx));
> > > + continue;
> > > + }
> > > + else
> > > + {
> > > + if (!(is_valid_fd_p (state) || (state == m_stop)))
> > > + {
> > > + if (!is_constant_fd_p (state))
> > > + sm_ctxt->warn (node, stmt, arg,
> > > + new unchecked_use_of_fd (*this,
> > > diag_arg,
> > > +
> > > callee_fndecl));
> > > + }
> > > + }
> > > + }
> > > + switch (fd_attr_access_dir)
> > > + {
> > > + case DIR_READ:
> > > + if (bitmap_bit_p (argmap, arg_idx)) // check if the arg
> > > is
> > > marked by
> > > + // reads_from_fd
> > > attribute
> > > + {
> > > + if (is_writeonly_fd_p (state))
> > > + {
> > > + sm_ctxt->warn (node, stmt, arg,
> > > + new fd_access_mode_mismatch
> > > (*this,
> > > diag_arg,
> > > +
> > > DIR_WRITE,
> > > +
> > > callee_fndecl, true, i));
> > > + }
> > > + }
> > > + break;
> > > + case DIR_WRITE:
> > > + if (bitmap_bit_p (argmap, arg_idx)) // check if the arg
> > > is
> > > marked by
> > > + // writes_to_fd
> > > attribute
> > > + {
> > > + if (is_readonly_fd_p (state))
> > > + {
> > > + sm_ctxt->warn (node, stmt, arg,
> > > + new fd_access_mode_mismatch (
> > > + *this, diag_arg, DIR_READ,
> > > callee_fndecl, true, i));
> > > + }
> > > + }
> > > + break;
> > > + default:
> > > + gcc_unreachable ();
> > > + }
> > > + }
> > > +}
> > > +
> > > +bitmap
> > > +fd_state_machine::get_fd_attrs (const char *attr_name, tree
> > > callee_fndecl) const
> > > +{
> > > + bitmap argmap = NULL;
> > > + tree attrs = TYPE_ATTRIBUTES (TREE_TYPE (callee_fndecl));
> > > + attrs = lookup_attribute (attr_name, attrs);
> > > + if (!attrs)
> > > + return argmap;
> > > +
> > > + if (!TREE_VALUE (attrs))
> > > + return argmap;
> > > +
> > > + argmap = BITMAP_ALLOC (NULL);
> > > +
> > > + for (tree idx = TREE_VALUE (attrs); idx; idx = TREE_CHAIN (idx))
> > > + {
> > > + unsigned int val = TREE_INT_CST_LOW (TREE_VALUE (idx)) - 1;
> > > + bitmap_set_bit (argmap, val);
> > > + }
> > > + return argmap;
> > > +}
> > > +
> > > +
> > > void
> > > fd_state_machine::on_open (sm_context *sm_ctxt, const supernode
> > > *node,
> > > const gimple *stmt, const gcall *call)
> > > const
> > > @@ -729,7 +918,7 @@ void
> > > fd_state_machine::check_for_open_fd (
> > > sm_context *sm_ctxt, const supernode *node, const gimple
> > > *stmt,
> > > const gcall *call, const tree callee_fndecl,
> > > - enum access_direction callee_fndecl_dir) const
> > > + enum fd_access_direction callee_fndecl_dir) const
> > > {
> > > tree arg = gimple_call_arg (call, 0);
> > > tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
> > > @@ -738,7 +927,7 @@ fd_state_machine::check_for_open_fd (
> > > if (is_closed_fd_p (state))
> > > {
> > > sm_ctxt->warn (node, stmt, arg,
> > > - new fd_use_after_close (*this, diag_arg,
> > > callee_fndecl));
> > > + new fd_use_after_close (*this, diag_arg,
> > > callee_fndecl, false, -1));
> > > }
> > >
> > > else
> > > @@ -758,7 +947,7 @@ fd_state_machine::check_for_open_fd (
> > > tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
> > > sm_ctxt->warn (node, stmt, arg,
> > > new fd_access_mode_mismatch (
> > > - *this, diag_arg, DIR_WRITE,
> > > callee_fndecl));
> > > + *this, diag_arg, DIR_WRITE,
> > > callee_fndecl, false, -1));
> > > }
> > >
> > > break;
> > > @@ -769,9 +958,11 @@ fd_state_machine::check_for_open_fd (
> > > tree diag_arg = sm_ctxt->get_diagnostic_tree (arg);
> > > sm_ctxt->warn (node, stmt, arg,
> > > new fd_access_mode_mismatch (
> > > - *this, diag_arg, DIR_READ,
> > > callee_fndecl));
> > > + *this, diag_arg, DIR_READ,
> > > callee_fndecl, false, -1));
> > > }
> > > break;
> > > + default:
> > > + gcc_unreachable ();
> > > }
> > > }
> > > }
> > > diff --git a/gcc/c-family/c-attribs.cc b/gcc/c-family/c-attribs.cc
> > > index c8d96723f4c..84f29987bc6 100644
> > > --- a/gcc/c-family/c-attribs.cc
> > > +++ b/gcc/c-family/c-attribs.cc
> > > @@ -173,6 +173,10 @@ static tree handle_objc_nullability_attribute
> > > (tree
> > > *, tree, tree, int, bool *);
> > > static tree handle_signed_bool_precision_attribute (tree *, tree,
> > > tree,
> > > int,
> > > bool *);
> > > static tree handle_retain_attribute (tree *, tree, tree, int, bool
> > > *);
> > > +static tree handle_accesses_fd_attribute (tree *, tree, tree, int,
> > > bool
> > > *);
> > > +static tree handle_reads_from_fd_attribute (tree *, tree, tree,
> > > int,
> > > bool
> > > *);
> > > +static tree handle_writes_to_fd_attribute (tree *, tree, tree,
> > > int,
> > > bool
> > > *);
> > > +
> > >
> > > /* Helper to define attribute exclusions. */
> > > #define ATTR_EXCL(name, function, type, variable) \
> > > @@ -427,6 +431,12 @@ const struct attribute_spec
> > > c_common_attribute_table[] =
> > > handle_tls_model_attribute, NULL },
> > > { "nonnull", 0, -1, false, true, true, false,
> > > handle_nonnull_attribute, NULL },
> > > + { "accesses_fd", 0, -1, false, true, true, false,
> > > + handle_accesses_fd_attribute, NULL},
> > > + { "reads_from_fd", 0, -1, false, true, true, false,
> > > + handle_reads_from_fd_attribute, NULL},
> > > + { "writes_to_fd", 0, -1, false, true, true, false,
> > > + handle_writes_to_fd_attribute, NULL},
> > > { "nonstring", 0, 0, true, false, false, false,
> > > handle_nonstring_attribute, NULL },
> > > { "nothrow", 0, 0, true, false, false, false,
> > > @@ -4521,6 +4531,59 @@ handle_nonnull_attribute (tree *node, tree
> > > name,
> > > return NULL_TREE;
> > > }
> > >
> > > +/* Handle the "accesses_fd" attribute */
> > > +static tree
> > > +handle_accesses_fd_attribute (tree *node, tree name, tree args,
> > > + int ARG_UNUSED (flags), bool
> > > *no_add_attrs)
> > > +{
> > > + tree type = *node;
> > > + if (!args)
> > > + {
> > > + if (!prototype_p (type))
> > > + {
> > > + error ("%qE attribute without arguments on a non-
> > > prototype",
> > > name);
> > > + *no_add_attrs = true;
> > > + }
> > > + return NULL_TREE;
> > > + }
> > > +
> > > + for (int i = 1; args; ++i)
> > > + {
> > > + tree pos = TREE_VALUE (args);
> > > + tree next = TREE_CHAIN (args);
> > > + if (tree val = positional_argument (type, name, pos,
> > > INTEGER_TYPE,
> > > + next || i > 1 ? i : 0))
> > > + TREE_VALUE (args) = val;
> > > + else
> > > + {
> > > + *no_add_attrs = true;
> > > + break;
> > > + }
> > > + args = next;
> > > + }
> > > +}
> > > +
> > > +
> > > +
> > > +static tree
> > > +handle_reads_from_fd_attribute (tree *node, tree name, tree
> > > ARG_UNUSED
> > > (args),
> > > + int ARG_UNUSED (flags), bool *no_add_attrs)
> > > +{
> > > + // TODO: valiadate usage of this attribute
> > > + return NULL_TREE;
> > > +}
> > > +
> > > +static tree
> > > +handle_writes_to_fd_attribute (tree *node, tree name, tree
> > > ARG_UNUSED
> > > (args),
> > > + int ARG_UNUSED (flags), bool *no_add_attrs)
> > > +{
> > > + // TODO: validate usage of this attribute
> > > + return NULL_TREE;
> > > +}
> > > +
> > > +
> > > +
> > > +
> > > /* Handle the "nonstring" variable attribute. */
> > >
> > > static tree
> > > diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-5.c
> > > b/gcc/testsuite/gcc.dg/analyzer/fd-5.c
> > > new file mode 100644
> > > index 00000000000..e597ba2367a
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/analyzer/fd-5.c
> > > @@ -0,0 +1,44 @@
> > > +int open(const char *, int mode);
> > > +void close(int fd);
> > > +int write (int fd, void *buf, int nbytes);
> > > +int read (int fd, void *buf, int nbytes);
> > > +
> > > +#define O_RDONLY 0
> > > +#define O_WRONLY 1
> > > +#define O_RDWR 2
> > > +
> > > +void f (int fd) __attribute__((accesses_fd(1))); /* { dg-message
> > > "argument 1 of 'f' must be an open file descriptor" } */
> > > +
> > > +void
> > > +test_1 (const char *path)
> > > +{
> > > + int fd = open (path, O_RDWR);
> > > + close(fd);
> > > + f(fd); /* { dg-warning "'f' on closed file descriptor 'fd'" }
> > > */
> > > + /* { dg-message "\\(3\\) 'f' on closed file descriptor 'fd';
> > > 'close' was at \\(2\\)" "" { target *-*-* } .-1 } */
> > > +}
> > > +
> > > +void g (int fd) __attribute__((reads_from_fd(1))); /* { dg-message
> > > "argument 1 of 'g' must be a read-only file descriptor" } */
> > > +
> > > +void
> > > +test_2 (const char *path)
> > > +{
> > > + int fd = open (path, O_WRONLY);
> > > + if (fd != -1)
> > > + {
> > > + g (fd); /* { dg-warning "'g' on 'write-only' file descriptor
> > > 'fd'" }
> > > */
> > > + }
> > > + close (fd);
> > > +}
> > > +
> > > +void h (int fd) __attribute__((writes_to_fd(1))); /* { dg-message
> > > "argument 1 of 'h' must be a write-only file descriptor" } */
> > > +void
> > > +test_3 (const char *path)
> > > +{
> > > + int fd = open (path, O_RDONLY);
> > > + if (fd != -1)
> > > + {
> > > + h (fd); /* { dg-warning "'h' on 'read-only' file descriptor
> > > 'fd'" } */
> > > + }
> > > + close(fd);
> > > +}
> > > \ No newline at end of file
> > > diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-6.c
> > > b/gcc/testsuite/gcc.dg/analyzer/fd-6.c
> > > new file mode 100644
> > > index 00000000000..9c0b55086a9
> > > --- /dev/null
> > > +++ b/gcc/testsuite/gcc.dg/analyzer/fd-6.c
> > > @@ -0,0 +1,4 @@
> > > +
> > > +int not_a_fn __attribute__ ((accesses_fd(1))); /* { dg-warning
> > > "'accesses_fd' attribute only applies to function types" } */
> > > +
> > > +void f (char *p) __attribute__ ((accesses_fd(1))); /* { dg-warning
> > > "'accesses_fd' attribute argument value '1' refers to parameter
> > > type
> > > 'char
> > > \\\*'" } */
> > > \ No newline at end of file
> > > --
> > > 2.25.1
> > >
> > >
>
next prev parent reply other threads:[~2022-07-12 22:25 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 [this message]
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
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=5769682d0d17579cbd72f72a4001bfa8444b80a8.camel@redhat.com \
--to=dmalcolm@redhat.com \
--cc=gcc@gcc.gnu.org \
--cc=libc-alpha@sourceware.org \
--cc=mirimnan017@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).