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

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

[Immad Mir]

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 <mirimmad@outlook.com>
---
 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);
+}
+
+
-- 
2.25.1

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

[David Malcolm]

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 <mirimmad@outlook.com>
> ---
>  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]

 > 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 <dmalcolm@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 <mirimmad@outlook.com>
> > ---
> >  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]

[David Malcolm]

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 <dmalcolm@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 <mirimmad@outlook.com>
> > > ---
> > >  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]

 > 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 <dmalcolm@redhat.com> 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 <dmalcolm@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 <mirimmad@outlook.com>
> > > > ---
> > > >  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]

[David Malcolm]

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 <dmalcolm@redhat.com>
> 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 <
> > > dmalcolm@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 <mirimmad@outlook.com>
> > > > > ---
> > > > >  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]

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 <dmalcolm@redhat.com> 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 <dmalcolm@redhat.com>
> > 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 <
> > > > dmalcolm@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 <mirimmad@outlook.com>
> > > > > > ---
> > > > > >  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
> > > > >
> > > > >
> > > > >
> > > > >
> > >
> > >
> > >
>
>
>

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

[Immad Mir]

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.
	* gcc.dg/analyzer/fd-uninit-1.c: Remove bogus
	warning.
Signed-off-by: Immad Mir <mirimmad@outlook.com>
---
 gcc/analyzer/sm-fd.cc                       | 10 +++++---
 gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c    | 27 ++++++++++++++++++++-
 gcc/testsuite/gcc.dg/analyzer/fd-uninit-1.c |  2 --
 3 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
index 8bb76d72b05..e02b86baad1 100644
--- a/gcc/analyzer/sm-fd.cc
+++ b/gcc/analyzer/sm-fd.cc
@@ -971,7 +971,8 @@ fd_state_machine::check_for_dup (sm_context *sm_ctxt, const supernode *node,
   state_t state_arg_1 = sm_ctxt->get_state (stmt, arg_1);
   if (state_arg_1 == m_stop)
     return;
-  if (!(is_constant_fd_p (state_arg_1) || is_valid_fd_p (state_arg_1)))
+  if (!(is_constant_fd_p (state_arg_1) || is_valid_fd_p (state_arg_1)
+	|| state_arg_1 == m_start))
     {
       check_for_open_fd (sm_ctxt, node, stmt, call, callee_fndecl,
 			 DIRS_READ_WRITE);
@@ -983,7 +984,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,
@@ -999,7 +1000,8 @@ fd_state_machine::check_for_dup (sm_context *sm_ctxt, const supernode *node,
       if (state_arg_2 == m_stop)
 	return;
       /* Check if -1 was passed as second argument to dup2.  */
-      if (!(is_constant_fd_p (state_arg_2) || is_valid_fd_p (state_arg_2)))
+      if (!(is_constant_fd_p (state_arg_2) || is_valid_fd_p (state_arg_2)
+	    || state_arg_2 == m_start))
 	{
 	  sm_ctxt->warn (
 	      node, stmt, arg_2,
@@ -1011,7 +1013,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..b971d31b1c7 100644
--- a/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
@@ -220,4 +220,29 @@ test_19 (const char *path, void *buf)
         close (fd);
     }
     
-}
\ No newline at end of file
+}
+
+extern int m;
+
+void
+test_20 ()
+{
+    int fd = dup (m); 
+    close (fd);
+}
+
+void
+test_21 ()
+{
+    int fd = dup2 (m, 1); 
+    close (fd);
+}
+
+void
+test_22 (int flags)
+{
+    int fd = dup3 (m, 1, flags);
+    close (fd);
+}
+
+
diff --git a/gcc/testsuite/gcc.dg/analyzer/fd-uninit-1.c b/gcc/testsuite/gcc.dg/analyzer/fd-uninit-1.c
index b5b189ece98..1084d1b4da2 100644
--- a/gcc/testsuite/gcc.dg/analyzer/fd-uninit-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/fd-uninit-1.c
@@ -9,8 +9,6 @@ test_1 ()
 {
   int m;
   return dup (m); /* { dg-warning "use of uninitialized value 'm'" "uninit" } */
-  /* { dg-bogus "'dup' on possibly invalid file descriptor 'm'" "invalid fd false +ve" { xfail *-*-* } .-1 } */
-  /* XFAIL: probably covered by fix for PR analyzer/106551.  */
 }
 
 int
-- 
2.25.1

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

[David Malcolm]

On Thu, 2022-08-11 at 14:41 +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.
>         * gcc.dg/analyzer/fd-uninit-1.c: Remove bogus
>         warning.
> Signed-off-by: Immad Mir <mirimmad@outlook.com>

Thanks for the updated patch; looks ready for trunk.

Dave

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

[Immad Mir]

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 <mirimmad@outlook.com>
---
 gcc/analyzer/sm-fd.cc                    |  4 ++--
 gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c | 27 +++++++++++++++++++++++-
 2 files changed, 28 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..bc823d1ce4e 100644
--- a/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/fd-dup-1.c
@@ -220,4 +220,29 @@ test_19 (const char *path, void *buf)
         close (fd);
     }
     
-}
\ No newline at end of file
+}
+
+extern int m;
+
+void
+test_20 ()
+{
+    int fd = dup (m); /* { dg-warning "'dup' on possibly invalid file descriptor 'm'" } */
+    close (fd);
+}
+
+void
+test_21 ()
+{
+    int fd = dup2 (m, 1); /* { dg-warning "'dup2' on possibly invalid file descriptor 'm'" } */
+    close (fd);
+}
+
+void
+test_22 (int flags)
+{
+    int fd = dup3 (m, 1, flags); /* { dg-warning "'dup3' on possibly invalid file descriptor 'm'" } */
+    close (fd);
+}
+
+
-- 
2.25.1

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

[Immad Mir]

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 <mirimmad@outlook.com>
---
 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,
-- 
2.25.1

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

[Immad Mir]

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 <mirimmad@outlook.com>
---
 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,
-- 
2.25.1

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

[David Malcolm]

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 <mirimmad@outlook.com>
> ---
>  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: fix ICE casued by dup2 in sm-fd.cc[PR106551]

[Mir Immad]

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 <dmalcolm@redhat.com> 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 <mirimmad@outlook.com>
> > ---
> >  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
>
>