[PATCH] declare bcmp, bcopy, and bzero nonnull (PR 80936)

[PATCH] declare bcmp, bcopy, and bzero nonnull (PR 80936)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 1430 bytes --]

GCC declares bcmp, bcopy, and bzero without the nonnull attribute.
This was a deliberate decision as is reflected in the comment in
builtins.def:

/* bcmp, bcopy and bzero have traditionally accepted NULL pointers
    when the length parameter is zero, so don't apply attribute 
"nonnull".  */

But the SUSv3, Issue 6 marks the functions as legacy and recommends
portable applications to replace calls to the functions with the C
standard equivalents as follows:

   #define bcmp(b1,b2,len) memcmp((b1), (b2), (size_t)(len))

(The legacy functions have been removed from POSIX in Issue 7.)

The C standard functions do require their arguments to be nonnull.

Some libcs (e.g., Glibc) implement the legacy functions in terms
of the C standard equivalents.  Others (e.g., libiberty) rely on
strictly undefined behavior in their implementation of these
function (null pointer arithmetic).

Finally, GCC has been transforming calls to the legacy functions
to the C standard equivalents for a few releases, and making
the same assumptions about their arguments (i.e., eliminating
tests for their pointer arguments being null subsequent to
the calls).

To help detect some of the same mistakes in calls to the legacy
functions as in the standard ones the attached adds the nonnull
attribute to all three of them.

Tested on x86_64-linux.  Compiling Binutils/GDB and Glibc with
the patch doesn't turn up any -Wnonnull instances.

Martin

[-- Attachment #2: gcc-80936.diff --]
[-- Type: text/x-patch, Size: 6694 bytes --]

PR tree-optimization/80936 - bcmp, bcopy, and bzero not declared nonnull

gcc/testsuite/ChangeLog:

	PR tree-optimization/80936
	* gcc.dg/Wnonnull-2.c: New test.
	* gcc.dg/Wnonnull-3.c: New test.
	* gcc.dg/nonnull-3.c: Expect more warnings.

gcc/ChangeLog:

	PR tree-optimization/80936
	* builtins.def (bcmp, bcopy, bzero): Declare nonnull.

Index: gcc/builtins.def
===================================================================
--- gcc/builtins.def	(revision 276183)
+++ gcc/builtins.def	(working copy)
@@ -687,11 +687,9 @@ DEF_C99_COMPL_BUILTIN        (BUILT_IN_CTANHL, "ct
 DEF_C99_COMPL_BUILTIN        (BUILT_IN_CTANL, "ctanl", BT_FN_COMPLEX_LONGDOUBLE_COMPLEX_LONGDOUBLE, ATTR_MATHFN_FPROUNDING)
 
 /* Category: string/memory builtins.  */
-/* bcmp, bcopy and bzero have traditionally accepted NULL pointers
-   when the length parameter is zero, so don't apply attribute "nonnull".  */
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_BCMP, "bcmp", BT_FN_INT_CONST_PTR_CONST_PTR_SIZE, ATTR_PURE_NOTHROW_LEAF_LIST)
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_BCOPY, "bcopy", BT_FN_VOID_CONST_PTR_PTR_SIZE, ATTR_NOTHROW_LEAF_LIST)
-DEF_EXT_LIB_BUILTIN    (BUILT_IN_BZERO, "bzero", BT_FN_VOID_PTR_SIZE, ATTR_NOTHROW_LEAF_LIST)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_BCMP, "bcmp", BT_FN_INT_CONST_PTR_CONST_PTR_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_BCOPY, "bcopy", BT_FN_VOID_CONST_PTR_PTR_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
+DEF_EXT_LIB_BUILTIN    (BUILT_IN_BZERO, "bzero", BT_FN_VOID_PTR_SIZE, ATTR_NOTHROW_NONNULL_LEAF)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_INDEX, "index", BT_FN_STRING_CONST_STRING_INT, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_MEMCHR, "memchr", BT_FN_PTR_CONST_PTR_INT_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
 DEF_LIB_BUILTIN        (BUILT_IN_MEMCMP, "memcmp", BT_FN_INT_CONST_PTR_CONST_PTR_SIZE, ATTR_PURE_NOTHROW_NONNULL_LEAF)
Index: gcc/testsuite/gcc.dg/Wnonnull-2.c
===================================================================
--- gcc/testsuite/gcc.dg/Wnonnull-2.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/Wnonnull-2.c	(working copy)
@@ -0,0 +1,55 @@
+/* PR middle-end/80936 - bcmp, bcopy, and bzero not declared nonnull
+   Verify that -Wnonnull is issued for calls with constant null pointers
+   with no optimization.
+   { dg-do compile }
+   { dg-options "-O0 -Wall" } */
+
+void zero0 (void *p, unsigned n)
+{
+  __builtin_memset (0, 0, n);           // { dg-warning "\\\[-Wnonnull]" }
+}
+
+void zero1 (void *p, unsigned n)
+{
+  __builtin_bzero (0, n);               // { dg-warning "\\\[-Wnonnull]" }
+}
+
+void copy0 (void *p, const void *q, unsigned n)
+{
+  __builtin_memcpy (0, q, n);           // { dg-warning "\\\[-Wnonnull]" }
+}
+
+void copy1 (void *p, const void *q, unsigned n)
+{
+  __builtin_memcpy (0, q, n);           // { dg-warning "\\\[-Wnonnull]" }
+}
+
+void copy2 (void *p, const void *q, unsigned n)
+{
+  __builtin_bcopy (q, 0, n);            // { dg-warning "\\\[-Wnonnull]" }
+}
+
+void copy3 (void *p, const void *q, unsigned n)
+{
+  __builtin_bcopy (q, 0, n);            // { dg-warning "\\\[-Wnonnull]" }
+}
+
+int cmp0 (const void *p, const void *q, unsigned n)
+{
+  return __builtin_memcmp (0, q, n);    // { dg-warning "\\\[-Wnonnull]" }
+}
+
+int cmp1 (const void *p, const void *q, unsigned n)
+{
+  return __builtin_memcmp (0, q, n);    // { dg-warning "\\\[-Wnonnull]" }
+}
+
+int cmp2 (const void *p, const void *q, unsigned n)
+{
+  return __builtin_bcmp (0, q, n);      // { dg-warning "\\\[-Wnonnull]" }
+}
+
+int cmp3 (const void *p, const void *q, unsigned n)
+{
+  return __builtin_bcmp (p, 0, n);      // { dg-warning "\\\[-Wnonnull]" }
+}
Index: gcc/testsuite/gcc.dg/Wnonnull-3.c
===================================================================
--- gcc/testsuite/gcc.dg/Wnonnull-3.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/Wnonnull-3.c	(working copy)
@@ -0,0 +1,71 @@
+/* PR middle-end/80936 - bcmp, bcopy, and bzero not declared nonnull
+   Verify that with optimization, -Wnonnull is issued for calls with
+   non-constant arguments determined to be null.
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+#define NOIPA __attribute__ ((noipa))
+
+NOIPA void zero0 (void *p, unsigned n)
+{
+  if (p == 0)
+    __builtin_memset (p, 0, n);         // { dg-warning "\\\[-Wnonnull]" }
+}
+
+NOIPA void zero1 (void *p, unsigned n)
+{
+  if (p == 0)
+    __builtin_bzero (p, n);             // { dg-warning "\\\[-Wnonnull]" }
+}
+
+NOIPA void copy0 (void *p, const void *q, unsigned n)
+{
+  if (p == 0)
+    __builtin_memcpy (p, q, n);         // { dg-warning "\\\[-Wnonnull]" }
+}
+
+NOIPA void copy1 (void *p, const void *q, unsigned n)
+{
+  if (q == 0)
+    __builtin_memcpy (p, q, n);         // { dg-warning "\\\[-Wnonnull]" }
+}
+
+NOIPA void copy2 (void *p, const void *q, unsigned n)
+{
+  if (p == 0)
+    __builtin_bcopy (q, p, n);          // { dg-warning "\\\[-Wnonnull]" }
+}
+
+NOIPA void copy3 (void *p, const void *q, unsigned n)
+{
+  if (q == 0)
+    __builtin_bcopy (q, p, n);          // { dg-warning "\\\[-Wnonnull]" }
+}
+
+NOIPA int cmp0 (const void *p, const void *q, unsigned n)
+{
+  if (p == 0)
+    return __builtin_memcmp (p, q, n);  // { dg-warning "\\\[-Wnonnull]" }
+  return 0;
+}
+
+NOIPA int cmp1 (const void *p, const void *q, unsigned n)
+{
+  if (q == 0)
+    return __builtin_memcmp (p, q, n);  // { dg-warning "\\\[-Wnonnull]" }
+  return 0;
+}
+
+NOIPA int cmp2 (const void *p, const void *q, unsigned n)
+{
+  if (p == 0)
+    return __builtin_bcmp (p, q, n);    // { dg-warning "\\\[-Wnonnull]" }
+  return 0;
+}
+
+NOIPA int cmp3 (const void *p, const void *q, unsigned n)
+{
+  if (q == 0)
+    return __builtin_bcmp (p, q, n);    // { dg-warning "\\\[-Wnonnull]" }
+  return 0;
+}
Index: gcc/testsuite/gcc.dg/nonnull-3.c
===================================================================
--- gcc/testsuite/gcc.dg/nonnull-3.c	(revision 276183)
+++ gcc/testsuite/gcc.dg/nonnull-3.c	(working copy)
@@ -9,11 +9,11 @@
 void
 foo (void *p, char *s)
 {
-  __builtin_bzero (NULL, 0);
-  __builtin_bcopy (NULL, p, 0);
-  __builtin_bcopy (p, NULL, 0);
-  __builtin_bcmp (NULL, p, 0);
-  __builtin_bcmp (p, NULL, 0);
+  __builtin_bzero (NULL, 0);  /* { dg-warning "null" "pr80936" } */
+  __builtin_bcopy (NULL, p, 0);  /* { dg-warning "null" "pr80936" } */
+  __builtin_bcopy (p, NULL, 0);  /* { dg-warning "null" "pr80936" } */
+  __builtin_bcmp (NULL, p, 0);  /* { dg-warning "null" "pr80936" } */
+  __builtin_bcmp (p, NULL, 0);  /* { dg-warning "null" "pr80936" } */
   __builtin_index (NULL, 16);  /* { dg-warning "null" "null pointer check" } */
   __builtin_rindex (NULL, 16);  /* { dg-warning "null" "null pointer check" } */
 

Re: [PATCH] declare bcmp, bcopy, and bzero nonnull (PR 80936)

[Jeff Law]

On 9/27/19 1:06 PM, Martin Sebor wrote:
> GCC declares bcmp, bcopy, and bzero without the nonnull attribute.
> This was a deliberate decision as is reflected in the comment in
> builtins.def:
> 
> /* bcmp, bcopy and bzero have traditionally accepted NULL pointers
>    when the length parameter is zero, so don't apply attribute
> "nonnull".  */
> 
> But the SUSv3, Issue 6 marks the functions as legacy and recommends
> portable applications to replace calls to the functions with the C
> standard equivalents as follows:
> 
>   #define bcmp(b1,b2,len) memcmp((b1), (b2), (size_t)(len))
> 
> (The legacy functions have been removed from POSIX in Issue 7.)
> 
> The C standard functions do require their arguments to be nonnull.
> 
> Some libcs (e.g., Glibc) implement the legacy functions in terms
> of the C standard equivalents.  Others (e.g., libiberty) rely on
> strictly undefined behavior in their implementation of these
> function (null pointer arithmetic).
> 
> Finally, GCC has been transforming calls to the legacy functions
> to the C standard equivalents for a few releases, and making
> the same assumptions about their arguments (i.e., eliminating
> tests for their pointer arguments being null subsequent to
> the calls).
> 
> To help detect some of the same mistakes in calls to the legacy
> functions as in the standard ones the attached adds the nonnull
> attribute to all three of them.
> 
> Tested on x86_64-linux.  Compiling Binutils/GDB and Glibc with
> the patch doesn't turn up any -Wnonnull instances.
> 
> Martin
> 
> gcc-80936.diff
> 
> PR tree-optimization/80936 - bcmp, bcopy, and bzero not declared nonnull
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/80936
> 	* gcc.dg/Wnonnull-2.c: New test.
> 	* gcc.dg/Wnonnull-3.c: New test.
> 	* gcc.dg/nonnull-3.c: Expect more warnings.
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/80936
> 	* builtins.def (bcmp, bcopy, bzero): Declare nonnull.
OK.  I'll be doing another Fedora rebuild relatively soon, so we'll have
a chance to see if there's fallout on a wider codebase.  I don't expect
significant issues.

jeff