From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x52c.google.com (mail-pg1-x52c.google.com [IPv6:2607:f8b0:4864:20::52c]) by sourceware.org (Postfix) with ESMTPS id C376C3857BA1 for ; Tue, 12 Jul 2022 17:33:35 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C376C3857BA1 Received: by mail-pg1-x52c.google.com with SMTP id p11so1446299pgr.12 for ; Tue, 12 Jul 2022 10:33:35 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:cc; bh=vCndOdDIloAo6/S5lSQGzLPclvPdjpS3f3uBE+W+Mmw=; b=LTNtVk9yBsvqXWQxBPhGrOPeVuo2J8SSx63tpigF6C5IcqbJBNlZOjgPh3r5XOdVuJ hu64qu8ri1wy9fidmbvdQAC/njoenIS8wwOCxBeMhvdM1wg7vQQkGylVBUoO90H4qEtv 3vF/hXyoXfkArdYZ8NGlHoYKjFKzeanMyiAiErP/a5B8PLG+lG9hqjdQsslZkqcLBXAI NoY9ylpuaOkwCvFJYQKfUmFNeJLUH782hX9zTS7zNl4WjdmfJJ4zRn1YV3NR73/iaG7O dFcFClRrJcLK1GjUrRNMikBIS/JwmlUUfthcjJ28g2UNh5sjUxmHR5YARiPnv1OTI+r6 8m4g== X-Gm-Message-State: AJIora9/EDdd48n5bsXCFv0+fVoNY/0xCBckwAUEkgUnkpkLWtGO+PfL YKEiKKxEHDWUGI2b98qcMavsVaNwvGhSQ4tFJ2rsYfdgU7eifV8U X-Received: by 2002:a63:f58:0:b0:416:492:267d with SMTP id 24-20020a630f58000000b004160492267dmt6325648pgp.22.1657647214233; Tue, 12 Jul 2022 10:33:34 -0700 (PDT) MIME-Version: 1.0 References: In-Reply-To: From: Mir Immad Date: Tue, 12 Jul 2022 23:03:23 +0530 Message-ID: Subject: Re: [PATCH] filedescriptor attribute Cc: gcc@gcc.gnu.org, David Malcolm X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, GIT_PATCH_0, HTML_MESSAGE, MISSING_HEADERS, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 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 17:33:40 -0000 Hi everyone, 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) 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) 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) 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) 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 > >