Re: GCC 10.4 Release Candidate available from gcc.gnu.org

[Iain Sandoe]

Hi Jakub,

> On 21 Jun 2022, at 12:33, Jakub Jelinek via Gcc  wrote:
> 
> The first release candidate for GCC 10.4 is available from
> 
> https://gcc.gnu.org/pub/gcc/snapshots/10.4.0-RC-20220621/
> ftp://gcc.gnu.org/pub/gcc/snapshots/10.4.0-RC-20220621/
> 
> and shortly its mirrors.  It has been generated from git commit
> r10-10862-g3c390f4ad27c3d79fd1817276a6d3217fd9bfb51.
> 
> I have so far bootstrapped and tested the release candidate on
> x86_64-linux.  Please test it and report any issues to bugzilla.
> 
> If all goes well, I'd like to release 10.4 on Tuesday, June 28th.
> 

I bootstrapped and tested on i686, powerpc and x86_64 Darwin
from darwin9 to darwin21 (archs as appropriate) - all supported
languages - no new issues seen.  I also checked that bootstrap
worked with Apple gcc-4.2.1 on darwin9, 10 and with clang 3.4
on darwin12 

Iain

Re: remove intl/ directory?

[Eric Gallager via Gcc]

On Thu, Jun 23, 2022 at 12:25 AM Bruno Haible  wrote:
>
> Iain Sandoe wrote:
> > Yes (
> > # We can use an in-tree build of libintl.
> > if test -f  ifelse([$1],,[../gettext-runtime],[$1])/uninstalled-config.sh; 
> > then
> >   
> > relative_builddir='ifelse([$1],,[${top_builddir}/..],[$1]/..)/gettext-runtime'
> >   .  ifelse([$1],,[../gettext-runtime],[$1])/uninstalled-config.sh
> > elif test -f  ifelse([$1],,[../intl],[$1])/config.intl; then
> >   .  ifelse([$1],,[../intl],[$1])/config.intl
> > fi
> > )
> > and it works ...
>
> Good!
>
> > … although now I see some configure warnings about not being able to access 
> > build-aux (which I do not recall seeing with the previous hack - but that 
> > could be just bad memory ;) )
>
> You can get warnings if you _move_ the gettext-runtime directory so that it
> becomes a sibling directory of 'gcc'. You should *not* get warnings if you
> create a symlink, sibling of the 'gcc' directory, to the
> gettext-20220620/gettext-runtime/ directory.
>
> > FWIW this following snippet would be just as broken on macOS as other noted 
> > platforms - it would need auto-foo-provided shared lib extension - or the 
> > equivalent to be used.
> > …  is there any reason that all platforms with non-’so’ suffixes would not 
> > work with that change?
>
> On macOS (with .dylib instead of .so) it would probably work.
>
> However, AIX and HP-UX will not work, because (as I understand it) if you want
> to have a binary, say cc1, which depends on libintl, then
>   - the cc1 that accesses /usr/local/lib/libintl.$suffix
> and
>   - the cc1 that accesses 
> /home/user/build/gcc-snap/gettext-runtime/intl/.libs/libintl.$suffix
> must necessarily be different. You cannot just install the second one in
> public locations, because it will have the wrong shared library filename
> hardcoded into it. This is why on these systems, libtool has to rebuild
> executables during "make install".
>
> Anyway, you said that for GCC, the important case is to build libintl as a
> static (non-shared) library.
>
> > > There is also a GCC specific quirk, that I upstreamed into GNU gettext:
> > > https://git.savannah.gnu.org/gitweb/?p=gettext.git;a=commitdiff;h=fdc2bd236a6a62b477c1fca4205df10b0e64266b
> >
> > IMHO we need to fix this ^ in GCC config - since gettext-runtime accepts 
> > “—with-pic” we should amend the GCC configury to pass —with-pic [to GMP et. 
> > al. as well, currently to build in-tree with host-shared, needs a manual 
> > —with-pic on the configure too]
>
> Indeed, the option '--with-pic' (from libtool) has the same effect as
> '--enable-host-shared'. If you can arrange to pass '--with-pic' instead
> of '--enable-host-shared', I can revert the addition of the option
> '--enable-host-shared' in gettext-runtime/intl from two days ago.
>
> > I think that we now need to deal with the GCC-side of the configury …
> >
> > 1) add logic [like GMP et. al.] to specify an external source of the 
> > library (when there is no-in-tree source present)
>
> Are you aware that gettext.m4 already introduces the configure options
>   --with-libintl-prefix[=DIR]  search for libintl in DIR/include and DIR/lib
>   --without-libintl-prefix don't search for libintl in includedir and 
> libdir
> ?
>

N.B. that there's already at least one issue open about those flags
that I can think of OTTOMH; it's part of the reason I wanted to go
about updating GCC configury in the first place:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78251

> Bruno
>
>
>

Re: remove intl/ directory?

[Iain Sandoe via Gcc]

> On 23 Jun 2022, at 07:51, Iain Sandoe via Gcc  wrote:
> 
>> On 23 Jun 2022, at 05:24, Bruno Haible  wrote:
>> 
>> Iain Sandoe wrote:
> 
>>> … although now I see some configure warnings about not being able to access 
>>> build-aux (which I do not recall seeing with the previous hack - but that 
>>> could be just bad memory ;) )
>> 
>> You can get warnings if you _move_ the gettext-runtime directory so that it
>> becomes a sibling directory of 'gcc'. You should *not* get warnings if you
>> create a symlink, sibling of the 'gcc' directory, to the
>> gettext-20220620/gettext-runtime/ directory.
> 
> I did symlink, and agree it should work - I’ll need to try and repeat when 
> next I have some time.
>> 
>>> FWIW this following snippet would be just as broken on macOS as other noted 
>>> platforms - it would need auto-foo-provided shared lib extension - or the 
>>> equivalent to be used.
>>> …  is there any reason that all platforms with non-’so’ suffixes would not 
>>> work with that change?
>> 
>> On macOS (with .dylib instead of .so) it would probably work.
>> 
>> However, AIX and HP-UX will not work, because (as I understand it) if you 
>> want
>> to have a binary, say cc1, which depends on libintl, then
>> - the cc1 that accesses /usr/local/lib/libintl.$suffix
>> and
>> - the cc1 that accesses 
>> /home/user/build/gcc-snap/gettext-runtime/intl/.libs/libintl.$suffix
>> must necessarily be different. You cannot just install the second one in
>> public locations, because it will have the wrong shared library filename
>> hardcoded into it. This is why on these systems, libtool has to rebuild
>> executables during "make install".
> 
> Ah, actually a similar situation might apply to the macOS case, you would 
> either need
> to build it “@rpath” and install the library in the exe’s dir or build and 
> install it into
> ‘prefix’ (that puts the full pathname into the dylib, in a similar way to AIX 
> / HP-UX).
> This is also requires a bit of juggling on macOS (I have patches in flight to 
> make all
> the runtimes for GCC built with ‘@rpath’ and using embedded rpaths in exe) 
> hopefully
> for GCC-13
> … so let’s quietly forget the shared case for now...
> 
>> Anyway, you said that for GCC, the important case is to build libintl as a
>> static (non-shared) library.
> 
> Yes, in a 1:1 replacement for ‘intl’ that’s the case, we can figure out 
> shared stuff as a follow-on.
> 
>>> I think that we now need to deal with the GCC-side of the configury …
>>> 
>>> 1) add logic [like GMP et. al.] to specify an external source of the 
>>> library (when there is no-in-tree source present)
>> 
>> Are you aware that gettext.m4 already introduces the configure options
>> --with-libintl-prefix[=DIR]  search for libintl in DIR/include and DIR/lib
>> --without-libintl-prefix don't search for libintl in includedir and 
>> libdir
> 
> Hmm - the following cases:
> a) there’s no gettext-runtime in the source tree and the user needs to 
> configure —with-libintl-prefix= 
> b) there is a  gettext-runtime in the source tree and the user decides to 
> configure —with-libintl-prefix= (which will be ignored if we take the way the 
> other in-tree builds are handled as ’status quo’
> c) there is a  gettext-runtime in the source tree  and no 
> —with-libintl-prefix= is given (we expect to pick up the in-tree build 
> silently and automatically).
> 
> … in case (a) we’d need to arrange for the gettext macro to be called in 
> configure, but I don’t think it will play nicely with gettext-sister .. so 
> there’s some work needed here.
> in case (b) I’m not sure what will happen - will the configure for libintl 
> just point the variables to the install suggested?

.. update on (b).
OK, so there are two issues I can see [let’s put the flags pollution issues to 
one side for now, since people sometimes forget that the configuration 
namespace is flat and overload save_CFLAGS et. al]

1. —with-libintl-prefix= is not going to work on macOS, when the prefix 
contains only a convenience lib (which is what I prefer for GCC).
  This is because the configure has no way to know that libintl.a depends on 
-framework CoreFoundation, AFAICS there’s nowhere it could even look - the info 
is not in the libtool lib (and that does not seem to get installed anyway for 
the snapshot) 
  I'd suppose that a shared library would work (since there are no hidden deps) 
.. 

AFAICT, ‘intl/‘ works OK with this (iff I manually add -framework 
CoreFoundation to the LDFLAGS) and LIBINTL gets set to the right line for using 
the installed variant.

2/ .. the current gettext-runtime snapshot does not work, I think because it 
assumes if we’re not using the in-tree case, then the lib must be coming from 
libc - which is not the case for most non-glibc clients (so it seems to ignore 
the —with… and populate the uninstalled-gettext.sh with the in-tree data anyway)



(we still need to deal with case (a) but case (b) could be fixable, perhaps at 
a cost of having to

Re: remove intl/ directory?

[Iain Sandoe via Gcc]

> On 23 Jun 2022, at 16:40, Iain Sandoe via Gcc  wrote:
> 
> 
> 
>> On 23 Jun 2022, at 07:51, Iain Sandoe via Gcc  wrote:
>> 
>>> On 23 Jun 2022, at 05:24, Bruno Haible  wrote:
>>> 
>>> Iain Sandoe wrote:
>> 
 … although now I see some configure warnings about not being able to 
 access build-aux (which I do not recall seeing with the previous hack - 
 but that could be just bad memory ;) )
>>> 
>>> You can get warnings if you _move_ the gettext-runtime directory so that it
>>> becomes a sibling directory of 'gcc'. You should *not* get warnings if you
>>> create a symlink, sibling of the 'gcc' directory, to the
>>> gettext-20220620/gettext-runtime/ directory.
>> 
>> I did symlink, and agree it should work - I’ll need to try and repeat when 
>> next I have some time.
>>> 
 FWIW this following snippet would be just as broken on macOS as other 
 noted platforms - it would need auto-foo-provided shared lib extension - 
 or the equivalent to be used.
 …  is there any reason that all platforms with non-’so’ suffixes would not 
 work with that change?
>>> 
>>> On macOS (with .dylib instead of .so) it would probably work.
>>> 
>>> However, AIX and HP-UX will not work, because (as I understand it) if you 
>>> want
>>> to have a binary, say cc1, which depends on libintl, then
>>> - the cc1 that accesses /usr/local/lib/libintl.$suffix
>>> and
>>> - the cc1 that accesses 
>>> /home/user/build/gcc-snap/gettext-runtime/intl/.libs/libintl.$suffix
>>> must necessarily be different. You cannot just install the second one in
>>> public locations, because it will have the wrong shared library filename
>>> hardcoded into it. This is why on these systems, libtool has to rebuild
>>> executables during "make install".
>> 
>> Ah, actually a similar situation might apply to the macOS case, you would 
>> either need
>> to build it “@rpath” and install the library in the exe’s dir or build and 
>> install it into
>> ‘prefix’ (that puts the full pathname into the dylib, in a similar way to 
>> AIX / HP-UX).
>> This is also requires a bit of juggling on macOS (I have patches in flight 
>> to make all
>> the runtimes for GCC built with ‘@rpath’ and using embedded rpaths in exe) 
>> hopefully
>> for GCC-13
>> … so let’s quietly forget the shared case for now...
>> 
>>> Anyway, you said that for GCC, the important case is to build libintl as a
>>> static (non-shared) library.
>> 
>> Yes, in a 1:1 replacement for ‘intl’ that’s the case, we can figure out 
>> shared stuff as a follow-on.
>> 
 I think that we now need to deal with the GCC-side of the configury …
 
 1) add logic [like GMP et. al.] to specify an external source of the 
 library (when there is no-in-tree source present)
>>> 
>>> Are you aware that gettext.m4 already introduces the configure options
>>> --with-libintl-prefix[=DIR]  search for libintl in DIR/include and DIR/lib
>>> --without-libintl-prefix don't search for libintl in includedir and 
>>> libdir
>> 
>> Hmm - the following cases:
>> a) there’s no gettext-runtime in the source tree and the user needs to 
>> configure —with-libintl-prefix= 
>> b) there is a  gettext-runtime in the source tree and the user decides to 
>> configure —with-libintl-prefix= (which will be ignored if we take the way 
>> the other in-tree builds are handled as ’status quo’
>> c) there is a  gettext-runtime in the source tree  and no 
>> —with-libintl-prefix= is given (we expect to pick up the in-tree build 
>> silently and automatically).
>> 
>> … in case (a) we’d need to arrange for the gettext macro to be called in 
>> configure, but I don’t think it will play nicely with gettext-sister .. so 
>> there’s some work needed here.
>> in case (b) I’m not sure what will happen - will the configure for libintl 
>> just point the variables to the install suggested?
> 
> .. update on (b).
> OK, so there are two issues I can see [let’s put the flags pollution issues 
> to one side for now, since people sometimes forget that the configuration 
> namespace is flat and overload save_CFLAGS et. al]
> 
> 1. —with-libintl-prefix= is not going to work on macOS, when the prefix 
> contains only a convenience lib (which is what I prefer for GCC).
>  This is because the configure has no way to know that libintl.a depends on 
> -framework CoreFoundation, AFAICS there’s nowhere it could even look - the 
> info is not in the libtool lib (and that does not seem to get installed 
> anyway for the snapshot) 
>  I'd suppose that a shared library would work (since there are no hidden 
> deps) .. 
> 
> AFAICT, ‘intl/‘ works OK with this (iff I manually add -framework 
> CoreFoundation to the LDFLAGS) and LIBINTL gets set to the right line for 
> using the installed variant.

this ^ is actually inconsistent with the other deps .. as noted  below.
> 
> 2/ .. the current gettext-runtime snapshot does not work, I think because it 
> assumes if we’re not using the in-tree case, then the lib must be coming from 
> libc 

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 pa

[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
+  

gcc-10-20220623 is now available

[GCC Administrator via Gcc]

Snapshot gcc-10-20220623 is now available on
  https://gcc.gnu.org/pub/gcc/snapshots/10-20220623/
and on various mirrors, see http://gcc.gnu.org/mirrors.html for details.

This snapshot has been generated from the GCC 10 git branch
with the following options: git://gcc.gnu.org/git/gcc.git branch 
releases/gcc-10 revision 94dd514748a75862dc123d5da0673dffbf48239e

You'll find:

 gcc-10-20220623.tar.xz   Complete GCC

  SHA256=a4417bc36aa036db13b551c21339f29177d3dd63e04eecaa13cd20a1d658f9e5
  SHA1=75a07f041dada80593c8a571f782861b6a44ede9

Diffs from 10-20220616 are available in the diffs/ subdirectory.

When a particular snapshot is ready for public consumption the LATEST-10
link is updated and a message is sent to the gcc list.  Please do not use
a snapshot before it has been announced that way.

Re: [PATCH] analyzer PR 106003

[David Malcolm via Gcc]

On Fri, 2022-06-24 at 00:00 +0530, Mir Immad wrote:

Thanks for the updated patch.

This is close to being ready.

One big issue is the target hook idea for isolating the target's
definition of the O_* flags as per Joseph's suggestion here:
  https://gcc.gnu.org/pipermail/gcc/2022-June/238961.html
Given Joseph's comment here:
  https://gcc.gnu.org/pipermail/gcc/2022-June/238954.html
I'm inclined to think that the target hook stuff could be done as a
followup.

Indentation has gone a bit wrong (presumably due to issue with the
mailer); the main thing to watch out for is to use tabs for 8 space
indentations within the file.  Enabling "visual whitespace" (or
whatever it's called) in your editor can help with this.

Every patch that adds any options (e.g. to gcc/analyzer/analyzer.opt)
needs to document those options, in gcc/doc/invoke.texi.  Grep for one
of the existing analyzer options in invoke.texi to see what you need to
add for the new options.

You'll need a ChangeLog entry before this can be pushed.  For better or
worse, the project requires ChangeLog entries.  You can
use the script ./contrib/mklog.py to help generate this.  Some more
information is here:
  https://www.gnu.org/prep/standards/html_node/Change-Logs.html
In GCC our ChangeLog entries are part of the git commit message, and we
have a serverside script to update the ChangeLog files in the source
tree (to avoid constantly having merge conflicts).

The Subject line for the commit should have an "analyzer: " prefix
and reference the bugzilla problem report (PR 106003)

Before you can push the patch, you'll have to test it with a full
bootstrap and regression test run.  Let me know if you need help with
that.

Various other comments inline below...

[...snip...]

> diff --git gcc/analyzer/analyzer.opt gcc/analyzer/analyzer.opt
> index 23dfc797cea..e2a629bb42c 100644
> --- gcc/analyzer/analyzer.opt
> +++ gcc/analyzer/analyzer.opt

[...snip...]
> +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.

These should be capitalized as English sentences, and the "must-be-a-
valid" wording doesn't feel quite right to me.

Reword this to something like:

"Warn about code paths in which a file descriptor is used without being
checked for validity."

or somesuch (all on one line).

[...snip...]

> 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 

We'll want to drop the #include  once we have a target hook.

[...snip...]
> +
> +  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;
> +  }

Please add coverage to the testsuite for the case of accessing negative
and non-negative constant integer values: what happens when we hit the
above cases?

In particular, IIRC 0, 1, and 2 are stdin, stdout, and stderr
respectively, and are already open at the start of "main" (though not
necessarily at the s