The above patch is bootstrapped, lightly tested (on x86_64 Linux) and approved for trunk by David.
On Tue, Aug 2, 2022 at 10:04 PM Immad Mir <mirim...@outlook.com> wrote: > This patch extends the state machine in sm-fd.cc to support > creat, dup, dup2 and dup3 functions. > > Lightly tested on x86_64 Linux. > > gcc/analyzer/ChangeLog: > PR analyzer/106298 > * sm-fd.cc (fd_state_machine::on_open): Add > creat, dup, dup2 and dup3 functions. > (enum dup): New. > (fd_state_machine::valid_to_unchecked_state): New. > (fd_state_machine::on_creat): New. > (fd_state_machine::on_dup): New. > > gcc/testsuite/ChangeLog: > PR analyzer/106298 > * gcc.dg/analyzer/fd-1.c: Add tests for 'creat'. > * gcc.dg/analyzer/fd-2.c: Likewise. > * gcc.dg/analyzer/fd-4.c: Likewise. > * gcc.dg/analyzer/fd-dup-1.c: New tests. > > Signed-off-by: Immad Mir <mirim...@outlook.com> > --- > gcc/analyzer/sm-fd.cc | 129 ++++++++++++- > gcc/testsuite/gcc.dg/analyzer/fd-1.c | 21 +++ > gcc/testsuite/gcc.dg/analyzer/fd-2.c | 15 ++ > gcc/testsuite/gcc.dg/analyzer/fd-4.c | 31 +++- > gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c | 223 +++++++++++++++++++++++ > 5 files changed, 415 insertions(+), 4 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c > > diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc > index ed923ade100..8bb76d72b05 100644 > --- a/gcc/analyzer/sm-fd.cc > +++ b/gcc/analyzer/sm-fd.cc > @@ -69,6 +69,14 @@ enum access_directions > DIRS_WRITE > }; > > +/* An enum for distinguishing between dup, dup2 and dup3. */ > +enum dup > +{ > + DUP_1, > + DUP_2, > + DUP_3 > +}; > + > class fd_state_machine : public state_machine > { > public: > @@ -114,7 +122,9 @@ 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; > - > + /* Function for one-to-one correspondence between valid > + and unchecked states. */ > + state_t valid_to_unchecked_state (state_t state) const; > /* State for a constant file descriptor (>= 0) */ > state_t m_constant_fd; > > @@ -147,6 +157,8 @@ public: > private: > void on_open (sm_context *sm_ctxt, const supernode *node, const gimple > *stmt, > const gcall *call) const; > + void on_creat (sm_context *sm_ctxt, const supernode *node, const gimple > *stmt, > + const gcall *call) const; > void on_close (sm_context *sm_ctxt, const supernode *node, const gimple > *stmt, > const gcall *call) const; > void on_read (sm_context *sm_ctxt, const supernode *node, const gimple > *stmt, > @@ -170,6 +182,9 @@ private: > const gimple *stmt, const gcall *call, > const tree callee_fndecl, const char *attr_name, > access_directions fd_attr_access_dir) const; > + void check_for_dup (sm_context *sm_ctxt, const supernode *node, > + const gimple *stmt, const gcall *call, const tree callee_fndecl, > + enum dup kind) const; > }; > > /* Base diagnostic class relative to fd_state_machine. */ > @@ -723,6 +738,20 @@ fd_state_machine::is_constant_fd_p (state_t state) > const > return (state == m_constant_fd); > } > > +fd_state_machine::state_t > +fd_state_machine::valid_to_unchecked_state (state_t state) const > +{ > + if (state == m_valid_read_write) > + return m_unchecked_read_write; > + else if (state == m_valid_write_only) > + return m_unchecked_write_only; > + else if (state == m_valid_read_only) > + return m_unchecked_read_only; > + else > + gcc_unreachable (); > + return NULL; > +} > + > bool > fd_state_machine::on_stmt (sm_context *sm_ctxt, const supernode *node, > const gimple *stmt) const > @@ -736,6 +765,11 @@ fd_state_machine::on_stmt (sm_context *sm_ctxt, const > supernode *node, > return true; > } // "open" > > + if (is_named_call_p (callee_fndecl, "creat", call, 2)) > + { > + on_creat (sm_ctxt, node, stmt, call); > + } // "creat" > + > if (is_named_call_p (callee_fndecl, "close", call, 1)) > { > on_close (sm_ctxt, node, stmt, call); > @@ -754,6 +788,23 @@ fd_state_machine::on_stmt (sm_context *sm_ctxt, const > supernode *node, > return true; > } // "read" > > + if (is_named_call_p (callee_fndecl, "dup", call, 1)) > + { > + check_for_dup (sm_ctxt, node, stmt, call, callee_fndecl, > DUP_1); > + return true; > + } > + > + if (is_named_call_p (callee_fndecl, "dup2", call, 2)) > + { > + check_for_dup (sm_ctxt, node, stmt, call, callee_fndecl, > DUP_2); > + return true; > + } > + > + if (is_named_call_p (callee_fndecl, "dup3", call, 3)) > + { > + check_for_dup (sm_ctxt, node, stmt, call, callee_fndecl, > DUP_3); > + return true; > + } > > { > // Handle __attribute__((fd_arg)) > @@ -899,6 +950,78 @@ fd_state_machine::on_open (sm_context *sm_ctxt, const > supernode *node, > } > } > > +void > +fd_state_machine::on_creat (sm_context *sm_ctxt, const supernode *node, > + const gimple *stmt, const gcall *call) const > +{ > + tree lhs = gimple_call_lhs (call); > + if (lhs) > + sm_ctxt->on_transition (node, stmt, lhs, m_start, > m_unchecked_write_only); > + else > + sm_ctxt->warn (node, stmt, NULL_TREE, new fd_leak (*this, NULL_TREE)); > +} > + > +void > +fd_state_machine::check_for_dup (sm_context *sm_ctxt, const supernode > *node, > + const gimple *stmt, const gcall *call, > + const tree callee_fndecl, enum dup kind) > const > +{ > + tree lhs = gimple_call_lhs (call); > + tree arg_1 = gimple_call_arg (call, 0); > + state_t state_arg_1 = sm_ctxt->get_state (stmt, arg_1); > + if (state_arg_1 == m_stop) > + return; > + if (!(is_constant_fd_p (state_arg_1) || is_valid_fd_p (state_arg_1))) > + { > + check_for_open_fd (sm_ctxt, node, stmt, call, callee_fndecl, > + DIRS_READ_WRITE); > + if (kind == DUP_1) > + return; > + } > + switch (kind) > + { > + case DUP_1: > + if (lhs) > + { > + if (is_constant_fd_p (state_arg_1)) > + sm_ctxt->set_next_state (stmt, lhs, m_unchecked_read_write); > + else > + sm_ctxt->set_next_state (stmt, lhs, > + valid_to_unchecked_state > (state_arg_1)); > + } > + break; > + > + case DUP_2: > + case DUP_3: > + tree arg_2 = gimple_call_arg (call, 1); > + state_t state_arg_2 = sm_ctxt->get_state (stmt, arg_2); > + tree diag_arg_2 = sm_ctxt->get_diagnostic_tree (arg_2); > + if (state_arg_2 == m_stop) > + return; > + /* Check if -1 was passed as second argument to dup2. */ > + if (!(is_constant_fd_p (state_arg_2) || is_valid_fd_p > (state_arg_2))) > + { > + sm_ctxt->warn ( > + node, stmt, arg_2, > + new fd_use_without_check (*this, diag_arg_2, callee_fndecl)); > + return; > + } > + /* dup2 returns value of its second argument on success.But, the > + access mode of the returned file descriptor depends on the > duplicated > + file descriptor i.e the first argument. */ > + if (lhs) > + { > + if (is_constant_fd_p (state_arg_1)) > + sm_ctxt->set_next_state (stmt, lhs, m_unchecked_read_write); > + else > + sm_ctxt->set_next_state (stmt, lhs, > + valid_to_unchecked_state > (state_arg_1)); > + } > + > + break; > + } > +} > + > void > fd_state_machine::on_close (sm_context *sm_ctxt, const supernode *node, > const gimple *stmt, const gcall *call) const > @@ -964,6 +1087,8 @@ fd_state_machine::check_for_open_fd ( > } > switch (callee_fndecl_dir) > { > + case DIRS_READ_WRITE: > + break; > case DIRS_READ: > if (is_writeonly_fd_p (state)) > { > @@ -984,8 +1109,6 @@ fd_state_machine::check_for_open_fd ( > *this, diag_arg, DIRS_READ, > callee_fndecl)); > } > break; > - default: > - gcc_unreachable (); > } > } > } > diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-1.c > b/gcc/testsuite/gcc.dg/analyzer/fd-1.c > index 8a72e63833c..5b85a3316e8 100644 > --- a/gcc/testsuite/gcc.dg/analyzer/fd-1.c > +++ b/gcc/testsuite/gcc.dg/analyzer/fd-1.c > @@ -3,6 +3,13 @@ int open(const char *, int mode); > #define O_WRONLY 1 > #define O_RDWR 2 > > +typedef enum { > + S_IRWXU > + // etc > +} mode_t; > + > +int creat (const char *, mode_t mode); > + > void > test_1 (const char *path) > { > @@ -37,3 +44,17 @@ void test_4 (const char *path) > /* { dg-message "\\(1\\) leaks here" "" { target *-*-* } .-1 } */ > } > > +void > +test_5 (const char *path, mode_t mode) > +{ > + creat (path, mode); /* { dg-warning "leak of file descriptor > \\\[CWE-775\\\]" } */ > +} > + > +void > +test_6 (const char *path, mode_t mode) > +{ > + int fd = creat (path, mode); > + return; /* { dg-warning "leak of file descriptor 'fd' \\\[CWE-775\\\]" > } */ > +} > + > + > diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-2.c > b/gcc/testsuite/gcc.dg/analyzer/fd-2.c > index d794b460a2e..10c9ecdb09d 100644 > --- a/gcc/testsuite/gcc.dg/analyzer/fd-2.c > +++ b/gcc/testsuite/gcc.dg/analyzer/fd-2.c > @@ -5,6 +5,13 @@ void close(int fd); > #define O_RDWR 2 > #define STDIN 0 > > +typedef enum { > + S_IRWXU > + // etc > +} mode_t; > + > +int creat (const char *, mode_t mode); > + > void > test_1 (const char *path) > { > @@ -46,4 +53,12 @@ test_4 () > int fd = -1; > close(fd); > close(fd); > +} > + > +void > +test_5 (const char *path, mode_t mode) > +{ > + int fd = creat (path, mode); > + close(fd); > + close(fd); /* { dg-warning "double 'close' of file descriptor 'fd' > \\\[CWE-1341\\\]" "warning" } */ > } > \ No newline at end of file > diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-4.c > b/gcc/testsuite/gcc.dg/analyzer/fd-4.c > index ecd787caff7..6b8fca5408f 100644 > --- a/gcc/testsuite/gcc.dg/analyzer/fd-4.c > +++ b/gcc/testsuite/gcc.dg/analyzer/fd-4.c > @@ -9,6 +9,12 @@ int read (int fd, void *buf, int nbytes); > #define O_WRONLY 1 > #define O_RDWR 2 > > +typedef enum { > + S_IRWXU > + // etc > +} mode_t; > + > +int creat (const char *, mode_t mode); > > void > test_1 (const char *path, void *buf) > @@ -69,4 +75,27 @@ test_5 (const char *path) > int fd = open (path, O_RDWR); > close(fd); > printf("%d", fd); /* { dg-bogus "'printf' on a closed file descriptor > 'fd'" } */ > -} > \ No newline at end of file > +} > + > + > +void > +test_6 (const char *path, mode_t mode, void *buf) > +{ > + int fd = creat (path, mode); > + if (fd != -1) > + { > + read (fd, buf, 1); /* { dg-warning "'read' on write-only file > descriptor 'fd'" } */ > + close(fd); > + } > +} > + > +void > +test_7 (const char *path, mode_t mode, void *buf) > +{ > + int fd = creat (path, mode); > + if (fd != -1) > + { > + write (fd, buf, 1); > + close(fd); > + } > +} > diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c > b/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c > new file mode 100644 > index 00000000000..eba2570568f > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c > @@ -0,0 +1,223 @@ > +int open(const char *, int mode); > +void close(int fd); > +int dup (int old_fd); > +int dup2 (int old_fd, int new_fd); > +int dup3 (int old_fd, int new_fd, int flags); > +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 test_1 (const char *path) > +{ > + int old_fd = open (path, O_RDWR); > + int new_fd = dup (old_fd); /* { dg-warning "'dup' on possibly invalid > file descriptor 'old_fd'" } */ > + close(old_fd); > + close(new_fd); > +} > + > +void test_2 (const char *path) > +{ > + int old_fd = open (path, O_RDWR); > + if (old_fd != -1) > + { > + int new_fd = dup (old_fd); > + close(old_fd); > + return; /* { dg-warning "leak of file descriptor 'new_fd' > \\\[CWE-775\\\]" } */ > + } > +} > + > +void test_3 (const char *path, void *buf) > +{ > + int old_fd = open (path, O_RDWR); > + if (old_fd != -1) > + { > + int new_fd = dup (old_fd); > + write (new_fd, buf, 1); /* { dg-warning "'write' on possibly > invalid file descriptor 'new_fd'" } */ > + close (new_fd); > + close(old_fd); > + } > +} > + > + > +void test_5 (const char *path, void *buf) > +{ > + int old_fd = open (path, O_RDWR); > + if (old_fd != -1) > + { > + int new_fd = dup (old_fd); > + if (new_fd != -1) > + { > + write (new_fd, buf, 1); > + close (new_fd); > + > + } > + close(old_fd); > + } > +} > + > + > +void test_7 (const char *path) > +{ > + int old_fd = open (path, O_RDWR); > + dup2 (old_fd, 4); /* { dg-warning "'dup2' on possibly invalid file > descriptor 'old_fd'" } */ > + close(old_fd); > +} > + > +void test_8 (const char *path) > +{ > + int old_fd = open (path, O_RDWR); > + int new_fd = open (path, O_RDWR); > + if (old_fd != -1) > + { > + dup2 (old_fd, new_fd); /* { dg-warning "'dup2' on possibly > invalid file descriptor 'new_fd'" } */ > + close (old_fd); > + } > + close (new_fd); > +} > + > +void test_9 (const char *path, void *buf) > +{ > + int old_fd = open (path, O_RDWR); > + > + if (old_fd != -1) > + { > + int new_fd = open (path, O_RDWR); > + if (new_fd != -1) > + { > + int lhs = dup2 (old_fd, new_fd); > + write (lhs, buf, 1); /* { dg-warning "'write' on possibly > invalid file descriptor 'lhs'" } */ > + close(new_fd); > + close(lhs); > + } > + close(old_fd); > + } > +} > + > +void test_10 (const char *path, int flags) > +{ > + int old_fd = open (path, O_RDWR); > + int new_fd = open (path, O_RDWR); > + if (old_fd != -1) > + { > + dup3 (old_fd, new_fd, flags); /* { dg-warning "'dup3' on possibly > invalid file descriptor 'new_fd'" } */ > + close(old_fd); > + > + } > + close(new_fd); > +} > + > +void test_11 (const char *path, int flags) > +{ > + int old_fd = open (path, O_RDWR); > + int new_fd = open (path, O_RDWR); > + if (new_fd != -1) > + { > + dup3 (old_fd, new_fd, flags); /* { dg-warning "'dup3' on possibly > invalid file descriptor 'old_fd'" } */ > + close(new_fd); > + > + } > + close(old_fd); > +} > + > +void test_12 (const char *path, void *buf) > +{ > + int old_fd = open (path, O_RDONLY); > + if (old_fd != -1) > + { > + int new_fd = dup (old_fd); > + if (new_fd != -1) > + { > + write (new_fd, buf, 1); /* { dg-warning "'write' on read-only > file descriptor 'new_fd'" } */ > + close(new_fd); > + } > + close(old_fd); > + } > +} > + > +void test_13 (const char *path, void *buf) > +{ > + int old_fd = open (path, O_WRONLY); > + if (old_fd != -1) > + { > + int new_fd = dup (old_fd); > + if (new_fd != -1) > + { > + read (new_fd, buf, 1); /* { dg-warning "'read' on write-only > file descriptor 'new_fd'" } */ > + close(new_fd); > + } > + close(old_fd); > + } > +} > + > +void test_14 (const char *path, void *buf) > +{ > + int old_fd = open (path, O_RDWR); > + if (old_fd != -1) > + { > + int new_fd = dup (old_fd); > + if (new_fd != -1) > + { > + write (new_fd, buf, 1); > + read (new_fd, buf, 1); > + close(new_fd); > + } > + close(old_fd); > + } > +} > + > +void test_15 (void *buf) > +{ > + int fd = dup(0); > + read (fd, buf, 1); /* { dg-warning "'read' on possibly invalid file > descriptor 'fd'" } */ > + close(fd); > +} > + > +void test_16 (void *buf) > +{ > + int fd = dup(1); > + if (fd != -1) > + { > + write (fd, buf, 1); > + close (fd); > + } > +} > + > +void test_17 (const char *path) > +{ > + int fd = open (path, O_RDWR); > + close(fd); > + dup (fd); /* { dg-warning "'dup' on closed file descriptor 'fd'" } */ > + dup2 (fd, 4); /* { dg-warning "'dup2' on closed file descriptor 'fd'" > } */ > +} > + > +void > +test_18 (const char *path, void *buf) > +{ > + int fd = open (path, O_RDWR); > + if (fd != -1) > + { > + int fd2 = dup2 (fd, 3); > + read (fd2, buf, 1); /* { dg-warning "'read' on possibly invalid > file descriptor 'fd2'" } */ > + close(fd); > + close(fd2); > + } > +} > + > +void > +test_19 (const char *path, void *buf) > +{ > + int fd = open (path, O_WRONLY); > + if (fd != -1) > + { > + int fd2 = dup2 (fd, 4); > + if (fd2 != -1) > + { > + read (fd2, buf, 1); /* { dg-warning "'read' on write-only > file descriptor 'fd2'" } */ > + close(fd2); > + } > + close (fd); > + } > + > +} > \ No newline at end of file > -- > 2.25.1 > >