PR c++/33160: wrong "NULL used in arithmetic" warning

PR c++/33160: wrong "NULL used in arithmetic" warning

[Manuel López-Ibáñez]

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

:ADDPATCH c++/diagnostics:

The following patch fixes a wrong "NULL used in arithmetic" warning.
However, I wonder whether this is the best solution. Alternatively, we
could modify POINTER_TYPE_P to return 1 for (TYPE_MODE (TREE_TYPE (t))
== ptr_mode). Any other ideas?

(I must admit that I am not sure what __attribute__(mode(pointer))
means exactly).

Anyway, bootstrapped and regression tested with --enable-languages=all
on x86_64-unknown-linux-gnu

2007-11-10  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
  PR c++/33160
cp/
 * typeck.c (build_binary_op): Check for mode(pointer).

testsuite/
  * g++.dg/warn/pr33160.C: New.

[-- Attachment #2: fix-pr33160.diff --]
[-- Type: text/plain, Size: 1514 bytes --]

Index: gcc/testsuite/g++.dg/warn/pr33160.C
===================================================================
--- gcc/testsuite/g++.dg/warn/pr33160.C	(revision 0)
+++ gcc/testsuite/g++.dg/warn/pr33160.C	(revision 0)
@@ -0,0 +1,12 @@
+// PR 33160
+// { dg-do compile }
+// { dg-options "-Wall -Wextra -Wpointer-arith" }
+
+typedef int __attribute__((mode(pointer))) intptr_t;
+int foo(void)
+{
+ intptr_t t = 0;
+ if (t != ((intptr_t)__null)) t = 1;
+ return 0;
+}
+
Index: gcc/cp/typeck.c
===================================================================
--- gcc/cp/typeck.c	(revision 129898)
+++ gcc/cp/typeck.c	(working copy)
@@ -3849,8 +3849,12 @@
 	  (null_ptr_cst_p (orig_op0) && null_ptr_cst_p (orig_op1) 
 	   && code != EQ_EXPR && code != NE_EXPR) 
 	  /* Or if one of OP0 or OP1 is neither a pointer nor NULL.  */
-	  || (!null_ptr_cst_p (orig_op0) && TREE_CODE (TREE_TYPE (op0)) != POINTER_TYPE)
-	  || (!null_ptr_cst_p (orig_op1) && TREE_CODE (TREE_TYPE (op1)) != POINTER_TYPE)))
+	  || (!null_ptr_cst_p (orig_op0) 
+              && TREE_CODE (TREE_TYPE (op0)) != POINTER_TYPE 
+              && TYPE_MODE (TREE_TYPE (op0)) != ptr_mode)
+	  || (!null_ptr_cst_p (orig_op1) 
+              && TREE_CODE (TREE_TYPE (op1)) != POINTER_TYPE
+              && TYPE_MODE (TREE_TYPE (op1)) != ptr_mode)))
     /* Some sort of arithmetic operation involving NULL was
        performed.  Note that pointer-difference and pointer-addition
        have already been handled above, and so we don't end up here in

Re: PR c++/33160: wrong "NULL used in arithmetic" warning

[Mark Mitchell]

Manuel López-Ibáñez wrote:

> 2007-11-10  Manuel Lopez-Ibanez  <manu@gcc.gnu.org>
>   PR c++/33160
> cp/
>  * typeck.c (build_binary_op): Check for mode(pointer).
> 
> testsuite/
>   * g++.dg/warn/pr33160.C: New.

This doesn't look right to me.  First, any front-end use of TYPE_MODE
(except in explicitly handling attributes) is supsicious; it's not a
front-end concept.  Second, (intptr_t)__null should not be null_node, so
the first check in this conditional:

 if ((orig_op0 == null_node || orig_op1 == null_node)

should be false.

The bug might be that something is thinking that (intptr_t)__null is a
no-op cast and is throwing away the cast -- even though it is important,
for exactly the reason shown here.  It would be OK to replace the cast
with a zero of the correct type (rather than leaving the cast itself
explicit).

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: PR c++/33160: wrong "NULL used in arithmetic" warning

[Manuel López-Ibáñez]

On 12/11/2007, Mark Mitchell <mark@codesourcery.com> wrote:
>  if ((orig_op0 == null_node || orig_op1 == null_node)
>
> should be false.
>
> The bug might be that something is thinking that (intptr_t)__null is a
> no-op cast and is throwing away the cast -- even though it is important,
> for exactly the reason shown here.  It would be OK to replace the cast
> with a zero of the correct type (rather than leaving the cast itself
> explicit).
>

That is exactly what I was thinking. Any hints where this may be happening?

Cheers,

Manuel.

Re: PR c++/33160: wrong "NULL used in arithmetic" warning

[Mark Mitchell]

Manuel López-Ibáñez wrote:
> On 12/11/2007, Mark Mitchell <mark@codesourcery.com> wrote:
>>  if ((orig_op0 == null_node || orig_op1 == null_node)
>>
>> should be false.
>>
>> The bug might be that something is thinking that (intptr_t)__null is a
>> no-op cast and is throwing away the cast -- even though it is important,
>> for exactly the reason shown here.  It would be OK to replace the cast
>> with a zero of the correct type (rather than leaving the cast itself
>> explicit).
>>

> That is exactly what I was thinking. Any hints where this may be happening?

No, sorry, I have no idea; you'll just have to step through the code
that handles casts...

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713