Re: [PATCH] analyzer: fix ICE casued by dup2 in sm-fd.cc[PR106551]

[Mir Immad via Gcc-patches]

With the fix for bogus warning in fd-uninit.c, the analyzer now does not
warning for the following code for which it would previously emit
-Wanalyzer-fd-use-without-check

extern int m;
test()
{
 int fd = dup2(m, 1);
 close(fd);
}

So I had to remove such warnings from fd-dup-1.c test_20,21,22 (in the
patch). Now these tests are only there to show fix for PR16551.

Sending an updated patch (passes style and commit checker).
Thanks.
Immad.

On Thu, Aug 11, 2022 at 12:14 AM David Malcolm  wrote:

> On Wed, 2022-08-10 at 22:51 +0530, Mir Immad wrote:
> >  > Can you please rebase and see if your patch
> > > does fix it?
> >
> > No, the patch that I sent did not attempt to fix this. Now that I
> > have made
> > the correction, XFAIL in fd-uninit-1.c has changed to XPASS.
>
> Great - that means that, with your fix, we no longer bogusly emit that
> false positive.
>
> >
> > Should i remove the dg-bogus warning from fd-uninit-1.c test_1?
>
> Yes please.
>
> Thanks
> Dave
>
> >
> > Thanks.
> > Immad.
> >
> >
> > On Wed, Aug 10, 2022 at 10:26 PM David Malcolm 
> > wrote:
> >
> > > On Wed, 2022-08-10 at 20:34 +0530, Mir Immad wrote:
> > > >  > if you convert the "int m;" locals into an extern global, like
> > > > in
> > > > > comment #0 of bug 106551, does that still trigger the crash on
> > > > > the
> > > > > unpatched sm-fd.cc?
> > > >
> > > > Yes, it does, since m would be in "m_start" state. I'm sending an
> > > > updated
> > > > patch.
> > >
> > > Great!
> > >
> > > Note that I recently committed a fix for bug 106573, which has an
> > > xfail
> > > on a dg-bogus to mark a false positive which your patch hopefully
> > > also
> > > fixes (in fd-uninit-1.c).  Can you please rebase and see if your
> > > patch
> > > does fix it?
> > >
> > > Thanks
> > > Dave
> > >
> > >
> > > >
> > > > Thanks
> > > > Immad.
> > > >
> > > > On Wed, Aug 10, 2022 at 1:32 AM David Malcolm <
> > > > dmalc...@redhat.com>
> > > > wrote:
> > > >
> > > > > On Tue, 2022-08-09 at 21:42 +0530, Immad Mir wrote:
> > > > > > This patch fixes the ICE caused by valid_to_unchecked_state,
> > > > > > at analyzer/sm-fd.cc by handling the m_start state in
> > > > > > check_for_dup.
> > > > > >
> > > > > > Tested lightly on x86_64.
> > > > > >
> > > > > > gcc/analyzer/ChangeLog:
> > > > > > PR analyzer/106551
> > > > > > * sm-fd.cc (check_for_dup): handle the m_start
> > > > > > state when transitioning the state of LHS
> > > > > > of dup, dup2 and dup3 call.
> > > > > >
> > > > > > gcc/testsuite/ChangeLog:
> > > > > > * gcc.dg/analyzer/fd-dup-1.c: New testcases.
> > > > > >
> > > > > > Signed-off-by: Immad Mir 
> > > > > > ---
> > > > > >  gcc/analyzer/sm-fd.cc|  4 ++--
> > > > > >  gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c | 28
> > > > > > +++-
> > > > > >  2 files changed, 29 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
> > > > > > index 8bb76d72b05..c8b9930a7b6 100644
> > > > > > --- a/gcc/analyzer/sm-fd.cc
> > > > > > +++ b/gcc/analyzer/sm-fd.cc
> > > > > > @@ -983,7 +983,7 @@ fd_state_machine::check_for_dup
> > > > > > (sm_context
> > > > > > *sm_ctxt, const supernode *node,
> > > > > >  case DUP_1:
> > > > > >if (lhs)
> > > > > > {
> > > > > > - if (is_constant_fd_p (state_arg_1))
> > > > > > + if (is_constant_fd_p (state_arg_1) || state_arg_1
> > > > > > ==
> > > > > > m_start)
> > > > > > sm_ctxt->set_next_state (stmt, lhs,
> > > > > > m_unchecked_read_write);
> > > > > >   else
> > > > > > sm_ctxt->set_next_state (stmt, lhs,
> > > > > > @@ -1011,7 +1011,7 @@ fd_state_machine::check_for_dup
> > > > > > (sm_context
> > > > > > *sm_ctxt, const supernode *node,
> > > > > >file descriptor i.e the first argument.  */
> > > > > >if (lhs)
> > > > > > {
> > > > > > - if (is_constant_fd_p (state_arg_1))
> > > > > > + if (is_constant_fd_p (state_arg_1) || state_arg_1
> > > > > > ==
> > > > > > m_start)
> > > > > > sm_ctxt->set_next_state (stmt, lhs,
> > > > > > m_unchecked_read_write);
> > > > > >   else
> > > > > > sm_ctxt->set_next_state (stmt, lhs,
> > > > > > diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
> > > > > > b/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
> > > > > > index eba2570568f..ed4d6de57db 100644
> > > > > > --- a/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
> > > > > > +++ b/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
> > > > > > @@ -220,4 +220,30 @@ test_19 (const char *path, void *buf)
> > > > > >  close (fd);
> > > > > >  }
> > > > > >
> > > > > > -}
> > > > > > \ No newline at end of file
> > > > > > +}
> > > > > > +
> > > > > > +void
> > > > > > +test_20 ()
> > > > > > +{
> > > > > > +int m;
> > > > > > +int fd = dup (m); /* { dg-warning "'dup' on possibly
> > > > > > invalid
> > > > > > file descriptor 'm'" } */
> > > > > > +

Re: [PATCH] analyzer: fix ICE casued by dup2 in sm-fd.cc[PR106551]

[Mir Immad via Gcc-patches]

 > Can you please rebase and see if your patch
> does fix it?

No, the patch that I sent did not attempt to fix this. Now that I have made
the correction, XFAIL in fd-uninit-1.c has changed to XPASS.

Should i remove the dg-bogus warning from fd-uninit-1.c test_1?

Thanks.
Immad.


On Wed, Aug 10, 2022 at 10:26 PM David Malcolm  wrote:

> On Wed, 2022-08-10 at 20:34 +0530, Mir Immad wrote:
> >  > if you convert the "int m;" locals into an extern global, like in
> > > comment #0 of bug 106551, does that still trigger the crash on the
> > > unpatched sm-fd.cc?
> >
> > Yes, it does, since m would be in "m_start" state. I'm sending an
> > updated
> > patch.
>
> Great!
>
> Note that I recently committed a fix for bug 106573, which has an xfail
> on a dg-bogus to mark a false positive which your patch hopefully also
> fixes (in fd-uninit-1.c).  Can you please rebase and see if your patch
> does fix it?
>
> Thanks
> Dave
>
>
> >
> > Thanks
> > Immad.
> >
> > On Wed, Aug 10, 2022 at 1:32 AM David Malcolm 
> > wrote:
> >
> > > On Tue, 2022-08-09 at 21:42 +0530, Immad Mir wrote:
> > > > This patch fixes the ICE caused by valid_to_unchecked_state,
> > > > at analyzer/sm-fd.cc by handling the m_start state in
> > > > check_for_dup.
> > > >
> > > > Tested lightly on x86_64.
> > > >
> > > > gcc/analyzer/ChangeLog:
> > > > PR analyzer/106551
> > > > * sm-fd.cc (check_for_dup): handle the m_start
> > > > state when transitioning the state of LHS
> > > > of dup, dup2 and dup3 call.
> > > >
> > > > gcc/testsuite/ChangeLog:
> > > > * gcc.dg/analyzer/fd-dup-1.c: New testcases.
> > > >
> > > > Signed-off-by: Immad Mir 
> > > > ---
> > > >  gcc/analyzer/sm-fd.cc|  4 ++--
> > > >  gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c | 28
> > > > +++-
> > > >  2 files changed, 29 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
> > > > index 8bb76d72b05..c8b9930a7b6 100644
> > > > --- a/gcc/analyzer/sm-fd.cc
> > > > +++ b/gcc/analyzer/sm-fd.cc
> > > > @@ -983,7 +983,7 @@ fd_state_machine::check_for_dup (sm_context
> > > > *sm_ctxt, const supernode *node,
> > > >  case DUP_1:
> > > >if (lhs)
> > > > {
> > > > - if (is_constant_fd_p (state_arg_1))
> > > > + if (is_constant_fd_p (state_arg_1) || state_arg_1 ==
> > > > m_start)
> > > > sm_ctxt->set_next_state (stmt, lhs,
> > > > m_unchecked_read_write);
> > > >   else
> > > > sm_ctxt->set_next_state (stmt, lhs,
> > > > @@ -1011,7 +1011,7 @@ fd_state_machine::check_for_dup (sm_context
> > > > *sm_ctxt, const supernode *node,
> > > >file descriptor i.e the first argument.  */
> > > >if (lhs)
> > > > {
> > > > - if (is_constant_fd_p (state_arg_1))
> > > > + if (is_constant_fd_p (state_arg_1) || state_arg_1 ==
> > > > m_start)
> > > > sm_ctxt->set_next_state (stmt, lhs,
> > > > m_unchecked_read_write);
> > > >   else
> > > > sm_ctxt->set_next_state (stmt, lhs,
> > > > diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
> > > > b/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
> > > > index eba2570568f..ed4d6de57db 100644
> > > > --- a/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
> > > > +++ b/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
> > > > @@ -220,4 +220,30 @@ test_19 (const char *path, void *buf)
> > > >  close (fd);
> > > >  }
> > > >
> > > > -}
> > > > \ No newline at end of file
> > > > +}
> > > > +
> > > > +void
> > > > +test_20 ()
> > > > +{
> > > > +int m;
> > > > +int fd = dup (m); /* { dg-warning "'dup' on possibly invalid
> > > > file descriptor 'm'" } */
> > > > +close (fd);
> > > > +}
> > > > +
> > > > +void
> > > > +test_21 ()
> > > > +{
> > > > +int m;
> > > > +int fd = dup2 (m, 1); /* { dg-warning "'dup2' on possibly
> > > > invalid file descriptor 'm'" } */
> > > > +close (fd);
> > > > +}
> > > > +
> > > > +void
> > > > +test_22 (int flags)
> > > > +{
> > > > +int m;
> > > > +int fd = dup3 (m, 1, flags); /* { dg-warning "'dup3' on
> > > > possibly
> > > > invalid file descriptor 'm'" } */
> > > > +close (fd);
> > > > +}
> > >
> > > Thanks for the updated patch.
> > >
> > > The test cases looked suspicious to me - I was wondering why the
> > > analyzer doesn't complain about the uninitialized values being
> > > passed
> > > to the various dup functions as parameters.  So your test cases
> > > seem to
> > > have uncovered a hidden pre-existing bug in the analyzer's
> > > uninitialized value detection, which I've filed for myself to deal
> > > with
> > > as PR analyzer/106573.
> > >
> > > If you convert the "int m;" locals into an extern global, like in
> > > comment #0 of bug 106551, does that still trigger the crash on the
> > > unpatched sm-fd.cc?  If so, then that's greatly preferable as a
> > > regression test, since otherwise I'll have to modify that test case
> > > 

Re: [PATCH] analyzer: fix ICE casued by dup2 in sm-fd.cc[PR106551]

[Mir Immad via Gcc-patches]

 > if you convert the "int m;" locals into an extern global, like in
> comment #0 of bug 106551, does that still trigger the crash on the
> unpatched sm-fd.cc?

Yes, it does, since m would be in "m_start" state. I'm sending an updated
patch.

Thanks
Immad.

On Wed, Aug 10, 2022 at 1:32 AM David Malcolm  wrote:

> On Tue, 2022-08-09 at 21:42 +0530, Immad Mir wrote:
> > This patch fixes the ICE caused by valid_to_unchecked_state,
> > at analyzer/sm-fd.cc by handling the m_start state in
> > check_for_dup.
> >
> > Tested lightly on x86_64.
> >
> > gcc/analyzer/ChangeLog:
> > PR analyzer/106551
> > * sm-fd.cc (check_for_dup): handle the m_start
> > state when transitioning the state of LHS
> > of dup, dup2 and dup3 call.
> >
> > gcc/testsuite/ChangeLog:
> > * gcc.dg/analyzer/fd-dup-1.c: New testcases.
> >
> > Signed-off-by: Immad Mir 
> > ---
> >  gcc/analyzer/sm-fd.cc|  4 ++--
> >  gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c | 28
> > +++-
> >  2 files changed, 29 insertions(+), 3 deletions(-)
> >
> > diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
> > index 8bb76d72b05..c8b9930a7b6 100644
> > --- a/gcc/analyzer/sm-fd.cc
> > +++ b/gcc/analyzer/sm-fd.cc
> > @@ -983,7 +983,7 @@ fd_state_machine::check_for_dup (sm_context
> > *sm_ctxt, const supernode *node,
> >  case DUP_1:
> >if (lhs)
> > {
> > - if (is_constant_fd_p (state_arg_1))
> > + if (is_constant_fd_p (state_arg_1) || state_arg_1 ==
> > m_start)
> > sm_ctxt->set_next_state (stmt, lhs,
> > m_unchecked_read_write);
> >   else
> > sm_ctxt->set_next_state (stmt, lhs,
> > @@ -1011,7 +1011,7 @@ fd_state_machine::check_for_dup (sm_context
> > *sm_ctxt, const supernode *node,
> >file descriptor i.e the first argument.  */
> >if (lhs)
> > {
> > - if (is_constant_fd_p (state_arg_1))
> > + if (is_constant_fd_p (state_arg_1) || state_arg_1 ==
> > m_start)
> > sm_ctxt->set_next_state (stmt, lhs,
> > m_unchecked_read_write);
> >   else
> > sm_ctxt->set_next_state (stmt, lhs,
> > diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
> > b/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
> > index eba2570568f..ed4d6de57db 100644
> > --- a/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
> > +++ b/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
> > @@ -220,4 +220,30 @@ test_19 (const char *path, void *buf)
> >  close (fd);
> >  }
> >
> > -}
> > \ No newline at end of file
> > +}
> > +
> > +void
> > +test_20 ()
> > +{
> > +int m;
> > +int fd = dup (m); /* { dg-warning "'dup' on possibly invalid
> > file descriptor 'm'" } */
> > +close (fd);
> > +}
> > +
> > +void
> > +test_21 ()
> > +{
> > +int m;
> > +int fd = dup2 (m, 1); /* { dg-warning "'dup2' on possibly
> > invalid file descriptor 'm'" } */
> > +close (fd);
> > +}
> > +
> > +void
> > +test_22 (int flags)
> > +{
> > +int m;
> > +int fd = dup3 (m, 1, flags); /* { dg-warning "'dup3' on possibly
> > invalid file descriptor 'm'" } */
> > +close (fd);
> > +}
>
> Thanks for the updated patch.
>
> The test cases looked suspicious to me - I was wondering why the
> analyzer doesn't complain about the uninitialized values being passed
> to the various dup functions as parameters.  So your test cases seem to
> have uncovered a hidden pre-existing bug in the analyzer's
> uninitialized value detection, which I've filed for myself to deal with
> as PR analyzer/106573.
>
> If you convert the "int m;" locals into an extern global, like in
> comment #0 of bug 106551, does that still trigger the crash on the
> unpatched sm-fd.cc?  If so, then that's greatly preferable as a
> regression test, since otherwise I'll have to modify that test case
> when I fix bug 106573.
>
> Dave
>
>
>
>

Re: [PATCH] analyzer: fix ICE casued by dup2 in sm-fd.cc[PR106551]

[Mir Immad via Gcc-patches]

Thanks. I've added few testcases that use uninitialized ints in dup, dup2,
and dup3.

Immad.

On Tue, Aug 9, 2022 at 8:43 PM David Malcolm  wrote:

> On Tue, 2022-08-09 at 13:16 +0530, Immad Mir wrote:
> > This patch fixes the ICE caused by valid_to_unchecked_state,
> > at analyzer/sm-fd.cc by handling the m_start state in
> > check_for_dup.
> >
> > Tested lightly on x86_64.
> >
> > gcc/analyzer/ChangeLog:
> > PR analyzer/106551
> > * sm-fd.cc (check_for_dup): handle the m_start
> > state when transitioning the state of LHS
> > of dup, dup2 and dup3 call.
> >
> > Signed-off-by: Immad Mir 
> > ---
> >  gcc/analyzer/sm-fd.cc | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
> > index 8bb76d72b05..c8b9930a7b6 100644
> > --- a/gcc/analyzer/sm-fd.cc
> > +++ b/gcc/analyzer/sm-fd.cc
> > @@ -983,7 +983,7 @@ fd_state_machine::check_for_dup (sm_context
> > *sm_ctxt, const supernode *node,
> >  case DUP_1:
> >if (lhs)
> > {
> > - if (is_constant_fd_p (state_arg_1))
> > + if (is_constant_fd_p (state_arg_1) || state_arg_1 ==
> > m_start)
> > sm_ctxt->set_next_state (stmt, lhs,
> > m_unchecked_read_write);
> >   else
> > sm_ctxt->set_next_state (stmt, lhs,
> > @@ -1011,7 +1011,7 @@ fd_state_machine::check_for_dup (sm_context
> > *sm_ctxt, const supernode *node,
> >file descriptor i.e the first argument.  */
> >if (lhs)
> > {
> > - if (is_constant_fd_p (state_arg_1))
> > + if (is_constant_fd_p (state_arg_1) || state_arg_1 ==
> > m_start)
> > sm_ctxt->set_next_state (stmt, lhs,
> > m_unchecked_read_write);
> >   else
> > sm_ctxt->set_next_state (stmt, lhs,
>
> Thanks.  The fix looks reasonable, but please can the patch also add a
> reproducer to the test suite, covering each of the three dup/dup2/dup3
> entrypoints - presumably the one from the bug can be used/adapted.
>
> Dave
>
>

Re: [PATCH] analyzer: support for creat, dup, dup2 and dup3 [PR106298]

[Mir Immad via Gcc-patches]

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

Re: [PATCH] Adding three new function attributes for static analysis of file descriptors

[Mir Immad via Gcc-patches]

 > Sorry to nitpick -- I assume stmt here refers to a call stmt ?
> In that case, I suppose it'd be better to use const gcall *stmt ?

Thanks for the catch, Prathamesh.

Immad.


On Wed, Jul 20, 2022 at 11:59 PM Prathamesh Kulkarni <
prathamesh.kulka...@linaro.org> wrote:

> On Wed, 20 Jul 2022 at 23:31, Immad Mir via Gcc-patches
>  wrote:
> >
> > This patch adds three new function attributes to GCC that
> > are used for static analysis of usage of file descriptors:
> >
> > 1) __attribute__ ((fd_arg(N))): The attributes may be applied to a
> function that
> > takes on open file descriptor at refrenced argument N.
> >
> > It indicates that the passed filedescriptor must not have been closed.
> > Therefore, when the analyzer is enabled with -fanalyzer, the
> > analyzer may emit a -Wanalyzer-fd-use-after-close diagnostic
> > if it detects a code path in which a function with this attribute is
> > called with a closed file descriptor.
> >
> > The attribute also indicates that the file descriptor must have been
> checked for
> > validity before usage. Therefore, analyzer may emit
> > -Wanalyzer-fd-use-without-check diagnostic if it detects a code path in
> > which a function with this attribute is called with a file descriptor
> that has
> > not been checked for validity.
> >
> > 2) __attribute__((fd_arg_read(N))): The attribute is identical to
> > fd_arg, but with the additional requirement that it might read from
> > the file descriptor, and thus, the file descriptor must not have been
> opened
> > as write-only.
> >
> > The analyzer may emit a -Wanalyzer-access-mode-mismatch
> > diagnostic if it detects a code path in which a function with this
> > attribute is called on a file descriptor opened with O_WRONLY.
> >
> > 3) __attribute__((fd_arg_write(N))): The attribute is identical to
> fd_arg_read
> > except that the analyzer may emit a -Wanalyzer-access-mode-mismatch
> diagnostic if
> > it detects a code path in which a function with this attribute is called
> on a
> > file descriptor opened with O_RDONLY.
> >
> > gcc/analyzer/ChangeLog:
> > * sm-fd.cc (fd_param_diagnostic): New diagnostic class.
> > (fd_access_mode_mismatch): Change inheritance from fd_diagnostic
> > to fd_param_diagnostic. Add new overloaded constructor.
> > (fd_use_after_close): Likewise.
> > (unchecked_use_of_fd): Likewise and also change name to
> fd_use_without_check.
> > (double_close): Change name to fd_double_close.
> > (enum access_directions): New.
> > (fd_state_machine::on_stmt): Handle calls to function with the
> > new three function attributes.
> > (fd_state_machine::check_for_fd_attrs): New.
> > (fd_state_machine::on_open): Use the new overloaded constructors
> > of diagnostic classes.
> >
> > gcc/c-family/ChangeLog:
> > * c-attribs.cc: (c_common_attribute_table): add three new
> attributes
> > namely: fd_arg, fd_arg_read and fd_arg_write.
> > (handle_fd_arg_attribute): New.
> >
> > gcc/ChangeLog:
> > * doc/extend.texi: Add fd_arg, fd_arg_read and fd_arg_write under
> > "Common Function Attributes" section.
> > * doc/invoke.texi: Add docs to
> -Wanalyzer-fd-access-mode-mismatch,
> > -Wanalyzer-use-after-close, -Wanalyzer-fd-use-without-check that
> these
> > warnings may be emitted through usage of three function
> attributes used
> > for static analysis of file descriptors namely fd_arg,
> fd_arg_read and
> > fd_arg_write.
> >
> > gcc/testsuite/ChangeLog:
> > * gcc.dg/analyzer/fd-5.c: New test.
> > * c-c++-common/attr-fd.c: New test.
> >
> > Signed-off-by: Immad Mir 
> > ---
> >  gcc/analyzer/sm-fd.cc| 335 +--
> >  gcc/c-family/c-attribs.cc|  32 +++
> >  gcc/doc/extend.texi  |  37 +++
> >  gcc/doc/invoke.texi  |  17 +-
> >  gcc/testsuite/c-c++-common/attr-fd.c |  18 ++
> >  gcc/testsuite/gcc.dg/analyzer/fd-5.c |  53 +
> >  6 files changed, 422 insertions(+), 70 deletions(-)
> >  create mode 100644 gcc/testsuite/c-c++-common/attr-fd.c
> >  create mode 100644 gcc/testsuite/gcc.dg/analyzer/fd-5.c
> >
> > diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
> > index 8e4300b06e2..bb89c471f7e 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 access_directions
> > +{
> > +  DIRS_READ_WRITE,
> > +  DIRS_READ,
> > +  

Re: Adding file descriptor attribute(s) to gcc and glibc

[Mir Immad via Gcc]

Hi everyone,

Reading through the thread, I feel the following attributes look good and
similar to what I've done:

__attribute__ ((fd_arg(N)))
__attribute__ ((fd_arg_read(N)))
__attribute__ ((fd_arg_write(N)))

I believe how to annotate the glibc headers is a separate discussion.

Dave:  From the GCC side of things, these three attributes are basic
functionalities that can be useful for any piece of code that passes around
file descriptors.

Thanks
Immad


On Wed, Jul 13, 2022 at 7:31 PM Florian Weimer  wrote:

> * David Malcolm:
>
> > On Wed, 2022-07-13 at 14:05 +0200, Florian Weimer wrote:
> >> * Szabolcs Nagy via Gcc:
> >
> > [adding Immad back to the CC list]
> >
> >>
> >> > to be honest, i'd expect interesting fd bugs to be
> >> > dynamic and not easy to statically analyze.
> >> > the use-after-unchecked-open maybe useful. i would
> >> > not expect the access direction to catch many bugs.
> >>
> >> You might be right.  But I think the annotations could help to catch
> >> use-after-close errors.
> >>
> >> By the way, I think it would help us if we didn't have to special-
> >> case
> >> AT_FDCWD using inline wrappers.
> >
> > Florian: I confess I wasn't familiar with AT_FDCWD until I read your
> > email and did a little reading a few minutes ago; it seems to be a
> > "magic number" for an FD that has special meaning; on my system it has
> > the value -100.
> >
> > GCC's current implementation of the various -Wanalyzer-fd-* warnings
> > will track state for constant integer values as well as symbolic
> > values; it doesn't have any special meanings for specific integer
> > values.  So e.g. it doesn't assume that 0, 1, and 2 have specific
> > meaning or are opened with specific flags (the analysis doesn't
> > necessarily begin its execution path at the start of "main", so there's
> > no guarantee that the standard FDs have their standard meaning).
>
> Ahh.  It might be useful to detect close (-1) etc. as a form of
> double-close, and ther AT_FDCWD is exceptional.
>
> > Presumably if someone attempts
> >   close (AT_FDCWD);
> > they'll get -1 and errno set to EBADFD, right?
>
> Correct
>
> > I don't think GCC's -fanalyzer needs to check for that.
>
> Not sure …
>
> > -fanalyzer's filedescriptor support doesn't yet have a concept of
> > "directory filedescriptors".  Should it?  (similarly, it doesn't yet
> > know about sockets)
> >
> > A possible way to annotate "openat":
> >
> >   int openat(int dirfd, const char *pathname, int flags)
> > __attr_fd_arg(1);
>
> openat is not the most general interface in this regard.  We have other
> *at functions which accept an O_PATH descriptor (or maybe even a
> different kind of non-directory descriptor) with pathname == "" and
> AT_EMPTY_PATH.  I'm not sure if modeling all this is beneficial.
>
> Thanks,
> Florian
>
>

Re: [PATCH] filedescriptor attribute

[Mir Immad via Gcc]

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

[PATCH 2/2] analyzer: reorder initialization of state m_invalid in sm-fd.cc [PR106184]

[Mir Immad via Gcc-patches]

>From 3de908fa0c3e515b49df24460f85924211802d6c Mon Sep 17 00:00:00 2001
From: Immad Mir 
Date: Tue, 5 Jul 2022 21:21:13 +0530
Subject: [PATCH 2/2] analyzer: reorder initialization of state m_invalid in
 sm-fd.cc [PR106184]

This patch reorders the initization of state m_invalid in sm-fd.cc
to conform with standard practice in C++.

gcc/analyzer/ChangeLog:
PR analyzer/106184
* sm-fd.cc (fd_state_machine): Change ordering of intilization
of state m_invalid.

Signed-off-by: Immad Mir 
---
 gcc/analyzer/sm-fd.cc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
index a85c95cc502..610261bdde5 100644
--- a/gcc/analyzer/sm-fd.cc
+++ b/gcc/analyzer/sm-fd.cc
@@ -551,11 +551,12 @@ fd_state_machine::fd_state_machine (logger *logger)
   m_unchecked_read_write (add_state ("fd-unchecked-read-write")),
   m_unchecked_read_only (add_state ("fd-unchecked-read-only")),
   m_unchecked_write_only (add_state ("fd-unchecked-write-only")),
-  m_invalid (add_state ("fd-invalid")),
   m_valid_read_write (add_state ("fd-valid-read-write")),
   m_valid_read_only (add_state ("fd-valid-read-only")),
   m_valid_write_only (add_state ("fd-valid-write-only")),
-  m_closed (add_state ("fd-closed")), m_stop (add_state ("fd-stop"))
+  m_invalid (add_state ("fd-invalid")),
+  m_closed (add_state ("fd-closed")),
+  m_stop (add_state ("fd-stop"))
 {
 }

-- 
2.25.1

[PATCH 1/2] analyzer: show close event for use_after_close diagnostic

[Mir Immad via Gcc-patches]

>From be60d5194068355ccdbf832d0de9dbfed1e0b074 Mon Sep 17 00:00:00 2001
From: Immad Mir 
Date: Tue, 5 Jul 2022 21:14:06 +0530
Subject: [PATCH 1/2] analyzer: show close event for use_after_close
diagnostic

This patch saves the "close" event in use_after_close diagnostic
and shows it where possible.

gcc/analyzer/ChangeLog:
* sm-fd.cc (use_after_close): save the "close" event and
show it where possible.

gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/fd-4.c (test_3): change the message note to conform to the
changes in analyzer/sm-fd.cc
(test_4): Likewise.

Signed-off-by: Immad Mir 
---
 gcc/analyzer/sm-fd.cc| 13 +++--
 gcc/testsuite/gcc.dg/analyzer/fd-4.c |  4 ++--
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
index 4058ac53308..a85c95cc502 100644
--- a/gcc/analyzer/sm-fd.cc
+++ b/gcc/analyzer/sm-fd.cc
@@ -454,7 +454,10 @@ public:
   return label_text::borrow ("opened here");

 if (change.m_new_state == m_sm.m_closed)
+{
+  m_first_close_event = change.m_event_id;
return change.formatted_print ("closed here");
+}

 return fd_diagnostic::describe_state_change (change);
   }
@@ -462,11 +465,17 @@ public:
   label_text
   describe_final_event (const evdesc::final_event ) final override
   {
-return ev.formatted_print ("%qE on closed file descriptor %qE here",
-   m_callee_fndecl, m_arg);
+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", _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;
 };

diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-4.c
b/gcc/testsuite/gcc.dg/analyzer/fd-4.c
index a973704f403..c992db619e7 100644
--- a/gcc/testsuite/gcc.dg/analyzer/fd-4.c
+++ b/gcc/testsuite/gcc.dg/analyzer/fd-4.c
@@ -45,7 +45,7 @@ test_3 (const char *path, void *buf)
 {
 close(fd); /* {dg-message "\\(2\\) closed here"} */
 read(fd, buf, 1); /* { dg-warning "'read' on closed file
descriptor 'fd'" }  */
-/* {dg-message "\\(3\\) 'read' on closed file descriptor 'fd'
here" "" {target *-*-*} .-1 } */
+/* {dg-message "\\(3\\) 'read' on closed file descriptor 'fd';
'close' was at \\(2\\)" "" {target *-*-*} .-1 } */
 }
 }

@@ -57,6 +57,6 @@ test_4 (const char *path, void *buf)
 {
 close(fd); /* {dg-message "\\(2\\) closed here"} */
 write(fd, buf, 1); /* { dg-warning "'write' on closed file
descriptor 'fd'" }  */
-/* {dg-message "\\(3\\) 'write' on closed file descriptor 'fd'
here" "" {target *-*-*} .-1 } */
+/* {dg-message "\\(3\\) 'write' on closed file descriptor 'fd';
'close' was at \\(2\\)" "" {target *-*-*} .-1 } */
 }
 }
-- 
2.25.1

Re: [PATCH] PR 106003

[Mir Immad via Gcc]

Thank you.
I've committed the patch, and it is covered by the  Developer Certificate
of Origin (DCO).

Immad.

On Sat, Jul 2, 2022 at 9:02 PM David Malcolm  wrote:

> On Sat, 2022-07-02 at 19:34 +0530, Mir Immad wrote:
> > From 62b7b7736975172f03b30783436fbc9217324223 Mon Sep 17 00:00:00 2001
> > From: mir 
> > Date: Sat, 2 Jul 2022 15:04:37 +0530
> > Subject: [PATCH] analyzer: implement five new warnings for misuse of
> > POSIX
> >  file descriptor APIs [PR106003].
> >
> > This patch adds a new state machine to the analyzer for checking usage
> > of
> > POSIX file descriptor
> > APIs with five new warnings.
> >
> > It adds:
> > - check for FD leaks (CWE 775).
> > - check for double "close" of a FD (CWE-1341).
> > - check for read/write of a closed file descriptor.
> > - check whether a file descriptor was used without being checked for
> > validity.
> > - check for read/write of a descriptor opened for just writing/reading.
> >
> > gcc/ChangeLog:
> > PR analyzer/106003
> > * Makefile.in (ANALYZER_OBJS): Add sm-fd.o.
> > * doc/invoke.texi:  Add -Wanalyzer-fd-double-close, -Wanalyzer-fd-leak,
> > -Wanalyzer-fd-access-mode-mismatch, -Wanalyzer-fd-use-without-check,
> > -Wanalyzer-fd-use-after-close.
> >
> > gcc/analyzer/ChangeLog:
> > PR analyzer/106003
> > * analyzer.opt (Wanalyzer-fd-leak): New option.
> > (Wanalyzer-fd-access-mode-mismatch): New option.
> > (Wanalyzer-fd-use-without-check): New option.
> > (Wanalyzer-fd-double-close): New option.
> > (Wanalyzer-fd-use-after-close): New option.
> > * sm.h (make_fd_state_machine): New decl.
> > * sm.cc (make_checkers): Call make_fd_state_machine.
> > * sm-fd.cc: New file.
> >
> > gcc/testsuite/ChangeLog:
> > PR analyzer/106003
> > * gcc.dg/analyzer/fd-1.c: New test.
> > * gcc.dg/analyzer/fd-2.c: New test.
> > * gcc.dg/analyzer/fd-3.c: New test.
> > * gcc.dg/analyzer/fd-4.c: New test.
>
> [...snip...]
>
> Hi Immad.
>
> Thanks for the updated patch.
>
> For everyone else, we've been discussing this patch off-list.  We've
> had some issues with gmail mangling patches; FWIW a pristine version of
> the patch can be seen at:
>   https://mirimmad.github.io/patch-02-07.txt
>
> As discussed off-list, you've successfully bootstrapped this patch and
> run the testsuite without regressions (and a bunch of extra PASSes), so
> this patch is ready for you to push it to the "master" git branch (aka
> trunk).  Please go ahead with that (or let me know if you need help
> [1]).
>
> Note that Tim's first analyzer patch is also ready to push, so there's
> a chance that your patches might conflict with each other (though I
> think you're touching different areas of the analyzer, so I'm hoping
> that won't happen).
>
> There's plenty of scope for followups, such as adding attributes for
> parameters that expect an open file-descriptor, or for handling socket
> APIs, etc.  Also, Murphy's Law means that there's sure to be at least
> something we missed in review :/
>
> Let's move followup patches to the gcc-patches mailing list, rather
> than the "gcc" list.
>
> Thanks
> Dave
>
> [1] though I'll only be checking email intermittently this weekend and
> on Monday (which is a holiday here in the USA).
>
>

Re: [PATCH] PR 106003

[Mir Immad via Gcc]

Hi everyone,

This is a patch for PR 106003
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=106003; tested on x86_64.

On Sat, Jul 2, 2022 at 7:34 PM Mir Immad  wrote:

> From 62b7b7736975172f03b30783436fbc9217324223 Mon Sep 17 00:00:00 2001
> From: mir 
> Date: Sat, 2 Jul 2022 15:04:37 +0530
> Subject: [PATCH] analyzer: implement five new warnings for misuse of POSIX
>  file descriptor APIs [PR106003].
>
> This patch adds a new state machine to the analyzer for checking usage of
> POSIX file descriptor
> APIs with five new warnings.
>
> It adds:
> - check for FD leaks (CWE 775).
> - check for double "close" of a FD (CWE-1341).
> - check for read/write of a closed file descriptor.
> - check whether a file descriptor was used without being checked for
> validity.
> - check for read/write of a descriptor opened for just writing/reading.
>
> gcc/ChangeLog:
> PR analyzer/106003
> * Makefile.in (ANALYZER_OBJS): Add sm-fd.o.
> * doc/invoke.texi:  Add -Wanalyzer-fd-double-close, -Wanalyzer-fd-leak,
> -Wanalyzer-fd-access-mode-mismatch, -Wanalyzer-fd-use-without-check,
> -Wanalyzer-fd-use-after-close.
>
> gcc/analyzer/ChangeLog:
> PR analyzer/106003
> * analyzer.opt (Wanalyzer-fd-leak): New option.
> (Wanalyzer-fd-access-mode-mismatch): New option.
> (Wanalyzer-fd-use-without-check): New option.
> (Wanalyzer-fd-double-close): New option.
> (Wanalyzer-fd-use-after-close): New option.
> * sm.h (make_fd_state_machine): New decl.
> * sm.cc (make_checkers): Call make_fd_state_machine.
> * sm-fd.cc: New file.
>
> gcc/testsuite/ChangeLog:
> PR analyzer/106003
> * gcc.dg/analyzer/fd-1.c: New test.
> * gcc.dg/analyzer/fd-2.c: New test.
> * gcc.dg/analyzer/fd-3.c: New test.
> * gcc.dg/analyzer/fd-4.c: New test.
> ---
>  gcc/Makefile.in  |   1 +
>  gcc/analyzer/analyzer.opt|  20 +
>  gcc/analyzer/sm-fd.cc| 847 +++
>  gcc/analyzer/sm.cc   |   1 +
>  gcc/analyzer/sm.h|   1 +
>  gcc/doc/invoke.texi  |  55 ++
>  gcc/testsuite/gcc.dg/analyzer/fd-1.c |  39 ++
>  gcc/testsuite/gcc.dg/analyzer/fd-2.c |  49 ++
>  gcc/testsuite/gcc.dg/analyzer/fd-3.c |  85 +++
>  gcc/testsuite/gcc.dg/analyzer/fd-4.c |  62 ++
>  10 files changed, 1160 insertions(+)
>  create mode 100644 gcc/analyzer/sm-fd.cc
>  create mode 100644 gcc/testsuite/gcc.dg/analyzer/fd-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/analyzer/fd-2.c
>  create mode 100644 gcc/testsuite/gcc.dg/analyzer/fd-3.c
>  create mode 100644 gcc/testsuite/gcc.dg/analyzer/fd-4.c
>
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index b6dcc45a58a..04631f737ea 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1269,6 +1269,7 @@ ANALYZER_OBJS = \
>   analyzer/region-model-reachability.o \
>   analyzer/sm.o \
>   analyzer/sm-file.o \
> + analyzer/sm-fd.o \
>   analyzer/sm-malloc.o \
>   analyzer/sm-pattern-test.o \
>   analyzer/sm-sensitive.o \
> diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
> index 23dfc797cea..479990b0e5e 100644
> --- a/gcc/analyzer/analyzer.opt
> +++ b/gcc/analyzer/analyzer.opt
> @@ -66,6 +66,26 @@ Wanalyzer-exposure-through-output-file
>  Common Var(warn_analyzer_exposure_through_output_file) Init(1) Warning
>  Warn about code paths in which sensitive data is written to a file.
>
> +Wanalyzer-fd-access-mode-mismatch
> +Common Var(warn_analyzer_fd_mode_mismatch) Init(1) Warning
> +Warn about code paths in which read on a write-only file descriptor is
> attempted, or vice versa.
> +
> +Wanalyzer-fd-double-close
> +Common Var(warn_analyzer_fd_double_close) Init(1) Warning
> +Warn about code paths in which a file descriptor can be closed more than
> once.
> +
> +Wanalyzer-fd-leak
> +Common Var(warn_analyzer_fd_leak) Init(1) Warning
> +Warn about code paths in which a file descriptor is not closed.
> +
> +Wanalyzer-fd-use-after-close
> +Common Var(warn_analyzer_fd_use_after_close) Init(1) Warning
> +Warn about code paths in which a read or write is performed on a closed
> file descriptor.
> +
> +Wanalyzer-fd-use-without-check
> +Common Var(warn_analyzer_fd_use_without_check) Init(1) Warning
> +Warn about code paths in which a file descriptor is used without being
> checked for validity.
> +
>  Wanalyzer-file-leak
>  Common Var(warn_analyzer_file_leak) Init(1) Warning
>  Warn about code paths in which a stdio FILE is not closed.
> diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
> new file mode 100644
> index 000..4058ac53308
> --- /dev/null
> +++ b/gcc/analyzer/sm-fd.cc
> @@ -0,0 +1,847 @@
> +/* A state machine for detecting misuses of POSIX file descriptor APIs.
> +   Copyright (C) 2019-2022 Free Software Foundation, Inc.
> +   Contributed by Immad Mir .
> +
> +This file is part of GCC.
> +
> +GCC is free software; you can redistribute it and/or modify it
> +under the terms of the GNU General Public License as published by
> +the Free Software Foundation; either version 3, or (at your option)
> +any later 

[PATCH] PR 106003

[Mir Immad via Gcc]

>From 62b7b7736975172f03b30783436fbc9217324223 Mon Sep 17 00:00:00 2001
From: mir 
Date: Sat, 2 Jul 2022 15:04:37 +0530
Subject: [PATCH] analyzer: implement five new warnings for misuse of POSIX
 file descriptor APIs [PR106003].

This patch adds a new state machine to the analyzer for checking usage of
POSIX file descriptor
APIs with five new warnings.

It adds:
- check for FD leaks (CWE 775).
- check for double "close" of a FD (CWE-1341).
- check for read/write of a closed file descriptor.
- check whether a file descriptor was used without being checked for
validity.
- check for read/write of a descriptor opened for just writing/reading.

gcc/ChangeLog:
PR analyzer/106003
* Makefile.in (ANALYZER_OBJS): Add sm-fd.o.
* doc/invoke.texi:  Add -Wanalyzer-fd-double-close, -Wanalyzer-fd-leak,
-Wanalyzer-fd-access-mode-mismatch, -Wanalyzer-fd-use-without-check,
-Wanalyzer-fd-use-after-close.

gcc/analyzer/ChangeLog:
PR analyzer/106003
* analyzer.opt (Wanalyzer-fd-leak): New option.
(Wanalyzer-fd-access-mode-mismatch): New option.
(Wanalyzer-fd-use-without-check): New option.
(Wanalyzer-fd-double-close): New option.
(Wanalyzer-fd-use-after-close): New option.
* sm.h (make_fd_state_machine): New decl.
* sm.cc (make_checkers): Call make_fd_state_machine.
* sm-fd.cc: New file.

gcc/testsuite/ChangeLog:
PR analyzer/106003
* gcc.dg/analyzer/fd-1.c: New test.
* gcc.dg/analyzer/fd-2.c: New test.
* gcc.dg/analyzer/fd-3.c: New test.
* gcc.dg/analyzer/fd-4.c: New test.
---
 gcc/Makefile.in  |   1 +
 gcc/analyzer/analyzer.opt|  20 +
 gcc/analyzer/sm-fd.cc| 847 +++
 gcc/analyzer/sm.cc   |   1 +
 gcc/analyzer/sm.h|   1 +
 gcc/doc/invoke.texi  |  55 ++
 gcc/testsuite/gcc.dg/analyzer/fd-1.c |  39 ++
 gcc/testsuite/gcc.dg/analyzer/fd-2.c |  49 ++
 gcc/testsuite/gcc.dg/analyzer/fd-3.c |  85 +++
 gcc/testsuite/gcc.dg/analyzer/fd-4.c |  62 ++
 10 files changed, 1160 insertions(+)
 create mode 100644 gcc/analyzer/sm-fd.cc
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/fd-1.c
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/fd-2.c
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/fd-3.c
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/fd-4.c

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index b6dcc45a58a..04631f737ea 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1269,6 +1269,7 @@ ANALYZER_OBJS = \
  analyzer/region-model-reachability.o \
  analyzer/sm.o \
  analyzer/sm-file.o \
+ analyzer/sm-fd.o \
  analyzer/sm-malloc.o \
  analyzer/sm-pattern-test.o \
  analyzer/sm-sensitive.o \
diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index 23dfc797cea..479990b0e5e 100644
--- a/gcc/analyzer/analyzer.opt
+++ b/gcc/analyzer/analyzer.opt
@@ -66,6 +66,26 @@ Wanalyzer-exposure-through-output-file
 Common Var(warn_analyzer_exposure_through_output_file) Init(1) Warning
 Warn about code paths in which sensitive data is written to a file.

+Wanalyzer-fd-access-mode-mismatch
+Common Var(warn_analyzer_fd_mode_mismatch) Init(1) Warning
+Warn about code paths in which read on a write-only file descriptor is
attempted, or vice versa.
+
+Wanalyzer-fd-double-close
+Common Var(warn_analyzer_fd_double_close) Init(1) Warning
+Warn about code paths in which a file descriptor can be closed more than
once.
+
+Wanalyzer-fd-leak
+Common Var(warn_analyzer_fd_leak) Init(1) Warning
+Warn about code paths in which a file descriptor is not closed.
+
+Wanalyzer-fd-use-after-close
+Common Var(warn_analyzer_fd_use_after_close) Init(1) Warning
+Warn about code paths in which a read or write is performed on a closed
file descriptor.
+
+Wanalyzer-fd-use-without-check
+Common Var(warn_analyzer_fd_use_without_check) Init(1) Warning
+Warn about code paths in which a file descriptor is used without being
checked for validity.
+
 Wanalyzer-file-leak
 Common Var(warn_analyzer_file_leak) Init(1) Warning
 Warn about code paths in which a stdio FILE is not closed.
diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
new file mode 100644
index 000..4058ac53308
--- /dev/null
+++ b/gcc/analyzer/sm-fd.cc
@@ -0,0 +1,847 @@
+/* A state machine for detecting misuses of POSIX file descriptor APIs.
+   Copyright (C) 2019-2022 Free Software Foundation, Inc.
+   Contributed by Immad Mir .
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+

[PATCH] analyzer PR 106003

[Mir Immad via Gcc]

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index b6dcc45a58a..04631f737ea 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1269,6 +1269,7 @@ ANALYZER_OBJS = \
  analyzer/region-model-reachability.o \
  analyzer/sm.o \
  analyzer/sm-file.o \
+ analyzer/sm-fd.o \
  analyzer/sm-malloc.o \
  analyzer/sm-pattern-test.o \
  analyzer/sm-sensitive.o \
diff --git a/gcc/analyzer/ChangeLog b/gcc/analyzer/ChangeLog
index 53b3ffb487b..d30e94f2f62 100644
--- a/gcc/analyzer/ChangeLog
+++ b/gcc/analyzer/ChangeLog
@@ -1,3 +1,6 @@
+2022-06-27 Immad Mir 
+ * sm-fd.cc: New file.
+
 2022-06-02  David Malcolm  

  * checker-path.cc (checker_event::get_meaning): New.
diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index 23dfc797cea..219f6180130 100644
--- a/gcc/analyzer/analyzer.opt
+++ b/gcc/analyzer/analyzer.opt
@@ -66,6 +66,26 @@ Wanalyzer-exposure-through-output-file
 Common Var(warn_analyzer_exposure_through_output_file) Init(1) Warning
 Warn about code paths in which sensitive data is written to a file.

+Wanalyzer-fd-access-mode-mismatch
+Common Var(warn_analyzer_fd_mode_mismatch) Init(1) Warning
+Warn about code paths in which read on write-only file descriptor is
attempted, or vice versa.
+
+Wanalyzer-fd-double-close
+Common Var(warn_analyzer_fd_double_close) Init(1) Warning
+Warn about code paths in which a file descriptor can be closed more than
once.
+
+Wanalyzer-fd-leak
+Common Var(warn_analyzer_fd_leak) Init(1) Warning
+Warn about code paths in which a file descriptor is not closed.
+
+Wanalyzer-fd-use-after-close
+Common Var(warn_analyzer_fd_use_after_close) Init(1) Warning
+Warn about code paths in which a read or write is performed on a closed
file descriptor.
+
+Wanalyzer-fd-use-without-check
+Common Var(warn_analyzer_fd_use_without_check) Init(1) Warning
+Warn about code paths in which a file descriptor is used without being
checked for validity.
+
 Wanalyzer-file-leak
 Common Var(warn_analyzer_file_leak) Init(1) Warning
 Warn about code paths in which a stdio FILE is not closed.
diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
new file mode 100644
index 000..0e9833a55a8
--- /dev/null
+++ b/gcc/analyzer/sm-fd.cc
@@ -0,0 +1,821 @@
+/* A state machine for detecting misuses of POSIX file descriptor APIs.
+   Copyright (C) 2019-2022 Free Software Foundation, Inc.
+   Contributed by Immad Mir .
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tree.h"
+#include "function.h"
+#include "basic-block.h"
+#include "gimple.h"
+#include "options.h"
+#include "diagnostic-path.h"
+#include "diagnostic-metadata.h"
+#include "function.h"
+#include "json.h"
+#include "analyzer/analyzer.h"
+#include "diagnostic-event-id.h"
+#include "analyzer/analyzer-logging.h"
+#include "analyzer/sm.h"
+#include "analyzer/pending-diagnostic.h"
+#include "analyzer/function-set.h"
+#include "analyzer/analyzer-selftests.h"
+#include "tristate.h"
+#include "selftest.h"
+#include "analyzer/call-string.h"
+#include "analyzer/program-point.h"
+#include "analyzer/store.h"
+#include "analyzer/region-model.h"
+
+#include 
+#if ENABLE_ANALYZER
+
+namespace ana
+{
+
+namespace
+{
+
+/* An enum for distinguishing between three different access modes. */
+
+enum access_mode
+{
+  READ_WRITE,
+  READ_ONLY,
+  WRITE_ONLY
+};
+
+/* An enum for describing the direction of an access to a file descriptor.
 */
+
+enum access_direction
+{
+  DIR_READ,
+  DIR_WRITE
+};
+
+class fd_state_machine : public state_machine
+{
+public:
+  fd_state_machine (logger *logger);
+
+  bool
+  inherited_state_p () const final override
+  {
+return false;
+  }
+
+  state_machine::state_t
+  get_default_state (const svalue *sval) const final override
+  {
+if (tree cst = sval->maybe_get_constant ())
+  {
+int val = TREE_INT_CST_LOW (cst);
+if (val >= 0)
+  return m_constant_fd;
+else
+  return m_invalid;
+  }
+return m_start;
+  }
+
+  bool on_stmt (sm_context *sm_ctxt, const supernode *node,
+const gimple *stmt) const final override;
+
+  void on_condition (sm_context *sm_ctxt, const supernode *node,
+ const gimple *stmt, const svalue *lhs, const
tree_code op,
+ const svalue *rhs) const final override;
+
+  bool can_purge_p (state_t s) const final override;
+  

Re: [PATCH] analyzer PR 106003

[Mir Immad via Gcc]

Thanks for the suggestions, Dave.

+}
> +  else
> +{
> +  /* FIXME: add leak reporting */
> +}
> +}
> +

> Please add a testcase for this, with an "xfail" in the dg-warning
> directive.

I fixed it and made other suggested changes. All the tests (for fds) are
passing.

Sending an updated patch in a separate email.

Thanks.

Immad.

On Sun, Jun 26, 2022 at 9:23 PM David Malcolm  wrote:

> Thanks for the updated patch.
>
> Various comments inline below.
>
> Sorry if this seems nitpicky in places.
>
> I think the patch is nearly ready; please write a ChangeLog entry for
> the next one (none of my proposed changes are going to affect what the
> ChangeLog will look like, apart from new test case files perhaps, given
> that much of it is going to be just:
>
> * sm-fd.cc: New file.
>
> [...snip...]
>
> > diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
> > new file mode 100644
> > index 000..83eb0df724d
> > --- /dev/null
> > +++ b/gcc/analyzer/sm-fd.cc
>
> [...snip...]
>
> > +class fd_leak : public fd_diagnostic
> > +{
> > +public:
> > +  fd_leak (const fd_state_machine , tree arg) : fd_diagnostic (sm,
> arg)
> > {}
> > +
> > +  const char *
> > +  get_kind () const final override
> > +  {
> > +return "fd_leak";
> > +  }
> > +
> > +  int
> > +  get_controlling_option () const final override
> > +  {
> > +return OPT_Wanalyzer_fd_leak;
> > +  }
> > +
> > +  bool
> > +  emit (rich_location *rich_loc) final override
> > +  {
> > +/*CWE-775: Missing Release of File Descriptor or Handle after
> Effective
> > + * Lifetime
> > + */
>
> Formatting nit: we don't put these "*" on contination lines in
> multiline comments; this should read:
>
> > +/* CWE-775: Missing Release of File Descriptor or Handle after
> Effective
> > +   Lifetime.  */
>
>
> [...snip...]
>
> > +class fd_access_mode_mismatch : public fd_diagnostic
> > +{
> > +public:
> > +  /* mode specfies whether read on write was attempted or vice versa.
>
> Typo: "specfies" -> "specifies"
>
> [...snip...]
>
> > +  bool
> > +  emit (rich_location *rich_loc) final override
> > +  {
> > +
> > +switch (m_mode)
> > +  {
>
> I like to put in a "default: gcc_unreachable ();" in switch statements,
> especially one that isn't covering all of the values in the enum, but
> this got me thinking: this class has "enum access_mode m_mode;", but
> that enum is describing a *set* of ways in which the FD can be validly
> accessed, whereas here we're trying to describe a particular way in
> which the FD is being invalidly accessed.
>
> analyzer.h has this enum:
>
> /* An enum for describing the direction of an access to memory.  */
>
> enum access_direction
> {
>   DIR_READ,
>   DIR_WRITE
> };
>
> which I think would be a much better fit here for
> fd_access_mode_mismatch, so please change fd_access_mode_mismatch's:
>   enum access_mode m_mode;
> to
>   enum access_direction m_access_dir;
> and update this switch accordingly:
>
> > +  case READ_ONLY:
> > +return warning_at (rich_loc, get_controlling_option (),
> > +   "%qE on % file descriptor %qE",
> > +   m_callee_fndecl, m_arg);
> > +  case WRITE_ONLY:
> > +return warning_at (rich_loc, get_controlling_option (),
> > +   "%qE on % file descriptor %qE",
> > +   m_callee_fndecl, m_arg);
> > +  }
>
> [...snip...]
>
> > +
> > +  label_text
> > +  describe_state_change (const evdesc::state_change ) override
> > +  {
> > +return fd_diagnostic::describe_state_change (change);
> > +  }
>
> This vfunc is redundant; it can be deleted so we simply inherit the
> fd_diagnostic implementation directly.
>
> > +
> > +  label_text
> > +  describe_final_event (const evdesc::final_event ) final override
> > +  {
> > +switch (m_mode)
> > +  {
> > +  case READ_ONLY:
> > +return ev.formatted_print ("%qE on % file descriptor
> > %qE",
> > +   m_callee_fndecl, m_arg);
> > +  case WRITE_ONLY:
> > +return ev.formatted_print ("%qE on % file
> descriptor
> > %qE",
> > +   m_callee_fndecl, m_arg);
> > +  }
> > +  }
>
> Similar comments here about the switch (m_mode) as per the emit vfunc.
>
>
> > +
> > +private:
> > +  enum access_mode m_mode;
>
> ...and this field.
>
> > +  const tree m_callee_fndecl;
> > +};
>
>
> [...snip...]
>
> > +  label_text
> > +  describe_state_change (const evdesc::state_change ) override
> > +  {
> > +if (m_sm.is_unchecked (change.m_new_state))
> > +  {
> > +return label_text::borrow ("opened here");
> > +  }
> > +
> > +if (change.m_new_state == m_sm.m_closed)
> > +  {
> > +return change.formatted_print ("closed here");
> > +  }
>
> Formatting nit: we don't add the { } when they're redundant, like in
> these "if" stmts (I dislike this rule in our conventions, but it's
> probably best to be 

[PATCH] analyzer PR 106003

[Mir Immad via Gcc]

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index b6dcc45a58a..04631f737ea 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1269,6 +1269,7 @@ ANALYZER_OBJS = \
  analyzer/region-model-reachability.o \
  analyzer/sm.o \
  analyzer/sm-file.o \
+ analyzer/sm-fd.o \
  analyzer/sm-malloc.o \
  analyzer/sm-pattern-test.o \
  analyzer/sm-sensitive.o \
diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index 23dfc797cea..219f6180130 100644
--- a/gcc/analyzer/analyzer.opt
+++ b/gcc/analyzer/analyzer.opt
@@ -66,6 +66,26 @@ Wanalyzer-exposure-through-output-file
 Common Var(warn_analyzer_exposure_through_output_file) Init(1) Warning
 Warn about code paths in which sensitive data is written to a file.

+Wanalyzer-fd-access-mode-mismatch
+Common Var(warn_analyzer_fd_mode_mismatch) Init(1) Warning
+Warn about code paths in which read on write-only file descriptor is
attempted, or vice versa.
+
+Wanalyzer-fd-double-close
+Common Var(warn_analyzer_fd_double_close) Init(1) Warning
+Warn about code paths in which a file descriptor can be closed more than
once.
+
+Wanalyzer-fd-leak
+Common Var(warn_analyzer_fd_leak) Init(1) Warning
+Warn about code paths in which a file descriptor is not closed.
+
+Wanalyzer-fd-use-after-close
+Common Var(warn_analyzer_fd_use_after_close) Init(1) Warning
+Warn about code paths in which a read or write is performed on a closed
file descriptor.
+
+Wanalyzer-fd-use-without-check
+Common Var(warn_analyzer_fd_use_without_check) Init(1) Warning
+Warn about code paths in which a file descriptor is used without being
checked for validity.
+
 Wanalyzer-file-leak
 Common Var(warn_analyzer_file_leak) Init(1) Warning
 Warn about code paths in which a stdio FILE is not closed.
diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
new file mode 100644
index 000..83eb0df724d
--- /dev/null
+++ b/gcc/analyzer/sm-fd.cc
@@ -0,0 +1,821 @@
+/* A state machine for detecting misuses of POSIX file descriptor APIs.
+   Copyright (C) 2019-2022 Free Software Foundation, Inc.
+   Contributed by Immad Mir .
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tree.h"
+#include "function.h"
+#include "basic-block.h"
+#include "gimple.h"
+#include "options.h"
+#include "diagnostic-path.h"
+#include "diagnostic-metadata.h"
+#include "function.h"
+#include "json.h"
+#include "analyzer/analyzer.h"
+#include "diagnostic-event-id.h"
+#include "analyzer/analyzer-logging.h"
+#include "analyzer/sm.h"
+#include "analyzer/pending-diagnostic.h"
+#include "analyzer/function-set.h"
+#include "analyzer/analyzer-selftests.h"
+#include "tristate.h"
+#include "selftest.h"
+#include "analyzer/call-string.h"
+#include "analyzer/program-point.h"
+#include "analyzer/store.h"
+#include "analyzer/region-model.h"
+
+#include 
+#if ENABLE_ANALYZER
+
+namespace ana
+{
+
+namespace
+{
+
+/* An enum for distinguishing between three different access modes. */
+
+enum access_mode
+{
+  READ_WRITE,
+  READ_ONLY,
+  WRITE_ONLY
+};
+
+class fd_state_machine : public state_machine
+{
+public:
+  fd_state_machine (logger *logger);
+
+  bool
+  inherited_state_p () const final override
+  {
+return false;
+  }
+
+  state_machine::state_t
+  get_default_state (const svalue *sval) const final override
+  {
+if (tree cst = sval->maybe_get_constant ())
+  {
+int val = TREE_INT_CST_LOW (cst);
+if (val >= 0)
+  return m_constant_fd;
+else
+  return m_invalid;
+  }
+return m_start;
+  }
+
+  bool on_stmt (sm_context *sm_ctxt, const supernode *node,
+const gimple *stmt) const final override;
+
+  void on_condition (sm_context *sm_ctxt, const supernode *node,
+ const gimple *stmt, const svalue *lhs, const
tree_code op,
+ const svalue *rhs) const final override;
+
+  bool can_purge_p (state_t s) const final override;
+  pending_diagnostic *on_leak (tree var) const final override;
+
+  bool is_unchecked (state_t s) const;
+  bool is_valid (state_t s) const;
+  enum access_mode get_access_mode_from_flag (int flag) const;
+  enum access_mode get_access_mode_from_state (state_t s) const;
+  bool check_for_closed_fd (state_t s) const;
+  bool check_for_invalid_fd (state_t s) const;
+  bool check_for_constant_fd (state_t s) const;
+
+  /* State for a constant file 

[PATCH] analyzer PR 106003

[Mir Immad via Gcc]

diff --git gcc/Makefile.in gcc/Makefile.in
index b6dcc45a58a..04631f737ea 100644
--- gcc/Makefile.in
+++ gcc/Makefile.in
@@ -1269,6 +1269,7 @@ ANALYZER_OBJS = \
  analyzer/region-model-reachability.o \
  analyzer/sm.o \
  analyzer/sm-file.o \
+ analyzer/sm-fd.o \
  analyzer/sm-malloc.o \
  analyzer/sm-pattern-test.o \
  analyzer/sm-sensitive.o \
diff --git gcc/analyzer/analyzer.opt gcc/analyzer/analyzer.opt
index 23dfc797cea..e2a629bb42c 100644
--- gcc/analyzer/analyzer.opt
+++ gcc/analyzer/analyzer.opt
@@ -66,6 +66,26 @@ Wanalyzer-exposure-through-output-file
 Common Var(warn_analyzer_exposure_through_output_file) Init(1) Warning
 Warn about code paths in which sensitive data is written to a file.

+Wanalyzer-fd-access-mode-mismatch
+Common Var(warn_analyzer_fd_mode_mismatch) Init(1) Warning
+Warn about code paths in which read on write-only file descriptor is
attempted, or vice versa.
+
+Wanalyzer-fd-double-close
+Common Var(warn_analyzer_fd_double_close) Init(1) Warning
+Warn about code paths in which a file descriptor can be closed more than
once.
+
+Wanalyzer-fd-leak
+Common Var(warn_analyzer_fd_leak) Init(1) Warning
+Warn about code paths in which a file descriptor is not closed.
+
+Wanalyzer-fd-use-after-close
+Common Var(warn_analyzer_fd_use_after_close) Init(1) Warning
+Warn about code paths in which a read or write is performed on a closed
file descriptor.
+
+Wanalyzer-fd-use-without-check
+Common Var(warn_analyzer_fd_use_without_check) Init(1) Warning
+warn about code paths in which a possibly invalid file descriptor is
passed to a must-be-a-valid file descriptor function argument.
+
 Wanalyzer-file-leak
 Common Var(warn_analyzer_file_leak) Init(1) Warning
 Warn about code paths in which a stdio FILE is not closed.
diff --git gcc/analyzer/sm-fd.cc gcc/analyzer/sm-fd.cc
new file mode 100644
index 000..048014d7a26
--- /dev/null
+++ gcc/analyzer/sm-fd.cc
@@ -0,0 +1,770 @@
+/* A state machine for detecting misuses of POSIX file descriptor APIs.
+   Copyright (C) 2019-2022 Free Software Foundation, Inc.
+   Contributed by Immad Mir .
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3.  If not see
+.  */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tree.h"
+#include "function.h"
+#include "basic-block.h"
+#include "gimple.h"
+#include "options.h"
+#include "diagnostic-path.h"
+#include "diagnostic-metadata.h"
+#include "function.h"
+#include "json.h"
+#include "analyzer/analyzer.h"
+#include "diagnostic-event-id.h"
+#include "analyzer/analyzer-logging.h"
+#include "analyzer/sm.h"
+#include "analyzer/pending-diagnostic.h"
+#include "analyzer/function-set.h"
+#include "analyzer/analyzer-selftests.h"
+#include "tristate.h"
+#include "selftest.h"
+#include "analyzer/call-string.h"
+#include "analyzer/program-point.h"
+#include "analyzer/store.h"
+#include "analyzer/region-model.h"
+
+#include 
+#if ENABLE_ANALYZER
+
+namespace ana
+{
+
+namespace
+{
+
+/* An enum for distinguishing between three different access modes. */
+
+enum access_mode
+{
+  READ_WRITE,
+  READ_ONLY,
+  WRITE_ONLY
+};
+
+class fd_state_machine : public state_machine
+{
+public:
+  fd_state_machine (logger *logger);
+
+  bool
+  inherited_state_p () const final override
+  {
+return false;
+  }
+
+  state_machine::state_t
+  get_default_state (const svalue *sval) const final override
+  {
+if (tree cst = sval->maybe_get_constant ())
+  {
+int val = TREE_INT_CST_LOW (cst);
+if (val < 0)
+  {
+return m_invalid;
+  }
+  }
+return m_start;
+  }
+
+  bool on_stmt (sm_context *sm_ctxt, const supernode *node,
+const gimple *stmt) const final override;
+
+  void on_condition (sm_context *sm_ctxt, const supernode *node,
+ const gimple *stmt, const svalue *lhs, const
tree_code op,
+ const svalue *rhs) const final override;
+
+  bool can_purge_p (state_t s) const final override;
+  pending_diagnostic *on_leak (tree var) const final override;
+
+  bool is_unchecked (state_t s) const;
+  bool is_valid (state_t s) const;
+  const char *get_string_for_access_mode (enum access_mode) const;
+  enum access_mode get_access_mode_from_flag (int flag) const;
+  enum access_mode get_access_mode_from_state (state_t s) const;
+  bool check_for_closed_fd (state_t s) const;
+
+  /* States representing a file descriptor that hasn't yet been
+  

Re: [PATCH] static analysis support for posix file desccriptor APIs

[Mir Immad via Gcc]

 Hi Dave,
Thanks for the suggestions,

I changed most of the things that you suggested, however reporting for
warnings like close of known invalid fd was problematic:

consider the following code:

if (fd >= 0)
{ write (fd,...); }
close(fd);

As I was checking the exploded graph for this; the "close(fd)" stmt when
visited by the FALSE edge of if stmt (fd < 0) finds fd to be in m_invalid
state; hence warns about "close on known invalid fd" which I believe is not
true as fd at that point is not *known* to be invalid. I spent quite some
time on this and decided not to add this diagnosis for now.

Also, when close transitions m_invalid to m_close; this leads to double
close even when the second close is outside the ( < 0 ) condition which
again does not seem right.
if (fd < 0)
close(fd):
close(fd); // double close here.

> Maybe consolidate on_read and on_write by adding a subroutine that
> checks for m_closed, and for checking access mode (maybe a
> "check_for_open_fd" that takes an access mode enum value amongst other
> params.  If you pass around caller_fndecl, I think much of this code
> can be shared that way between on_read and on_write (which will help
> simplify things if we want to support further functions)

I hope I got this right.


> > +}
> > +}
> > +  else
> > +{
>  >+  /* FIXME: add leak reporting */

> Do you have a testcase that exhibits this behavior?

I was thinking of the following case:
void test()
{
 open(..);
}
Here the resources are leaked because there is no way to free them.

In "read" and "write" funcs, I'm warning for unchecked_use_of_fd and
access_mode_mismatch only when we know fd is not in closed state.
Otherwise, such code leads to lot of irrelevant warnings, example:

void test1(const char *path, void *buf) {
  int fd = open(path, O_RDONLY);
  if (fd >= 0)
  {
  close(fd);
  read(fd, buf, 1); // read on closed fd + read on possibly invalid fd
  write(fd, buf, 1); // write on closed fd + write on possibly invlid fd
  }
}


Adding docs for new options still remains pending. I added some new tests;
and all tests are passing. The stuff about O_* macros is left as-is.

 I'm sending a patch in another email.

Thanks a lot.

On Wed, Jun 22, 2022 at 12:04 AM David Malcolm  wrote:

> Hi Immad, thanks for this patch.
>
> Overall, looks like you're making good progress.
>
> Various notes and nitpicks inline below, throughout...
>
> On Tue, 2022-06-21 at 22:00 +0530, Mir Immad wrote:
>
> [...snip...]
>
> >
> > diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
> > index 23dfc797cea..d99acfbb069 100644
> > --- a/gcc/analyzer/analyzer.opt
> > +++ b/gcc/analyzer/analyzer.opt
> > @@ -54,6 +54,10 @@ The minimum number of supernodes within a function
> > for
> > the analyzer to consider
> >  Common Joined UInteger Var(param_analyzer_max_enodes_for_full_dump)
> > Init(200) Param
> >  The maximum depth of exploded nodes that should appear in a dot dump
> > before switching to a less verbose format.
>
> I'm not yet sure if this is a good idea, but I wonder if all of these
> warnings ought to have a "-Wanalyzer-fd-" prefix?  "file-descriptor" is
> rather long, and the analyzer's warnings are already tending to be on
> the long side, and having a consistent prefix might make it easier for
> users to grok them.
>
> (though this feels like a "what color do we paint the bikeshed?" issue;
> see e.g. https://bikeshed.org/ )
>
> >
> > +Wanalyzer-double-close
> > +Common Var(warn_analyzer_double_close) Init(1) Warning
> > +Warn about code paths in which a file descriptor can be closed more
> > than
> > once.
>
> ...so this could be Wanalyzer-fd-double-close
>
> > +
> >  Wanalyzer-double-fclose
> >  Common Var(warn_analyzer_double_fclose) Init(1) Warning
> >  Warn about code paths in which a stdio FILE can be closed more than
> > once.
> > @@ -66,6 +70,10 @@ Wanalyzer-exposure-through-output-file
> >  Common Var(warn_analyzer_exposure_through_output_file) Init(1) Warning
> >  Warn about code paths in which sensitive data is written to a file.
> >
> > +Wanalyzer-file-descriptor-leak
> > +Common Var(warn_analyzer_file_descriptor_leak) Init(1) Warning
> > +Warn about code paths in which a file descriptor is not closed.
>
> ...and Wanalyzer-fd-leak
>
> > +
> >  Wanalyzer-file-leak
> >  Common Var(warn_analyzer_file_leak) Init(1) Warning
> >  Warn about code paths in which a stdio FILE is not closed.
> > @@ -82,6 +90,14 @@ Wanalyzer-mismatching-deallocation
> >  Common Var(warn_analyzer_mismatching_deallocation) Init(1) Warning
> >  Warn about code paths in which the wrong deallocation function is
> > called.
> >
> > +Wanalyzer-mismatching-operation-on-file-descriptor
> > +Common Var(warn_analyzer_mismatching_operation_on_file_descriptor)
> > Init(1)
> > Warning
> > +Warn about the code paths in which read on write-only file descriptor
> > or
> > write on read-only file descriptor is called.
>
> Maybe:
>   Wanalyzer-fd-access-mode-mismatch
> ?
>
> Lose the "the" in "the code 

Re: [PATCH] static analysis support for posix file desccriptor APIs

[Mir Immad via Gcc]

 This is a patch for extending static analysis support for posix file
descriptor APIs which is a part of my GSoC project. I've written a few
testcases, which are all passing. There are a few TODOs like adding the
copyright header and adding docs to gcc/doc/invoke.texi.

I'm looking for suggestions for whether the names of warnings I have chosen
are appropriate, and configuring my vsocde editor to follow the GNU coding
conventions.

- Immad

[PATCH] static analysis support for posix file desccriptor APIs

[Mir Immad via Gcc]

diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index b6dcc45a58a..04631f737ea 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -1269,6 +1269,7 @@ ANALYZER_OBJS = \
  analyzer/region-model-reachability.o \
  analyzer/sm.o \
  analyzer/sm-file.o \
+ analyzer/sm-fd.o \
  analyzer/sm-malloc.o \
  analyzer/sm-pattern-test.o \
  analyzer/sm-sensitive.o \
diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt
index 23dfc797cea..d99acfbb069 100644
--- a/gcc/analyzer/analyzer.opt
+++ b/gcc/analyzer/analyzer.opt
@@ -54,6 +54,10 @@ The minimum number of supernodes within a function for
the analyzer to consider
 Common Joined UInteger Var(param_analyzer_max_enodes_for_full_dump)
Init(200) Param
 The maximum depth of exploded nodes that should appear in a dot dump
before switching to a less verbose format.

+Wanalyzer-double-close
+Common Var(warn_analyzer_double_close) Init(1) Warning
+Warn about code paths in which a file descriptor can be closed more than
once.
+
 Wanalyzer-double-fclose
 Common Var(warn_analyzer_double_fclose) Init(1) Warning
 Warn about code paths in which a stdio FILE can be closed more than once.
@@ -66,6 +70,10 @@ Wanalyzer-exposure-through-output-file
 Common Var(warn_analyzer_exposure_through_output_file) Init(1) Warning
 Warn about code paths in which sensitive data is written to a file.

+Wanalyzer-file-descriptor-leak
+Common Var(warn_analyzer_file_descriptor_leak) Init(1) Warning
+Warn about code paths in which a file descriptor is not closed.
+
 Wanalyzer-file-leak
 Common Var(warn_analyzer_file_leak) Init(1) Warning
 Warn about code paths in which a stdio FILE is not closed.
@@ -82,6 +90,14 @@ Wanalyzer-mismatching-deallocation
 Common Var(warn_analyzer_mismatching_deallocation) Init(1) Warning
 Warn about code paths in which the wrong deallocation function is called.

+Wanalyzer-mismatching-operation-on-file-descriptor
+Common Var(warn_analyzer_mismatching_operation_on_file_descriptor) Init(1)
Warning
+Warn about the code paths in which read on write-only file descriptor or
write on read-only file descriptor is called.
+
+Wanalyzer-possible-invalid-file-descriptor
+Common Var(warn_analyzer_possible_invalid_file_descriptor) Init(1) Warning
+warn about code paths in which a possibly invalid file descriptor is
passed to a must-be-a-valid file descriptor function argument.
+
 Wanalyzer-possible-null-argument
 Common Var(warn_analyzer_possible_null_argument) Init(1) Warning
 Warn about code paths in which a possibly-NULL value is passed to a
must-not-be-NULL function argument.
@@ -90,6 +106,10 @@ Wanalyzer-possible-null-dereference
 Common Var(warn_analyzer_possible_null_dereference) Init(1) Warning
 Warn about code paths in which a possibly-NULL pointer is dereferenced.

+Wanalyzer-read-write-on-closed-file-descriptor
+Common Var(warn_analyzer_read_write_on_closed_file_descriptor) Init(1)
Warning
+Warn about code paths in which a read on write in performed on a closed
file descriptor.
+
 Wanalyzer-unsafe-call-within-signal-handler
 Common Var(warn_analyzer_unsafe_call_within_signal_handler) Init(1) Warning
 Warn about code paths in which an async-signal-unsafe function is called
from a signal handler.
diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
new file mode 100644
index 000..23e79e3e16a
--- /dev/null
+++ b/gcc/analyzer/sm-fd.cc
@@ -0,0 +1,697 @@
+/* FIXME: add copyright header. */
+
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tree.h"
+#include "function.h"
+#include "basic-block.h"
+#include "gimple.h"
+#include "options.h"
+#include "diagnostic-path.h"
+#include "diagnostic-metadata.h"
+#include "function.h"
+#include "json.h"
+#include "analyzer/analyzer.h"
+#include "diagnostic-event-id.h"
+#include "analyzer/analyzer-logging.h"
+#include "analyzer/sm.h"
+#include "analyzer/pending-diagnostic.h"
+#include "analyzer/function-set.h"
+#include "analyzer/analyzer-selftests.h"
+#include "tristate.h"
+#include "selftest.h"
+#include "analyzer/call-string.h"
+#include "analyzer/program-point.h"
+#include "analyzer/store.h"
+#include "analyzer/region-model.h"
+
+#include 
+#if ENABLE_ANALYZER
+
+namespace ana
+{
+
+namespace
+{
+class fd_state_machine : public state_machine
+{
+public:
+  fd_state_machine (logger *logger);
+
+  bool
+  inherited_state_p () const final override
+  {
+return false;
+  }
+
+  state_machine::state_t
+  get_default_state (const svalue *sval) const final override
+  {
+if (tree cst = sval->maybe_get_constant ())
+  {
+int val = TREE_INT_CST_LOW (cst);
+if (val < 0)
+  {
+return m_null;
+  }
+  }
+return m_start;
+  }
+
+  bool on_stmt (sm_context *sm_ctxt, const supernode *node,
+const gimple *stmt) const final override;
+
+  void on_condition (sm_context *sm_ctxt, const supernode *node,
+ const gimple *stmt, const svalue *lhs, const
tree_code op,
+ const svalue *rhs) const 

GSoC: Getting started

[Mir Immad via Gcc]

HI everyone,

I'm Immad Mir -- one of the GSoC students this year. I'll be working on
adding static analysis support for POSIX file description APIs this summer.

The plan is to let the static analyzer know about the FD APIs through the
use of function attributes, although initially we might hardcode the logic
to get started working. I'm looking for the suggestions on this from the
people who have experience working with file-descriptors.

Thank you.
Immad

Re: GSoC proposal for extending static analyzer

[Mir Immad via Gcc]

I've updated the link on the repo -- https://mirimmad.github.io/zeta-lang.

> You don't give many specifics in your personal decription.  One thing
> I'm not seeing is a sense of how proficient you are in various
> programming languages.  In particular, how is your C and C++?  How
> familiar are you with the debugger?  Looking at your github, you seem
> to have relevant experience in compilers, which is great, but all your
> code appears to be with "managed" languages such as Ruby, Java, and
> Python  [and Zeta :)].

 I'm pretty comfortable with both C and C++ and manual mem management .
Unfortunately, I don't have any project in C/C++ to show. Maybe I can use
the time before May 20 to write something in cpp?
About the debugger, I'm okay-ish with it. In fact, it was due to the
debugger (and your blog on it) that I was initially able to walk through
the codebase.


> That said, I got
> the sense from your previous emails that you're not very familiar with
> the APIs, and that you chose them because that was the suggestion I had
> made on the wiki page.

thats right.


>  Obviously it's something you can learn on the
>  way, but it would be better to accurately identify which areas you're
> going need to learn along the way, and the timetable and scope should
> reflect that.

If I understand the statement correctly; currently I'm thinking of
extending the support for open() (for creating the fd), write/read (for
working on the fd) and close(). This is quite analogous to what we have in
sm-file. Please let me know how do you want the analyzer to be extended and
if you expect support for any other FD APIs  too as I understand there are
many other APIs for creating and working wiith FDs?

Thank you.



On Fri, Apr 15, 2022 at 9:35 PM David Malcolm  wrote:

> On Fri, 2022-04-15 at 19:58 +0530, Mir Immad wrote:
> > I've submitted a proposal for extending the static analyzer to support
> > posix fd APIs on GSoC website. Here is the Google docs link (gdocs
> > <
> >
> https://docs.google.com/document/d/188zxPUsuYcF-uGVYL_G1s2RVtHhJSZeQ4sha40H7374/edit?usp=sharing
> > >).
> >
> >
> > Please take a look and let me know what you think.
> >
> > Thank you.
>
> Thanks.
>
> FWIW, I'm getting an error when trying the URL given in your github
> repo: http://mirimmad.me/
> but https://mirimmad.github.io/ seems to work  - but it's almost empty.
>
> You don't give many specifics in your personal decription.  One thing
> I'm not seeing is a sense of how proficient you are in various
> programming languages.  In particular, how is your C and C++?  How
> familiar are you with the debugger?  Looking at your github, you seem
> to have relevant experience in compilers, which is great, but all your
> code appears to be with "managed" languages such as Ruby, Java, and
> Python  [and Zeta :)].
>
> Also, the proposal is to extend the analyzer to cover a specific
> domain: various POSIX APIs.  Can you please give a sense of your level
> of expertise with these APIs?  I was pleased at your initiative in
> trying to reuse the existing code to work with them.  That said, I got
> the sense from your previous emails that you're not very familiar with
> the APIs, and that you chose them because that was the suggestion I had
> made on the wiki page.  Obviously it's something you can learn on the
> way, but it would be better to accurately identify which areas you're
> going need to learn along the way, and the timetable and scope should
> reflect that.
>
> Hope this is constructive
> Dave
>
>
>

GSoC proposal for extending static analyzer

[Mir Immad via Gcc]

I've submitted a proposal for extending the static analyzer to support
posix fd APIs on GSoC website. Here is the Google docs link (gdocs
).


Please take a look and let me know what you think.

Thank you.

Re: GSoC: Working on the static analyzer

[Mir Immad via Gcc]

 > > good GSoC project.  Maybe find an old CVE that someone has
> > > > > > > written
> > > > > > > a
> > > > > > > good write-up for, and think about "how could GCC/-
> > > > > > > fanalyzer
> > > > > > > have
> > > > > > > spotted it?"
> > > > > > >
> > > > > > > (ii) add leak-detection for POSIX file descriptors: i.e.
> > > > > > > the
> > > > > > > integer
> > > > > > > values returned by "open", "dup", etc.  It would be good to
> > > > > > > have a
> > > > > > > check that the user's code doesn't leak these values e.g.
> > > > > > > on
> > > > > > > error-
> > > > > > > handling paths, by failing to close a file-descriptor (and
> > > > > > > not
> > > > > > > storing
> > > > > > > it anywhere).  I think that much of this could be done by
> > > > > > > analogy
> > > > > > > with
> > > > > > > the sm-file.cc code.
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > Also, I didn't realize the complexity of adding SARIF
> > > > > > > > when I
> > > > > > > > mentioned
> > > > > > > > it.
> > > > > > > > I'd rather work on adding more checkers.
> > > > > > >
> > > > > > > Fair enough.
> > > > > > >
> > > > > > > Hope this above is constructive.
> > > > > > >
> > > > > > > Dave
> > > > > > >
> > > > > > > >
> > > > > > > > Regards.
> > > > > > > >
> > > > > > > > Mir Immad
> > > > > > > >
> > > > > > > > On Sun, Jan 23, 2022, 11:04 PM Mir Immad <
> > > > > > > > mirimnan...@gmail.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi Sir,
> > > > > > > > >
> > > > > > > > > I've been trying to understand the static analyzer's
> > > > > > > > > code.
> > > > > > > > > I
> > > > > > > > > spent
> > > > > > > > > most of
> > > > > > > > > my time learning the state machine's API. I learned how
> > > > > > > > > state
> > > > > > > > > machine's
> > > > > > > > > on_stmt is supposed to "recognize" specific functions
> > > > > > > > > and
> > > > > > > > > takes
> > > > > > > > > a
> > > > > > > > > specific
> > > > > > > > > tree from one state to another, and how the captured
> > > > > > > > > states
> > > > > > > > > are
> > > > > > > > > used
> > > > > > > > > by
> > > > > > > > > pending_diagnostics to report the errors. Furthermore,
> > > > > > > > > I
> > > > > > > > > was
> > > > > > > > > able to
> > > > > > > > > create
> > > > > > > > > a dummy checker that mimicked the behaviour of sm-
> > > > > > > > > file's
> > > > > > > > > double_fclose and
> > > > > > > > > compile GCC with these changes. Is this the right way
> > > > > > > > > of
> > > > > > > > > learning?
> > > > > > > > >
> > > > > > > > > As you've mentioned on the projects page that you would
> > > > > > > > > like to
> > > > > > > > > add
> > > > > > > > > more
> > > > > > > > > support for some POSIX APIs. Can you please write (or
> > > > > > > > > refer
> > > > > > > > > me
> > > > > > > > > to a)
> > > > > > > > > a
> > > > > > > > > simple C program that uses such an API (and also what
> > > > > > > > > the
> > > > > > > > > analyzer
> > > > > > > > > should
> > > > > > > > > have done) so that I can attempt to add such a checker
> > > > > > > > > to
> > > > > > > > > the
> > > > > > > > > analyzer.
> > > > > > > > >
> > > > > > > > > Also, I didn't realize the complexity of adding SARIF
> > > > > > > > > when
> > > > > > > > > I
> > > > > > > > > mentioned it.
> > > > > > > > > I'd rather work on adding more checkers.
> > > > > > > > >
> > > > > > > > > Regards.
> > > > > > > > > Mir Immad
> > > > > > > > >
> > > > > > > > > On Mon, Jan 17, 2022 at 5:41 AM David Malcolm <
> > > > > > > > > dmalc...@redhat.com>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > On Fri, 2022-01-14 at 22:15 +0530, Mir Immad wrote:
> > > > > > > > > > > HI David,
> > > > > > > > > > > I've been tinkering with the static analyzer for
> > > > > > > > > > > the
> > > > > > > > > > > last
> > > > > > > > > > > few
> > > > > > > > > > > days. I
> > > > > > > > > > > find
> > > > > > > > > > > the project of adding SARIF output to the analyzer
> > > > > > > > > > > intresting.
> > > > > > > > > > > I'm
> > > > > > > > > > > writing
> > > > > > > > > > > this to let you know that I'm trying to learn the
> > > > > > > > > > > codebase.
> > > > > > > > > > > Thank you.
> > > > > > > > > >
> > > > > > > > > > Excellent.
> > > > > > > > > >
> > > > > > > > > > BTW, I think adding SARIF output would involve
> > > > > > > > > > working
> > > > > > > > > > more
> > > > > > > > > > with
> > > > > > > > > > GCC's
> > > > > > > > > > diagnostics subsystem than with the static analyzer,
> > > > > > > > > > since
> > > > > > > > > > (in
> > > > > > > > > > theory)
> > > > > > > > > > all of the static analyzer's output is passing
> > > > > > > > > > through
> > > > > > > > > > the
> > > > > > > > > > diagnostics
> > > > > > > > > > subsystem - though the static analyzer is probably
> > > > > > > > > > the
> > > > > > > > > > only
> > > > > > > > > > GCC
> > > > > > > > > > component generating diagnostic paths.
> > > > > > > > > >
> > > > > > > > > > I'm happy to mentor such a project as I maintain both
> > > > > > > > > > subsystems
> > > > > > > > > > and
> > > > > > > > > > SARIF output would benefit both - but it would be
> > > > > > > > > > rather
> > > > > > > > > > tangential
> > > > > > > > > > to
> > > > > > > > > > the analyzer - so if you had specifically wanted to
> > > > > > > > > > be
> > > > > > > > > > working on
> > > > > > > > > > the
> > > > > > > > > > guts of the analyzer itself, you may want to pick a
> > > > > > > > > > different
> > > > > > > > > > subproject.
> > > > > > > > > >
> > > > > > > > > > The SARIF standard is rather long and complicated,
> > > > > > > > > > and we
> > > > > > > > > > would
> > > > > > > > > > want to
> > > > > > > > > > be compatible with clang's implementation.
> > > > > > > > > >
> > > > > > > > > > It would be very cool if gcc could also accept SARIF
> > > > > > > > > > files as
> > > > > > > > > > an
> > > > > > > > > > *input* format, and emit them as diagnostics; that
> > > > > > > > > > might
> > > > > > > > > > help
> > > > > > > > > > with
> > > > > > > > > > debugging SARIF output.   (I have a old patch for
> > > > > > > > > > adding
> > > > > > > > > > JSON
> > > > > > > > > > parsing
> > > > > > > > > > support to GCC that could be used as a starting point
> > > > > > > > > > for
> > > > > > > > > > this).
> > > > > > > > > >
> > > > > > > > > > Hope the above makes sense
> > > > > > > > > > Dave
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > On Tue, Jan 11, 2022, 7:09 PM David Malcolm <
> > > > > > > > > > > dmalc...@redhat.com>
> > > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > On Tue, 2022-01-11 at 11:03 +0530, Mir Immad via
> > > > > > > > > > > > Gcc
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > > Hi everyone,
> > > > > > > > > > > >
> > > > > > > > > > > > Hi, and welcome.
> > > > > > > > > > > >
> > > > > > > > > > > > > I intend to work on the static analyzer. Are
> > > > > > > > > > > > > these
> > > > > > > > > > > > > documents
> > > > > > > > > > > > > enough to
> > > > > > > > > > > > > get
> > > > > > > > > > > > > started:
> > > > > > > > > > > > > https://gcc.gnu.org/onlinedocs/gccint and
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > >
> > > > > > >
> > > > >
> https://gcc.gnu.org/onlinedocs/gccint/Analyzer-Internals.html#Analyzer-Internals
> > > > > > > > > > > >
> > > > > > > > > > > > Yes.
> > > > > > > > > > > >
> > > > > > > > > > > > There are also some high-level notes here:
> > > > > > > > > > > >
> > > > > > > > > > > > https://gcc.gnu.org/wiki/DavidMalcolm/StaticAnalyzer
> > > > > > > > > > > >
> > > > > > > > > > > > Also, given that the analyzer is part of GCC, the
> > > > > > > > > > > > more
> > > > > > > > > > > > general
> > > > > > > > > > > > introductions to hacking on GCC will be useful.
> > > > > > > > > > > >
> > > > > > > > > > > > I recommend creating a trivial C source file with
> > > > > > > > > > > > a
> > > > > > > > > > > > bug
> > > > > > > > > > > > in it
> > > > > > > > > > > > (e.g.
> > > > > > > > > > > > a
> > > > > > > > > > > > 3-line function with a use-after-free), and
> > > > > > > > > > > > stepping
> > > > > > > > > > > > through
> > > > > > > > > > > > the
> > > > > > > > > > > > analyzer to get a sense of how it works.
> > > > > > > > > > > >
> > > > > > > > > > > > Hope this is helpful; don't hesitate to ask
> > > > > > > > > > > > questions.
> > > > > > > > > > > > Dave
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > >
> > > > >
> > > > >
> > > > >
> >
>
>
>

Re: GSoC: Working on the static analyzer

[Mir Immad via Gcc]

d like to
>> > > > > add
>> > > > > more
>> > > > > support for some POSIX APIs. Can you please write (or refer me
>> > > > > to a)
>> > > > > a
>> > > > > simple C program that uses such an API (and also what the
>> > > > > analyzer
>> > > > > should
>> > > > > have done) so that I can attempt to add such a checker to the
>> > > > > analyzer.
>> > > > >
>> > > > > Also, I didn't realize the complexity of adding SARIF when I
>> > > > > mentioned it.
>> > > > > I'd rather work on adding more checkers.
>> > > > >
>> > > > > Regards.
>> > > > > Mir Immad
>> > > > >
>> > > > > On Mon, Jan 17, 2022 at 5:41 AM David Malcolm <
>> > > > > dmalc...@redhat.com>
>> > > > > wrote:
>> > > > >
>> > > > > > On Fri, 2022-01-14 at 22:15 +0530, Mir Immad wrote:
>> > > > > > > HI David,
>> > > > > > > I've been tinkering with the static analyzer for the last
>> > > > > > > few
>> > > > > > > days. I
>> > > > > > > find
>> > > > > > > the project of adding SARIF output to the analyzer
>> > > > > > > intresting.
>> > > > > > > I'm
>> > > > > > > writing
>> > > > > > > this to let you know that I'm trying to learn the codebase.
>> > > > > > > Thank you.
>> > > > > >
>> > > > > > Excellent.
>> > > > > >
>> > > > > > BTW, I think adding SARIF output would involve working more
>> > > > > > with
>> > > > > > GCC's
>> > > > > > diagnostics subsystem than with the static analyzer, since
>> > > > > > (in
>> > > > > > theory)
>> > > > > > all of the static analyzer's output is passing through the
>> > > > > > diagnostics
>> > > > > > subsystem - though the static analyzer is probably the only
>> > > > > > GCC
>> > > > > > component generating diagnostic paths.
>> > > > > >
>> > > > > > I'm happy to mentor such a project as I maintain both
>> > > > > > subsystems
>> > > > > > and
>> > > > > > SARIF output would benefit both - but it would be rather
>> > > > > > tangential
>> > > > > > to
>> > > > > > the analyzer - so if you had specifically wanted to be
>> > > > > > working on
>> > > > > > the
>> > > > > > guts of the analyzer itself, you may want to pick a different
>> > > > > > subproject.
>> > > > > >
>> > > > > > The SARIF standard is rather long and complicated, and we
>> > > > > > would
>> > > > > > want to
>> > > > > > be compatible with clang's implementation.
>> > > > > >
>> > > > > > It would be very cool if gcc could also accept SARIF files as
>> > > > > > an
>> > > > > > *input* format, and emit them as diagnostics; that might help
>> > > > > > with
>> > > > > > debugging SARIF output.   (I have a old patch for adding JSON
>> > > > > > parsing
>> > > > > > support to GCC that could be used as a starting point for
>> > > > > > this).
>> > > > > >
>> > > > > > Hope the above makes sense
>> > > > > > Dave
>> > > > > >
>> > > > > > >
>> > > > > > > On Tue, Jan 11, 2022, 7:09 PM David Malcolm <
>> > > > > > > dmalc...@redhat.com>
>> > > > > > > wrote:
>> > > > > > >
>> > > > > > > > On Tue, 2022-01-11 at 11:03 +0530, Mir Immad via Gcc
>> > > > > > > > wrote:
>> > > > > > > > > Hi everyone,
>> > > > > > > >
>> > > > > > > > Hi, and welcome.
>> > > > > > > >
>> > > > > > > > > I intend to work on the static analyzer. Are these
>> > > > > > > > > documents
>> > > > > > > > > enough to
>> > > > > > > > > get
>> > > > > > > > > started: https://gcc.gnu.org/onlinedocs/gccint and
>> > > > > > > > >
>> > > > > > > >
>> > > > > >
>> > > > > >
>> > >
>> > >
>> https://gcc.gnu.org/onlinedocs/gccint/Analyzer-Internals.html#Analyzer-Internals
>> > > > > > > >
>> > > > > > > > Yes.
>> > > > > > > >
>> > > > > > > > There are also some high-level notes here:
>> > > > > > > >   https://gcc.gnu.org/wiki/DavidMalcolm/StaticAnalyzer
>> > > > > > > >
>> > > > > > > > Also, given that the analyzer is part of GCC, the more
>> > > > > > > > general
>> > > > > > > > introductions to hacking on GCC will be useful.
>> > > > > > > >
>> > > > > > > > I recommend creating a trivial C source file with a bug
>> > > > > > > > in it
>> > > > > > > > (e.g.
>> > > > > > > > a
>> > > > > > > > 3-line function with a use-after-free), and stepping
>> > > > > > > > through
>> > > > > > > > the
>> > > > > > > > analyzer to get a sense of how it works.
>> > > > > > > >
>> > > > > > > > Hope this is helpful; don't hesitate to ask questions.
>> > > > > > > > Dave
>> > > > > > > >
>> > > > > > > >
>> > > > > >
>> > > > > >
>> > > > > >
>> > >
>> > >
>> > >
>>
>>
>>

Re: Compiling GCC source

[Mir Immad via Gcc]

The build system probably first tries to make sure if everything is in
place and if there were any changes and re-compiles the new/changed files.

How much time does it take when you try to rebuild?

On Mon, Feb 7, 2022, 12:43 AM Mohamed Atef 
wrote:

> After i built it
> I tried make - j 8 and it's recompiling now i thought i will have messege
> like
> Every thing is up to date or there are no change done
> But it actually started to compile again
>
>
> في الأحد، ٦ فبراير، ٢٠٢٢ ٩:٠٨ م Mir Immad  كتب:
>
>> Yes, that is right.
>>
>> On Mon, Feb 7, 2022, 12:38 AM Mohamed Atef 
>> wrote:
>>
>>> Hello,
>>>   Only modified files will be recompiled, won't it?
>>>
>>>
>>> في الأحد، ٦ فبراير، ٢٠٢٢ ٩:٠٥ م Mir Immad  كتب:
>>>
 Make sure to use all the cores available.

 make -j N

 e.g; make -j 8


 On Mon, Feb 7, 2022, 12:26 AM Mohamed Atef via Gcc 
 wrote:

> Hello everyone,
> I built gcc from the repo and it took around 2 hours but I am
> wondering should I wait two hours after every modification?
> Is there any way to recompile faster.
> That's very important as we will add some files and tests  for OMPD.
> Thanks
>

Re: Compiling GCC source

[Mir Immad via Gcc]

Yes, that is right.

On Mon, Feb 7, 2022, 12:38 AM Mohamed Atef 
wrote:

> Hello,
>   Only modified files will be recompiled, won't it?
>
>
> في الأحد، ٦ فبراير، ٢٠٢٢ ٩:٠٥ م Mir Immad  كتب:
>
>> Make sure to use all the cores available.
>>
>> make -j N
>>
>> e.g; make -j 8
>>
>>
>> On Mon, Feb 7, 2022, 12:26 AM Mohamed Atef via Gcc 
>> wrote:
>>
>>> Hello everyone,
>>> I built gcc from the repo and it took around 2 hours but I am
>>> wondering should I wait two hours after every modification?
>>> Is there any way to recompile faster.
>>> That's very important as we will add some files and tests  for OMPD.
>>> Thanks
>>>
>>

Re: Compiling GCC source

[Mir Immad via Gcc]

Make sure to use all the cores available.

make -j N

e.g; make -j 8


On Mon, Feb 7, 2022, 12:26 AM Mohamed Atef via Gcc  wrote:

> Hello everyone,
> I built gcc from the repo and it took around 2 hours but I am
> wondering should I wait two hours after every modification?
> Is there any way to recompile faster.
> That's very important as we will add some files and tests  for OMPD.
> Thanks
>

Re: GSoC: Working on the static analyzer

[Mir Immad via Gcc]

 done) so that I can attempt to add such a checker to the
> > > > analyzer.
> > >
> > > A couple of project ideas:
> > >
> > > (i) treat data coming from a network connection as tainted, by
> > > somehow
> > > teaching the analyzer about networking APIs.  Ideally: look at some
> > > subset of historical CVEs involving network-facing attacks on user-
> > > space daemons, and find a way to detect them in the analyzer (need
> > > to
> > > find a way to mark the incoming data as tainted, so that the
> > > analyer
> > > "know" about the trust boundary - that the incoming data needs to
> > > be
> > > sanitized and treated with extra caution; see
> > > https://gcc.gnu.org/pipermail/gcc-patches/2021-November/584372.html
> > > for my attempts to do this for the Linux kernel).
> > >
> > > Obviously this is potentially a huge project, so maybe just picking
> > > a
> > > tiny subset and getting that working as a proof-of-concept would be
> > > a
> > > good GSoC project.  Maybe find an old CVE that someone has written
> > > a
> > > good write-up for, and think about "how could GCC/-fanalyzer have
> > > spotted it?"
> > >
> > > (ii) add leak-detection for POSIX file descriptors: i.e. the
> > > integer
> > > values returned by "open", "dup", etc.  It would be good to have a
> > > check that the user's code doesn't leak these values e.g. on error-
> > > handling paths, by failing to close a file-descriptor (and not
> > > storing
> > > it anywhere).  I think that much of this could be done by analogy
> > > with
> > > the sm-file.cc code.
> > >
> > >
> > > >
> > > > Also, I didn't realize the complexity of adding SARIF when I
> > > > mentioned
> > > > it.
> > > > I'd rather work on adding more checkers.
> > >
> > > Fair enough.
> > >
> > > Hope this above is constructive.
> > >
> > > Dave
> > >
> > > >
> > > > Regards.
> > > >
> > > > Mir Immad
> > > >
> > > > On Sun, Jan 23, 2022, 11:04 PM Mir Immad 
> > > > wrote:
> > > >
> > > > > Hi Sir,
> > > > >
> > > > > I've been trying to understand the static analyzer's code. I
> > > > > spent
> > > > > most of
> > > > > my time learning the state machine's API. I learned how state
> > > > > machine's
> > > > > on_stmt is supposed to "recognize" specific functions and takes
> > > > > a
> > > > > specific
> > > > > tree from one state to another, and how the captured states are
> > > > > used
> > > > > by
> > > > > pending_diagnostics to report the errors. Furthermore, I was
> > > > > able to
> > > > > create
> > > > > a dummy checker that mimicked the behaviour of sm-file's
> > > > > double_fclose and
> > > > > compile GCC with these changes. Is this the right way of
> > > > > learning?
> > > > >
> > > > > As you've mentioned on the projects page that you would like to
> > > > > add
> > > > > more
> > > > > support for some POSIX APIs. Can you please write (or refer me
> > > > > to a)
> > > > > a
> > > > > simple C program that uses such an API (and also what the
> > > > > analyzer
> > > > > should
> > > > > have done) so that I can attempt to add such a checker to the
> > > > > analyzer.
> > > > >
> > > > > Also, I didn't realize the complexity of adding SARIF when I
> > > > > mentioned it.
> > > > > I'd rather work on adding more checkers.
> > > > >
> > > > > Regards.
> > > > > Mir Immad
> > > > >
> > > > > On Mon, Jan 17, 2022 at 5:41 AM David Malcolm <
> > > > > dmalc...@redhat.com>
> > > > > wrote:
> > > > >
> > > > > > On Fri, 2022-01-14 at 22:15 +0530, Mir Immad wrote:
> > > > > > > HI David,
> > > > > > > I've been tinkering with the static analyzer for the last
> > > > > > > few
> > > > > > > days. 

Re: GSoC: Working on the static analyzer

[Mir Immad via Gcc]

> Also, I didn't realize the complexity of adding SARIF when I
> > > mentioned it.
> > > I'd rather work on adding more checkers.
> > >
> > > Regards.
> > > Mir Immad
> > >
> > > On Mon, Jan 17, 2022 at 5:41 AM David Malcolm 
> > > wrote:
> > >
> > > > On Fri, 2022-01-14 at 22:15 +0530, Mir Immad wrote:
> > > > > HI David,
> > > > > I've been tinkering with the static analyzer for the last few
> > > > > days. I
> > > > > find
> > > > > the project of adding SARIF output to the analyzer intresting.
> > > > > I'm
> > > > > writing
> > > > > this to let you know that I'm trying to learn the codebase.
> > > > > Thank you.
> > > >
> > > > Excellent.
> > > >
> > > > BTW, I think adding SARIF output would involve working more with
> > > > GCC's
> > > > diagnostics subsystem than with the static analyzer, since (in
> > > > theory)
> > > > all of the static analyzer's output is passing through the
> > > > diagnostics
> > > > subsystem - though the static analyzer is probably the only GCC
> > > > component generating diagnostic paths.
> > > >
> > > > I'm happy to mentor such a project as I maintain both subsystems
> > > > and
> > > > SARIF output would benefit both - but it would be rather tangential
> > > > to
> > > > the analyzer - so if you had specifically wanted to be working on
> > > > the
> > > > guts of the analyzer itself, you may want to pick a different
> > > > subproject.
> > > >
> > > > The SARIF standard is rather long and complicated, and we would
> > > > want to
> > > > be compatible with clang's implementation.
> > > >
> > > > It would be very cool if gcc could also accept SARIF files as an
> > > > *input* format, and emit them as diagnostics; that might help with
> > > > debugging SARIF output.   (I have a old patch for adding JSON
> > > > parsing
> > > > support to GCC that could be used as a starting point for this).
> > > >
> > > > Hope the above makes sense
> > > > Dave
> > > >
> > > > >
> > > > > On Tue, Jan 11, 2022, 7:09 PM David Malcolm <
> > > > > dmalc...@redhat.com>
> > > > > wrote:
> > > > >
> > > > > > On Tue, 2022-01-11 at 11:03 +0530, Mir Immad via Gcc wrote:
> > > > > > > Hi everyone,
> > > > > >
> > > > > > Hi, and welcome.
> > > > > >
> > > > > > > I intend to work on the static analyzer. Are these documents
> > > > > > > enough to
> > > > > > > get
> > > > > > > started: https://gcc.gnu.org/onlinedocs/gccint and
> > > > > > >
> > > > > >
> > > >
> > > >
> https://gcc.gnu.org/onlinedocs/gccint/Analyzer-Internals.html#Analyzer-Internals
> > > > > >
> > > > > > Yes.
> > > > > >
> > > > > > There are also some high-level notes here:
> > > > > >   https://gcc.gnu.org/wiki/DavidMalcolm/StaticAnalyzer
> > > > > >
> > > > > > Also, given that the analyzer is part of GCC, the more general
> > > > > > introductions to hacking on GCC will be useful.
> > > > > >
> > > > > > I recommend creating a trivial C source file with a bug in it
> > > > > > (e.g.
> > > > > > a
> > > > > > 3-line function with a use-after-free), and stepping through
> > > > > > the
> > > > > > analyzer to get a sense of how it works.
> > > > > >
> > > > > > Hope this is helpful; don't hesitate to ask questions.
> > > > > > Dave
> > > > > >
> > > > > >
> > > >
> > > >
> > > >
>
>
>
$ cat double-close.c

#include 
#include 
void test()
{
  int fd = open("test.txt", O_RDONLY);
  close(fd);
  close(fd);
}

int main() {}

$ gcc-11.2.0 double-close.c -fanalyzer

double-close.c: In function ‘test’:
double-close.c:7:3: warning: double close on fd [-Wanalyzer-double-fclose]
7 |   close(fd);
  |   ^
  ‘test’: even

Re: GSoC: Working on the static analyzer

[Mir Immad via Gcc]

Hi, sir.

I've been trying to understand the static analyzer's code. I spent most of
my time learning the state machine's API. I learned how state machine's
on_stmt is supposed to "recognize" specific functions and how on_transition
takes a specific tree from one state to another, and how the captured
states are used by pending_diagnostics to report the errors. Furthermore, I
was able to create a dummy checker that mimicked the behaviour of sm-file's
double_fclose and compile GCC with these changes. Is this the right way of
learning?

As you've mentioned on the projects page that you would like to add more
support for some POSIX APIs. Can you please write (or refer me to a) a
simple C program that uses such an API (and also what the analyzer should
have done) so that I can attempt to add such a checker to the analyzer.

Also, I didn't realize the complexity of adding SARIF when I mentioned it.
I'd rather work on adding more checkers.

Regards.

Mir Immad

On Sun, Jan 23, 2022, 11:04 PM Mir Immad  wrote:

> Hi Sir,
>
> I've been trying to understand the static analyzer's code. I spent most of
> my time learning the state machine's API. I learned how state machine's
> on_stmt is supposed to "recognize" specific functions and takes a  specific
> tree from one state to another, and how the captured states are used by
> pending_diagnostics to report the errors. Furthermore, I was able to create
> a dummy checker that mimicked the behaviour of sm-file's double_fclose and
> compile GCC with these changes. Is this the right way of learning?
>
> As you've mentioned on the projects page that you would like to add more
> support for some POSIX APIs. Can you please write (or refer me to a) a
> simple C program that uses such an API (and also what the analyzer should
> have done) so that I can attempt to add such a checker to the analyzer.
>
> Also, I didn't realize the complexity of adding SARIF when I mentioned it.
> I'd rather work on adding more checkers.
>
> Regards.
> Mir Immad
>
> On Mon, Jan 17, 2022 at 5:41 AM David Malcolm  wrote:
>
>> On Fri, 2022-01-14 at 22:15 +0530, Mir Immad wrote:
>> > HI David,
>> > I've been tinkering with the static analyzer for the last few days. I
>> > find
>> > the project of adding SARIF output to the analyzer intresting. I'm
>> > writing
>> > this to let you know that I'm trying to learn the codebase.
>> > Thank you.
>>
>> Excellent.
>>
>> BTW, I think adding SARIF output would involve working more with GCC's
>> diagnostics subsystem than with the static analyzer, since (in theory)
>> all of the static analyzer's output is passing through the diagnostics
>> subsystem - though the static analyzer is probably the only GCC
>> component generating diagnostic paths.
>>
>> I'm happy to mentor such a project as I maintain both subsystems and
>> SARIF output would benefit both - but it would be rather tangential to
>> the analyzer - so if you had specifically wanted to be working on the
>> guts of the analyzer itself, you may want to pick a different
>> subproject.
>>
>> The SARIF standard is rather long and complicated, and we would want to
>> be compatible with clang's implementation.
>>
>> It would be very cool if gcc could also accept SARIF files as an
>> *input* format, and emit them as diagnostics; that might help with
>> debugging SARIF output.   (I have a old patch for adding JSON parsing
>> support to GCC that could be used as a starting point for this).
>>
>> Hope the above makes sense
>> Dave
>>
>> >
>> > On Tue, Jan 11, 2022, 7:09 PM David Malcolm 
>> > wrote:
>> >
>> > > On Tue, 2022-01-11 at 11:03 +0530, Mir Immad via Gcc wrote:
>> > > > Hi everyone,
>> > >
>> > > Hi, and welcome.
>> > >
>> > > > I intend to work on the static analyzer. Are these documents
>> > > > enough to
>> > > > get
>> > > > started: https://gcc.gnu.org/onlinedocs/gccint and
>> > > >
>> > >
>> https://gcc.gnu.org/onlinedocs/gccint/Analyzer-Internals.html#Analyzer-Internals
>> > >
>> > > Yes.
>> > >
>> > > There are also some high-level notes here:
>> > >   https://gcc.gnu.org/wiki/DavidMalcolm/StaticAnalyzer
>> > >
>> > > Also, given that the analyzer is part of GCC, the more general
>> > > introductions to hacking on GCC will be useful.
>> > >
>> > > I recommend creating a trivial C source file with a bug in it (e.g.
>> > > a
>> > > 3-line function with a use-after-free), and stepping through the
>> > > analyzer to get a sense of how it works.
>> > >
>> > > Hope this is helpful; don't hesitate to ask questions.
>> > > Dave
>> > >
>> > >
>>
>>
>>

Re: GSoC: Working on the static analyzer

[Mir Immad via Gcc]

HI David,
I've been tinkering with the static analyzer for the last few days. I find
the project of adding SARIF output to the analyzer intresting. I'm writing
this to let you know that I'm trying to learn the codebase.
Thank you.

On Tue, Jan 11, 2022, 7:09 PM David Malcolm  wrote:

> On Tue, 2022-01-11 at 11:03 +0530, Mir Immad via Gcc wrote:
> > Hi everyone,
>
> Hi, and welcome.
>
> > I intend to work on the static analyzer. Are these documents enough to
> > get
> > started: https://gcc.gnu.org/onlinedocs/gccint and
> >
> https://gcc.gnu.org/onlinedocs/gccint/Analyzer-Internals.html#Analyzer-Internals
>
> Yes.
>
> There are also some high-level notes here:
>   https://gcc.gnu.org/wiki/DavidMalcolm/StaticAnalyzer
>
> Also, given that the analyzer is part of GCC, the more general
> introductions to hacking on GCC will be useful.
>
> I recommend creating a trivial C source file with a bug in it (e.g. a
> 3-line function with a use-after-free), and stepping through the
> analyzer to get a sense of how it works.
>
> Hope this is helpful; don't hesitate to ask questions.
> Dave
>
>

GSoC: Working on the static analyzer

[Mir Immad via Gcc]

Hi everyone,

I intend to work on the static analyzer. Are these documents enough to get
started: https://gcc.gnu.org/onlinedocs/gccint and
https://gcc.gnu.org/onlinedocs/gccint/Analyzer-Internals.html#Analyzer-Internals
.

Thank you.