[PATCH] Emit -Waddress warnings for comparing address of reference against NULL

[PATCH] Emit -Waddress warnings for comparing address of reference against NULL

[Patrick Palka]

Implementation is pretty straightforward.  The only catch is that the
middle-end doesn't actually assume that REFERENCE_TYPEs are non-NULL so
code like

    int &a = *(int *)0;
    if (&a != 0)

will warn that &a will never be NULL yet the middle-end will fold the
conditional to false instead of true anyway.  But I guess that's not a
big deal.

Does this look OK after testing?

gcc/c-family/ChangeLog:
	* c-common.c (c_common_truthvalue_conversion): Warn when
	converting an address of a reference to a truth value.

gcc/cp/ChangeLog:
	* typeck.c (cp_build_binary_op): Warn when comparing an address
	of a reference against NULL.

gcc/testsuite/ChangeLog:
	g++.dg/warn/Walways-true-3.C: New test.
---
 gcc/c-family/c-common.c                    | 12 ++++++++++++
 gcc/cp/typeck.c                            | 10 ++++++++--
 gcc/testsuite/g++.dg/warn/Walways-true-3.C | 30 ++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Walways-true-3.C

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 7fe7fa6..89bbdc9 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -4784,6 +4784,18 @@ c_common_truthvalue_conversion (location_t location, tree expr)
 	tree totype = TREE_TYPE (expr);
 	tree fromtype = TREE_TYPE (TREE_OPERAND (expr, 0));
 
+	if (POINTER_TYPE_P (totype)
+	    && TREE_CODE (fromtype) == REFERENCE_TYPE)
+	  {
+	    tree inner = TREE_OPERAND (expr, 0);
+	    if (decl_with_nonnull_addr_p (inner))
+	      warning_at (location,
+			  OPT_Waddress,
+			  "the address of reference %qD may be assumed to "
+			  "always evaluate to %<true%>",
+			  inner);
+	  }
+
 	/* Don't cancel the effect of a CONVERT_EXPR from a REFERENCE_TYPE,
 	   since that affects how `default_conversion' will behave.  */
 	if (TREE_CODE (totype) == REFERENCE_TYPE
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 250b5d6..0d177b7 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4415,7 +4415,10 @@ cp_build_binary_op (location_t location,
 	  else
 	    result_type = type0;
 
-	  if (TREE_CODE (op0) == ADDR_EXPR
+	  if ((TREE_CODE (op0) == ADDR_EXPR
+	       || (CONVERT_EXPR_P (op0)
+		   && TREE_CODE (TREE_TYPE (TREE_OPERAND (op0, 0)))
+		      == REFERENCE_TYPE))
 	      && decl_with_nonnull_addr_p (TREE_OPERAND (op0, 0)))
 	    {
 	      if ((complain & tf_warning)
@@ -4437,7 +4440,10 @@ cp_build_binary_op (location_t location,
 	  else
 	    result_type = type1;
 
-	  if (TREE_CODE (op1) == ADDR_EXPR 
+	  if ((TREE_CODE (op1) == ADDR_EXPR
+	       || (CONVERT_EXPR_P (op1)
+		   && TREE_CODE (TREE_TYPE (TREE_OPERAND (op1, 0)))
+		      == REFERENCE_TYPE))
 	      && decl_with_nonnull_addr_p (TREE_OPERAND (op1, 0)))
 	    {
 	      if ((complain & tf_warning)
diff --git a/gcc/testsuite/g++.dg/warn/Walways-true-3.C b/gcc/testsuite/g++.dg/warn/Walways-true-3.C
new file mode 100644
index 0000000..a145932
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Walways-true-3.C
@@ -0,0 +1,30 @@
+// { dg-options "-Waddress" }
+
+void foo (void);
+
+int d;
+int &c = d;
+
+void
+bar (int &a)
+{
+  int &b = a;
+
+  if (&a) // { dg-warning "address of reference" }
+    foo ();
+
+  if (&b) // { dg-warning "address of reference" }
+    foo ();
+
+  if (&c) // { dg-warning "address of reference" }
+    foo ();
+
+  if (&a == 0) // { dg-warning "never be NULL" }
+    foo ();
+
+  if (&b != 0) // { dg-warning "never be NULL" }
+    foo ();
+
+  if (0 == &c) // { dg-warning "never be NULL" }
+    foo ();
+}
-- 
2.4.0.rc2.31.g7c597ef

Re: [PATCH] Emit -Waddress warnings for comparing address of reference against NULL

[Jason Merrill]

On 04/20/2015 10:36 PM, Patrick Palka wrote:
> +	    if (decl_with_nonnull_addr_p (inner))

Using decl_with_nonnull_addr_p doesn't make sense for reference 
variables, since we're using their pointer value rather than their address.

> +	      warning_at (location,
> +			  OPT_Waddress,
> +			  "the address of reference %qD may be assumed to "
> +			  "always evaluate to %<true%>",
> +			  inner);

Perhaps "the compiler can assume that the address of reference %qD will 
always evaluate to %<true%>"?

> -	  if (TREE_CODE (op0) == ADDR_EXPR
> +	  if ((TREE_CODE (op0) == ADDR_EXPR
> +	       || (CONVERT_EXPR_P (op0)
> +		   && TREE_CODE (TREE_TYPE (TREE_OPERAND (op0, 0)))
> +		      == REFERENCE_TYPE))
>   	      && decl_with_nonnull_addr_p (TREE_OPERAND (op0, 0)))
>   	    {
>   	      if ((complain & tf_warning)
> @@ -4437,7 +4440,10 @@ cp_build_binary_op (location_t location,
>   	  else
>   	    result_type = type1;
>
> -	  if (TREE_CODE (op1) == ADDR_EXPR
> +	  if ((TREE_CODE (op1) == ADDR_EXPR
> +	       || (CONVERT_EXPR_P (op1)
> +		   && TREE_CODE (TREE_TYPE (TREE_OPERAND (op1, 0)))
> +		      == REFERENCE_TYPE))

Let's distinguish between address of object vs reference in these 
warnings, too.

Jason

Re: Re: [PATCH] Emit -Waddress warnings for comparing address of reference against NULL

[Manuel López-Ibáñez]

On 04/23/2015 05:12 PM, Jason Merrill wrote:
> On 04/20/2015 10:36 PM, Patrick Palka wrote:
> Implementation is pretty straightforward.  The only catch is that the
> middle-end doesn't actually assume that REFERENCE_TYPEs are non-NULL so
> code like
>
>     int &a = *(int *)0;
>     if (&a != 0)
>
> will warn that &a will never be NULL yet the middle-end will fold the
> conditional to false instead of true anyway.  But I guess that's not a
> big deal.

Is this actually correct? Is it because of undefined behavior?

It seems also weird we do not warn directly for '*(int *)0' in the C/C++ FE.


>> +        if (decl_with_nonnull_addr_p (inner))
>
> Using decl_with_nonnull_addr_p doesn't make sense for reference variables,
> since we're using their pointer value rather than their address.

Is an extra check needed at all (can &reference ever be false)?

>
>> +          warning_at (location,
>> +              OPT_Waddress,
>> +              "the address of reference %qD may be assumed to "
>> +              "always evaluate to %<true%>",
>> +              inner);
>
> Perhaps "the compiler can assume that the address of reference %qD will always
> evaluate to %<true%>"?

The discussion (and perhaps the patch) at https://gcc.gnu.org/bugzilla/PR65168 
may be relevant.

Jonathan suggests to match what we say for:

/home/manuel/test.c:3:21: warning: the address of ‘i’ will always evaluate as 
‘true’ [-Waddress]
    int i; bool b = !&i;
                      ^

I think this case requires a slightly different text because the address may 
not evaluate to 'true' and also because it is not actually the address of the 
reference but the object bounded to the reference. Clang says:

warning: reference cannot be bound to dereferenced null pointer in well-defined 
C++ code; pointer may be assumed to always convert to true 
[-Wundefined-bool-conversion]

which is in my opinion even less clear.

The testcases:

int fii(int *p) {
   int &r=*p;
   return !&r;
}

int fii(int p) {
   int &r=p;
   return !&r;
}

should also generate the same warning in my opinion.

Cheers,

Manuel.

Re: [PATCH] Emit -Waddress warnings for comparing address of reference against NULL

[Jason Merrill]

On 04/23/2015 11:34 AM, Manuel López-Ibáñez wrote:
> It seems also weird we do not warn directly for '*(int *)0' in the C/C++
> FE.

Agreed.

>> Using decl_with_nonnull_addr_p doesn't make sense for reference variables,
>> since we're using their pointer value rather than their address.
>
> Is an extra check needed at all (can &reference ever be false)?

No, I don't think we need any extra check here.

> The discussion (and perhaps the patch) at
> https://gcc.gnu.org/bugzilla/PR65168 may be relevant.

Yes, I thought this sounded familiar...

> I think this case requires a slightly different text because the address
> may not evaluate to 'true' and also because it is not actually the
> address of the reference but the object bounded to the reference.

I agree with the first point, but C++ (unfortunately) uses "address" to 
refer both to the actual numerical address and the result of the & 
operator.  So I think it's OK to use the word in the latter sense here.

> The testcases:
>
> int fii(int *p) {
>    int &r=*p;
>    return !&r;
> }
>
> int fii(int p) {
>    int &r=p;
>    return !&r;
> }
>
> should also generate the same warning in my opinion.

Agreed.

Jason

Re: Re: [PATCH] Emit -Waddress warnings for comparing address of reference against NULL

[Patrick Palka]

On Thu, Apr 23, 2015 at 11:34 AM, Manuel López-Ibáñez
<lopezibanez@gmail.com> wrote:
> On 04/23/2015 05:12 PM, Jason Merrill wrote:
>>
>> On 04/20/2015 10:36 PM, Patrick Palka wrote:
>> Implementation is pretty straightforward.  The only catch is that the
>> middle-end doesn't actually assume that REFERENCE_TYPEs are non-NULL so
>> code like
>>
>>     int &a = *(int *)0;
>>     if (&a != 0)
>>
>> will warn that &a will never be NULL yet the middle-end will fold the
>> conditional to false instead of true anyway.  But I guess that's not a
>> big deal.
>
>
> Is this actually correct? Is it because of undefined behavior?

I would think that the assignment is UB due to the null-pointer
dereference but if it's not then we may have to fold the comparison to
true always.  According to section 8.3.2 of the C++11 standard:

A reference shall be initialized to refer to a valid object or
function. [ Note: in particular, a null reference cannot exist in a
well-defined program, because the only way to create such a reference
would be to bind it to the “object” obtained by dereferencing a null
pointer, which causes undefined behavior. As described in 9.6, a
reference cannot be bound directly to a bit-field. — end note ]

So it looks like it's not incorrect to fold the comparison to false.

>
> It seems also weird we do not warn directly for '*(int *)0' in the C/C++ FE.

That wouldn't be too hard to add probably.  I'll take a look at this.

>
>
>>> +        if (decl_with_nonnull_addr_p (inner))
>>
>>
>> Using decl_with_nonnull_addr_p doesn't make sense for reference variables,
>> since we're using their pointer value rather than their address.
>
>
> Is an extra check needed at all (can &reference ever be false)?

Consider it removed. Somehow I didn't catch the redundancy of this check..

>
>>
>>> +          warning_at (location,
>>> +              OPT_Waddress,
>>> +              "the address of reference %qD may be assumed to "
>>> +              "always evaluate to %<true%>",
>>> +              inner);
>>
>>
>> Perhaps "the compiler can assume that the address of reference %qD will
>> always
>> evaluate to %<true%>"?
>
>
> The discussion (and perhaps the patch) at
> https://gcc.gnu.org/bugzilla/PR65168 may be relevant.
>
> Jonathan suggests to match what we say for:
>
> /home/manuel/test.c:3:21: warning: the address of ‘i’ will always evaluate
> as ‘true’ [-Waddress]
>    int i; bool b = !&i;
>                      ^
>
> I think this case requires a slightly different text because the address may
> not evaluate to 'true' and also because it is not actually the address of
> the reference but the object bounded to the reference. Clang says:

OK.

>
> warning: reference cannot be bound to dereferenced null pointer in
> well-defined C++ code; pointer may be assumed to always convert to true
> [-Wundefined-bool-conversion]
>
> which is in my opinion even less clear.
>
> The testcases:
>
> int fii(int *p) {
>   int &r=*p;
>   return !&r;
> }
>
> int fii(int p) {
>   int &r=p;
>   return !&r;
> }
>
> should also generate the same warning in my opinion.

Good idea.

Thanks guys for the feedback.  I'll post a new version of this patch
in about 24 hours or so.  So many issues for such a small patch!

[PATCH] Emit -Waddress warnings for comparing address of reference against NULL

[Patrick Palka]

Here is an updated version of the patch with, hopefully, all your
suggestions made.  I decided to add calls to STRIP_NOPS before emitting
the warning so that we properly warn for cases where there's a cast in
between the whole thing, e.g.

  if (!&(int &)a)

I also added guards to emit the warnings only when the stripped operand
is actually a decl so that we don't pass a non-decl argument to
warning_at() which can happen in a case like

  if (!&(int &)*(int *)0)

Does this look OK now after testing?

gcc/c-family/ChangeLog:

	PR c++/65168
	* c-common.c (c_common_truthvalue_conversion): Warn when
	converting an address of a reference to a truth value.

gcc/cp/ChangeLog:

	PR c++/65168
	* typeck.c (cp_build_binary_op): Warn when comparing an address
	of a reference against NULL.

gcc/testsuite/ChangeLog:

	PR c++/65168
	g++.dg/warn/Walways-true-3.C: New test.
---
 gcc/c-family/c-common.c                    | 14 ++++++++++
 gcc/cp/typeck.c                            | 34 ++++++++++++++++++++++
 gcc/testsuite/g++.dg/warn/Walways-true-3.C | 45 ++++++++++++++++++++++++++++++
 3 files changed, 93 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/warn/Walways-true-3.C

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 9797e17..c353c72 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -4806,6 +4806,20 @@ c_common_truthvalue_conversion (location_t location, tree expr)
 	tree totype = TREE_TYPE (expr);
 	tree fromtype = TREE_TYPE (TREE_OPERAND (expr, 0));
 
+	if (POINTER_TYPE_P (totype)
+	    && TREE_CODE (fromtype) == REFERENCE_TYPE)
+	  {
+	    tree inner = expr;
+	    STRIP_NOPS (inner);
+
+	    if (DECL_P (inner))
+	      warning_at (location,
+			  OPT_Waddress,
+			  "the compiler can assume that the address of "
+			  "%qD will always evaluate to %<true%>",
+			  inner);
+	  }
+
 	/* Don't cancel the effect of a CONVERT_EXPR from a REFERENCE_TYPE,
 	   since that affects how `default_conversion' will behave.  */
 	if (TREE_CODE (totype) == REFERENCE_TYPE
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index 91db32a..13fb401 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4423,6 +4423,23 @@ cp_build_binary_op (location_t location,
 		warning (OPT_Waddress, "the address of %qD will never be NULL",
 			 TREE_OPERAND (op0, 0));
 	    }
+
+	  if (CONVERT_EXPR_P (op0)
+	      && TREE_CODE (TREE_TYPE (TREE_OPERAND (op0, 0)))
+		 == REFERENCE_TYPE)
+	    {
+	      tree inner_op0 = op0;
+	      STRIP_NOPS (inner_op0);
+
+	      if ((complain & tf_warning)
+		  && c_inhibit_evaluation_warnings == 0
+		  && !TREE_NO_WARNING (op0)
+		  && DECL_P (inner_op0))
+		warning (OPT_Waddress,
+			 "the compiler can assume that the address of "
+			 "%qD will never be NULL",
+			 inner_op0);
+	    }
 	}
       else if (((code1 == POINTER_TYPE || TYPE_PTRDATAMEM_P (type1))
 		&& null_ptr_cst_p (op0))
@@ -4445,6 +4462,23 @@ cp_build_binary_op (location_t location,
 		warning (OPT_Waddress, "the address of %qD will never be NULL",
 			 TREE_OPERAND (op1, 0));
 	    }
+
+	  if (CONVERT_EXPR_P (op1)
+	      && TREE_CODE (TREE_TYPE (TREE_OPERAND (op1, 0)))
+		 == REFERENCE_TYPE)
+	    {
+	      tree inner_op1 = op1;
+	      STRIP_NOPS (inner_op1);
+
+	      if ((complain & tf_warning)
+		  && c_inhibit_evaluation_warnings == 0
+		  && !TREE_NO_WARNING (op1)
+		  && DECL_P (inner_op1))
+		warning (OPT_Waddress,
+			 "the compiler can assume that the address of "
+			 "%qD will never be NULL",
+			 inner_op1);
+	    }
 	}
       else if ((code0 == POINTER_TYPE && code1 == POINTER_TYPE)
 	       || (TYPE_PTRDATAMEM_P (type0) && TYPE_PTRDATAMEM_P (type1)))
diff --git a/gcc/testsuite/g++.dg/warn/Walways-true-3.C b/gcc/testsuite/g++.dg/warn/Walways-true-3.C
new file mode 100644
index 0000000..0d13d3f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Walways-true-3.C
@@ -0,0 +1,45 @@
+// { dg-options "-Waddress" }
+
+void foo (void);
+
+int d;
+int &c = d;
+
+void
+bar (int &a)
+{
+  int &b = a;
+
+  if ((int *)&a) // { dg-warning "address of" }
+    foo ();
+
+  if (&b) // { dg-warning "address of" }
+    foo ();
+
+  if (!&c) // { dg-warning "address of" }
+    foo ();
+
+  if (!&(int &)(int &)a) // { dg-warning "address of" }
+    foo ();
+
+  if (&a == 0) // { dg-warning "never be NULL" }
+    foo ();
+
+  if (&b != 0) // { dg-warning "never be NULL" }
+    foo ();
+
+  if (0 == &(int &)(int &)c) // { dg-warning "never be NULL" }
+    foo ();
+
+  if (&a != (int *)0) // { dg-warning "never be NULL" }
+    foo ();
+}
+
+bool
+bar_1 (int &a)
+{
+  if (d == 5)
+    return &a; // { dg-warning "address of" }
+  else
+    return !&(int &)(int &)a; // { dg-warning "address of" }
+}
-- 
2.4.0.rc2.31.g7c597ef

Re: [PATCH] Emit -Waddress warnings for comparing address of reference against NULL

[Patrick Palka]

On Sun, Apr 26, 2015 at 8:56 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> Here is an updated version of the patch with, hopefully, all your
> suggestions made.  I decided to add calls to STRIP_NOPS before emitting
> the warning so that we properly warn for cases where there's a cast in
> between the whole thing, e.g.
>
>   if (!&(int &)a)
>
> I also added guards to emit the warnings only when the stripped operand
> is actually a decl so that we don't pass a non-decl argument to
> warning_at() which can happen in a case like
>
>   if (!&(int &)*(int *)0)
>
> Does this look OK now after testing?
>
> gcc/c-family/ChangeLog:
>
>         PR c++/65168
>         * c-common.c (c_common_truthvalue_conversion): Warn when
>         converting an address of a reference to a truth value.
>
> gcc/cp/ChangeLog:
>
>         PR c++/65168
>         * typeck.c (cp_build_binary_op): Warn when comparing an address
>         of a reference against NULL.
>
> gcc/testsuite/ChangeLog:
>
>         PR c++/65168
>         g++.dg/warn/Walways-true-3.C: New test.
> ---
>  gcc/c-family/c-common.c                    | 14 ++++++++++
>  gcc/cp/typeck.c                            | 34 ++++++++++++++++++++++
>  gcc/testsuite/g++.dg/warn/Walways-true-3.C | 45 ++++++++++++++++++++++++++++++
>  3 files changed, 93 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/warn/Walways-true-3.C
>
> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
> index 9797e17..c353c72 100644
> --- a/gcc/c-family/c-common.c
> +++ b/gcc/c-family/c-common.c
> @@ -4806,6 +4806,20 @@ c_common_truthvalue_conversion (location_t location, tree expr)
>         tree totype = TREE_TYPE (expr);
>         tree fromtype = TREE_TYPE (TREE_OPERAND (expr, 0));
>
> +       if (POINTER_TYPE_P (totype)
> +           && TREE_CODE (fromtype) == REFERENCE_TYPE)
> +         {
> +           tree inner = expr;
> +           STRIP_NOPS (inner);
> +
> +           if (DECL_P (inner))
> +             warning_at (location,
> +                         OPT_Waddress,
> +                         "the compiler can assume that the address of "
> +                         "%qD will always evaluate to %<true%>",
> +                         inner);
> +         }
> +
>         /* Don't cancel the effect of a CONVERT_EXPR from a REFERENCE_TYPE,
>            since that affects how `default_conversion' will behave.  */
>         if (TREE_CODE (totype) == REFERENCE_TYPE
> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
> index 91db32a..13fb401 100644
> --- a/gcc/cp/typeck.c
> +++ b/gcc/cp/typeck.c
> @@ -4423,6 +4423,23 @@ cp_build_binary_op (location_t location,
>                 warning (OPT_Waddress, "the address of %qD will never be NULL",
>                          TREE_OPERAND (op0, 0));
>             }
> +
> +         if (CONVERT_EXPR_P (op0)
> +             && TREE_CODE (TREE_TYPE (TREE_OPERAND (op0, 0)))
> +                == REFERENCE_TYPE)
> +           {
> +             tree inner_op0 = op0;
> +             STRIP_NOPS (inner_op0);
> +
> +             if ((complain & tf_warning)
> +                 && c_inhibit_evaluation_warnings == 0
> +                 && !TREE_NO_WARNING (op0)
> +                 && DECL_P (inner_op0))
> +               warning (OPT_Waddress,
> +                        "the compiler can assume that the address of "
> +                        "%qD will never be NULL",
> +                        inner_op0);
> +           }
>         }
>        else if (((code1 == POINTER_TYPE || TYPE_PTRDATAMEM_P (type1))
>                 && null_ptr_cst_p (op0))
> @@ -4445,6 +4462,23 @@ cp_build_binary_op (location_t location,
>                 warning (OPT_Waddress, "the address of %qD will never be NULL",
>                          TREE_OPERAND (op1, 0));
>             }
> +
> +         if (CONVERT_EXPR_P (op1)
> +             && TREE_CODE (TREE_TYPE (TREE_OPERAND (op1, 0)))
> +                == REFERENCE_TYPE)
> +           {
> +             tree inner_op1 = op1;
> +             STRIP_NOPS (inner_op1);
> +
> +             if ((complain & tf_warning)
> +                 && c_inhibit_evaluation_warnings == 0
> +                 && !TREE_NO_WARNING (op1)
> +                 && DECL_P (inner_op1))
> +               warning (OPT_Waddress,
> +                        "the compiler can assume that the address of "
> +                        "%qD will never be NULL",
> +                        inner_op1);
> +           }
>         }
>        else if ((code0 == POINTER_TYPE && code1 == POINTER_TYPE)
>                || (TYPE_PTRDATAMEM_P (type0) && TYPE_PTRDATAMEM_P (type1)))
> diff --git a/gcc/testsuite/g++.dg/warn/Walways-true-3.C b/gcc/testsuite/g++.dg/warn/Walways-true-3.C
> new file mode 100644
> index 0000000..0d13d3f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/warn/Walways-true-3.C
> @@ -0,0 +1,45 @@
> +// { dg-options "-Waddress" }
> +
> +void foo (void);
> +
> +int d;
> +int &c = d;
> +
> +void
> +bar (int &a)
> +{
> +  int &b = a;
> +
> +  if ((int *)&a) // { dg-warning "address of" }
> +    foo ();
> +
> +  if (&b) // { dg-warning "address of" }
> +    foo ();
> +
> +  if (!&c) // { dg-warning "address of" }
> +    foo ();
> +
> +  if (!&(int &)(int &)a) // { dg-warning "address of" }
> +    foo ();
> +
> +  if (&a == 0) // { dg-warning "never be NULL" }
> +    foo ();
> +
> +  if (&b != 0) // { dg-warning "never be NULL" }
> +    foo ();
> +
> +  if (0 == &(int &)(int &)c) // { dg-warning "never be NULL" }
> +    foo ();
> +
> +  if (&a != (int *)0) // { dg-warning "never be NULL" }
> +    foo ();
> +}
> +
> +bool
> +bar_1 (int &a)
> +{
> +  if (d == 5)
> +    return &a; // { dg-warning "address of" }
> +  else
> +    return !&(int &)(int &)a; // { dg-warning "address of" }
> +}
> --
> 2.4.0.rc2.31.g7c597ef
>

Ping.

Re: [PATCH] Emit -Waddress warnings for comparing address of reference against NULL

[Patrick Palka]

On Sun, May 3, 2015 at 5:29 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Sun, Apr 26, 2015 at 8:56 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>> Here is an updated version of the patch with, hopefully, all your
>> suggestions made.  I decided to add calls to STRIP_NOPS before emitting
>> the warning so that we properly warn for cases where there's a cast in
>> between the whole thing, e.g.
>>
>>   if (!&(int &)a)
>>
>> I also added guards to emit the warnings only when the stripped operand
>> is actually a decl so that we don't pass a non-decl argument to
>> warning_at() which can happen in a case like
>>
>>   if (!&(int &)*(int *)0)
>>
>> Does this look OK now after testing?
>>
>> gcc/c-family/ChangeLog:
>>
>>         PR c++/65168
>>         * c-common.c (c_common_truthvalue_conversion): Warn when
>>         converting an address of a reference to a truth value.
>>
>> gcc/cp/ChangeLog:
>>
>>         PR c++/65168
>>         * typeck.c (cp_build_binary_op): Warn when comparing an address
>>         of a reference against NULL.
>>
>> gcc/testsuite/ChangeLog:
>>
>>         PR c++/65168
>>         g++.dg/warn/Walways-true-3.C: New test.
>> ---
>>  gcc/c-family/c-common.c                    | 14 ++++++++++
>>  gcc/cp/typeck.c                            | 34 ++++++++++++++++++++++
>>  gcc/testsuite/g++.dg/warn/Walways-true-3.C | 45 ++++++++++++++++++++++++++++++
>>  3 files changed, 93 insertions(+)
>>  create mode 100644 gcc/testsuite/g++.dg/warn/Walways-true-3.C
>>
>> diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
>> index 9797e17..c353c72 100644
>> --- a/gcc/c-family/c-common.c
>> +++ b/gcc/c-family/c-common.c
>> @@ -4806,6 +4806,20 @@ c_common_truthvalue_conversion (location_t location, tree expr)
>>         tree totype = TREE_TYPE (expr);
>>         tree fromtype = TREE_TYPE (TREE_OPERAND (expr, 0));
>>
>> +       if (POINTER_TYPE_P (totype)
>> +           && TREE_CODE (fromtype) == REFERENCE_TYPE)
>> +         {
>> +           tree inner = expr;
>> +           STRIP_NOPS (inner);
>> +
>> +           if (DECL_P (inner))
>> +             warning_at (location,
>> +                         OPT_Waddress,
>> +                         "the compiler can assume that the address of "
>> +                         "%qD will always evaluate to %<true%>",
>> +                         inner);
>> +         }
>> +
>>         /* Don't cancel the effect of a CONVERT_EXPR from a REFERENCE_TYPE,
>>            since that affects how `default_conversion' will behave.  */
>>         if (TREE_CODE (totype) == REFERENCE_TYPE
>> diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
>> index 91db32a..13fb401 100644
>> --- a/gcc/cp/typeck.c
>> +++ b/gcc/cp/typeck.c
>> @@ -4423,6 +4423,23 @@ cp_build_binary_op (location_t location,
>>                 warning (OPT_Waddress, "the address of %qD will never be NULL",
>>                          TREE_OPERAND (op0, 0));
>>             }
>> +
>> +         if (CONVERT_EXPR_P (op0)
>> +             && TREE_CODE (TREE_TYPE (TREE_OPERAND (op0, 0)))
>> +                == REFERENCE_TYPE)
>> +           {
>> +             tree inner_op0 = op0;
>> +             STRIP_NOPS (inner_op0);
>> +
>> +             if ((complain & tf_warning)
>> +                 && c_inhibit_evaluation_warnings == 0
>> +                 && !TREE_NO_WARNING (op0)
>> +                 && DECL_P (inner_op0))
>> +               warning (OPT_Waddress,
>> +                        "the compiler can assume that the address of "
>> +                        "%qD will never be NULL",
>> +                        inner_op0);
>> +           }
>>         }
>>        else if (((code1 == POINTER_TYPE || TYPE_PTRDATAMEM_P (type1))
>>                 && null_ptr_cst_p (op0))
>> @@ -4445,6 +4462,23 @@ cp_build_binary_op (location_t location,
>>                 warning (OPT_Waddress, "the address of %qD will never be NULL",
>>                          TREE_OPERAND (op1, 0));
>>             }
>> +
>> +         if (CONVERT_EXPR_P (op1)
>> +             && TREE_CODE (TREE_TYPE (TREE_OPERAND (op1, 0)))
>> +                == REFERENCE_TYPE)
>> +           {
>> +             tree inner_op1 = op1;
>> +             STRIP_NOPS (inner_op1);
>> +
>> +             if ((complain & tf_warning)
>> +                 && c_inhibit_evaluation_warnings == 0
>> +                 && !TREE_NO_WARNING (op1)
>> +                 && DECL_P (inner_op1))
>> +               warning (OPT_Waddress,
>> +                        "the compiler can assume that the address of "
>> +                        "%qD will never be NULL",
>> +                        inner_op1);
>> +           }
>>         }
>>        else if ((code0 == POINTER_TYPE && code1 == POINTER_TYPE)
>>                || (TYPE_PTRDATAMEM_P (type0) && TYPE_PTRDATAMEM_P (type1)))
>> diff --git a/gcc/testsuite/g++.dg/warn/Walways-true-3.C b/gcc/testsuite/g++.dg/warn/Walways-true-3.C
>> new file mode 100644
>> index 0000000..0d13d3f
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/warn/Walways-true-3.C
>> @@ -0,0 +1,45 @@
>> +// { dg-options "-Waddress" }
>> +
>> +void foo (void);
>> +
>> +int d;
>> +int &c = d;
>> +
>> +void
>> +bar (int &a)
>> +{
>> +  int &b = a;
>> +
>> +  if ((int *)&a) // { dg-warning "address of" }
>> +    foo ();
>> +
>> +  if (&b) // { dg-warning "address of" }
>> +    foo ();
>> +
>> +  if (!&c) // { dg-warning "address of" }
>> +    foo ();
>> +
>> +  if (!&(int &)(int &)a) // { dg-warning "address of" }
>> +    foo ();
>> +
>> +  if (&a == 0) // { dg-warning "never be NULL" }
>> +    foo ();
>> +
>> +  if (&b != 0) // { dg-warning "never be NULL" }
>> +    foo ();
>> +
>> +  if (0 == &(int &)(int &)c) // { dg-warning "never be NULL" }
>> +    foo ();
>> +
>> +  if (&a != (int *)0) // { dg-warning "never be NULL" }
>> +    foo ();
>> +}
>> +
>> +bool
>> +bar_1 (int &a)
>> +{
>> +  if (d == 5)
>> +    return &a; // { dg-warning "address of" }
>> +  else
>> +    return !&(int &)(int &)a; // { dg-warning "address of" }
>> +}
>> --
>> 2.4.0.rc2.31.g7c597ef
>>
>
> Ping.

Ping.

Re: [PATCH] Emit -Waddress warnings for comparing address of reference against NULL

[Jason Merrill]

Let's use explicit location for the C++ front end warnings, too.  OK 
with that change.

Jason