avoid false negatives in attr-nonstring-3.c (PR 83131)

Message ID bb57fb9f-054b-b601-c062-63a785501152@gmail.com
State New
Headers show
Series
  • avoid false negatives in attr-nonstring-3.c (PR 83131)
Related show

Commit Message

Martin Sebor Dec. 11, 2017, 10:54 p.m.
The attr-nonstring-3.c test fails on targets that expand
the calls to some of the tested string functions in builtins.c,
before they reach the checker in calls.c.  The failures were
reported on powrrpc64le but tests can be constructed that fail
even on other targets (including x86_64).

To fix these failures the checker needs to be invoked both
in builtins.c when the expansion takes place and in calls.c
otherwise.

The attached patch does that.  Since it also adjusts
the indentation in the changed functions, I used diff -w
to leave the whitespace changes out of it.

Bootstrapped and tested on x86_64-linux.  I verified the tests
pass using a powerpc64le-linux cross-compiler.

Martin
PR testsuite/83131 - c-c++/common/attr-nonstring-3 failure for strcmp tests on PowerPC

gcc/ChangeLog:

	PR testsuite/83131
	* builtins.c (expand_builtin_strlen): Use get_callee_fndecl.
	(expand_builtin_strcmp): Call maybe_warn_nonstring_arg.	
	(expand_builtin_strncmp): Same.

gcc/testsuite/ChangeLog:

	PR testsuite/83131
	* c-c++-common/attr-nonstring-4.c: New test.

Comments

Martin Sebor Dec. 18, 2017, 10:41 p.m. | #1
Ping: https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00665.html

On 12/11/2017 03:54 PM, Martin Sebor wrote:
> The attr-nonstring-3.c test fails on targets that expand

> the calls to some of the tested string functions in builtins.c,

> before they reach the checker in calls.c.  The failures were

> reported on powrrpc64le but tests can be constructed that fail

> even on other targets (including x86_64).

>

> To fix these failures the checker needs to be invoked both

> in builtins.c when the expansion takes place and in calls.c

> otherwise.

>

> The attached patch does that.  Since it also adjusts

> the indentation in the changed functions, I used diff -w

> to leave the whitespace changes out of it.

>

> Bootstrapped and tested on x86_64-linux.  I verified the tests

> pass using a powerpc64le-linux cross-compiler.

>

> Martin
Jeff Law Dec. 18, 2017, 11:06 p.m. | #2
On 12/11/2017 03:54 PM, Martin Sebor wrote:
> The attr-nonstring-3.c test fails on targets that expand

> the calls to some of the tested string functions in builtins.c,

> before they reach the checker in calls.c.  The failures were

> reported on powrrpc64le but tests can be constructed that fail

> even on other targets (including x86_64).

> 

> To fix these failures the checker needs to be invoked both

> in builtins.c when the expansion takes place and in calls.c

> otherwise.

> 

> The attached patch does that.  Since it also adjusts

> the indentation in the changed functions, I used diff -w

> to leave the whitespace changes out of it.

> 

> Bootstrapped and tested on x86_64-linux.  I verified the tests

> pass using a powerpc64le-linux cross-compiler.

> 

> Martin

> 

> gcc-83131.diff-w

> 

> 

> PR testsuite/83131 - c-c++/common/attr-nonstring-3 failure for strcmp tests on PowerPC

> 

> gcc/ChangeLog:

> 

> 	PR testsuite/83131

> 	* builtins.c (expand_builtin_strlen): Use get_callee_fndecl.

> 	(expand_builtin_strcmp): Call maybe_warn_nonstring_arg.	

> 	(expand_builtin_strncmp): Same.

> 

> gcc/testsuite/ChangeLog:

> 

> 	PR testsuite/83131

> 	* c-c++-common/attr-nonstring-4.c: New test.

OK.  Thanks for the -w output, it certainly makes this one easier to read.



jeff

Patch

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 6b25253..79616df 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -2819,8 +2819,7 @@  expand_builtin_strlen (tree exp, rtx target,
 {
   if (!validate_arglist (exp, POINTER_TYPE, VOID_TYPE))
     return NULL_RTX;
-  else
-    {
+
   struct expand_operand ops[4];
   rtx pat;
   tree len;
@@ -2883,7 +2882,7 @@  expand_builtin_strlen (tree exp, rtx target,
   /* Check to see if the argument was declared attribute nonstring
      and if so, issue a warning since at this point it's not known
      to be nul-terminated.  */
-      maybe_warn_nonstring_arg (TREE_OPERAND (CALL_EXPR_FN (exp), 0), exp);
+  maybe_warn_nonstring_arg (get_callee_fndecl (exp), exp);
 
   /* Now that we are assured of success, expand the source.  */
   start_sequence ();
@@ -2915,7 +2914,6 @@  expand_builtin_strlen (tree exp, rtx target,
 
   return target;
 }
-}
 
 /* Callback routine for store_by_pieces.  Read GET_MODE_BITSIZE (MODE)
    bytes from constant string DATA + OFFSET and return it as target
@@ -4426,13 +4424,11 @@  expand_builtin_strcmp (tree exp, ATTRIBUTE_UNUSED rtx target)
 
   insn_code cmpstr_icode = direct_optab_handler (cmpstr_optab, SImode);
   insn_code cmpstrn_icode = direct_optab_handler (cmpstrn_optab, SImode);
-  if (cmpstr_icode != CODE_FOR_nothing || cmpstrn_icode != CODE_FOR_nothing)
-    {
-      rtx arg1_rtx, arg2_rtx;
-      tree fndecl, fn;
+  if (cmpstr_icode == CODE_FOR_nothing && cmpstrn_icode == CODE_FOR_nothing)
+    return NULL_RTX;
+
   tree arg1 = CALL_EXPR_ARG (exp, 0);
   tree arg2 = CALL_EXPR_ARG (exp, 1);
-      rtx result = NULL_RTX;
 
   unsigned int arg1_align = get_pointer_alignment (arg1) / BITS_PER_UNIT;
   unsigned int arg2_align = get_pointer_alignment (arg2) / BITS_PER_UNIT;
@@ -4445,9 +4441,10 @@  expand_builtin_strcmp (tree exp, ATTRIBUTE_UNUSED rtx target)
   arg1 = builtin_save_expr (arg1);
   arg2 = builtin_save_expr (arg2);
 
-      arg1_rtx = get_memory_rtx (arg1, NULL);
-      arg2_rtx = get_memory_rtx (arg2, NULL);
+  rtx arg1_rtx = get_memory_rtx (arg1, NULL);
+  rtx arg2_rtx = get_memory_rtx (arg2, NULL);
 
+  rtx result = NULL_RTX;
   /* Try to call cmpstrsi.  */
   if (cmpstr_icode != CODE_FOR_nothing)
     result = expand_cmpstr (cmpstr_icode, target, arg1_rtx, arg2_rtx,
@@ -4501,6 +4498,12 @@  expand_builtin_strcmp (tree exp, ATTRIBUTE_UNUSED rtx target)
 	}
     }
 
+  /* Check to see if the argument was declared attribute nonstring
+     and if so, issue a warning since at this point it's not known
+     to be nul-terminated.  */
+  tree fndecl = get_callee_fndecl (exp);
+  maybe_warn_nonstring_arg (fndecl, exp);
+
   if (result)
     {
       /* Return the value in the proper mode for this function.  */
@@ -4515,14 +4518,11 @@  expand_builtin_strcmp (tree exp, ATTRIBUTE_UNUSED rtx target)
 
   /* Expand the library call ourselves using a stabilized argument
      list to avoid re-evaluating the function's arguments twice.  */
-      fndecl = get_callee_fndecl (exp);
-      fn = build_call_nofold_loc (EXPR_LOCATION (exp), fndecl, 2, arg1, arg2);
+  tree fn = build_call_nofold_loc (EXPR_LOCATION (exp), fndecl, 2, arg1, arg2);
   gcc_assert (TREE_CODE (fn) == CALL_EXPR);
   CALL_EXPR_TAILCALL (fn) = CALL_EXPR_TAILCALL (exp);
   return expand_call (fn, target, target == const0_rtx);
 }
-  return NULL_RTX;
-}
 
 /* Expand expression EXP, which is a call to the strncmp builtin. Return
    NULL_RTX if we failed the caller should emit a normal call, otherwise try to get
@@ -4532,8 +4532,6 @@  static rtx
 expand_builtin_strncmp (tree exp, ATTRIBUTE_UNUSED rtx target,
 			ATTRIBUTE_UNUSED machine_mode mode)
 {
-  location_t loc ATTRIBUTE_UNUSED = EXPR_LOCATION (exp);
-
   if (!validate_arglist (exp,
  			 POINTER_TYPE, POINTER_TYPE, INTEGER_TYPE, VOID_TYPE))
     return NULL_RTX;
@@ -4542,12 +4540,11 @@  expand_builtin_strncmp (tree exp, ATTRIBUTE_UNUSED rtx target,
      lengths, and it doesn't have side effects, then emit cmpstrnsi
      using length MIN(strlen(string)+1, arg3).  */
   insn_code cmpstrn_icode = direct_optab_handler (cmpstrn_optab, SImode);
-  if (cmpstrn_icode != CODE_FOR_nothing)
-  {
-    tree len, len1, len2, len3;
-    rtx arg1_rtx, arg2_rtx, arg3_rtx;
-    rtx result;
-    tree fndecl, fn;
+  if (cmpstrn_icode == CODE_FOR_nothing)
+    return NULL_RTX;
+
+  tree len;
+
   tree arg1 = CALL_EXPR_ARG (exp, 0);
   tree arg2 = CALL_EXPR_ARG (exp, 1);
   tree arg3 = CALL_EXPR_ARG (exp, 2);
@@ -4555,15 +4552,17 @@  expand_builtin_strncmp (tree exp, ATTRIBUTE_UNUSED rtx target,
   unsigned int arg1_align = get_pointer_alignment (arg1) / BITS_PER_UNIT;
   unsigned int arg2_align = get_pointer_alignment (arg2) / BITS_PER_UNIT;
 
-    len1 = c_strlen (arg1, 1);
-    len2 = c_strlen (arg2, 1);
+  tree len1 = c_strlen (arg1, 1);
+  tree len2 = c_strlen (arg2, 1);
+
+  location_t loc = EXPR_LOCATION (exp);
 
   if (len1)
     len1 = size_binop_loc (loc, PLUS_EXPR, ssize_int (1), len1);
   if (len2)
     len2 = size_binop_loc (loc, PLUS_EXPR, ssize_int (1), len2);
 
-    len3 = fold_convert_loc (loc, sizetype, arg3);
+  tree len3 = fold_convert_loc (loc, sizetype, arg3);
 
   /* If we don't have a constant length for the first, use the length
      of the second, if we know it.  If neither string is constant length,
@@ -4596,12 +4595,19 @@  expand_builtin_strncmp (tree exp, ATTRIBUTE_UNUSED rtx target,
      The actual new length parameter will be MIN(len,arg3) in this case.  */
   if (len != len3)
     len = fold_build2_loc (loc, MIN_EXPR, TREE_TYPE (len), len, len3);
-    arg1_rtx = get_memory_rtx (arg1, len);
-    arg2_rtx = get_memory_rtx (arg2, len);
-    arg3_rtx = expand_normal (len);
-    result = expand_cmpstrn_or_cmpmem (cmpstrn_icode, target, arg1_rtx,
+  rtx arg1_rtx = get_memory_rtx (arg1, len);
+  rtx arg2_rtx = get_memory_rtx (arg2, len);
+  rtx arg3_rtx = expand_normal (len);
+  rtx result = expand_cmpstrn_or_cmpmem (cmpstrn_icode, target, arg1_rtx,
 					 arg2_rtx, TREE_TYPE (len), arg3_rtx,
 					 MIN (arg1_align, arg2_align));
+
+  /* Check to see if the argument was declared attribute nonstring
+     and if so, issue a warning since at this point it's not known
+     to be nul-terminated.  */
+  tree fndecl = get_callee_fndecl (exp);
+  maybe_warn_nonstring_arg (fndecl, exp);
+
   if (result)
     {
       /* Return the value in the proper mode for this function.  */
@@ -4616,15 +4622,11 @@  expand_builtin_strncmp (tree exp, ATTRIBUTE_UNUSED rtx target,
 
   /* Expand the library call ourselves using a stabilized argument
      list to avoid re-evaluating the function's arguments twice.  */
-    fndecl = get_callee_fndecl (exp);
-    fn = build_call_nofold_loc (EXPR_LOCATION (exp), fndecl, 3,
-				arg1, arg2, len);
+  tree fn = build_call_nofold_loc (loc, fndecl, 3, arg1, arg2, len);
   gcc_assert (TREE_CODE (fn) == CALL_EXPR);
   CALL_EXPR_TAILCALL (fn) = CALL_EXPR_TAILCALL (exp);
   return expand_call (fn, target, target == const0_rtx);
 }
-  return NULL_RTX;
-}
 
 /* Expand a call to __builtin_saveregs, generating the result in TARGET,
    if that's convenient.  */
diff --git a/gcc/testsuite/c-c++-common/attr-nonstring-4.c b/gcc/testsuite/c-c++-common/attr-nonstring-4.c
new file mode 100644
index 0000000..786e647
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/attr-nonstring-4.c
@@ -0,0 +1,71 @@ 
+/* PR middle-end/83131 - c-c++/common/attr-nonstring-3 failure for strcmp
+   tests on PowerPC
+   { dg-do compile }
+   { dg-options "-O2 -Wstringop-overflow -ftrack-macro-expansion=0" }  */
+
+typedef __SIZE_TYPE__ size_t;
+
+extern int strcmp (const char*, const char*);
+extern int strncmp (const char*, const char*, size_t);
+
+extern char arx[] __attribute__ ((nonstring));
+extern char ar5[5] __attribute__ ((nonstring));
+extern char str[];
+
+enum { N = sizeof ar5 };
+enum { X = sizeof ar5 + 1 };
+
+
+int warn_strcmp_cst_1 (void)
+{
+  return strcmp ("bar", arx);       /* { dg-warning "argument 2 declared attribute .nonstring." } */
+}
+
+int warn_strcmp_cst_2 (void)
+{
+  return strcmp (arx, "foo");       /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+int warn_strncmp_cst_1 (void)
+{
+  return strncmp ("bar", ar5, X);   /* { dg-warning "argument 2 declared attribute .nonstring." } */
+}
+
+int warn_strncmp_cst_2 (void)
+{
+  return strncmp (ar5, "foo", X);   /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+int nowarn_strncmp_cst_1 (void)
+{
+  return strncmp ("bar", ar5, N);
+}
+
+int nowarn_strncmp_cst_2 (void)
+{
+  return strncmp (ar5, "foo", N);
+}
+
+
+int warn_strncmp_var_1 (void)
+{
+  return strncmp (str, ar5, X);     /* { dg-warning "argument 2 declared attribute .nonstring." } */
+}
+
+int warn_strncmp_var_2 (void)
+{
+  return strncmp (ar5, str, X);     /* { dg-warning "argument 1 declared attribute .nonstring." } */
+}
+
+
+int nowarn_strncmp_var_1 (void)
+{
+  return strncmp (str, ar5, N);
+}
+
+int nowarn_strncmp_var_2 (void)
+{
+  return strncmp (ar5, str, N);
+}