| Message ID | CY4PR1801MB1910435B13A30726A728D065C69D9@CY4PR1801MB1910.namprd18.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | analyzer: support for creat, dup, dup2 and dup3 [PR106298] | expand |
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 <mirimmad@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 <mirimmad@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 > >
On Tue, 2022-08-02 at 22:08 +0530, Mir Immad wrote: > The above patch is bootstrapped, lightly tested (on x86_64 Linux) and > approved for trunk by David. For reference, Immad sent that version to me off-list to me for review, and I approved it. He's committed it to trunk now (as r13-1936-g6a11f2d974a912). Dave
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
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 <mirimmad@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