[pushed] analyzer: fix -Wanalyzer-use-of-uninitialized-value false +ve on "read" [PR108661]

[David Malcolm <dmalcolm@redhat.com>]

My integration testing shows many false positives from
-Wanalyzer-use-of-uninitialized-value.

One cause turns out to be that as of r13-1404-g97baacba963c06
fd_state_machine::on_stmt recognizes calls to "read", and returns true,
so that region_model::on_call_post doesn't call handle_unrecognized_call
on them, and so the analyzer erroneously "thinks" that the buffer
pointed to by "read" is never touched by the "read" call.

This works for "fread" because sm-file.cc implements kf_fread, which
handles calls to "fread" by clobbering the buffer pointed to.  In the
long term we should probably be smarter about this and bifurcate the
analysis to consider e.g. errors vs full reads vs partial reads, etc
(which I'm tracking in PR analyzer/108689).

In the meantime, this patch adds a kf_read for "read" analogous to the
one for "fread", fixing 6 false positives seen in git-2.39.0 and
2 in haproxy-2.7.1.

Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r13-5733-gc300e251f5b22d.

gcc/analyzer/ChangeLog:
	PR analyzer/108661
	* sm-fd.cc (class kf_read): New.
	(register_known_fd_functions): Register "read".
	* sm-file.cc (class kf_fread): Update comment.

gcc/testsuite/ChangeLog:
	PR analyzer/108661
	* gcc.dg/analyzer/fread-pr108661.c: New test.
	* gcc.dg/analyzer/read-pr108661.c: New test.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
---
 gcc/analyzer/sm-fd.cc                         | 33 +++++++++++++++
 gcc/analyzer/sm-file.cc                       | 10 ++++-
 .../gcc.dg/analyzer/fread-pr108661.c          | 40 +++++++++++++++++++
 gcc/testsuite/gcc.dg/analyzer/read-pr108661.c | 33 +++++++++++++++
 4 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/fread-pr108661.c
 create mode 100644 gcc/testsuite/gcc.dg/analyzer/read-pr108661.c

diff --git a/gcc/analyzer/sm-fd.cc b/gcc/analyzer/sm-fd.cc
index 494d802a1d4..d107390bc00 100644
--- a/gcc/analyzer/sm-fd.cc
+++ b/gcc/analyzer/sm-fd.cc
@@ -2659,6 +2659,38 @@ private:
   unsigned m_num_args;
 };
 
+/* Handler for "read".
+     ssize_t read(int fildes, void *buf, size_t nbyte);
+   See e.g. https://man7.org/linux/man-pages/man2/read.2.html   */
+
+class kf_read : public known_function
+{
+public:
+  bool matches_call_types_p (const call_details &cd) const final override
+  {
+    return (cd.num_args () == 3
+	    && cd.arg_is_pointer_p (1)
+	    && cd.arg_is_size_p (2));
+  }
+
+  /* For now, assume that any call to "read" fully clobbers the buffer
+     passed in.  This isn't quite correct (e.g. errors, partial reads;
+     see PR analyzer/108689), but at least stops us falsely complaining
+     about the buffer being uninitialized.  */
+  void impl_call_pre (const call_details &cd) const final override
+  {
+    region_model *model = cd.get_model ();
+    const svalue *ptr_sval = cd.get_arg_svalue (1);
+    if (const region *reg = ptr_sval->maybe_get_region ())
+      {
+	const region *base_reg = reg->get_base_region ();
+	const svalue *new_sval = cd.get_or_create_conjured_svalue (base_reg);
+	model->set_value (base_reg, new_sval, cd.get_ctxt ());
+      }
+  }
+};
+
+
 /* Populate KFM with instances of known functions relating to
    file descriptors.  */
 
@@ -2672,6 +2704,7 @@ register_known_fd_functions (known_function_manager &kfm)
   kfm.add ("listen", make_unique<kf_listen> ());
   kfm.add ("pipe", make_unique<kf_pipe> (1));
   kfm.add ("pipe2", make_unique<kf_pipe> (2));
+  kfm.add ("read", make_unique<kf_read> ());
   kfm.add ("socket", make_unique<kf_socket> ());
 }
 
diff --git a/gcc/analyzer/sm-file.cc b/gcc/analyzer/sm-file.cc
index 9cb4e32e286..d99a09b76c4 100644
--- a/gcc/analyzer/sm-file.cc
+++ b/gcc/analyzer/sm-file.cc
@@ -560,7 +560,11 @@ public:
   }
 };
 
-/* Handler for "fread"".  */
+/* Handler for "fread".
+     size_t fread(void *restrict buffer, size_t size, size_t count,
+		  FILE *restrict stream);
+   See e.g. https://en.cppreference.com/w/c/io/fread
+   and https://www.man7.org/linux/man-pages/man3/fread.3.html */
 
 class kf_fread : public known_function
 {
@@ -574,6 +578,10 @@ public:
 	    && cd.arg_is_pointer_p (3));
   }
 
+  /* For now, assume that any call to "fread" fully clobbers the buffer
+     passed in.  This isn't quite correct (e.g. errors, partial reads;
+     see PR analyzer/108689), but at least stops us falsely complaining
+     about the buffer being uninitialized.  */
   void impl_call_pre (const call_details &cd) const final override
   {
     region_model *model = cd.get_model ();
diff --git a/gcc/testsuite/gcc.dg/analyzer/fread-pr108661.c b/gcc/testsuite/gcc.dg/analyzer/fread-pr108661.c
new file mode 100644
index 00000000000..b51cf41ec2a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/fread-pr108661.c
@@ -0,0 +1,40 @@
+typedef __SIZE_TYPE__ size_t;
+
+extern size_t fread (void *, size_t, size_t, void *);
+
+struct ring
+{
+  char buf[1024];
+};
+
+int
+test_one_large_item (void *fp)
+{
+  struct ring ring;
+  int ret;
+
+  ret = fread(&ring, sizeof(ring), 1, fp);
+
+  if (ret != 1)
+    return 1;
+
+  if (ring.buf[0] > 1) /* { dg-bogus "use of uninitialized value" } */
+    return 2;
+  return 3;
+}
+
+int
+test_many_small_items (void *fp)
+{
+  struct ring ring;
+  int ret;
+
+  ret = fread(&ring, 1, sizeof(ring), fp);
+
+  if (ret != sizeof(ring))
+    return 1;
+
+  if (ring.buf[0] > 1) /* { dg-bogus "use of uninitialized value" } */
+    return 2;
+  return 3;
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/read-pr108661.c b/gcc/testsuite/gcc.dg/analyzer/read-pr108661.c
new file mode 100644
index 00000000000..70335e6fef1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/read-pr108661.c
@@ -0,0 +1,33 @@
+typedef long int ssize_t;
+typedef long unsigned int size_t;
+
+extern int open(const char* __file, int __oflag, ...) __attribute__((__nonnull__(1)));
+extern int close(int __fd);
+extern ssize_t read(int __fd, void* __buf, size_t __nbytes);
+
+struct ring
+{
+  char buf[1024];
+};
+
+int
+test(const char* name)
+{
+  struct ring ring;
+  int fd;
+  int ret;
+
+  fd = open(name, 00);
+  if (fd < 0)
+    return 0;
+
+  ret = read(fd, &ring, sizeof(ring));
+  close(fd);
+
+  if (ret != sizeof(ring))
+    return 1;
+
+  if (ring.buf[0] > 1) /* { dg-bogus "use of uninitialized value" } */
+    return 2;
+  return 3;
+}
-- 
2.26.3