This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] avoid warning on constant strncpy until next statement is reachable (PR 87028)


On 08/25/2018 11:24 PM, Jeff Law wrote:
On 08/24/2018 09:58 AM, Martin Sebor wrote:
The warning suppression for -Wstringop-truncation looks for
the next statement after a truncating strncpy to see if it
adds a terminating nul.  This only works when the next
statement can be reached using the Gimple statement iterator
which isn't until after gimplification.  As a result, strncpy
calls that truncate their constant argument that are being
folded to memcpy this early get diagnosed even if they are
followed by the nul assignment:

  const char s[] = "12345";
  char d[3];

  void f (void)
  {
    strncpy (d, s, sizeof d - 1);   // -Wstringop-truncation
    d[sizeof d - 1] = 0;
  }

To avoid the warning I propose to defer folding strncpy to
memcpy until the pointer to the basic block the strnpy call
is in can be used to try to reach the next statement (this
happens as early as ccp1).  I'm aware of the preference to
fold things early but in the case of strncpy (a relatively
rarely used function that is often misused), getting
the warning right while folding a bit later but still fairly
early on seems like a reasonable compromise.  I fear that
otherwise, the false positives will drive users to adopt
other unsafe solutions (like memcpy) where these kinds of
bugs cannot be as readily detected.

Tested on x86_64-linux.

Martin

PS There still are outstanding cases where the warning can
be avoided.  I xfailed them in the test for now but will
still try to get them to work for GCC 9.

gcc-87028.diff


PR tree-optimization/87028 - false positive -Wstringop-truncation strncpy with global variable source string
gcc/ChangeLog:

	PR tree-optimization/87028
	* gimple-fold.c (gimple_fold_builtin_strncpy): Avoid folding when
	statement doesn't belong to a basic block.
	* tree-ssa-strlen.c (maybe_diag_stxncpy_trunc): Handle MEM_REF on
	the left hand side of assignment.

gcc/testsuite/ChangeLog:

	PR tree-optimization/87028
	* c-c++-common/Wstringop-truncation.c: Remove xfails.
	* gcc.dg/Wstringop-truncation-5.c: New test.

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 07341eb..284c2fb 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -1702,6 +1702,11 @@ gimple_fold_builtin_strncpy (gimple_stmt_iterator *gsi,
   if (tree_int_cst_lt (ssize, len))
     return false;

+  /* Defer warning (and folding) until the next statement in the basic
+     block is reachable.  */
+  if (!gimple_bb (stmt))
+    return false;
I think you want cfun->cfg as the test here.  They should be equivalent
in practice.


diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index d0792aa..f1988f6 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -1981,6 +1981,23 @@ maybe_diag_stxncpy_trunc (gimple_stmt_iterator gsi, tree src, tree cnt)
 	  && known_eq (dstoff, lhsoff)
 	  && operand_equal_p (dstbase, lhsbase, 0))
 	return false;
+
+      if (code == MEM_REF
+	  && TREE_CODE (lhsbase) == SSA_NAME
+	  && known_eq (dstoff, lhsoff))
+	{
+	  /* Extract the referenced variable from something like
+	       MEM[(char *)d_3(D) + 3B] = 0;  */
+	  gimple *def = SSA_NAME_DEF_STMT (lhsbase);
+	  if (gimple_nop_p (def))
+	    {
+	      lhsbase = SSA_NAME_VAR (lhsbase);
+	      if (lhsbase
+		  && dstbase
+		  && operand_equal_p (dstbase, lhsbase, 0))
+		return false;
+	    }
+	}
If you find yourself looking at SSA_NAME_VAR, you're usually barking up
the wrong tree.  It'd be easier to suggest something here if I could see
the gimple (with virtual operands).  BUt at some level what you really
want to do is make sure the base of the MEM_REF is the same as what got
passed as the destination of the strncpy.  You'd want to be testing
SSA_NAMEs in that case.

I replied to Richard with the code that this hunk handles:

  https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01697.html

I couldn't find any other way to determine that d_3(D) in

  MEM[(char *)d_3(D) + 3B] = 0;

is the same as the first argument in:

  __builtin_strncpy (d_3(D), "12345", 4);

The MEM_REF operand is an SSA_NAME whose DEF_STMT is
a GIMPLE_NOP and whose SSA_NAME_VAR is the PARAM_DECL d.
Where else can I get the variable from?

Martin


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]