From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 67D0038560BD for ; Tue, 12 Jul 2022 22:17:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 67D0038560BD Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-468-VKzga3VnMXOQrKop85Ejyw-1; Tue, 12 Jul 2022 18:16:57 -0400 X-MC-Unique: VKzga3VnMXOQrKop85Ejyw-1 Received: by mail-qv1-f69.google.com with SMTP id mh1-20020a056214564100b00472fcc5ae9eso2557923qvb.11 for ; Tue, 12 Jul 2022 15:16:57 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:user-agent:mime-version:content-transfer-encoding; bh=xLgE15XgpcUE3deqDGV5HqzI7u+IMO48C6N6W+YBsp8=; b=ljnznY5BS8Y6jyB1Wguo6AumTuMzsQO09KzqqCDyEgIvuMo8QuCYPcuur6qvgR6h2b 859ac+t/cpsnorfqqOXdmHuryGfPr3QbkMr4Vmw9STPeL9j7a9FGWAtZDCaH36N5P38z KrKp+hNoiEagF5RbiBftFxrHQXBB6B7/yojwFc4nDC8smRVUDm3Xu3etotLnnm3c9JO8 XpMaQe4q384hArJmjpGwVQfOEIjaxd78cBOwPmchtih1LdJ2ZQRBatSozK/h9VvreMn4 C6XWDbnwBpR8r1LSEXOnidZ3iC2tsE/J9WQNw4C8yQS3Bx4yQo7Vw452g4ojrbtaA9Kx cS6g== X-Gm-Message-State: AJIora8Nr+1h+n7Ohj+VGm+SL1LE7yqh3eomJc0QeHPkk/r55L9Xyiym olI7Org9HwuVywfQTcM5KA3+gFTVciDXsIY3oHHhDyoWXolY9BoVm/z5BGfPHG41FGv0i5tpBik K1Yb0lN4= X-Received: by 2002:a05:620a:2452:b0:6b5:b107:63d9 with SMTP id h18-20020a05620a245200b006b5b10763d9mr365054qkn.167.1657664216333; Tue, 12 Jul 2022 15:16:56 -0700 (PDT) X-Google-Smtp-Source: AGRyM1tGBDVKrLizfcoyX/I9p7BujnyZ66wAJ80YUDw87FuhDxZY17nAW3GyHX6OuM0B45P59ACBjg== X-Received: by 2002:a05:620a:2452:b0:6b5:b107:63d9 with SMTP id h18-20020a05620a245200b006b5b10763d9mr365036qkn.167.1657664215902; Tue, 12 Jul 2022 15:16:55 -0700 (PDT) Received: from t14s.localdomain (c-73-69-212-193.hsd1.nh.comcast.net. [73.69.212.193]) by smtp.gmail.com with ESMTPSA id k10-20020ac8604a000000b0031eb51dd72csm5065589qtm.85.2022.07.12.15.16.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 12 Jul 2022 15:16:55 -0700 (PDT) Message-ID: <260f0b41c663133cea96eb64bd85e8ba16d8a936.camel@redhat.com> Subject: Adding file descriptor attribute(s) to gcc and glibc From: David Malcolm To: Mir Immad Cc: gcc@gcc.gnu.org, libc-alpha@sourceware.org Date: Tue, 12 Jul 2022 18:16:53 -0400 In-Reply-To: References: User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00, BODY_8BITS, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Jul 2022 22:17:08 -0000 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 > 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 % 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 % 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 > > > >