diff mbox series

[V2] Add warning options -W[no-]compare-distinct-pointer-types

Message ID 87a681d7tf.fsf_-_@oracle.com
State New, archived
Headers
Series [V2] Add warning options -W[no-]compare-distinct-pointer-types |

Commit Message

Jose E. Marchesi Aug. 18, 2022, 1:01 p.m. UTC 
  Hi Joseph.

> On Fri, 5 Aug 2022, Jose E. Marchesi via Gcc-patches wrote:
>
>> +Wcompare-distinct-pointer-types
>> +C C++ Var(warn_compare_distinct_pointer_types) Warning Init(1)
>> +Warn if pointers of distinct types are compared without a cast.
>
> There's no implementation for C++ in this patch, so the option shouldn't 
> be supported for C++ in c.opt.  However, C options are normally supported 
> for Objective-C; unless you have a specific reason why Objective-C support 
> for this option would be a bad idea, "C ObjC" would be appropriate for the 
> languages.

Thanks for the review!
See a V2 of the patch with the suggested change below.

----

GCC emits pedwarns unconditionally when comparing pointers of
different types, for example:

  int xdp_context (struct xdp_md *xdp)
    {
        void *data = (void *)(long)xdp->data;
        __u32 *metadata = (void *)(long)xdp->data_meta;
        __u32 ret;

        if (metadata + 1 > data)
          return 0;
        return 1;
   }

  /home/jemarch/foo.c: In function ‘xdp_context’:
  /home/jemarch/foo.c:15:20: warning: comparison of distinct pointer types lacks a cast
         15 |   if (metadata + 1 > data)
                 |                    ^

LLVM supports an option -W[no-]compare-distinct-pointer-types that can
be used in order to enable or disable the emission of such warnings.
It is enabled by default.

This patch adds the same options to GCC.

Documentation and testsuite updated included.
Regtested in x86_64-linu-gnu.
No regressions observed.

gcc/ChangeLog:

	PR c/106537
	* doc/invoke.texi (Option Summary): Mention
	-Wcompare-distinct-pointer-types under `Warning Options'.
	(Warning Options): Document -Wcompare-distinct-pointer-types.

gcc/c-family/ChangeLog:

	PR c/106537
	* c.opt (Wcompare-distinct-pointer-types): New option.

gcc/c/ChangeLog:

	PR c/106537
	* c-typeck.cc (build_binary_op): Warning on comparing distinct
	pointer types only when -Wcompare-distinct-pointer-types.

gcc/testsuite/ChangeLog:

	PR c/106537
	* gcc.c-torture/compile/pr106537-1.c: New test.
	* gcc.c-torture/compile/pr106537-2.c: Likewise.
	* gcc.c-torture/compile/pr106537-3.c: Likewise.
---
 gcc/c-family/c.opt                            |  4 ++++
 gcc/c/c-typeck.cc                             |  8 ++++---
 gcc/doc/invoke.texi                           |  6 +++++
 .../gcc.c-torture/compile/pr106537-1.c        | 23 +++++++++++++++++++
 .../gcc.c-torture/compile/pr106537-2.c        | 21 +++++++++++++++++
 .../gcc.c-torture/compile/pr106537-3.c        | 21 +++++++++++++++++
 6 files changed, 80 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr106537-1.c
 create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr106537-2.c
 create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr106537-3.c

Comments

Joseph Myers Aug. 18, 2022, 3:34 p.m. UTC | #1 
On Thu, 18 Aug 2022, Jose E. Marchesi via Gcc-patches wrote:

> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
> index de8780a1502..04af02add37 100644
> --- a/gcc/c/c-typeck.cc
> +++ b/gcc/c/c-typeck.cc
> @@ -12397,7 +12397,8 @@ build_binary_op (location_t location, enum tree_code code,
>  	    }
>  	  else
>  	    /* Avoid warning about the volatile ObjC EH puts on decls.  */
> -	    if (!objc_ok)
> +	    if (!objc_ok
> +                && warn_compare_distinct_pointer_types)
>  	      pedwarn (location, 0,
>  		       "comparison of distinct pointer types lacks a cast");
>  
> @@ -12517,8 +12518,9 @@ build_binary_op (location_t location, enum tree_code code,
>  	      int qual = ENCODE_QUAL_ADDR_SPACE (as_common);
>  	      result_type = build_pointer_type
>  			      (build_qualified_type (void_type_node, qual));
> -	      pedwarn (location, 0,
> -		       "comparison of distinct pointer types lacks a cast");
> +              if (warn_compare_distinct_pointer_types)
> +                pedwarn (location, 0,
> +                         "comparison of distinct pointer types lacks a cast");

I think this should use OPT_Wcompare_distinct_pointer_types in place of 0, 
and then you shouldn't need to check warn_compare_distinct_pointer_types 
(as well as the diagnostic then automatically telling the user what option 
controls it).
Jose E. Marchesi Aug. 26, 2022, 3:45 p.m. UTC | #2 
> On Thu, 18 Aug 2022, Jose E. Marchesi via Gcc-patches wrote:
>
>> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
>> index de8780a1502..04af02add37 100644
>> --- a/gcc/c/c-typeck.cc
>> +++ b/gcc/c/c-typeck.cc
>> @@ -12397,7 +12397,8 @@ build_binary_op (location_t location, enum tree_code code,
>>  	    }
>>  	  else
>>  	    /* Avoid warning about the volatile ObjC EH puts on decls.  */
>> -	    if (!objc_ok)
>> +	    if (!objc_ok
>> +                && warn_compare_distinct_pointer_types)
>>  	      pedwarn (location, 0,
>>  		       "comparison of distinct pointer types lacks a cast");
>>  
>> @@ -12517,8 +12518,9 @@ build_binary_op (location_t location, enum tree_code code,
>>  	      int qual = ENCODE_QUAL_ADDR_SPACE (as_common);
>>  	      result_type = build_pointer_type
>>  			      (build_qualified_type (void_type_node, qual));
>> -	      pedwarn (location, 0,
>> -		       "comparison of distinct pointer types lacks a cast");
>> +              if (warn_compare_distinct_pointer_types)
>> +                pedwarn (location, 0,
>> +                         "comparison of distinct pointer types lacks a cast");
>
> I think this should use OPT_Wcompare_distinct_pointer_types in place of 0, 
> and then you shouldn't need to check warn_compare_distinct_pointer_types 
> (as well as the diagnostic then automatically telling the user what option 
> controls it).

Ouch, better to use pedwarn the way it is intended to be used yes.
Sorry for the silly overlook :)

Sending a V3 with this modification.
diff mbox series

Patch

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index dfdebd596ef..c401c06ec0b 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1844,6 +1844,10 @@  Winvalid-imported-macros
 C++ ObjC++ Var(warn_imported_macros) Warning
 Warn about macros that have conflicting header units definitions.
 
+Wcompare-distinct-pointer-types
+C ObjC Var(warn_compare_distinct_pointer_types) Warning Init(1)
+Warn if pointers of distinct types are compared without a cast.
+
 flang-info-include-translate
 C++ Var(note_include_translate_yes)
 Note #include directives translated to import declarations.
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index de8780a1502..04af02add37 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -12397,7 +12397,8 @@  build_binary_op (location_t location, enum tree_code code,
 	    }
 	  else
 	    /* Avoid warning about the volatile ObjC EH puts on decls.  */
-	    if (!objc_ok)
+	    if (!objc_ok
+                && warn_compare_distinct_pointer_types)
 	      pedwarn (location, 0,
 		       "comparison of distinct pointer types lacks a cast");
 
@@ -12517,8 +12518,9 @@  build_binary_op (location_t location, enum tree_code code,
 	      int qual = ENCODE_QUAL_ADDR_SPACE (as_common);
 	      result_type = build_pointer_type
 			      (build_qualified_type (void_type_node, qual));
-	      pedwarn (location, 0,
-		       "comparison of distinct pointer types lacks a cast");
+              if (warn_compare_distinct_pointer_types)
+                pedwarn (location, 0,
+                         "comparison of distinct pointer types lacks a cast");
 	    }
 	}
       else if (code0 == POINTER_TYPE && null_pointer_constant_p (orig_op1))
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 1ac81ad0bb4..88b4af14d8c 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -341,6 +341,7 @@  Objective-C and Objective-C++ Dialects}.
 -Wcast-align  -Wcast-align=strict  -Wcast-function-type  -Wcast-qual  @gol
 -Wchar-subscripts @gol
 -Wclobbered  -Wcomment @gol
+-Wcompare-distinct-pointer-types @gol
 -Wconversion  -Wno-coverage-mismatch  -Wno-cpp @gol
 -Wdangling-else  -Wdangling-pointer  -Wdangling-pointer=@var{n}  @gol
 -Wdate-time @gol
@@ -8631,6 +8632,11 @@  programs.
 Warn for variables that might be changed by @code{longjmp} or
 @code{vfork}.  This warning is also enabled by @option{-Wextra}.
 
+@item -Wcompare-distinct-pointer-types
+@opindex Wcompare-distinct-pointer-types
+Warn if pointers of distinct types are compared without a cast.  This
+warning is enabled by default.
+
 @item -Wconversion
 @opindex Wconversion
 @opindex Wno-conversion
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr106537-1.c b/gcc/testsuite/gcc.c-torture/compile/pr106537-1.c
new file mode 100644
index 00000000000..56c8deeb4fe
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr106537-1.c
@@ -0,0 +1,23 @@ 
+/* { dg-do compile }
+   { dg-options "" }
+   This testcase checks that warn_compare_distinct_pointer_types is enabled by
+   default.  */
+
+typedef int __u32;
+
+struct xdp_md
+{
+  char *data;
+  char *data_meta;
+};
+
+int xdp_context (struct xdp_md *xdp)
+{
+  void *data = (void *)(long)xdp->data;
+  __u32 *metadata = (void *)(long)xdp->data_meta;
+  __u32 ret;
+
+  if (metadata + 1 > data) /* { dg-warning "comparison of distinct pointer types" } */
+    return 0;
+  return 1;
+}
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr106537-2.c b/gcc/testsuite/gcc.c-torture/compile/pr106537-2.c
new file mode 100644
index 00000000000..7a9b2aec0b8
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr106537-2.c
@@ -0,0 +1,21 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Wcompare-distinct-pointer-types" } */
+
+typedef int __u32;
+
+struct xdp_md
+{
+  char *data;
+  char *data_meta;
+};
+
+int xdp_context (struct xdp_md *xdp)
+{
+  void *data = (void *)(long)xdp->data;
+  __u32 *metadata = (void *)(long)xdp->data_meta;
+  __u32 ret;
+
+  if (metadata + 1 > data) /* { dg-warning "comparison of distinct pointer types" } */
+    return 0;
+  return 1;
+}
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr106537-3.c b/gcc/testsuite/gcc.c-torture/compile/pr106537-3.c
new file mode 100644
index 00000000000..dc5dd46e3cb
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr106537-3.c
@@ -0,0 +1,21 @@ 
+/* { dg-do compile } */
+/* { dg-options "-Wno-compare-distinct-pointer-types" } */
+
+typedef int __u32;
+
+struct xdp_md
+{
+  char *data;
+  char *data_meta;
+};
+
+int xdp_context (struct xdp_md *xdp)
+{
+  void *data = (void *)(long)xdp->data;
+  __u32 *metadata = (void *)(long)xdp->data_meta;
+  __u32 ret;
+
+  if (metadata + 1 > data) /* There shouldn't be a warning here.  */
+    return 0;
+  return 1;
+}