[PATCH v2] middle-end/104854: Limit strncmp overread warnings

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

The size argument in strncmp only describe the maximum length to which
to compare two strings and is not an indication of sizes of the two
source strings.  Do not warn if it is larger than the two input strings
because it is entirely likely that the size argument is a conservative
maximum to accommodate inputs of different lengths and only a subset is
reachable through the current code path or that it is some other
application-specific property completely unrelated to the sizes of the
input strings.

gcc/ChangeLog:

	middle-end/104854
	* gimple-ssa-warn-access.cc
	(pass_waccess::warn_zero_sized_strncmp_inputs): New function.
	(pass_waccess::check_strncmp): Use it.

gcc/testsuite/ChangeLog:

	middle-end/104854
	* gcc.dg/Wstringop-overread.c (test_strncmp_array): Don't expect
	failures for non-zero sizes.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---

Changes from v1:

A little better approach, ensuring that it tries to warn on zero length
inputs if the size of at least one of the two sources is known.

Also cc'ing Martin so that we can discuss approach on the list instead
of on the bug.  To summarize the discussion so far, Martin suggests that
the warning be split into levels but I'm contesting the utility of the
heuristics as a compiler warning given the looseness of the relationship
between the size argument and the inputs in the case of these functions.


 gcc/gimple-ssa-warn-access.cc             | 69 +++++++++--------------
 gcc/testsuite/gcc.dg/Wstringop-overread.c |  2 +-
 2 files changed, 28 insertions(+), 43 deletions(-)

diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc
index 75297ed7c9e..15299770e29 100644
--- a/gcc/gimple-ssa-warn-access.cc
+++ b/gcc/gimple-ssa-warn-access.cc
@@ -2137,6 +2137,9 @@ private:
   /* Return true if use follows an invalidating statement.  */
   bool use_after_inval_p (gimple *, gimple *, bool = false);
 
+  /* Emit an overread warning for zero sized inputs to strncmp.  */
+  void warn_zero_sized_strncmp_inputs (gimple *, tree *, access_data *);
+
   /* A pointer_query object to store information about pointers and
      their targets in.  */
   pointer_query m_ptr_qry;
@@ -2619,8 +2622,20 @@ pass_waccess::check_stxncpy (gcall *stmt)
 		data.mode, &data, m_ptr_qry.rvals);
 }
 
-/* Check a call STMT to stpncpy() or strncpy() for overflow and warn
-   if it does.  */
+/* Warn for strncmp on a zero sized source or when an argument isn't
+   nul-terminated.  */
+void
+pass_waccess::warn_zero_sized_strncmp_inputs (gimple *stmt, tree *bndrng,
+					      access_data *pad)
+{
+  tree func = get_callee_fndecl (stmt);
+  location_t loc = gimple_location (stmt);
+  maybe_warn_for_bound (OPT_Wstringop_overread, loc, stmt, func, bndrng,
+			size_zero_node, pad);
+}
+
+/* Check a call STMT to strncmp () for overflow and warn if it does.  This is
+   limited to checking for NUL terminated arrays for now.  */
 
 void
 pass_waccess::check_strncmp (gcall *stmt)
@@ -2678,46 +2693,16 @@ pass_waccess::check_strncmp (gcall *stmt)
   if (!bndrng[0] || integer_zerop (bndrng[0]))
     return;
 
-  if (len1 && tree_int_cst_lt (len1, bndrng[0]))
-    bndrng[0] = len1;
-  if (len2 && tree_int_cst_lt (len2, bndrng[0]))
-    bndrng[0] = len2;
-
-  /* compute_objsize almost never fails (and ultimately should never
-     fail).  Don't bother to handle the rare case when it does.  */
-  if (!compute_objsize (arg1, stmt, 1, &adata1.src, &m_ptr_qry)
-      || !compute_objsize (arg2, stmt, 1, &adata2.src, &m_ptr_qry))
-    return;
-
-  /* Compute the size of the remaining space in each array after
-     subtracting any offset into it.  */
-  offset_int rem1 = adata1.src.size_remaining ();
-  offset_int rem2 = adata2.src.size_remaining ();
-
-  /* Cap REM1 and REM2 at the other if the other's argument is known
-     to be an unterminated array, either because there's no space
-     left in it after adding its offset or because it's constant and
-     has no nul.  */
-  if (rem1 == 0 || (rem1 < rem2 && lendata1.decl))
-    rem2 = rem1;
-  else if (rem2 == 0 || (rem2 < rem1 && lendata2.decl))
-    rem1 = rem2;
-
-  /* Point PAD at the array to reference in the note if a warning
-     is issued.  */
-  access_data *pad = len1 ? &adata2 : &adata1;
-  offset_int maxrem = wi::max (rem1, rem2, UNSIGNED);
-  if (lendata1.decl || lendata2.decl
-      || maxrem < wi::to_offset (bndrng[0]))
-    {
-      /* Warn when either argument isn't nul-terminated or the maximum
-	 remaining space in the two arrays is less than the bound.  */
-      tree func = get_callee_fndecl (stmt);
-      location_t loc = gimple_location (stmt);
-      maybe_warn_for_bound (OPT_Wstringop_overread, loc, stmt, func,
-			    bndrng, wide_int_to_tree (sizetype, maxrem),
-			    pad);
-    }
+  /* compute_objsize almost never fails (and ultimately should never fail).
+     Don't bother to handle the rare case when it does.  Warn if either the
+     source or destination has zero size, since the minimum bound is non-zero,
+     hence guaranteeing an overread.  */
+  if (compute_objsize (arg1, stmt, 1, &adata1.src, &m_ptr_qry)
+      && adata1.src.size_remaining () == 0)
+    warn_zero_sized_strncmp_inputs (stmt, bndrng, &adata1);
+  if (compute_objsize (arg2, stmt, 1, &adata2.src, &m_ptr_qry)
+      && adata2.src.size_remaining () == 0)
+    warn_zero_sized_strncmp_inputs (stmt, bndrng, &adata2);
 }
 
 /* Determine and check the sizes of the source and the destination
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overread.c b/gcc/testsuite/gcc.dg/Wstringop-overread.c
index 7db74029819..fb8e626439d 100644
--- a/gcc/testsuite/gcc.dg/Wstringop-overread.c
+++ b/gcc/testsuite/gcc.dg/Wstringop-overread.c
@@ -431,7 +431,7 @@ void test_strncmp_array (const char *s, int i)
 
   T (strncmp (a1, b1, 0));
   T (strncmp (a1, b1, 1));
-  T (strncmp (a1, b1, 2));      // { dg-warning "'strncmp' specified bound 2 exceeds source size 1" }
+  T (strncmp (a1, b1, 2));
 }
 
 
-- 
2.35.1