public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
From: David Malcolm <dmalcolm@redhat.com>
To: Mir Immad <mirimnan017@gmail.com>
Cc: gcc@gcc.gnu.org, libc-alpha@sourceware.org
Subject: Adding file descriptor attribute(s) to gcc and glibc
Date: Tue, 12 Jul 2022 18:16:53 -0400	[thread overview]
Message-ID: <260f0b41c663133cea96eb64bd85e8ba16d8a936.camel@redhat.com> (raw)
In-Reply-To: <CAE1-7oyLmZJAue2S39fsvvJPH3KabPDNqhL6jjwTv4RybeAQEQ@mail.gmail.com>

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.

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
> > 
> > 



  reply	other threads:[~2022-07-12 22:17 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   ` David Malcolm [this message]
2022-07-12 22:25     ` Adding file descriptor attribute(s) to gcc and glibc 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

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=260f0b41c663133cea96eb64bd85e8ba16d8a936.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).