[v2] c: add Wzero-as-null-pointer-constant [PR117059]

This patch enables the Wzero-as-null-pointer-constant for C.
The second version added more tests and fixes one condition
to not incorrectly warn for nullptr. 


Bootstrapped and regression tested on x86_64.


    c: add Wzero-as-null-pointer-constant [PR117059]
    
    Add warnings for the use of zero as a null pointer constant to the C FE.
    
            PR c/117059
    
    gcc/c-family/ChangeLog:
            * c.opt (Wzero-as-null-pointer-constant): Enable for C and ObjC.
    
    gcc/c/ChangeLog:
            * c-typeck.cc (parse_build_binary_op): Add warning.
            (build_conditional_expr): Add warning.
            (convert_for_assignment): Add warning.
    
    gcc/testsuite/ChangeLog:
            * gcc.dg/Wzero-as-null-pointer-constant.c: New test.


Suggested-by: Alejandro Colomar <alx@kernel.org>
Acked-by: Alejandro Colomar <alx@kernel.org>

Hi Martin,

On Sat, Nov 09, 2024 at 06:15:35PM GMT, Martin Uecker wrote:
> 
> This patch enables the Wzero-as-null-pointer-constant for C.
> The second version added more tests and fixes one condition
> to not incorrectly warn for nullptr. 
> 
> 
> Bootstrapped and regression tested on x86_64.
> 
> 
>     c: add Wzero-as-null-pointer-constant [PR117059]
>     
>     Add warnings for the use of zero as a null pointer constant to the C FE.
>     
>             PR c/117059
>     
>     gcc/c-family/ChangeLog:
>             * c.opt (Wzero-as-null-pointer-constant): Enable for C and ObjC.
>     
>     gcc/c/ChangeLog:
>             * c-typeck.cc (parse_build_binary_op): Add warning.
>             (build_conditional_expr): Add warning.
>             (convert_for_assignment): Add warning.
>     
>     gcc/testsuite/ChangeLog:
>             * gcc.dg/Wzero-as-null-pointer-constant.c: New test.
> 
> 
> Suggested-by: Alejandro Colomar <alx@kernel.org>
> Acked-by: Alejandro Colomar <alx@kernel.org>
> 
> 
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 9b9f5e744f6..b4e967ce000 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -1632,7 +1632,7 @@ C ObjC C++ ObjC++ Var(warn_xor_used_as_pow) Warning Init(1)
>  Warn about xor operators where it appears the user meant exponentiation.
>  
>  Wzero-as-null-pointer-constant
> -C++ ObjC++ Var(warn_zero_as_null_pointer_constant) Warning
> +C ObjC C++ ObjC++ Var(warn_zero_as_null_pointer_constant) Warning
>  Warn when a literal '0' is used as null pointer.
>  
>  Wzero-length-bounds
> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> index 3fdf3437bf5..ced58c2be84 100644
> --- a/gcc/c/c-typeck.cc
> +++ b/gcc/c/c-typeck.cc
> @@ -4600,15 +4600,24 @@ parser_build_binary_op (location_t location, enum tree_code code,
>  		"comparison with string literal results in unspecified "
>  		"behavior");
>  
> -  if (true
> -      && TREE_CODE_CLASS (code) == tcc_comparison
> -      && ((TREE_CODE (type1) == POINTER_TYPE
> -	   && TREE_CODE (type2) == INTEGER_TYPE
> -	   && null_pointer_constant_p (arg2.value))
> -	  || (TREE_CODE (type2) == POINTER_TYPE
> -	      && TREE_CODE (type1) == INTEGER_TYPE
> -	       && null_pointer_constant_p (arg1.value))))
> -     warning_at (location, 0, "zero as null pointer constant");
> +  if (warn_zero_as_null_pointer_constant
> +      && c_inhibit_evaluation_warnings == 0
> +      && TREE_CODE_CLASS (code) == tcc_comparison)
> +    {
> +      if ((TREE_CODE (type1) == POINTER_TYPE
> +	   || TREE_CODE (type1) == NULLPTR_TYPE)
> +	  && TREE_CODE (type2) == INTEGER_TYPE
> +	  && null_pointer_constant_p (arg2.value))
> +	warning_at (arg2.get_location(), OPT_Wzero_as_null_pointer_constant,
> +		    "zero as null pointer constant");
> +
> +      if ((TREE_CODE (type2) == POINTER_TYPE
> +	   || TREE_CODE (type2) == NULLPTR_TYPE)
> +	  && TREE_CODE (type1) == INTEGER_TYPE
> +	  && null_pointer_constant_p (arg1.value))
> +	warning_at (arg1.get_location(), OPT_Wzero_as_null_pointer_constant,
> +		    "zero as null pointer constant");
> +    }
>  
>    if (warn_array_compare
>        && TREE_CODE_CLASS (code) == tcc_comparison
> @@ -5975,6 +5984,20 @@ build_conditional_expr (location_t colon_loc, tree ifexp, bool ifexp_bcp,
>  		    t1, t2);
>      }
>  
> +  if (warn_zero_as_null_pointer_constant
> +      && c_inhibit_evaluation_warnings == 0)
> +    {
> +      if ((code1 == POINTER_TYPE || code1 == NULLPTR_TYPE)
> +	  && code2 == INTEGER_TYPE && null_pointer_constant_p (orig_op2))
> +	warning_at (op2_loc, OPT_Wzero_as_null_pointer_constant,
> +		    "zero as null pointer constant");
> +
> +      if ((code2 == POINTER_TYPE || code2 == NULLPTR_TYPE)
> +	  && code1 == INTEGER_TYPE && null_pointer_constant_p (orig_op1))
> +	warning_at (op1_loc, OPT_Wzero_as_null_pointer_constant,
> +		    "zero as null pointer constant");
> +    }
> +
>    /* Quickly detect the usual case where op1 and op2 have the same type
>       after promotion.  */
>    if (TYPE_MAIN_VARIANT (type1) == TYPE_MAIN_VARIANT (type2))
> @@ -8406,8 +8429,10 @@ convert_for_assignment (location_t location, location_t expr_loc, tree type,
>  	       || coder == NULLPTR_TYPE
>  	       || coder == BITINT_TYPE))
>      {
> -      if (null_pointer_constant)
> -	warning_at (location, 0, "zero as null pointer constant");
> +      if (null_pointer_constant && c_inhibit_evaluation_warnings == 0
> +	  && coder == INTEGER_TYPE)
> +	warning_at (location, OPT_Wzero_as_null_pointer_constant,
> +		    "zero as null pointer constant");
>  
>        /* An explicit constant 0 or type nullptr_t can convert to a pointer,
>  	 or one that results from arithmetic, even including a cast to
> diff --git a/gcc/testsuite/gcc.dg/Wzero-as-null-pointer-constant.c b/gcc/testsuite/gcc.dg/Wzero-as-null-pointer-constant.c
> new file mode 100644
> index 00000000000..372756faeb9
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/Wzero-as-null-pointer-constant.c
> @@ -0,0 +1,37 @@
> +/* { dg-do compile } */
> +/* { dg-options "-std=c23 -Wzero-as-null-pointer-constant" } */
> +
> +#define NULL (void*)0
> +
> +void foo(void*);
> +void bar()
> +{
> +	foo(0);				/* { dg-warning "zero as null pointer constant" } */
> +	foo(NULL);
> +	foo(nullptr);
> +
> +	void *p = 0;			/* { dg-warning "zero as null pointer constant" } */
> +	void *r = NULL;
> +	void *t = nullptr;
> +	void *q = { 0 };		/* { dg-warning "zero as null pointer constant" } */
> +	void *s = { NULL };
> +	void *u = { nullptr };
> +	struct { void *q; } x = { 0 };	/* { dg-warning "zero as null pointer constant" } */
> +	struct { void *q; } y = { NULL };
> +	struct { void *q; } z = { nullptr };
> +	struct { int a; void *b; } a = { 0 };
> +	struct { int a; int b; void *c; } b = { 0, 0 };
> +
> +	1 ? 0 : p;			/* { dg-warning "zero as null pointer constant" } */
> +	1 ? p : 0;			/* { dg-warning "zero as null pointer constant" } */
> +	1 ? 0 : NULL;			/* { dg-warning "zero as null pointer constant" } */
> +	1 ? NULL : 0;			/* { dg-warning "zero as null pointer constant" } */
> +
> +	if (p == 0);			/* { dg-warning "zero as null pointer constant" } */
> +	if (0 == p);			/* { dg-warning "zero as null pointer constant" } */
> +	if (NULL == 0);			/* { dg-warning "zero as null pointer constant" } */
> +	if (0 == NULL);			/* { dg-warning "zero as null pointer constant" } */
> +	if (0 == nullptr);		/* { dg-warning "zero as null pointer constant" } */
> +	if (nullptr == 0);		/* { dg-warning "zero as null pointer constant" } */

Maybe we should also check a few obvious cases that shouldn't be warned
because 0 is an int?  If some bug causes these to trigger, we'd know
something is very wrong.

	if (0 == 0);

	1 ? 0 : 0;

	int i = 0;

It might also be interesting to have at least one test where we use a
hardcoded (void*)0 without the name NULL.

Cheers,
Alex

> +}
> +
>

Other cases to test: the != operator (should warn in same cases as ==); 
boolean uses of pointers such as if (p), !p, p?x:y, and implicit or 
explicit conversion to bool (none of those boolean uses of pointers should 
warn since there's no explicit integer null pointer constant involved in 
the implicit comparison to 0).  Also test that initializing a pointer with 
empty braces does not warn.